Skip to content

fix(openshift): apply route IP allowlist to production only - #226

Open
alex-struk wants to merge 5 commits into
developfrom
fix/route-ip-whitelist-bcgov-range
Open

fix(openshift): apply route IP allowlist to production only#226
alex-struk wants to merge 5 commits into
developfrom
fix/route-ip-whitelist-bcgov-range

Conversation

@alex-struk

@alex-struk alex-struk commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Intent

Source-IP restrictions should apply to production only. Ephemeral PR, test, and dev instances should stay open (reachable without VPN). The original design tried to do this with overlays/prod, but that overlay is never applied — so prod got the wrong (base) value and the restriction leaked to every environment.

Root cause (why the old overlay never worked)

Prod and test are deployed by the Deploy Instance workflow (.github/workflows/deploy-instance.yml), which renders from overlays/instance-template via scripts/lib/generate-overlay.sh and runs kustomize build | oc apply. instance-template inherits base; nothing applies overlays/{prod,test,dev}. So every deploy used the base allowlist (142.16.0.0/11), which also happened to exclude 142.32.0.0/16–142.36.0.0/16 (a coworker on 142.32.84.136 got HTTP 403).

Change

Hook the restriction into the real deploy path using the existing prod-only component mechanism (the same one that raises prod memory limits / PVC sizes):

  • base/*/route.yml — remove the IP allowlist entirely. Base is now open, so test/dev/PR instances need no VPN. Backend keeps the /metrics deny-list.
  • components/prod-resources/route-allowlist-{backend,frontend}.yml (new) — apply haproxy.router.openshift.io/ip_whitelist = 142.22.0.0/16–142.36.0.0/16 (BC Gov ARIN PBC-51-Z; covers VPN egress + Silver/Gold/Gold-DR NAT pools in 142.34.0.0/16). generate-overlay.sh auto-includes this component only for *-prod namespaces.
  • Removed the dead overlays/{prod,test}/route-allowlist-patch.yml and their kustomization refs.
  • Docs updated.

Annotation key: ip_allowlist

Uses haproxy.router.openshift.io/ip_allowlist, the inclusive-naming rename adopted in PR #218 / AI-1341. The base routes carry no allowlist at all, so the enforcement annotation exists only in components/prod-resources (prod-only).

Verification (kustomize build)

Render Result
base no allowlist (open) ✅
fd34fb-prod (via generate-overlay) ip_whitelist on both bcgov-di-{backend-services,frontend}
fd34fb-test (via generate-overlay) no allowlist (open) ✅
fd34fb-prod memory/PVC patches memory: 2Gi, storage: 5Gi still applied ✅

Operational notes

  • Live prod was hotfixed via oc annotate (ip_whitelist, equivalent range) and is currently restricted correctly. Once this reaches main, a prod deploy sets the same key/value from the component.
  • Live test may still carry a stale allowlist annotation from earlier deploys. If a redeploy from develop doesn't prune it, clear it once with:
    oc annotate route <name> haproxy.router.openshift.io/ip_whitelist- haproxy.router.openshift.io/ip_allowlist- -n fd34fb-test

🤖 Generated with Claude Code

alex-struk and others added 2 commits July 2, 2026 15:04
The base Route allowlist used 142.16.0.0/11, which a /11 mask expands to
142.0.0.0–142.31.255.255. That silently excluded BC Gov clients in
142.32.0.0/16–142.36.0.0/16 (e.g. a VPN client on 142.32.84.136 got HTTP 403
at the router despite being on a legitimate BC Gov network).

Replace it with the documented BC Gov public 142.x allocation (ARIN org
PBC-51-Z): 142.22.0.0/16–142.36.0.0/16. This covers VPN egress and the
Silver/Gold/Gold-DR NAT pools (all within 142.34.0.0/16), so the previously
enumerated 142.34.* NAT IPs are now redundant and dropped.

Move the allowlist to a single source of truth in base/ so every environment
(dev, test, prod, instance-template) inherits the correct value, and remove the
now-redundant per-overlay route-allowlist-patch.yml files from prod and test.

Docs: update KUSTOMIZE_INSTANCE_TEMPLATE.md to describe the corrected range.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reworks the previous approach: instead of restricting every environment via a
base allowlist, restrictions now apply to production only, matching the intent
that test/dev/PR instances stay open (reachable without VPN).

- base/{backend-services,frontend}/route.yml: remove the ip allowlist entirely
  so base (inherited by all instance-template deployments) is open. Backend keeps
  the /metrics deny-list.
- components/prod-resources: add route-allowlist-{backend,frontend}.yml applying
  haproxy.router.openshift.io/ip_whitelist = 142.22.0.0/16-142.36.0.0/16 (BC Gov
  ARIN PBC-51-Z). This component is auto-included by scripts/lib/generate-overlay.sh
  only for *-prod namespaces, so it hooks into the real deploy path (Deploy Instance
  workflow -> instance-template) rather than the dead overlays/prod.
- Use the ip_whitelist key (not ip_allowlist): ip_whitelist is verified to enforce
  on Silver's OpenShift 4.18 router; ip_allowlist support is unconfirmed and using
  an ignored key on prod would silently drop the restriction.

Verified via kustomize build: base open; fd34fb-prod renders ip_whitelist on both
prefixed routes; fd34fb-test renders no allowlist; prod memory/PVC patches intact.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@alex-struk alex-struk changed the title fix(openshift): correct route ip_allowlist to full BC Gov 142.x range fix(openshift): apply route IP allowlist to production only Jul 2, 2026
alex-struk and others added 2 commits July 2, 2026 16:06
…ction

Adopt the ip_allowlist annotation name (renamed from ip_whitelist in PR #218 /
AI-1341 for inclusive naming) for the production-only route restriction added by
components/prod-resources, and align the base-route comments and docs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
overlays/{prod,test,dev} are not part of any deploy path — the Deploy Instance
workflow renders from overlays/instance-template. They were stale/misleading
(their route patches never reached any cluster). Remove them and repoint the
README's OpenShift deploy snippet at the real mechanism (Deploy Instance /
instance-template).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@alex-struk
alex-struk marked this pull request as ready for review July 2, 2026 23:14

@dbarkowsky dbarkowsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to open the IP ranges that wide?
The previous OpenShift egress IPs came from this resource: https://digital.gov.bc.ca/technology/cloud/private/internal-resources/topology/
Can we confirm that the wider cidr notation still only encompasses BC Gov addresses?

Comment thread deployments/openshift/kustomize/overlays/prod/kustomization.yml
@alex-struk

alex-struk commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

Good question @dbarkowsky — I dug into this and the short version is the new notation is actually narrower and more precise than the old one, and it's provably all BC Gov.

Where the list comes from: the 15 blocks 142.22.0.0/16142.36.0.0/16 are exactly the 142.x allocation registered to the Province of BC (ARIN org PBC-51-Z). I confirmed it straight from ARIN (https://whois.arin.net/rest/org/PBC-51-Z/nets) — those 15 contiguous /16s are the entire 142.x space assigned to the province, nothing outside it. It's not one wide CIDR; it's 15 discrete /16 blocks that match the registry 1:1.

Why it's not wider than before: the old value was 142.16.0.0/11 + a handful of explicit 142.34 NAT /32s. /11 is misaligned and normalizes to 142.0.0.0142.31.255.255, which (a) over-allowed 142.0142.21 (not BC Gov) and (b) excluded 142.32142.36 (which are BC Gov — that's why a coworker on 142.32.84.136 was getting 403s, and why the 142.34 NAT IPs had to be hand-listed). By the numbers: old ≈ 2.1M addresses, new ≈ 983K. The new list drops the non-BC-Gov space and lines up exactly with the ARIN allocation.

The three cluster NAT/egress pools from the topology page (Silver 142.34.194.121–124, Gold 142.34.229.6–9, Gold-DR 142.34.64.6–9) all sit inside 142.34.0.0/16, so they're still fully covered — plus VPN egress — without needing to enumerate them by hand.

One honest caveat: this does admit BC Gov's whole 142.x allocation, not just our three NAT pools + VPN, so it's broader than the strict minimum. Everything in it is BC Gov, but it's whole-of-government rather than app-specific. I went this route because it's stable against NAT-pool changes and matches the openshift-wiki's documented approach; happy to tighten it back to VPN-range + the specific NAT /32s if you'd prefer the minimal surface.

Restore the fixed-overlay files removed earlier in this PR and comment them
out instead of deleting. Per review discussion with Kaegan, keep them as a
legible reference for a possible future hybrid deploy model (fixed overlays
for the stable test/prod envs alongside the instance-template script used for
ephemeral branch instances). Nothing in the pipeline builds these files; they
are inert. Decision tracked in AI-1687.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dbarkowsky

Copy link
Copy Markdown
Collaborator

Good question @dbarkowsky — I dug into this and the short version is the new notation is actually narrower and more precise than the old one, and it's provably all BC Gov.

Fair, I wasn't thinking of this /11 spread, just the exact IPs below.

image

Nice to confirm that we own that whole 142 range.
I would say to proceed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants