deploy/chart: add static NetworkPolicies for CatalogSource gRPC ingress and bundle unpack egress#3863
Conversation
…ss and bundle unpack egress The Helm chart's default-deny-all-traffic policy blocked two critical traffic paths that are not covered by the existing static NetworkPolicies: 1. CatalogSource registry pods need to accept inbound gRPC connections on port 50051 from within the cluster. The catalog-operator reconciler already creates per-CatalogSource NetworkPolicies for this, but there is a bootstrapping gap between when default-deny-all-traffic is applied and when the controller first reconciles each CatalogSource. 2. Bundle-unpack Job pods need egress to reach the Kubernetes API server and container registries. The API server port is not statically specifiable because it varies across Kubernetes implementations, so a wildcard egress rule is used. Adds two new NetworkPolicies to the chart: - catalog-source-grpc-server: selects all pods carrying the olm.catalogSource label and allows ingress on the gRPC port. - bundle-unpack-egress: selects all pods carrying both the olm.managed=true and operatorframework.io/bundle-unpack-ref labels and allows unrestricted egress. Fixes: operator-framework#3676 Signed-off-by: grokspawn <jordan@nimblewidget.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR updates the OLM Helm chart’s static NetworkPolicy set to avoid connectivity failures caused by the chart’s default-deny-all-traffic policy during the bootstrapping window (before the catalog controller has reconciled per-CatalogSource policies). It specifically targets allowing gRPC ingress to CatalogSource registry pods and enabling bundle-unpack job egress.
Changes:
- Add
catalog-source-grpc-serverNetworkPolicyto allow ingress to pods labeledolm.catalogSourceon the configured gRPC port. - Add
bundle-unpack-egressNetworkPolicyto allow wildcard egress from bundle-unpack job pods (selected byolm.managed=trueandoperatorframework.io/bundle-unpack-ref).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ck NPs CatalogSource registry pods and bundle-unpack Jobs run in the namespace determined by .Values.catalog_namespace, not .Values.namespace. When a user overrides catalog_namespace to differ from namespace, the NetworkPolicies must be created in catalog_namespace to actually apply to those pods. Fixes review feedback on operator-framework#3863. Signed-off-by: grokspawn <jordan@nimblewidget.com>
perdasilva
left a comment
There was a problem hiding this comment.
Code review — 3 findings on the new NetworkPolicy templates.
| apiVersion: networking.k8s.io/v1 | ||
| kind: NetworkPolicy | ||
| metadata: | ||
| name: catalog-source-grpc-server |
There was a problem hiding this comment.
NP name collision risk
The dynamic NP uses fmt.Sprintf("%s-grpc-server", catalogSource.GetName()) (see pkg/controller/registry/reconciler/helpers.go:24). A CatalogSource named catalog-source in catalog_namespace would produce a dynamic NP named catalog-source-grpc-server — identical to this static Helm NP.
Since they have different specs (matchExpressions vs matchLabels, different labels/ownerReferences), the controller will overwrite the static NP on reconciliation and Helm will fight back on upgrade, causing reconciliation churn.
Consider prefixing the static NP name, e.g. olm-static-catalog-source-grpc-server or similar, to avoid any possible collision with the <name>-grpc-server naming pattern.
There was a problem hiding this comment.
The only way to truly de-fang this challenge is to use a unique construction, which would make an accidental overlap less likely but do nothing to prevent a bad actor from seeing the public construction details and manufacturing a conflict.
So how likely do we feel this would be? Do we think that we could dump the resources on the cluster and see that there's a catalogsource called 'catalog-source' and determine the network policies are impacting?
I don't want to underplay this, but I'd like to understand the goal here. Just using a more complex or static name does not avoid the issue.
| kind: NetworkPolicy | ||
| metadata: | ||
| name: catalog-source-grpc-server | ||
| namespace: {{ .Values.catalog_namespace }} |
There was a problem hiding this comment.
NPs become no-ops when catalog_namespace != namespace
The default-deny-all-traffic NP (line 5) is created only in {{ .Values.namespace }}. These new NPs target {{ .Values.catalog_namespace }}. When these values differ, Kubernetes allows all traffic by default in catalog_namespace — the allow-style NPs grant nothing because there is no deny-all baseline to punch through.
The bootstrap gap these NPs are meant to close remains open in split-namespace configurations.
Options:
- Add a
default-deny-all-trafficNP incatalog_namespacewhen it differs fromnamespace - Or document this as a known limitation
| egress: | ||
| - { } | ||
| --- | ||
| # Complements per-CatalogSource NPs from the controller; covers the bootstrapping window before reconciliation. |
There was a problem hiding this comment.
Static broad-selector NPs persist permanently and cannot be narrowed by the controller
This NP allows TCP ingress on catalogGrpcPodPort from any source (no from restriction) for all pods with the olm.catalogSource label (Exists match). Since Kubernetes NetworkPolicies are additive (union semantics), if the controller later tightens its per-CatalogSource NPs to restrict ingress to specific source pods (e.g., only from catalog-operator), this static NP silently defeats that restriction — and the controller has no code path to delete or update Helm-managed resources.
This is acceptable if the ingress source is intentionally unrestricted long-term. Worth noting as a design constraint.
Summary
Fixes #3676.
The Helm chart's
default-deny-all-trafficNetworkPolicy blocked two traffic paths not covered by the existing static policies:default-deny-all-traffictakes effect immediately on install/upgrade, while the per-CatalogSource policies only exist after the controller reconciles each object.Two new NetworkPolicies are added to the chart:
catalog-source-grpc-serverolm.catalogSourcelabel (any value)catalogGrpcPodPort(50051)bundle-unpack-egressolm.managed=true+operatorframework.io/bundle-unpack-reflabelThe
catalog-source-grpc-serverpolicy only listsIngressinpolicyTypes, so it does not restrict egress of catalog pods (that remains under the per-CatalogSource policies from the controller). Thebundle-unpack-egresspolicy only listsEgress, so it does not affect ingress on those pods.Test plan
quay.io/operatorhubio/catalog:latest; verifylastObservedStatereachesREADYBundleLookupFailedcatalog-source-grpc-serverandbundle-unpack-egressNetworkPolicies are present in the OLM namespace afterhelm install🤖 Generated with Claude Code