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

feat: expose HARBOR_REGISTRY_INTERNAL to CI agents #159

Merged
forgejo_admin merged 2 commits from 135-harbor-internal-url into main 2026-03-24 20:58:33 +00:00
Conversation 3 Commits 2 Files changed 1 +1
forgejo_admin commented 2026-03-24 20:43:53 +00:00
Owner
Copy link

Summary

Adds HARBOR_REGISTRY_INTERNAL environment variable to the Woodpecker agent configuration so CI pipelines can push images via cluster-local DNS (harbor-core.harbor.svc.cluster.local) instead of the external Tailscale URL that routes through DERP relay.

Changes

  • terraform/main.tf: Added HARBOR_REGISTRY_INTERNAL = "harbor-core.harbor.svc.cluster.local" to the Woodpecker agent env block

Test Plan

  • tofu fmt -- no formatting changes needed
  • tofu validate -- passes
  • After apply, verify the Woodpecker agent pod has HARBOR_REGISTRY_INTERNAL in its environment
  • Service repo pipelines can reference $HARBOR_REGISTRY_INTERNAL for image push targets

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive

Related

## Summary Adds `HARBOR_REGISTRY_INTERNAL` environment variable to the Woodpecker agent configuration so CI pipelines can push images via cluster-local DNS (`harbor-core.harbor.svc.cluster.local`) instead of the external Tailscale URL that routes through DERP relay. ## Changes - `terraform/main.tf`: Added `HARBOR_REGISTRY_INTERNAL = "harbor-core.harbor.svc.cluster.local"` to the Woodpecker agent `env` block ## Test Plan - [x] `tofu fmt` -- no formatting changes needed - [x] `tofu validate` -- passes - [ ] After apply, verify the Woodpecker agent pod has `HARBOR_REGISTRY_INTERNAL` in its environment - [ ] Service repo pipelines can reference `$HARBOR_REGISTRY_INTERNAL` for image push targets ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related - Closes #135
feat: expose HARBOR_REGISTRY_INTERNAL env var to Woodpecker CI agents
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
fef3e47b8c 
CI pipelines currently push images to the external Harbor URL which
routes through Tailscale DERP relay from inside the cluster, causing
unreliable image pushes. Adding the internal k8s service URL as an
environment variable lets pipelines use cluster-local DNS instead.

Closes #135

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

Automated Review -- LGTM

Single-line addition, correctly placed in the Woodpecker agent env block. Value uses proper k8s internal service DNS. No secrets, no unnecessary changes. tofu fmt and tofu validate both pass.

**Automated Review -- LGTM** Single-line addition, correctly placed in the Woodpecker agent `env` block. Value uses proper k8s internal service DNS. No secrets, no unnecessary changes. `tofu fmt` and `tofu validate` both pass.
forgejo_admin commented 2026-03-24 20:46:23 +00:00
Author
Owner
Copy link

PR #159 Review

DOMAIN REVIEW

Tech stack: OpenTofu / Helm provider / k8s (Woodpecker CI Helm chart values)

Hostname mismatch -- wrong Harbor service for image push

The env var sets HARBOR_REGISTRY_INTERNAL = "harbor-core.harbor.svc.cluster.local". However, harbor-core is the Harbor core API service. For Docker registry push operations (the stated purpose of this env var), the correct internal service is the Harbor nginx frontend: harbor.harbor.svc.cluster.local.

Evidence from five pipelines that already use cluster-local push:

Repo .woodpecker.yaml registry field
pal-e-app harbor.harbor.svc.cluster.local
basketball-api harbor.harbor.svc.cluster.local
pal-e-mail harbor.harbor.svc.cluster.local
pal-e-docs harbor.harbor.svc.cluster.local
westside-app harbor.harbor.svc.cluster.local

The Blackbox health check at line 430 of terraform/main.tf correctly uses harbor-core.harbor.svc.cluster.local:80/api/v2.0/health because it is probing the API directly. But registry push traffic must go through the nginx service that handles v2 protocol routing.

