forgejo_admin/pal-e-platform
1
0
Fork 0
Code Issues 33 Pull requests Projects Releases Packages Wiki Activity Actions

fix: remove redundant :80 from internal Forgejo URLs #217

Merged
forgejo_admin merged 1 commit from 203-remove-port-80-from-internal-forgejo-urls into main 2026-03-28 05:28:40 +00:00
Conversation 2 Commits 1 Files changed 5 +8 -8
forgejo_admin commented 2026-03-28 04:59:44 +00:00
Owner
Copy link

Summary

Removes the explicit :80 port from all internal Forgejo URLs in pal-e-platform. Port 80 is the HTTP default and redundant, but ArgoCD does string comparison on repo URLs for credential matching -- explicit :80 causes mismatches with credential templates that omit it. This aligns pal-e-platform with the same fix already merged in pal-e-services (PR #37, commit 6b4267d).

Changes

  • terraform/modules/ci/main.tf -- remove :80 from WOODPECKER_FORGEJO_URL env var
  • terraform/modules/monitoring/main.tf -- remove :80 from Blackbox exporter Forgejo uptime target and DORA exporter FORGEJO_URL config map entry
  • .woodpecker.yaml -- remove :80 from clone step git remote, plan comment API call, and issue creation API call (3 occurrences)
  • scripts/update-kustomize-tag.sh -- remove :80 from git clone URL template
  • scripts/woodpecker-update-tag-step.yaml -- remove :80 from script download URL

Expected tofu plan impact on next apply:

  • Woodpecker Helm release: in-place update (env var change)
  • Blackbox exporter Helm release: in-place update (target URL change)
  • DORA exporter ConfigMap: in-place update (data change)

Test Plan

  • tofu fmt -check passes on both affected modules
  • CI pipeline clone step works with no-port URL (HTTP default port 80 is implicit)
  • Forgejo API calls in CI steps work without explicit port
  • After tofu apply: Woodpecker can reach Forgejo, Blackbox uptime probes succeed, DORA exporter collects metrics

Review Checklist

  • Only Forgejo URLs changed -- non-Forgejo :80 URLs (woodpecker-server, grafana, harbor, argocd, keycloak) left untouched to avoid scope creep
  • Spike docs (docs/spikes/125-ci-bootstrap-resilience.md) intentionally left unchanged -- historical documentation
  • No HCL structural changes, string-only substitution
  • Consistent with pal-e-services fix (PR #37)

Related Notes

  • Closes #203
  • pal-e-services PR #37 (merged) -- same fix for ArgoCD app repo_url and repository_credentials
  • pal-e-services #36 (closed) -- the :80 bug discovery
  • pal-e-platform #143 -- original internal URL migration (incomplete)
## Summary Removes the explicit `:80` port from all internal Forgejo URLs in pal-e-platform. Port 80 is the HTTP default and redundant, but ArgoCD does string comparison on repo URLs for credential matching -- explicit `:80` causes mismatches with credential templates that omit it. This aligns pal-e-platform with the same fix already merged in pal-e-services (PR #37, commit 6b4267d). ## Changes - `terraform/modules/ci/main.tf` -- remove `:80` from `WOODPECKER_FORGEJO_URL` env var - `terraform/modules/monitoring/main.tf` -- remove `:80` from Blackbox exporter Forgejo uptime target and DORA exporter `FORGEJO_URL` config map entry - `.woodpecker.yaml` -- remove `:80` from clone step git remote, plan comment API call, and issue creation API call (3 occurrences) - `scripts/update-kustomize-tag.sh` -- remove `:80` from git clone URL template - `scripts/woodpecker-update-tag-step.yaml` -- remove `:80` from script download URL Expected tofu plan impact on next apply: - Woodpecker Helm release: in-place update (env var change) - Blackbox exporter Helm release: in-place update (target URL change) - DORA exporter ConfigMap: in-place update (data change) ## Test Plan - [x] `tofu fmt -check` passes on both affected modules - [ ] CI pipeline clone step works with no-port URL (HTTP default port 80 is implicit) - [ ] Forgejo API calls in CI steps work without explicit port - [ ] After `tofu apply`: Woodpecker can reach Forgejo, Blackbox uptime probes succeed, DORA exporter collects metrics ## Review Checklist - [x] Only Forgejo URLs changed -- non-Forgejo `:80` URLs (woodpecker-server, grafana, harbor, argocd, keycloak) left untouched to avoid scope creep - [x] Spike docs (`docs/spikes/125-ci-bootstrap-resilience.md`) intentionally left unchanged -- historical documentation - [x] No HCL structural changes, string-only substitution - [x] Consistent with pal-e-services fix (PR #37) ## Related Notes - Closes #203 - pal-e-services PR #37 (merged) -- same fix for ArgoCD app repo_url and repository_credentials - pal-e-services #36 (closed) -- the :80 bug discovery - pal-e-platform #143 -- original internal URL migration (incomplete)
fix: remove redundant :80 from internal Forgejo URLs
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
ci/woodpecker/pull_request_closed/woodpecker Pipeline was successful
Details
e1d40f1f54 
Port 80 is the HTTP default and is redundant in URLs. ArgoCD does
string comparison on repo URLs for credential matching, so explicit
:80 causes mismatches with credential templates that omit it. This
aligns pal-e-platform with the same fix already applied in
pal-e-services (PR #37).

Removes :80 from Forgejo internal URLs in:
- Woodpecker Forgejo URL (ci module)
- Blackbox exporter uptime target (monitoring module)
- DORA exporter config map (monitoring module)
- CI clone step (.woodpecker.yaml)
- CI plan comment step (.woodpecker.yaml)
- CI issue creation step (.woodpecker.yaml)
- Kustomize tag update script and step template

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
forgejo_admin commented 2026-03-28 05:00:36 +00:00
Author
Owner
Copy link

QA Review

Scope: 5 files, 8 additions, 8 deletions. All changes are string-only substitutions removing :80 from internal Forgejo HTTP URLs.

Checks

Check Result
All Forgejo :80 URLs removed from code files (tf, yaml, sh) PASS -- grep confirms zero remaining matches
Non-Forgejo :80 URLs untouched (woodpecker-server, grafana, harbor, argocd, keycloak) PASS -- correctly scoped
Historical spike docs left unchanged PASS -- docs/spikes/125-ci-bootstrap-resilience.md preserves original URLs
tofu fmt -check -recursive on modules PASS
Consistent with pal-e-services PR #37 PASS -- same pattern (:80 removal from internal Forgejo URLs)
PR body has all required template sections PASS
Closes #203 present PASS
No unrelated changes PASS

Nits

  • WOODPECKER_URL in the DORA exporter ConfigMap (monitoring/main.tf:446) still has :80 for woodpecker-server.woodpecker.svc.cluster.local:80. This is not a Forgejo URL so it is correctly out of scope for this PR, but could be a follow-up consistency fix.

VERDICT: APPROVE

## QA Review **Scope**: 5 files, 8 additions, 8 deletions. All changes are string-only substitutions removing `:80` from internal Forgejo HTTP URLs. ### Checks | Check | Result | |-------|--------| | All Forgejo `:80` URLs removed from code files (tf, yaml, sh) | PASS -- grep confirms zero remaining matches | | Non-Forgejo `:80` URLs untouched (woodpecker-server, grafana, harbor, argocd, keycloak) | PASS -- correctly scoped | | Historical spike docs left unchanged | PASS -- `docs/spikes/125-ci-bootstrap-resilience.md` preserves original URLs | | `tofu fmt -check -recursive` on modules | PASS | | Consistent with pal-e-services PR #37 | PASS -- same pattern (`:80` removal from internal Forgejo URLs) | | PR body has all required template sections | PASS | | `Closes #203` present | PASS | | No unrelated changes | PASS | ### Nits - `WOODPECKER_URL` in the DORA exporter ConfigMap (`monitoring/main.tf:446`) still has `:80` for `woodpecker-server.woodpecker.svc.cluster.local:80`. This is not a Forgejo URL so it is correctly out of scope for this PR, but could be a follow-up consistency fix. ### VERDICT: APPROVE
forgejo_admin commented 2026-03-28 05:03:07 +00:00
Author
Owner
Copy link

PR #217 Review

DOMAIN REVIEW

Tech stack: Terraform/HCL (Helm provider), Woodpecker CI YAML, shell scripts. This is an infrastructure string substitution PR -- removing redundant :80 from HTTP Forgejo URLs across 5 files to fix ArgoCD credential string-matching.

Correctness: All 8 substitutions are mechanically correct -- each removes only the :80 suffix from http://forgejo-http.forgejo.svc.cluster.local:80 URLs. Port 80 is the HTTP default; omitting it produces a semantically identical URL. The fix aligns with the already-merged pal-e-services PR #37.

Scope discipline: Verified that non-Forgejo :80 URLs are correctly untouched. The following services retain their explicit :80 and are out of scope: woodpecker-server, grafana, harbor-core, argocd-server, keycloak, westside-dev, platform-validation. This is the right call -- the bug is specifically about ArgoCD repo URL credential matching against Forgejo.

Terraform impact: Changes touch only string values inside yamlencode() blocks and kubernetes_secret_v1.data. No HCL structural changes. Expected tofu plan: in-place updates to Woodpecker Helm release, Blackbox exporter Helm release, and DORA exporter ConfigMap. No destroy/recreate risk.

Shell script (update-kustomize-tag.sh): The :80 was appended to the ${FORGEJO_HOST} variable. Removal is correct -- the default value for FORGEJO_HOST on line 31 is forgejo-http.forgejo.svc.cluster.local (no port), so the old code was adding :80 redundantly.

Spike docs: docs/spikes/125-ci-bootstrap-resilience.md correctly left unchanged -- historical documentation should not be rewritten.

BLOCKERS

None.

  • No new functionality introduced, so no test coverage required -- this is a pure string substitution bug fix.
  • No secrets or credentials added or exposed.
  • No user input handling changes.
  • No auth/security path duplication.

NITS

  1. DORA exporter WOODPECKER_URL still has :80 (terraform/modules/monitoring/main.tf line 446): "http://woodpecker-server.woodpecker.svc.cluster.local:80". While this PR correctly scopes to Forgejo-only URLs (and this is a Woodpecker URL, not Forgejo), the same redundant :80 pattern exists for Woodpecker. If Woodpecker also does string comparison on URLs for any internal matching, this could bite later. Consider a follow-up ticket to normalize all :80 on HTTP services, or leave a comment explaining why only Forgejo URLs needed this fix.

  2. Blackbox targets retain :80 on non-Forgejo services: Same class of inconsistency for woodpecker, grafana, harbor, argocd, keycloak, westside-dev, platform-validation. Low risk since Blackbox just HTTP-probes these and does not do string matching, but the inconsistency in style is worth a tracking note.

SOP COMPLIANCE

  • Branch named after issue: 203-remove-port-80-from-internal-forgejo-urls references issue #203
  • PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related Notes all present
  • Related section references parent issue: Closes #203 plus cross-references to pal-e-services PR #37, issue #36, and platform issue #143
  • No secrets committed: diff is string-only substitution on URL patterns
  • No unnecessary file changes: 5 files, 8 additions / 8 deletions, all directly related to the Forgejo :80 removal
  • Commit messages are descriptive (PR title: fix: remove redundant :80 from internal Forgejo URLs)
  • Related section does not reference a plan slug (not applicable -- this is a standalone bug fix, no plan required)

PROCESS OBSERVATIONS

  • Change failure risk: Very low. Pure string substitution with no structural changes. The fix has already been validated in the pal-e-services sibling repo (PR #37). Worst case on failure: Woodpecker cannot reach Forgejo (easily detected by blackbox probe / CI failure), rollback is trivial.
  • Deployment frequency: Positive impact. Fixing the ArgoCD credential mismatch unblocks automated deployments that were falling back to wrong source repos or external URLs.
  • Test plan: Two items are unchecked (CI clone and API calls without port). These will be validated on merge when the CI pipeline runs against itself. Acceptable for this type of change.

VERDICT: APPROVED

## PR #217 Review ### DOMAIN REVIEW **Tech stack**: Terraform/HCL (Helm provider), Woodpecker CI YAML, shell scripts. This is an infrastructure string substitution PR -- removing redundant `:80` from HTTP Forgejo URLs across 5 files to fix ArgoCD credential string-matching. **Correctness**: All 8 substitutions are mechanically correct -- each removes only the `:80` suffix from `http://forgejo-http.forgejo.svc.cluster.local:80` URLs. Port 80 is the HTTP default; omitting it produces a semantically identical URL. The fix aligns with the already-merged pal-e-services PR #37. **Scope discipline**: Verified that non-Forgejo `:80` URLs are correctly untouched. The following services retain their explicit `:80` and are out of scope: woodpecker-server, grafana, harbor-core, argocd-server, keycloak, westside-dev, platform-validation. This is the right call -- the bug is specifically about ArgoCD repo URL credential matching against Forgejo. **Terraform impact**: Changes touch only string values inside `yamlencode()` blocks and `kubernetes_secret_v1.data`. No HCL structural changes. Expected tofu plan: in-place updates to Woodpecker Helm release, Blackbox exporter Helm release, and DORA exporter ConfigMap. No destroy/recreate risk. **Shell script (`update-kustomize-tag.sh`)**: The `:80` was appended to the `${FORGEJO_HOST}` variable. Removal is correct -- the default value for `FORGEJO_HOST` on line 31 is `forgejo-http.forgejo.svc.cluster.local` (no port), so the old code was adding `:80` redundantly. **Spike docs**: `docs/spikes/125-ci-bootstrap-resilience.md` correctly left unchanged -- historical documentation should not be rewritten. ### BLOCKERS None. - No new functionality introduced, so no test coverage required -- this is a pure string substitution bug fix. - No secrets or credentials added or exposed. - No user input handling changes. - No auth/security path duplication. ### NITS 1. **DORA exporter `WOODPECKER_URL` still has `:80`** (`terraform/modules/monitoring/main.tf` line 446): `"http://woodpecker-server.woodpecker.svc.cluster.local:80"`. While this PR correctly scopes to Forgejo-only URLs (and this is a Woodpecker URL, not Forgejo), the same redundant `:80` pattern exists for Woodpecker. If Woodpecker also does string comparison on URLs for any internal matching, this could bite later. Consider a follow-up ticket to normalize all `:80` on HTTP services, or leave a comment explaining why only Forgejo URLs needed this fix. 2. **Blackbox targets retain `:80` on non-Forgejo services**: Same class of inconsistency for woodpecker, grafana, harbor, argocd, keycloak, westside-dev, platform-validation. Low risk since Blackbox just HTTP-probes these and does not do string matching, but the inconsistency in style is worth a tracking note. ### SOP COMPLIANCE - [x] Branch named after issue: `203-remove-port-80-from-internal-forgejo-urls` references issue #203 - [x] PR body follows template: Summary, Changes, Test Plan, Review Checklist, Related Notes all present - [x] Related section references parent issue: `Closes #203` plus cross-references to pal-e-services PR #37, issue #36, and platform issue #143 - [x] No secrets committed: diff is string-only substitution on URL patterns - [x] No unnecessary file changes: 5 files, 8 additions / 8 deletions, all directly related to the Forgejo `:80` removal - [x] Commit messages are descriptive (PR title: `fix: remove redundant :80 from internal Forgejo URLs`) - [ ] Related section does not reference a plan slug (not applicable -- this is a standalone bug fix, no plan required) ### PROCESS OBSERVATIONS - **Change failure risk**: Very low. Pure string substitution with no structural changes. The fix has already been validated in the pal-e-services sibling repo (PR #37). Worst case on failure: Woodpecker cannot reach Forgejo (easily detected by blackbox probe / CI failure), rollback is trivial. - **Deployment frequency**: Positive impact. Fixing the ArgoCD credential mismatch unblocks automated deployments that were falling back to wrong source repos or external URLs. - **Test plan**: Two items are unchecked (CI clone and API calls without port). These will be validated on merge when the CI pipeline runs against itself. Acceptable for this type of change. ### VERDICT: APPROVED
forgejo_admin deleted branch 203-remove-port-80-from-internal-forgejo-urls 2026-03-28 05:28:40 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
domain:backend

   
domain:devops

   
domain:frontend

   
status:approved

QA passed, awaiting merge approval

  
status:in-progress

Dev agent is actively working

  
status:needs-fix

QA found issues, back to dev

  
status:qa

PR submitted, awaiting QA review

  
type:bug

Bug fix

  
type:devops

Infrastructure/CI/config work

  
type:feature

New functionality

No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo_admin/pal-e-platform!217
No description provided.