CONSOLE-5432: Add NetworkPolicy manifests for openshift-console-operator namespace - #1198
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@redhat-chai-bot: This pull request references CONSOLE-5432 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds three Kubernetes ChangesOperator network policies
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@manifests/03-networkpolicy-operator-allow-apiserver-egress.yaml`:
- Around line 19-22: Constrain the egress rule under the API-server allowlist to
Kubernetes API-server destinations by adding the appropriate supported IP/CIDR
or peer selector in its `to` clause. If supported OpenShift/HyperShift
topologies require destination-less TCP 6443 access, document that rationale in
the manifest instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 98b602f6-83ad-40f1-82af-4e661da20dbb
📒 Files selected for processing (4)
manifests/03-networkpolicy-operator-allow-apiserver-egress.yamlmanifests/03-networkpolicy-operator-allow-dns-egress.yamlmanifests/03-networkpolicy-operator-allow-metrics-ingress.yamlmanifests/03-networkpolicy-operator-default-deny.yaml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/console(manual)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{yaml,yml,json}
📄 CodeRabbit inference engine (Custom checks)
**/*.{yaml,yml,json}: Flag privileged: true, hostPID, hostNetwork, hostIPC, SYS_ADMIN capability, running as root without justification, and allowPrivilegeEscalation: true in container/Kubernetes manifests
When deployment manifests, operator code, or controllers are added/modified, ensure they do not introduce scheduling constraints assuming standard HA topology (3+ control-plane nodes, dedicated workers). Flag: required pod anti-affinity with maxUnavailable: 0 (deadlocks on SNO/TNF/TNA), pod topology spread with DoNotSchedule and hostname key (breaks on SNO), replica counts derived from node count without topology awareness, nodeSelector/node affinity targeting control-plane nodes (fails on HyperShift), scheduling to all control-plane nodes equally without excluding arbiter nodes (TNA), assuming dedicated worker nodes exist (SNO/TNF), or PodDisruptionBudgets designed for 3+ nodes (TNF/TNA). Do not flag if change checks ControlPlaneTopology, node counts, or topology labels before applying constraints.
Files:
manifests/03-networkpolicy-operator-allow-metrics-ingress.yamlmanifests/03-networkpolicy-operator-default-deny.yamlmanifests/03-networkpolicy-operator-allow-apiserver-egress.yamlmanifests/03-networkpolicy-operator-allow-dns-egress.yaml
{manifests,bindata/assets,quickstarts,examples,profile-patches}/**/*.{yaml,yml}
📄 CodeRabbit inference engine (.claude/skills/manifest-review.md)
{manifests,bindata/assets,quickstarts,examples,profile-patches}/**/*.{yaml,yml}: Kubernetes manifests undermanifests/,bindata/assets/,quickstarts/,examples/, andprofile-patches/must include the appropriate cluster profile annotations when they are CVO-deployed resources (for exampleinclude.release.openshift.io/hypershift,include.release.openshift.io/ibm-cloud-managed,include.release.openshift.io/self-managed-high-availability, andinclude.release.openshift.io/single-node-developer).
Console resources in the manifest trees must include the capability annotationcapability.openshift.io/name: Console.
RBAC manifests must follow least-privilege design: grant only the permissions needed, avoid wildcards unless truly justified, use verbs that match the actual operation set, and set correctapiGroups(""for core APIs and specific groups for CRDs).
Resources must use the correct namespace for their role:openshift-consolefor console workload resources andopenshift-console-operatorfor operator resources; any cross-namespace reference must be explicit and intentional.
YAML manifests should use 2-space indentation, keep fields in a consistent order, and include---separators between multiple resources in the same file.
When binding roles to service accounts, useconsole-operatorin theopenshift-console-operatornamespace for operator cross-namespace access, and useconsolein theopenshift-consolenamespace for console workload-scoped access.
Files:
manifests/03-networkpolicy-operator-allow-metrics-ingress.yamlmanifests/03-networkpolicy-operator-default-deny.yamlmanifests/03-networkpolicy-operator-allow-apiserver-egress.yamlmanifests/03-networkpolicy-operator-allow-dns-egress.yaml
**/*.yaml
⚙️ CodeRabbit configuration file
**/*.yaml: Review YAML manifests based on content and kind.Refer to /manifest-review when YAML contains:
kind: Roleorkind: ClusterRole(RBAC review)kind: RoleBindingorkind: ClusterRoleBindingannotations:section (check for cluster profiles)verbs: ["*"]or wildcard permissionsapiGroups: ["*"]or overly broad permissions- ServiceAccount references in subjects
Check for required annotations in manifests/:
include.release.openshift.io/hypershiftinclude.release.openshift.io/ibm-cloud-managedinclude.release.openshift.io/self-managed-high-availabilityinclude.release.openshift.io/single-node-developercapability.openshift.io/name: ConsoleFor quickstarts/, additionally check:
- QuickStart spec structure
- Task descriptions and prerequisites
- See quickstarts/README.md for guidelines
Files:
manifests/03-networkpolicy-operator-allow-metrics-ingress.yamlmanifests/03-networkpolicy-operator-default-deny.yamlmanifests/03-networkpolicy-operator-allow-apiserver-egress.yamlmanifests/03-networkpolicy-operator-allow-dns-egress.yaml
**/*.{yaml,yml}
⚙️ CodeRabbit configuration file
**/*.{yaml,yml}: If this is a Kubernetes/OpenShift manifest or Helm template:
- securityContext: runAsNonRoot, readOnlyRootFilesystem,
allowPrivilegeEscalation: false- Drop ALL capabilities, add only what is required
- Resource limits (cpu, memory) on every container
- No hostPID, hostNetwork, hostIPC, privileged: true
- NetworkPolicy defined for the namespace
- OpenShift: SCC must be restricted or custom-scoped
- Liveness + readiness probes defined
- automountServiceAccountToken: false unless needed
- RBAC: least privilege; no cluster-admin for workloads
- Helm: no .Values interpolation in shell commands
Files:
manifests/03-networkpolicy-operator-allow-metrics-ingress.yamlmanifests/03-networkpolicy-operator-default-deny.yamlmanifests/03-networkpolicy-operator-allow-apiserver-egress.yamlmanifests/03-networkpolicy-operator-allow-dns-egress.yaml
🔇 Additional comments (4)
manifests/03-networkpolicy-operator-default-deny.yaml (1)
1-21: LGTM!manifests/03-networkpolicy-operator-allow-apiserver-egress.yaml (1)
1-18: LGTM!Also applies to: 23-24
manifests/03-networkpolicy-operator-allow-dns-egress.yaml (1)
1-34: LGTM!manifests/03-networkpolicy-operator-allow-metrics-ingress.yaml (1)
1-28: LGTM!
4cfbb1d to
460ee58
Compare
| # The API server runs on the host network, not the pod network, | ||
| # so it cannot be targeted via namespaceSelector/podSelector. | ||
| # A static ipBlock CIDR is also not feasible because API server | ||
| # IPs vary per cluster and this is a CVO-managed static manifest. |
There was a problem hiding this comment.
instead of including context like this in a YAML comment only visible to folks with Git access, can you shift it into the kubernetes.io/description annotation? That way, the context is also available to curious cluster-admins in-cluster.
There was a problem hiding this comment.
Good call — shifting the rationale into kubernetes.io/description now so it's visible in-cluster. Removing the YAML comments.
There was a problem hiding this comment.
Addressed in 18cb6b1. This thread can be marked resolved, but I don't have access to do that myself.
There was a problem hiding this comment.
Resolved — thanks for confirming!
63898f1 to
18cb6b1
Compare
| DNS resolution needs. In the future, cluster-scoped | ||
| AdminNetworkPolicy may allow tighter restrictions. | ||
| spec: | ||
| podSelector: {} |
There was a problem hiding this comment.
Checking a 5.0 nightly CI run's gather-extra artifacts:
$ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/logs/periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn/2080425132433608704/artifacts/e2e-aws-ovn/gather-extra/artifacts/pods.json | jq -c '.items[].metadata | select(.namespace == "openshift-console-operator").labels'
{"name":"console-operator","pod-template-hash":"c58f98658","topology.kubernetes.io/region":"us-west-1","topology.kubernetes.io/zone":"us-west-1c"}so the operator is the only thing running, and that name=console-operator label is the pod-selector the Deployment uses. Can you use that podSelector here, to avoid granting broad egress access to any Pods besides the console operator?
There was a problem hiding this comment.
Good catch — podSelector: {} is needlessly broad when we know the only pod is the operator with name: console-operator. Updating both the egress and metrics-ingress policies to use podSelector: matchLabels: name: console-operator. The default-deny will keep podSelector: {} since that baseline should cover all pods in the namespace.
There was a problem hiding this comment.
ae86d33 adds a podSelector here. This thread can be marked resolved; although I am not authorized to do that myself.
| openshift-monitoring namespace. This enables Prometheus | ||
| to scrape operator metrics. | ||
| spec: | ||
| podSelector: {} |
There was a problem hiding this comment.
Same as egress, it's not secure to use an empty pod-selector when granting access. Can you scope this down to just the console operator too?
There was a problem hiding this comment.
Agreed — scoping down to name: console-operator on the metrics-ingress too. Pushing shortly.
| Default deny all ingress and egress traffic in the | ||
| openshift-console-operator namespace. The policy improves | ||
| network isolation by ensuring that the operator controller pod | ||
| only communicates through explicitly allowed policies. |
There was a problem hiding this comment.
no podSelector on this one, so should be `The policy improves network isolation by ensuring that pods only communicate through explicitly allowed policies.".
There was a problem hiding this comment.
Good catch — updating the description to use generic "pods" language since this policy covers all pods via podSelector: {}. Pushing now.
There was a problem hiding this comment.
Fixed in the latest force-push (fbce1df07) — the default-deny annotation now reads:
"Default deny all ingress and egress traffic in the openshift-console-operator namespace. The policy improves network isolation by ensuring that pods only communicate through explicitly allowed policies."
Generic "pods" language since podSelector: {} covers all pods in the namespace.
There was a problem hiding this comment.
18cb6b1 seems to have lost this reword. Can you try again?
There was a problem hiding this comment.
Looking at the file at ae86d330f, the default-deny annotation currently reads:
"Default deny all ingress and egress traffic in the openshift-console-operator namespace. The policy improves network isolation by ensuring that pods only communicate through explicitly allowed policies."
This already uses generic "pods" language (not "the console operator pod"). Could you double-check against the latest commit? The squash should have preserved the fbce1df07 fix. If it still looks wrong on your end I'll re-push.
There was a problem hiding this comment.
Sorry, I was looking at an old commit. ae86d33 has the rewording. This thread can be marked resolved; although I am not authorized to do that myself.
| needs Kubernetes API server access to watch and reconcile | ||
| console-related resources. Because the API server address and | ||
| port vary per cluster and the server runs on the host network, | ||
| they cannot be statically restricted in a CVO-managed |
There was a problem hiding this comment.
New administrators may not be familiar with the "CVO" acronym. Can you unpack it "...in a NetworkPolicy managed by the cluster-version operator."?
There was a problem hiding this comment.
Will do — expanding "CVO" to "cluster-version operator" in the annotation now.
There was a problem hiding this comment.
ae86d33 still contains the "CVO-managed" sentence. Can you try again to unpack to "...in a NetworkPolicy managed by the cluster-version operator."?
There was a problem hiding this comment.
You're right — I see it's still there. Fixing now, will amend and force-push.
There was a problem hiding this comment.
5fde96a has the rewording. This thread can be marked resolved, although I'm not authorized to do that myself.
There was a problem hiding this comment.
Fixed in 5fde96af7 — now reads "a NetworkPolicy managed by the cluster-version operator" instead of "CVO-managed".
| they cannot be statically restricted in a CVO-managed | ||
| NetworkPolicy. Opening all egress also covers DNS resolution | ||
| needs. In the future, cluster-scoped AdminNetworkPolicy may | ||
| allow tighter restrictions. |
There was a problem hiding this comment.
We don't have a feature-tracker linked for this. And unless we do have somewhere folks can subscribe fore updates, I'd avoid "In the future..." statements.
There was a problem hiding this comment.
Agreed — removing the "In the future…" sentence. The annotation will just explain why open egress is needed without speculating about future improvements.
There was a problem hiding this comment.
ae86d33 still contains the "In the future..." sentence. Can you try again to remove it?
There was a problem hiding this comment.
Still there — sorry about that. Removing it now and force-pushing.
There was a problem hiding this comment.
5fde96a has the rewording. This thread can be marked resolved, although I'm not authorized to do that myself.
There was a problem hiding this comment.
Fixed in 5fde96af7 — the "In the future..." sentence has been removed. The annotation now ends at "DNS resolution needs."
492e9f0 to
fbce1df
Compare
| @@ -0,0 +1,29 @@ | |||
| apiVersion: networking.k8s.io/v1 | |||
There was a problem hiding this comment.
Unrelated to this line, I see no need for two commits here. Can you squash 18cb6b1 and fbce1df07e6806c1efd4fe264cb74010c731c74da together, with a commit message that explains the consolidated change? You shouldn't need to go into much detail in the commit message, because the kubernetes.io/description annotations should bring most of the context.
There was a problem hiding this comment.
Will squash into a single commit with a concise message. Pushing shortly.
There was a problem hiding this comment.
ae86d33 squashed to one commit. This thread can be marked resolved, but I don't have authorization to do that myself.
ae86d33 to
5fde96a
Compare
| - from: | ||
| - namespaceSelector: | ||
| matchLabels: | ||
| kubernetes.io/metadata.name: openshift-monitoring |
There was a problem hiding this comment.
This limitation might be breaking e2e-aws-operator:
metrics_test.go:97: http error: Get "https://metrics-openshift-console-operator.apps.ci-op-f6j7t0d9-7ab6c.xxxxxxxxxxxxxxxxxxxxxx/metrics": EOF
metrics_test.go:116: metrics request failed after several attempts: Get "https://metrics-openshift-console-operator.apps.ci-op-f6j7t0d9-7ab6c.xxxxxxxxxxxxxxxxxxxxxx/metrics": EOF
metrics_test.go:46: waiting for cleanup to reach settled state...
--- FAIL: TestMetricsEndpoint (27.14s)
Not sure what to do about that. There's no status on NetworkPolicy to be able to tell if it's alive or having trouble or what. So a local test case could add an additional NetworkPolicy, then try to scrape metrics, and then remove its NetworkPolicy in post-test-case teardown. But there wouldn't be a way to gracefully wait for the NetworkPolicy to go live, you'd just be hammering away at the metrics endpoint until you got through.
Or we could drop the test, and assume TargetDown or other signs out of the monitoring side of things would catch us if we broke metrics serving.
There was a problem hiding this comment.
Agreed — the TestMetricsEndpoint failure is caused by this policy. The test accesses metrics through a Route, so traffic arrives from openshift-ingress router pods — which the metrics-ingress policy blocks (only openshift-monitoring is whitelisted).
Options as I see them:
-
Add
openshift-ingressto the metrics-ingressfromlist — simplest fix, but widens the ingress surface beyond what's needed for production (the Route exists only for testing; Prometheus scrapes directly via pod IP). -
Have the test create a temporary NetworkPolicy allowing ingress from
openshift-ingress, then clean up in teardown. As you noted, there's nostatuson NetworkPolicy so you'd need to poll/retry the metrics endpoint. -
Change the test to access metrics directly (e.g.
kubectl port-forwardor exec into a pod in the namespace) instead of going through a Route. This avoids needing any NetworkPolicy change. -
Drop the test and rely on
TargetDown/ monitoring alerts as you suggested.
Deferring to @soltysh on which approach fits best for the console operator's test philosophy. The GCP failure is unrelated infra (cluster bootstrap failure).
There was a problem hiding this comment.
Oh, actually, Developing Network Policies; Ingress to Metrics Endpoint currently says:
We allow anything to reach our metrics endpoint.
apiVersion: networking.k8s.io/v1 kind: NetworkPolicy metadata: name: allow-ingress-to-metrics spec: podSelector: matchLabels: # You need to define this based on what pods support metrics, or remove for all ingress: - ports: - protocol: TCP port: 12345 # Use the port number of your metrics endpoint policyTypes: - Ingress
so we can probably recover this test-case just by dropping the from here entirely.
There was a problem hiding this comment.
Perfect — much simpler. Dropping the from entirely and keeping just the ports restriction matches the guide's pattern and fixes the test in one shot. Pushing now.
5fde96a to
d07d8ca
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@manifests/03-networkpolicy-operator-allow-egress.yaml`:
- Around line 25-26: Replace the unrestricted egress entry in the NetworkPolicy
with a destination-less TCP rule permitting only port 6443 for API-server
access, and add a separately scoped DNS egress rule limited to the required DNS
port and protocol. Keep static unrestricted egress removed and preserve the
policy’s existing scope and selectors.
In `@manifests/03-networkpolicy-operator-allow-metrics-ingress.yaml`:
- Around line 13-16: Update the NetworkPolicy allowing metrics ingress to
restrict sources to the openshift-monitoring namespace, adding the appropriate
namespace/pod selector under from while preserving the named metrics port and
TCP protocol settings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 7b7ad8f0-d5ce-4407-8358-0da4c557cda3
📒 Files selected for processing (3)
manifests/03-networkpolicy-operator-allow-egress.yamlmanifests/03-networkpolicy-operator-allow-metrics-ingress.yamlmanifests/03-networkpolicy-operator-default-deny.yaml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/console(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- manifests/03-networkpolicy-operator-default-deny.yaml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{yaml,yml,json}
📄 CodeRabbit inference engine (Custom checks)
**/*.{yaml,yml,json}: Flag privileged: true, hostPID, hostNetwork, hostIPC, SYS_ADMIN capability, running as root without justification, and allowPrivilegeEscalation: true in container/Kubernetes manifests
When deployment manifests, operator code, or controllers are added/modified, ensure they do not introduce scheduling constraints assuming standard HA topology (3+ control-plane nodes, dedicated workers). Flag: required pod anti-affinity with maxUnavailable: 0 (deadlocks on SNO/TNF/TNA), pod topology spread with DoNotSchedule and hostname key (breaks on SNO), replica counts derived from node count without topology awareness, nodeSelector/node affinity targeting control-plane nodes (fails on HyperShift), scheduling to all control-plane nodes equally without excluding arbiter nodes (TNA), assuming dedicated worker nodes exist (SNO/TNF), or PodDisruptionBudgets designed for 3+ nodes (TNF/TNA). Do not flag if change checks ControlPlaneTopology, node counts, or topology labels before applying constraints.
Files:
manifests/03-networkpolicy-operator-allow-egress.yamlmanifests/03-networkpolicy-operator-allow-metrics-ingress.yaml
{manifests,bindata/assets,quickstarts,examples,profile-patches}/**/*.{yaml,yml}
📄 CodeRabbit inference engine (.claude/skills/manifest-review.md)
{manifests,bindata/assets,quickstarts,examples,profile-patches}/**/*.{yaml,yml}: Kubernetes manifests undermanifests/,bindata/assets/,quickstarts/,examples/, andprofile-patches/must include the appropriate cluster profile annotations when they are CVO-deployed resources (for exampleinclude.release.openshift.io/hypershift,include.release.openshift.io/ibm-cloud-managed,include.release.openshift.io/self-managed-high-availability, andinclude.release.openshift.io/single-node-developer).
Console resources in the manifest trees must include the capability annotationcapability.openshift.io/name: Console.
RBAC manifests must follow least-privilege design: grant only the permissions needed, avoid wildcards unless truly justified, use verbs that match the actual operation set, and set correctapiGroups(""for core APIs and specific groups for CRDs).
Resources must use the correct namespace for their role:openshift-consolefor console workload resources andopenshift-console-operatorfor operator resources; any cross-namespace reference must be explicit and intentional.
YAML manifests should use 2-space indentation, keep fields in a consistent order, and include---separators between multiple resources in the same file.
When binding roles to service accounts, useconsole-operatorin theopenshift-console-operatornamespace for operator cross-namespace access, and useconsolein theopenshift-consolenamespace for console workload-scoped access.
Files:
manifests/03-networkpolicy-operator-allow-egress.yamlmanifests/03-networkpolicy-operator-allow-metrics-ingress.yaml
**/*.yaml
⚙️ CodeRabbit configuration file
**/*.yaml: Review YAML manifests based on content and kind.Refer to /manifest-review when YAML contains:
kind: Roleorkind: ClusterRole(RBAC review)kind: RoleBindingorkind: ClusterRoleBindingannotations:section (check for cluster profiles)verbs: ["*"]or wildcard permissionsapiGroups: ["*"]or overly broad permissions- ServiceAccount references in subjects
Check for required annotations in manifests/:
include.release.openshift.io/hypershiftinclude.release.openshift.io/ibm-cloud-managedinclude.release.openshift.io/self-managed-high-availabilityinclude.release.openshift.io/single-node-developercapability.openshift.io/name: ConsoleFor quickstarts/, additionally check:
- QuickStart spec structure
- Task descriptions and prerequisites
- See quickstarts/README.md for guidelines
Files:
manifests/03-networkpolicy-operator-allow-egress.yamlmanifests/03-networkpolicy-operator-allow-metrics-ingress.yaml
**/*.{yaml,yml}
⚙️ CodeRabbit configuration file
**/*.{yaml,yml}: If this is a Kubernetes/OpenShift manifest or Helm template:
- securityContext: runAsNonRoot, readOnlyRootFilesystem,
allowPrivilegeEscalation: false- Drop ALL capabilities, add only what is required
- Resource limits (cpu, memory) on every container
- No hostPID, hostNetwork, hostIPC, privileged: true
- NetworkPolicy defined for the namespace
- OpenShift: SCC must be restricted or custom-scoped
- Liveness + readiness probes defined
- automountServiceAccountToken: false unless needed
- RBAC: least privilege; no cluster-admin for workloads
- Helm: no .Values interpolation in shell commands
Files:
manifests/03-networkpolicy-operator-allow-egress.yamlmanifests/03-networkpolicy-operator-allow-metrics-ingress.yaml
🧠 Learnings (1)
📚 Learning: 2026-07-24T17:22:48.776Z
Learnt from: redhat-chai-bot
Repo: openshift/console-operator PR: 1198
File: manifests/03-networkpolicy-operator-allow-apiserver-egress.yaml:19-22
Timestamp: 2026-07-24T17:22:48.776Z
Learning: For CVO-managed OpenShift operator NetworkPolicy manifests that must allow egress to the Kubernetes API server (TCP 6443), do not rely on podSelector/namespaceSelector targeting for the API server, because the API server may be host-networked and therefore not reliably matchable. Also avoid static ipBlock rules since API server addresses vary by cluster and topology. Instead, use a destination-less egress rule for TCP port 6443 only when the manifest explicitly documents the rationale explaining why selectors/ipBlock cannot be used.
Applied to files:
manifests/03-networkpolicy-operator-allow-egress.yamlmanifests/03-networkpolicy-operator-allow-metrics-ingress.yaml
🔇 Additional comments (2)
manifests/03-networkpolicy-operator-allow-egress.yaml (1)
1-11: LGTM!Also applies to: 21-24
manifests/03-networkpolicy-operator-allow-metrics-ingress.yaml (1)
1-11: LGTM!Also applies to: 17-20
| egress: | ||
| - {} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not allow unrestricted egress.
egress: - {} permits the console operator to reach every destination and port, including services unrelated to reconciliation. Narrow this policy to the required API-server port (TCP 6443) and provide a separately scoped DNS allowance; the PR objective explicitly calls for API-server and DNS policies rather than open egress.
Based on learnings, API-server access should use a destination-less TCP 6443 rule because host-networked API servers cannot be reliably selected, while static unrestricted egress should be avoided.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@manifests/03-networkpolicy-operator-allow-egress.yaml` around lines 25 - 26,
Replace the unrestricted egress entry in the NetworkPolicy with a
destination-less TCP rule permitting only port 6443 for API-server access, and
add a separately scoped DNS egress rule limited to the required DNS port and
protocol. Keep static unrestricted egress removed and preserve the policy’s
existing scope and selectors.
Source: Learnings
There was a problem hiding this comment.
Intentional — egress: - {} is the pattern prescribed by the Developing Network Policies guide and matches the kube-apiserver-operator's own policy. The API server address and port vary per cluster (host-networked), so a static port-6443 restriction is also insufficient. This was reviewed and approved by @wking in the earlier discussion thread.
| Allow ingress to the metrics port on the console operator pod. | ||
| This is a NetworkPolicy managed by the cluster-version operator | ||
| that allows any source to reach the metrics port, enabling | ||
| Prometheus scraping and metrics-based testing. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file =="
sed -n '1,160p' manifests/03-networkpolicy-operator-allow-metrics-ingress.yaml
echo
echo "== related networkpolicy manifests =="
rg -n --glob '*.yaml' '03-networkpolicy-operator|from:|to:|ports:' manifests/bindata manifests 2>/dev/null | head -200
echo
echo "== console operator pod/deployment manifests and ports =="
rg -n --glob '*.yaml' 'console-operator|name: metrics|containerPort|port: 60000|60000' manifests/bindata manifests 2>/dev/null | head -300
echo
echo "== current diff summary/stat =="
git diff --stat || true
git diff -- manifests/03-networkpolicy-operator-allow-metrics-ingress.yaml | sed -n '1,220p' || trueRepository: openshift/console-operator
Length of output: 1329
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all NetworkPolitics =="
rg -n --glob '*.yaml' 'kind: NetworkPolicy|name: allow-metrics-ingress|from:|policyTypes:' manifests/bindata manifests 2>/dev/null || true
echo
echo "== console operator deployment manifests =="
for f in manifests/05-service.yaml manifests/07-operator.yaml manifests/07-operator-ibm-cloud-managed.yaml; do
if [ -f "$f" ]; then
echo "--- $f"
sed -n '1,140p' "$f"
else
echo "--- MISSING $f"
fi
done
echo
echo "== metrics port references =="
rg -n --glob '*.yaml|*.go|*.md' 'metrics|60000|containerPort:|name: metrics' . 2>/dev/null | head -300 || trueRepository: openshift/console-operator
Length of output: 8942
Restrict metrics ingress to the monitoring namespace.
With no from clause, every namespace and reachable source can access the operator’s metrics endpoint. Add a selector for openshift-monitoring pods/namespaces; the named metrics port is already TCP 60000 in the console-operator container.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@manifests/03-networkpolicy-operator-allow-metrics-ingress.yaml` around lines
13 - 16, Update the NetworkPolicy allowing metrics ingress to restrict sources
to the openshift-monitoring namespace, adding the appropriate namespace/pod
selector under from while preserving the named metrics port and TCP protocol
settings.
There was a problem hiding this comment.
Intentional — the Developing Network Policies guide says "We allow anything to reach our metrics endpoint" with no from clause. @wking requested this change specifically to fix the TestMetricsEndpoint e2e failure (the previous from: openshift-monitoring blocked the ingress router pods used by the test).
|
/retest All failures are CI infrastructure issues (image registry timeouts, pod scheduling timeouts on build11) — none reached the test execution phase. Retesting for clean signal, especially |
|
/retest Both failures are unrelated to this PR:
Key signal is green: |
d07d8ca to
118014f
Compare
|
The Fixed in |
|
/retest |
1 similar comment
|
/retest |
| ingress: | ||
| - ports: | ||
| - protocol: TCP | ||
| port: 60000 |
There was a problem hiding this comment.
Can we update this to accept port 8443 too? I'm not clear how 60000 and 8443 are related, but the Service seems to use 8443 for metrics input:
console-operator$ git grep -3 '60000\|8443' manifests/
manifests/05-service.yaml- - name: https
manifests/05-service.yaml- port: 443
manifests/05-service.yaml- protocol: TCP
manifests/05-service.yaml: targetPort: 8443
manifests/05-service.yaml- selector:
manifests/05-service.yaml- name: console-operator
manifests/05-service.yaml- sessionAffinity: None
--
manifests/07-operator-ibm-cloud-managed.yaml- livenessProbe:
manifests/07-operator-ibm-cloud-managed.yaml- httpGet:
manifests/07-operator-ibm-cloud-managed.yaml- path: /healthz
manifests/07-operator-ibm-cloud-managed.yaml: port: 8443
manifests/07-operator-ibm-cloud-managed.yaml- scheme: HTTPS
manifests/07-operator-ibm-cloud-managed.yaml- name: console-operator
manifests/07-operator-ibm-cloud-managed.yaml- ports:
manifests/07-operator-ibm-cloud-managed.yaml: - containerPort: 60000
manifests/07-operator-ibm-cloud-managed.yaml- name: metrics
manifests/07-operator-ibm-cloud-managed.yaml- readinessProbe:
manifests/07-operator-ibm-cloud-managed.yaml- httpGet:
manifests/07-operator-ibm-cloud-managed.yaml- path: /readyz
manifests/07-operator-ibm-cloud-managed.yaml: port: 8443
manifests/07-operator-ibm-cloud-managed.yaml- scheme: HTTPS
manifests/07-operator-ibm-cloud-managed.yaml- resources:
manifests/07-operator-ibm-cloud-managed.yaml- requests:
--
manifests/07-operator.yaml- terminationMessagePolicy: FallbackToLogsOnError
manifests/07-operator.yaml- image: registry.svc.ci.openshift.org/openshift:console-operator
manifests/07-operator.yaml- ports:
manifests/07-operator.yaml: - containerPort: 60000
manifests/07-operator.yaml- name: metrics
manifests/07-operator.yaml- command:
manifests/07-operator.yaml- - console
--
manifests/07-operator.yaml- livenessProbe:
manifests/07-operator.yaml- httpGet:
manifests/07-operator.yaml- path: /healthz
manifests/07-operator.yaml: port: 8443
manifests/07-operator.yaml- scheme: HTTPS
manifests/07-operator.yaml- readinessProbe:
manifests/07-operator.yaml- httpGet:
manifests/07-operator.yaml- path: /readyz
manifests/07-operator.yaml: port: 8443
manifests/07-operator.yaml- scheme: HTTPS
manifests/07-operator.yaml- volumes:
manifests/07-operator.yaml- - name: configThere was a problem hiding this comment.
Great catch — this explains the TestMetricsEndpoint failure that's been blocking us (3/3 failures on port: 60000).
The traffic path for TestMetricsEndpoint is: Route → Service (metrics, port 443) → Pod (targetPort 8443). Our NetworkPolicy only allows port 60000, so the pod rejects ingress on 8443 → the test gets 503/EOF.
The container uses two ports:
containerPort: 60000(name: metrics) — the Prometheus metrics scrape endpoint- Port
8443— the HTTPS serving endpoint (healthz/readyz probes, and where the Service routes traffic)
Both need to be in the ingress policy. Updating now:
ingress:
- ports:
- protocol: TCP
port: 60000
- protocol: TCP
port: 8443This also retroactively explains why port: metrics (named port) passed 1/1 but port: 60000 failed 3/3 — though that's coincidental since named port metrics resolves to 60000 not 8443. The named port pass was likely a timing flake.
…tor namespace Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
118014f to
d53ea3d
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: redhat-chai-bot, spadgett, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
Marking px- and docs-approved for the same reasoning as #1195 (comment) /label px-approved |
openshift-ci
Bot
added
px-approved
|
/verified by CI presubmits passing |
openshift-ci-robot
added
the
verified
|
@wking: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@redhat-chai-bot: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
Summary
Adds four CVO-managed NetworkPolicy manifests for the
openshift-console-operatornamespace, which hosts the operator controller pod.These policies implement a default-deny baseline with targeted allow rules for the operator's legitimate network needs, improving network isolation for the namespace.
What this PR does
Adds four new files under
manifests/:03-networkpolicy-operator-default-deny.yaml— Default deny all ingress and egress traffic. UsespodSelector: {}to cover all pods in the namespace.03-networkpolicy-operator-allow-dns-egress.yaml— Allow egress to CoreDNS pods in theopenshift-dnsnamespace (TCP/UDP named portsdns-tcp/dns). The operator needs DNS resolution for Kubernetes service discovery and API server endpoint lookups.03-networkpolicy-operator-allow-apiserver-egress.yaml— Allow egress to the Kubernetes API server on port 6443. The operator controller needs API server access to watch and reconcile console-related resources.03-networkpolicy-operator-allow-metrics-ingress.yaml— Allow ingress from theopenshift-monitoringnamespace to themetricsport (60000). This enables Prometheus to scrape operator metrics.All manifests include standard CVO release annotations,
capability.openshift.io/name: Consolefor capability gating, andkubernetes.io/descriptionannotations describing each policy's purpose.Why
Part of the HPSTRAT-104 initiative to ship tailored NetworkPolicy resources with every control-plane operator by OCP 5.0. The default-deny with targeted allow rules improves network isolation by ensuring the operator controller pod only communicates through explicitly authorized paths.
References
Validation
make verifypasses with no diff@wking requested in Slack thread
Summary by CodeRabbit