If pipelines use $HARBOR_REGISTRY_INTERNAL for kaniko or docker push, they will fail because harbor-core does not serve the Docker v2 registry API directly.

Correct value: harbor.harbor.svc.cluster.local

HCL formatting: The alignment of = signs is consistent with the existing block. tofu fmt compliant. No issues.

Scope: Single-line addition in the correct block (agent.env within the Woodpecker Helm values). No unrelated changes. 1 file, 1 addition, 0 deletions.

BLOCKERS

  1. Wrong internal hostname: harbor-core.harbor.svc.cluster.local is the core API service, not the registry ingress. Must be harbor.harbor.svc.cluster.local to match the 5 pipelines that already use cluster-local push. This will cause push failures for any pipeline that references $HARBOR_REGISTRY_INTERNAL.

NITS

  1. Missing tofu plan output: PR conventions (CLAUDE.md) require tofu plan output for Terraform changes. The PR body has tofu fmt and tofu validate checks but no plan output. Low risk for a single env var addition, but noted for process compliance.

SOP COMPLIANCE

  • Branch named after issue (135-harbor-internal-url references issue #135)
  • PR body follows template (Summary, Changes, Test Plan, Related sections present)
  • Related references plan slug -- PR body has Closes #135 but no plan slug reference. Acceptable if this is standalone board work without a parent plan.
  • No secrets committed (value is a cluster-internal DNS name, not a credential)
  • No unnecessary file changes (1 file, 1 line)
  • Commit messages are descriptive

PROCESS OBSERVATIONS

  • Change failure risk: HIGH if merged as-is. The wrong hostname will silently propagate to any pipeline that adopts $HARBOR_REGISTRY_INTERNAL, causing push failures. The 5 pipelines that already hardcode the correct internal URL (harbor.harbor.svc.cluster.local) would not be affected, but the whole point of this env var is to give a canonical reference for migration.
  • Deployment frequency: No impact -- single env var addition, fast to fix and re-apply.
  • MTTR context: This PR stems from issue #135 (Harbor unreachable from CI pods). Getting the hostname right is critical to actually resolving that connectivity issue.

VERDICT: NOT APPROVED

One blocker: the internal hostname must be harbor.harbor.svc.cluster.local (the nginx frontend that serves Docker v2 registry protocol), not harbor-core.harbor.svc.cluster.local (the core API service). Fix the value and this is ready to merge.

## PR #159 Review ### DOMAIN REVIEW **Tech stack**: OpenTofu / Helm provider / k8s (Woodpecker CI Helm chart values) **Hostname mismatch -- wrong Harbor service for image push** The env var sets `HARBOR_REGISTRY_INTERNAL = "harbor-core.harbor.svc.cluster.local"`. However, `harbor-core` is the Harbor **core API** service. For Docker registry push operations (the stated purpose of this env var), the correct internal service is the Harbor **nginx frontend**: `harbor.harbor.svc.cluster.local`. Evidence from five pipelines that already use cluster-local push: | Repo | `.woodpecker.yaml` registry field | |------|----------------------------------| | `pal-e-app` | `harbor.harbor.svc.cluster.local` | | `basketball-api` | `harbor.harbor.svc.cluster.local` | | `pal-e-mail` | `harbor.harbor.svc.cluster.local` | | `pal-e-docs` | `harbor.harbor.svc.cluster.local` | | `westside-app` | `harbor.harbor.svc.cluster.local` | The Blackbox health check at line 430 of `terraform/main.tf` correctly uses `harbor-core.harbor.svc.cluster.local:80/api/v2.0/health` because it is probing the API directly. But registry push traffic must go through the nginx service that handles v2 protocol routing. If pipelines use `$HARBOR_REGISTRY_INTERNAL` for `kaniko` or `docker push`, they will fail because `harbor-core` does not serve the Docker v2 registry API directly. **Correct value**: `harbor.harbor.svc.cluster.local` **HCL formatting**: The alignment of `=` signs is consistent with the existing block. `tofu fmt` compliant. No issues. **Scope**: Single-line addition in the correct block (`agent.env` within the Woodpecker Helm values). No unrelated changes. 1 file, 1 addition, 0 deletions. ### BLOCKERS 1. **Wrong internal hostname**: `harbor-core.harbor.svc.cluster.local` is the core API service, not the registry ingress. Must be `harbor.harbor.svc.cluster.local` to match the 5 pipelines that already use cluster-local push. This will cause push failures for any pipeline that references `$HARBOR_REGISTRY_INTERNAL`. ### NITS 1. **Missing `tofu plan` output**: PR conventions (`CLAUDE.md`) require `tofu plan` output for Terraform changes. The PR body has `tofu fmt` and `tofu validate` checks but no plan output. Low risk for a single env var addition, but noted for process compliance. ### SOP COMPLIANCE - [x] Branch named after issue (`135-harbor-internal-url` references issue #135) - [x] PR body follows template (Summary, Changes, Test Plan, Related sections present) - [ ] Related references plan slug -- PR body has `Closes #135` but no plan slug reference. Acceptable if this is standalone board work without a parent plan. - [x] No secrets committed (value is a cluster-internal DNS name, not a credential) - [x] No unnecessary file changes (1 file, 1 line) - [x] Commit messages are descriptive ### PROCESS OBSERVATIONS - **Change failure risk**: HIGH if merged as-is. The wrong hostname will silently propagate to any pipeline that adopts `$HARBOR_REGISTRY_INTERNAL`, causing push failures. The 5 pipelines that already hardcode the correct internal URL (`harbor.harbor.svc.cluster.local`) would not be affected, but the whole point of this env var is to give a canonical reference for migration. - **Deployment frequency**: No impact -- single env var addition, fast to fix and re-apply. - **MTTR context**: This PR stems from issue #135 (Harbor unreachable from CI pods). Getting the hostname right is critical to actually resolving that connectivity issue. ### VERDICT: NOT APPROVED One blocker: the internal hostname must be `harbor.harbor.svc.cluster.local` (the nginx frontend that serves Docker v2 registry protocol), not `harbor-core.harbor.svc.cluster.local` (the core API service). Fix the value and this is ready to merge.
fix: use harbor nginx frontend URL, not core API
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
ce013c3ae0 
QA caught that harbor-core.harbor.svc.cluster.local is the core API
service (for health checks), not the registry push endpoint. All 5
existing service repos use harbor.harbor.svc.cluster.local (nginx
frontend) for Docker push. Match the established convention.

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

PR #159 Review (Re-review)

DOMAIN REVIEW

Tech stack: OpenTofu / Helm provider / Kubernetes (Woodpecker CI agent configuration).

The change adds a single environment variable HARBOR_REGISTRY_INTERNAL to the Woodpecker agent Helm values. The value harbor.harbor.svc.cluster.local is the Harbor nginx frontend service -- the correct endpoint for Docker push/pull within the cluster.

Validation against existing usage:

  • pal-e-docs/.woodpecker.yaml line 54: registry: harbor.harbor.svc.cluster.local -- matches
  • basketball-api/.woodpecker.yaml line 44: registry: harbor.harbor.svc.cluster.local -- matches
  • mcd-tracker-api/.woodpecker.yaml line 34: still uses external URL harbor.tail5b443a.ts.net -- a candidate to consume this new env var

The previous review flagged that harbor-core.harbor.svc.cluster.local was wrong for Docker push. The fix commit correctly changed it to harbor.harbor.svc.cluster.local. Note that harbor-core on line 430 is still correct in its own context -- that is the health check API endpoint, not the Docker registry frontend. These are distinct Harbor services.

Formatting: The env var key alignment (padding with spaces) is consistent with the surrounding WOODPECKER_BACKEND_* entries. tofu fmt compliant.

BLOCKERS

None.

NITS

  1. Stale PR description: The Summary and Changes sections still reference harbor-core.harbor.svc.cluster.local from the original commit, but the actual code now uses harbor.harbor.svc.cluster.local. Non-blocking, but the description should be updated to avoid confusion for anyone reading the PR history.

  2. No tofu plan output: The PR conventions in CLAUDE.md say to include tofu plan output for Terraform changes. The Test Plan has checkboxes for post-apply verification but no plan output. Non-blocking for a single env var addition.

SOP COMPLIANCE

  • Branch named after issue (135-harbor-internal-url references issue #135)
  • PR body has Summary, Changes, Test Plan, Related sections
  • Related references closes #135
  • Plan slug not applicable (bug fix, not plan work)
  • No secrets committed (value is a cluster-internal DNS name, not a credential)
  • No unnecessary file changes (1 file, 1 line addition)
  • Commit messages are descriptive

PROCESS OBSERVATIONS

This is a straightforward infrastructure fix that gives CI pipelines a stable env var for cluster-local Harbor access, avoiding DERP relay hairpin. The mcd-tracker-api pipeline still hardcodes the external URL and could be migrated to use $HARBOR_REGISTRY_INTERNAL in a follow-up.

VERDICT: APPROVED

## PR #159 Review (Re-review) ### DOMAIN REVIEW **Tech stack**: OpenTofu / Helm provider / Kubernetes (Woodpecker CI agent configuration). The change adds a single environment variable `HARBOR_REGISTRY_INTERNAL` to the Woodpecker agent Helm values. The value `harbor.harbor.svc.cluster.local` is the Harbor nginx frontend service -- the correct endpoint for Docker push/pull within the cluster. **Validation against existing usage**: - `pal-e-docs/.woodpecker.yaml` line 54: `registry: harbor.harbor.svc.cluster.local` -- matches - `basketball-api/.woodpecker.yaml` line 44: `registry: harbor.harbor.svc.cluster.local` -- matches - `mcd-tracker-api/.woodpecker.yaml` line 34: still uses external URL `harbor.tail5b443a.ts.net` -- a candidate to consume this new env var The previous review flagged that `harbor-core.harbor.svc.cluster.local` was wrong for Docker push. The fix commit correctly changed it to `harbor.harbor.svc.cluster.local`. Note that `harbor-core` on line 430 is still correct in its own context -- that is the health check API endpoint, not the Docker registry frontend. These are distinct Harbor services. **Formatting**: The env var key alignment (padding with spaces) is consistent with the surrounding `WOODPECKER_BACKEND_*` entries. `tofu fmt` compliant. ### BLOCKERS None. ### NITS 1. **Stale PR description**: The Summary and Changes sections still reference `harbor-core.harbor.svc.cluster.local` from the original commit, but the actual code now uses `harbor.harbor.svc.cluster.local`. Non-blocking, but the description should be updated to avoid confusion for anyone reading the PR history. 2. **No `tofu plan` output**: The PR conventions in CLAUDE.md say to include `tofu plan` output for Terraform changes. The Test Plan has checkboxes for post-apply verification but no plan output. Non-blocking for a single env var addition. ### SOP COMPLIANCE - [x] Branch named after issue (`135-harbor-internal-url` references issue #135) - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] Related references closes #135 - [ ] Plan slug not applicable (bug fix, not plan work) - [x] No secrets committed (value is a cluster-internal DNS name, not a credential) - [x] No unnecessary file changes (1 file, 1 line addition) - [x] Commit messages are descriptive ### PROCESS OBSERVATIONS This is a straightforward infrastructure fix that gives CI pipelines a stable env var for cluster-local Harbor access, avoiding DERP relay hairpin. The `mcd-tracker-api` pipeline still hardcodes the external URL and could be migrated to use `$HARBOR_REGISTRY_INTERNAL` in a follow-up. ### VERDICT: APPROVED
forgejo_admin deleted branch 135-harbor-internal-url 2026-03-24 20:58:33 +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!159
No description provided.