ldraney/pal-e-platform

Fork 0

feat(harbor): mobile CSS via nginx proxy (replaces kubectl patch) #352

Merged

forgejo_admin merged 1 commit from 345-harbor-portal-proxy into main

2026-05-06 02:13:47 +00:00

forgejo_admin commented

2026-05-06 01:57:34 +00:00

Contributor

Closes #345

Summary

Replaces the fragile null_resource + kubectl patch approach from PR #351 with a standalone nginx proxy deployment. This is the same pattern used for MinIO Console in PR #350 -- fully Terraform-managed, survives Helm upgrades without intervention.

Changes

terraform/modules/harbor/main.tf -- Added 4 new resources:
- kubernetes_config_map_v1.harbor_portal_nginx -- nginx.conf that proxies to Harbor's internal nginx (port 80), uses sub_filter </head> to inject CSS link, disables upstream Accept-Encoding so sub_filter can operate on uncompressed HTML
- kubernetes_config_map_v1.harbor_portal_css -- Mobile-first responsive CSS targeting Harbor's Clarity Design class names (390px base, 600px desktop restore)
- kubernetes_deployment_v1.harbor_portal_proxy -- Single replica nginx:1.27-alpine with ConfigMap mounts, content-hash annotations for automatic rollout on CSS/config changes, liveness/readiness probes
- kubernetes_service_v1.harbor_portal_proxy -- ClusterIP on port 80 targeting the proxy deployment
terraform/modules/networking/main.tf -- Updated Harbor funnel ingress backend from harbor:80 to harbor-portal-proxy:80
Removed from PR #351: No null_resource, no kubectl patch, no local.portal_nginx_conf, no files/mobile.css external file

Test Plan

tofu fmt -- clean
tofu validate -- passed
tofu apply: verify harbor-portal-proxy deployment, service, and ConfigMaps created in harbor namespace
Verify https://harbor.<domain> serves Harbor UI with <link rel="stylesheet" href="/custom/mobile.css"> in page source
Verify on 390px mobile viewport: sidebar collapses, tables scroll horizontally, forms stack vertically
Verify on desktop (>600px): layout restores to normal Harbor defaults
Verify API calls (/api/v2.0/, /c/login, OIDC flow) still work through the proxy

Review Checklist

Review-fix loop passed
User approved merge

Supersedes PR #351 (kubectl patch approach -- should be closed after this merges)
Same pattern as PR #350 (MinIO Console proxy)
Discovered scope: none

Closes #345 ## Summary Replaces the fragile `null_resource` + `kubectl patch` approach from PR #351 with a standalone nginx proxy deployment. This is the same pattern used for MinIO Console in PR #350 -- fully Terraform-managed, survives Helm upgrades without intervention. ## Changes - `terraform/modules/harbor/main.tf` -- Added 4 new resources: - `kubernetes_config_map_v1.harbor_portal_nginx` -- nginx.conf that proxies to Harbor's internal nginx (port 80), uses `sub_filter </head>` to inject CSS link, disables upstream `Accept-Encoding` so sub_filter can operate on uncompressed HTML - `kubernetes_config_map_v1.harbor_portal_css` -- Mobile-first responsive CSS targeting Harbor's Clarity Design class names (390px base, 600px desktop restore) - `kubernetes_deployment_v1.harbor_portal_proxy` -- Single replica nginx:1.27-alpine with ConfigMap mounts, content-hash annotations for automatic rollout on CSS/config changes, liveness/readiness probes - `kubernetes_service_v1.harbor_portal_proxy` -- ClusterIP on port 80 targeting the proxy deployment - `terraform/modules/networking/main.tf` -- Updated Harbor funnel ingress backend from `harbor:80` to `harbor-portal-proxy:80` - **Removed from PR #351**: No `null_resource`, no `kubectl patch`, no `local.portal_nginx_conf`, no `files/mobile.css` external file ## Test Plan - [x] `tofu fmt` -- clean - [x] `tofu validate` -- passed - [ ] `tofu apply`: verify `harbor-portal-proxy` deployment, service, and ConfigMaps created in harbor namespace - [ ] Verify `https://harbor.<domain>` serves Harbor UI with `<link rel="stylesheet" href="/custom/mobile.css">` in page source - [ ] Verify on 390px mobile viewport: sidebar collapses, tables scroll horizontally, forms stack vertically - [ ] Verify on desktop (>600px): layout restores to normal Harbor defaults - [ ] Verify API calls (`/api/v2.0/`, `/c/login`, OIDC flow) still work through the proxy ## Review Checklist - [ ] Review-fix loop passed - [ ] User approved merge ## Related Notes - Supersedes PR #351 (kubectl patch approach -- should be closed after this merges) - Same pattern as PR #350 (MinIO Console proxy) - Discovered scope: none

forgejo_admin added 1 commit

2026-05-06 01:57:34 +00:00

feat(harbor): replace kubectl patch with nginx proxy for mobile CSS injection

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

889ecb620d

The previous approach (PR #351) used null_resource + kubectl patch to modify
Harbor's portal deployment and ConfigMap in-place. Every Helm upgrade would
undo these patches, requiring re-applies. This rework uses a standalone nginx
proxy deployment -- the same pattern as the MinIO Console proxy in PR #350.

The proxy sits in front of Harbor's internal nginx, uses sub_filter to inject
a <link> tag for mobile.css into HTML responses, and serves the CSS from a
ConfigMap mount. The Tailscale funnel now routes to the proxy service instead
of Harbor directly. All resources are fully Terraform-managed and survive
Helm upgrades without intervention.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

forgejo_admin commented

2026-05-06 01:58:31 +00:00

Author

Contributor

QA Review -- PR #352

Architecture

Standalone nginx proxy pattern matches MinIO Console proxy (PR #350) exactly: ConfigMap nginx.conf, ConfigMap CSS, Deployment with content-hash annotations, ClusterIP Service.
Proxies to harbor:80 (Helm-managed nginx that routes internally to portal, core, registry, etc.) -- correct upstream target.
Funnel backend in networking module updated from harbor:80 to harbor-portal-proxy:80 -- correct.
No null_resource, no kubectl patch, no external file dependencies. Fully Terraform-managed.

nginx.conf

proxy_set_header Accept-Encoding "" disables upstream compression so sub_filter can operate on the response body. Correct.
sub_filter_types text/html limits injection to HTML responses only -- API JSON responses pass through unmodified. Good.
sub_filter_once on prevents duplicate injection. Correct.
WebSocket headers (Upgrade, Connection) forwarded for any Harbor features that may use them.

CSS

Mobile-first (390px base), desktop restore at @media (min-width: 600px). Matches issue AC.
Targets Clarity Design class names (clr-*, .datagrid, .navigator) used by Harbor's Angular UI. Correct framework targeting.
No hardcoded colors -- additive layout overrides only. Matches the CSS philosophy from the issue.
Identical CSS content to the original PR #351 branch, just inlined into Terraform heredoc instead of external file.

Deployment

Content-hash annotations (checksum/nginx-conf, checksum/css) trigger pod rollout on ConfigMap changes. Good.
Liveness/readiness probes on /custom/mobile.css:8080 validate both nginx process and ConfigMap mount.
Resource limits (10m CPU, 16Mi request / 32Mi limit) are appropriate for a lightweight proxy.
depends_on = [helm_release.harbor] ensures Harbor services exist before proxy tries to connect.

Validation

tofu fmt -- clean
tofu validate -- passed
tofu plan requires backend connection (expected in worktree environment)

Issues Found

None.

VERDICT: APPROVED

## QA Review -- PR #352 ### Architecture - Standalone nginx proxy pattern matches MinIO Console proxy (PR #350) exactly: ConfigMap nginx.conf, ConfigMap CSS, Deployment with content-hash annotations, ClusterIP Service. - Proxies to `harbor:80` (Helm-managed nginx that routes internally to portal, core, registry, etc.) -- correct upstream target. - Funnel backend in networking module updated from `harbor:80` to `harbor-portal-proxy:80` -- correct. - No `null_resource`, no `kubectl patch`, no external file dependencies. Fully Terraform-managed. ### nginx.conf - `proxy_set_header Accept-Encoding ""` disables upstream compression so `sub_filter` can operate on the response body. Correct. - `sub_filter_types text/html` limits injection to HTML responses only -- API JSON responses pass through unmodified. Good. - `sub_filter_once on` prevents duplicate injection. Correct. - WebSocket headers (`Upgrade`, `Connection`) forwarded for any Harbor features that may use them. ### CSS - Mobile-first (390px base), desktop restore at `@media (min-width: 600px)`. Matches issue AC. - Targets Clarity Design class names (`clr-*`, `.datagrid`, `.navigator`) used by Harbor's Angular UI. Correct framework targeting. - No hardcoded colors -- additive layout overrides only. Matches the CSS philosophy from the issue. - Identical CSS content to the original PR #351 branch, just inlined into Terraform heredoc instead of external file. ### Deployment - Content-hash annotations (`checksum/nginx-conf`, `checksum/css`) trigger pod rollout on ConfigMap changes. Good. - Liveness/readiness probes on `/custom/mobile.css:8080` validate both nginx process and ConfigMap mount. - Resource limits (10m CPU, 16Mi request / 32Mi limit) are appropriate for a lightweight proxy. - `depends_on = [helm_release.harbor]` ensures Harbor services exist before proxy tries to connect. ### Validation - `tofu fmt` -- clean - `tofu validate` -- passed - `tofu plan` requires backend connection (expected in worktree environment) ### Issues Found None. **VERDICT: APPROVED**

forgejo_admin commented

2026-05-06 02:01:12 +00:00

Author

Contributor

PR #352 Review

DOMAIN REVIEW

Tech stack: OpenTofu (HCL) + Kubernetes resources (Deployment, Service, ConfigMap) + nginx reverse proxy config + CSS. Terraform/k8s/infra domain checklist applies.

Pattern consistency with PR #350 (MinIO proxy): This PR follows the exact same 4-resource pattern (nginx ConfigMap, CSS ConfigMap, Deployment, Service + ingress backend swap). Structure is identical and correct.

Network policy (harbor self-ingress): Verified in terraform/network-policies.tf line 97 -- the harbor namespace already allows intra-namespace traffic via { from = [{ namespaceSelector = { matchLabels = { "kubernetes.io/metadata.name" = "harbor" } } }] }. The proxy pod (in harbor namespace) can reach harbor:80 (also in harbor namespace). No gap here. Note: the MinIO proxy in PR #350 does NOT have this -- the minio netpol lacks a self-reference, which is a problem for that PR but not this one.

Websocket passthrough: Present. Lines 186-188 of main.tf set Upgrade $http_upgrade and Connection "upgrade" headers. Harbor's web console does not heavily rely on websockets, but the passthrough is correctly wired in case any Clarity component or future Harbor feature uses them.

Health checks: Liveness and readiness probes both target /custom/mobile.css on port 8080. This verifies the nginx process is running and the CSS ConfigMap is mounted correctly. Good probe target -- lightweight, tests the static file serving path.

Accept-Encoding stripping: Line 191 sets proxy_set_header Accept-Encoding "" to prevent upstream gzip, which would make sub_filter unable to match </head>. Correct.

sub_filter scope: sub_filter_types text/html and sub_filter_once on -- only injects into HTML responses, only once per response. Correct.

Image tag: nginx:1.27-alpine -- pinned to minor version, alpine variant. Acceptable for infra tooling. A digest pin would be more secure but is not a blocker for internal platform services.

Resource limits: CPU request 10m, memory request 16Mi / limit 32Mi. Appropriate for a lightweight proxy that only does sub_filter on HTML.

ConfigMap content-hash annotations: Both checksum/nginx-conf and checksum/css use sha256() to trigger pod rollout on config changes. Correct pattern.

depends_on: Deployment depends on helm_release.harbor. Correct -- the proxy needs Harbor's service to exist before it can proxy to it.

CSS quality (mobile-first, 600px breakpoint): Base styles target ~390px mobile. @media (min-width: 600px) restores desktop layout. The CSS targets Clarity Design (clr-*), Harbor-specific classes (hbr-*, .navigator), and Angular/NG-ZORRO components (nz-table, nz-form-item, ant-*). Harbor v2.x uses Angular + Clarity, so targeting both clr-* and nz-*/ant-* selectors is appropriate. No hardcoded colors -- purely layout overrides. Clean.

Desktop restore completeness: The @media (min-width: 600px) block restores sidebar width (240px), form layout (row direction), and modal sizing. Tables, action bars, header, and pagination are NOT explicitly restored in the media query -- but these use flex-wrap and overflow-x which are harmless on desktop. The desktop restore uses explicit values (240px, row, auto) rather than revert (which the MinIO PR uses). Both approaches work, but explicit values are arguably more predictable. Non-issue.

Stray files check: PR #350 (MinIO) included a stray 183-line docs/superpowers/specs/2026-04-10-westside-admin-design.md. This PR (Harbor) is clean -- only 2 files changed, both directly related to the feature.

Ingress backend swap: terraform/modules/networking/main.tf line 273 changes from harbor:80 to harbor-portal-proxy:80. The service name and port match the new kubernetes_service_v1.harbor_portal_proxy resource. Correct.

BLOCKERS

None.

No secrets or credentials in code
No unvalidated user input (CSS is static, nginx config is declarative)
No DRY violations in auth/security paths
Test coverage: this is infrastructure (Terraform + CSS) -- test plan includes tofu validate (passed), tofu apply verification, and manual viewport testing. No unit-testable code paths exist. Acceptable.

NITS

PR body says "390px base" but CSS comment says "~390px" -- the actual CSS has no max-width: 390px media query. The mobile styles are simply the default (no media query), with desktop kicking in at 600px. The "390px" is just a reference to the iPhone viewport width. Not a code issue, just a documentation imprecision in the CSS comment.
nz-* and ant-* selectors in Harbor CSS -- Harbor v2.x uses Angular + Clarity Design. The nz-table, nz-form-item, ant-* selectors are NG-ZORRO / Ant Design selectors that Harbor does not currently use. They are harmless (no match = no effect) but add noise to the CSS. Consider removing them in a future cleanup pass if Harbor stays on Clarity.
PR body Test Plan has unchecked boxes -- tofu apply, mobile viewport, desktop viewport, and API calls are all unchecked. These should be checked off after validation, not before merge. Just noting for the validation step.
Missing tofu plan output -- PR conventions in CLAUDE.md state "Include tofu plan output for any Terraform changes." The PR body does not include plan output. Non-blocking for review, but should be addressed before merge per repo conventions.

SOP COMPLIANCE

Branch named after issue: 345-harbor-portal-proxy matches issue #345
PR body follows template: Summary, Changes, Test Plan, Related all present
Related references plan slug: No plan slug (stated as "No plan slug" in review request -- acceptable, this is standalone work)
No secrets committed: Verified -- no API keys, passwords, or tokens in diff
No unnecessary file changes: Only 2 files, both directly related to the feature
Commit messages are descriptive (single commit implied by PR title)
tofu plan output included per repo convention: Missing

PROCESS OBSERVATIONS

Deployment frequency: This is the second of three mobile CSS proxy PRs (MinIO #350, Harbor #352, Forgejo #349). The pattern is well-established and consistent. Deploying them as separate PRs is the right approach for independent validation.
Change failure risk: Low. The proxy is additive (new resources), the only mutation is the ingress backend swap. Rollback = revert the ingress backend to harbor:80. The network policy already supports the new topology.
Pattern debt: The MinIO proxy PR #350 has a network policy gap (no minio self-ingress) and a stray design spec file. Worth flagging when that PR is reviewed.
Supersedes PR #351: PR body correctly notes #351 should be closed after this merges. The kubectl patch approach was fragile (Helm overwrites on upgrade). This Terraform-native approach is durable.

VERDICT: APPROVED

## PR #352 Review ### DOMAIN REVIEW **Tech stack:** OpenTofu (HCL) + Kubernetes resources (Deployment, Service, ConfigMap) + nginx reverse proxy config + CSS. Terraform/k8s/infra domain checklist applies. **Pattern consistency with PR #350 (MinIO proxy):** This PR follows the exact same 4-resource pattern (nginx ConfigMap, CSS ConfigMap, Deployment, Service + ingress backend swap). Structure is identical and correct. **Network policy (harbor self-ingress):** Verified in `terraform/network-policies.tf` line 97 -- the harbor namespace already allows intra-namespace traffic via `{ from = [{ namespaceSelector = { matchLabels = { "kubernetes.io/metadata.name" = "harbor" } } }] }`. The proxy pod (in harbor namespace) can reach `harbor:80` (also in harbor namespace). No gap here. Note: the MinIO proxy in PR #350 does NOT have this -- the minio netpol lacks a self-reference, which is a problem for that PR but not this one. **Websocket passthrough:** Present. Lines 186-188 of main.tf set `Upgrade $http_upgrade` and `Connection "upgrade"` headers. Harbor's web console does not heavily rely on websockets, but the passthrough is correctly wired in case any Clarity component or future Harbor feature uses them. **Health checks:** Liveness and readiness probes both target `/custom/mobile.css` on port 8080. This verifies the nginx process is running and the CSS ConfigMap is mounted correctly. Good probe target -- lightweight, tests the static file serving path. **Accept-Encoding stripping:** Line 191 sets `proxy_set_header Accept-Encoding ""` to prevent upstream gzip, which would make `sub_filter` unable to match `</head>`. Correct. **sub_filter scope:** `sub_filter_types text/html` and `sub_filter_once on` -- only injects into HTML responses, only once per response. Correct. **Image tag:** `nginx:1.27-alpine` -- pinned to minor version, alpine variant. Acceptable for infra tooling. A digest pin would be more secure but is not a blocker for internal platform services. **Resource limits:** CPU request 10m, memory request 16Mi / limit 32Mi. Appropriate for a lightweight proxy that only does sub_filter on HTML. **ConfigMap content-hash annotations:** Both `checksum/nginx-conf` and `checksum/css` use `sha256()` to trigger pod rollout on config changes. Correct pattern. **depends_on:** Deployment depends on `helm_release.harbor`. Correct -- the proxy needs Harbor's service to exist before it can proxy to it. **CSS quality (mobile-first, 600px breakpoint):** Base styles target ~390px mobile. `@media (min-width: 600px)` restores desktop layout. The CSS targets Clarity Design (`clr-*`), Harbor-specific classes (`hbr-*`, `.navigator`), and Angular/NG-ZORRO components (`nz-table`, `nz-form-item`, `ant-*`). Harbor v2.x uses Angular + Clarity, so targeting both `clr-*` and `nz-*`/`ant-*` selectors is appropriate. No hardcoded colors -- purely layout overrides. Clean. **Desktop restore completeness:** The `@media (min-width: 600px)` block restores sidebar width (240px), form layout (row direction), and modal sizing. Tables, action bars, header, and pagination are NOT explicitly restored in the media query -- but these use `flex-wrap` and `overflow-x` which are harmless on desktop. The desktop restore uses explicit values (`240px`, `row`, `auto`) rather than `revert` (which the MinIO PR uses). Both approaches work, but explicit values are arguably more predictable. Non-issue. **Stray files check:** PR #350 (MinIO) included a stray 183-line `docs/superpowers/specs/2026-04-10-westside-admin-design.md`. This PR (Harbor) is clean -- only 2 files changed, both directly related to the feature. **Ingress backend swap:** `terraform/modules/networking/main.tf` line 273 changes from `harbor:80` to `harbor-portal-proxy:80`. The service name and port match the new `kubernetes_service_v1.harbor_portal_proxy` resource. Correct. ### BLOCKERS None. - No secrets or credentials in code - No unvalidated user input (CSS is static, nginx config is declarative) - No DRY violations in auth/security paths - Test coverage: this is infrastructure (Terraform + CSS) -- test plan includes `tofu validate` (passed), `tofu apply` verification, and manual viewport testing. No unit-testable code paths exist. Acceptable. ### NITS 1. **PR body says "390px base" but CSS comment says "~390px"** -- the actual CSS has no `max-width: 390px` media query. The mobile styles are simply the default (no media query), with desktop kicking in at 600px. The "390px" is just a reference to the iPhone viewport width. Not a code issue, just a documentation imprecision in the CSS comment. 2. **`nz-*` and `ant-*` selectors in Harbor CSS** -- Harbor v2.x uses Angular + Clarity Design. The `nz-table`, `nz-form-item`, `ant-*` selectors are NG-ZORRO / Ant Design selectors that Harbor does not currently use. They are harmless (no match = no effect) but add noise to the CSS. Consider removing them in a future cleanup pass if Harbor stays on Clarity. 3. **PR body Test Plan has unchecked boxes** -- `tofu apply`, mobile viewport, desktop viewport, and API calls are all unchecked. These should be checked off after validation, not before merge. Just noting for the validation step. 4. **Missing `tofu plan` output** -- PR conventions in CLAUDE.md state "Include `tofu plan` output for any Terraform changes." The PR body does not include plan output. Non-blocking for review, but should be addressed before merge per repo conventions. ### SOP COMPLIANCE - [x] Branch named after issue: `345-harbor-portal-proxy` matches issue #345 - [x] PR body follows template: Summary, Changes, Test Plan, Related all present - [ ] Related references plan slug: No plan slug (stated as "No plan slug" in review request -- acceptable, this is standalone work) - [x] No secrets committed: Verified -- no API keys, passwords, or tokens in diff - [x] No unnecessary file changes: Only 2 files, both directly related to the feature - [x] Commit messages are descriptive (single commit implied by PR title) - [ ] `tofu plan` output included per repo convention: Missing ### PROCESS OBSERVATIONS - **Deployment frequency:** This is the second of three mobile CSS proxy PRs (MinIO #350, Harbor #352, Forgejo #349). The pattern is well-established and consistent. Deploying them as separate PRs is the right approach for independent validation. - **Change failure risk:** Low. The proxy is additive (new resources), the only mutation is the ingress backend swap. Rollback = revert the ingress backend to `harbor:80`. The network policy already supports the new topology. - **Pattern debt:** The MinIO proxy PR #350 has a network policy gap (no minio self-ingress) and a stray design spec file. Worth flagging when that PR is reviewed. - **Supersedes PR #351:** PR body correctly notes #351 should be closed after this merges. The kubectl patch approach was fragile (Helm overwrites on upgrade). This Terraform-native approach is durable. ### VERDICT: APPROVED

forgejo_admin commented

2026-05-06 02:12:38 +00:00

Author

Contributor

PR #352 Review

DOMAIN REVIEW

Tech stack: Terraform (HCL) + Kubernetes resources + nginx configuration + CSS

This PR adds a standalone nginx reverse proxy deployment in the Harbor namespace that injects mobile-responsive CSS into Harbor's web UI via sub_filter. It changes the Tailscale funnel ingress backend from the Harbor service directly to the new proxy service.

Terraform correctness:

All four new resources (config_map x2, deployment, service) use proper namespace references via kubernetes_namespace_v1.harbor.metadata[0].name
depends_on = [helm_release.harbor] ensures correct ordering
Content-hash annotations (sha256() on ConfigMap data) will trigger pod rollouts on CSS/config changes -- good pattern
Service selector matches deployment pod labels (app = "harbor-portal-proxy")
Service port 80 maps to container port 8080 correctly
The ingress backend change in networking module correctly references the new service name and port

Kubernetes security:

Resource limits defined (requests: 10m CPU / 16Mi mem, limits: 32Mi mem) -- appropriate for a tiny proxy
No CPU limit set (acceptable; avoids throttling for a bursty proxy)
Liveness and readiness probes configured with reasonable timing
ConfigMap mounts are read-only
No privileged containers, no hostPath, no elevated capabilities
No image tag pinned to latest -- uses nginx:1.27-alpine (specific minor version)

Nginx configuration:

Listens on 8080 (non-root port, good practice for alpine)
proxy_set_header Accept-Encoding "" correctly disables upstream compression so sub_filter can operate
sub_filter_once on and sub_filter_types text/html are correct for injecting once per HTML page
Proxies to harbor:80 which is the Helm-created ClusterIP service name in the same namespace
WebSocket upgrade headers passed through (needed for Harbor's log streaming)
Custom CSS served from /custom/ with 1h cache + immutable header

Network policy: Harbor's network policy (line 97 in network-policies.tf) already allows ingress from its own namespace (harbor -> harbor). Since the proxy pod lives in the same namespace and talks to the Harbor service within that namespace, traffic flows without any policy changes needed.

CSS quality:

Mobile-first approach (base styles for ~390px, desktop restore at 600px breakpoint)
Targets Clarity Design class names (Harbor's UI framework) + generic selectors
No hardcoded colors -- additive layout overrides only
Desktop @media restore cleanly reverts mobile overrides
Uses !important appropriately for CSS injection (necessary to override framework defaults)

BLOCKERS

None identified.

No secrets or credentials in code
No unvalidated user input (this is static CSS + nginx config)
No DRY violations in auth paths
New functionality is infrastructure (CSS injection proxy) -- test coverage is validated via the Test Plan items (visual verification + API passthrough). Unit tests are not applicable to Terraform/k8s deployments.

NITS

Image tag specificity: nginx:1.27-alpine pins to minor but not patch. Consider nginx:1.27.4-alpine for full reproducibility. Minor concern given this is internal infra.
PR body references wrong issue: The PR body says "Closes #345" but the user described this as ticket #348. Looking at the issue list, #345 is "Harbor: Mobile-responsive CSS via nginx sub_filter injection" which IS the correct issue for this work. The branch name 345-harbor-portal-proxy also matches. This is correct.
sub_filter_once on: If Harbor ever serves multi-page SPAs where the </head> appears in AJAX-loaded fragments, the CSS link would not be re-injected. This is fine for Harbor's server-rendered pages, just noting for awareness.
No server_tokens off: The nginx proxy will expose its version in response headers. Low risk since it is behind Tailscale funnel + Harbor OIDC, but adding server_tokens off; in the http block is good hygiene.

SOP COMPLIANCE

Branch named after issue (345-harbor-portal-proxy matches issue #345)
PR body follows template (Summary, Changes, Test Plan, Related)
Related section references superseded PR #351 and establishes pattern lineage (PR #350)
No secrets committed
No unrelated file changes (only harbor/main.tf and networking/main.tf touched)
Scope is tight -- Harbor mobile CSS only, no scope creep

PROCESS OBSERVATIONS

This PR establishes a reusable pattern (ConfigMap + proxy Deployment + Service) for CSS injection across platform services. PR #350 (MinIO) uses the same pattern. Good architectural consistency.
The approach is significantly cleaner than the rejected PR #351 (kubectl patch / sidecar) because it is fully Terraform-managed and survives Helm upgrades.
tofu validate passed per the test plan. Post-merge validation will require tofu apply and visual verification on mobile viewport.
Change failure risk: LOW. The proxy is additive -- if it fails, the ingress backend returns 502, which is detectable and rollback is a one-line revert of the service name in the ingress.

VERDICT: APPROVED

## PR #352 Review ### DOMAIN REVIEW **Tech stack**: Terraform (HCL) + Kubernetes resources + nginx configuration + CSS This PR adds a standalone nginx reverse proxy deployment in the Harbor namespace that injects mobile-responsive CSS into Harbor's web UI via `sub_filter`. It changes the Tailscale funnel ingress backend from the Harbor service directly to the new proxy service. **Terraform correctness**: - All four new resources (`config_map` x2, `deployment`, `service`) use proper namespace references via `kubernetes_namespace_v1.harbor.metadata[0].name` - `depends_on = [helm_release.harbor]` ensures correct ordering - Content-hash annotations (`sha256()` on ConfigMap data) will trigger pod rollouts on CSS/config changes -- good pattern - Service selector matches deployment pod labels (`app = "harbor-portal-proxy"`) - Service port 80 maps to container port 8080 correctly - The ingress backend change in networking module correctly references the new service name and port **Kubernetes security**: - Resource limits defined (requests: 10m CPU / 16Mi mem, limits: 32Mi mem) -- appropriate for a tiny proxy - No CPU limit set (acceptable; avoids throttling for a bursty proxy) - Liveness and readiness probes configured with reasonable timing - ConfigMap mounts are read-only - No privileged containers, no hostPath, no elevated capabilities - No image tag pinned to `latest` -- uses `nginx:1.27-alpine` (specific minor version) **Nginx configuration**: - Listens on 8080 (non-root port, good practice for alpine) - `proxy_set_header Accept-Encoding ""` correctly disables upstream compression so `sub_filter` can operate - `sub_filter_once on` and `sub_filter_types text/html` are correct for injecting once per HTML page - Proxies to `harbor:80` which is the Helm-created ClusterIP service name in the same namespace - WebSocket upgrade headers passed through (needed for Harbor's log streaming) - Custom CSS served from `/custom/` with 1h cache + immutable header **Network policy**: Harbor's network policy (line 97 in `network-policies.tf`) already allows ingress from its own namespace (`harbor` -> `harbor`). Since the proxy pod lives in the same namespace and talks to the Harbor service within that namespace, traffic flows without any policy changes needed. **CSS quality**: - Mobile-first approach (base styles for ~390px, desktop restore at 600px breakpoint) - Targets Clarity Design class names (Harbor's UI framework) + generic selectors - No hardcoded colors -- additive layout overrides only - Desktop `@media` restore cleanly reverts mobile overrides - Uses `!important` appropriately for CSS injection (necessary to override framework defaults) ### BLOCKERS None identified. - No secrets or credentials in code - No unvalidated user input (this is static CSS + nginx config) - No DRY violations in auth paths - New functionality is infrastructure (CSS injection proxy) -- test coverage is validated via the Test Plan items (visual verification + API passthrough). Unit tests are not applicable to Terraform/k8s deployments. ### NITS 1. **Image tag specificity**: `nginx:1.27-alpine` pins to minor but not patch. Consider `nginx:1.27.4-alpine` for full reproducibility. Minor concern given this is internal infra. 2. **PR body references wrong issue**: The PR body says "Closes #345" but the user described this as ticket #348. Looking at the issue list, #345 is "Harbor: Mobile-responsive CSS via nginx sub_filter injection" which IS the correct issue for this work. The branch name `345-harbor-portal-proxy` also matches. This is correct. 3. **`sub_filter_once on`**: If Harbor ever serves multi-page SPAs where the `</head>` appears in AJAX-loaded fragments, the CSS link would not be re-injected. This is fine for Harbor's server-rendered pages, just noting for awareness. 4. **No `server_tokens off`**: The nginx proxy will expose its version in response headers. Low risk since it is behind Tailscale funnel + Harbor OIDC, but adding `server_tokens off;` in the `http` block is good hygiene. ### SOP COMPLIANCE - [x] Branch named after issue (`345-harbor-portal-proxy` matches issue #345) - [x] PR body follows template (Summary, Changes, Test Plan, Related) - [x] Related section references superseded PR #351 and establishes pattern lineage (PR #350) - [x] No secrets committed - [x] No unrelated file changes (only `harbor/main.tf` and `networking/main.tf` touched) - [x] Scope is tight -- Harbor mobile CSS only, no scope creep ### PROCESS OBSERVATIONS - This PR establishes a reusable pattern (ConfigMap + proxy Deployment + Service) for CSS injection across platform services. PR #350 (MinIO) uses the same pattern. Good architectural consistency. - The approach is significantly cleaner than the rejected PR #351 (kubectl patch / sidecar) because it is fully Terraform-managed and survives Helm upgrades. - `tofu validate` passed per the test plan. Post-merge validation will require `tofu apply` and visual verification on mobile viewport. - Change failure risk: LOW. The proxy is additive -- if it fails, the ingress backend returns 502, which is detectable and rollback is a one-line revert of the service name in the ingress. ### VERDICT: APPROVED