ESO-222:Adds the metrics configuration in network-policy #77
Conversation
siddhibhor-56
commented
Oct 9, 2025
siddhibhor-56
commented
- Added the "from" clause in the network policy of the operator.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: siddhibhor-56 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughRemoves the metrics-specific NetworkPolicy, updates the remaining allow-network-traffic policy to drop port 8080, add ingress from the openshift-monitoring namespace, and retains port 8443. Updates kustomization to remove the deleted resource. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–15 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
siddhibhor-56
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@siddhibhor-56: This pull request references ESO-222 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. In 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. |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
config/network-policy/allow-metrics-traffic.yaml(0 hunks)config/network-policy/allow-network-traffic.yaml(1 hunks)config/network-policy/kustomization.yaml(0 hunks)
💤 Files with no reviewable changes (2)
- config/network-policy/kustomization.yaml
- config/network-policy/allow-metrics-traffic.yaml
| - from: | ||
| - namespaceSelector: | ||
| matchLabels: | ||
| name: openshift-monitoring | ||
| - ports: | ||
| - protocol: TCP | ||
| port: 8443 | ||
| - ports: | ||
| - protocol: TCP | ||
| port: 8080 |
There was a problem hiding this comment.
🧩 Analysis chain
Ingress rules split incorrectly: results in overly permissive access; also 8080 still open and namespace label likely wrong
- A rule with only from allows all ports from openshift-monitoring.
- A rule with only ports allows 8443/8080 from anywhere.
- Combined, this is broader than intended. Also, 8080 is still exposed despite the PR description/summary. Finally, namespaces don’t have a default name label; use kubernetes.io/metadata.name.
Fix by combining from and ports into a single rule, dropping 8080, and using the stable namespace label:
ingress:
- - from:
- - namespaceSelector:
- matchLabels:
- name: openshift-monitoring
- - ports:
- - protocol: TCP
- port: 8443
- - protocol: TCP
- port: 8080
+ - from:
+ - namespaceSelector:
+ matchLabels:
+ kubernetes.io/metadata.name: openshift-monitoring
+ ports:
+ - protocol: TCP
+ port: 8443Please confirm:
- 8080 should indeed be removed.
- The target namespace is openshift-monitoring (or adjust the selector accordingly).
🏁 Script executed:
#!/bin/bash
sed -n '15,40p' config/network-policy/allow-network-traffic.yamlLength of output: 374
Combine ingress rules into one, correct namespaceSelector label, and remove port 8080
- Separate
fromandportsentries result in all ports allowed from the monitoring namespace and 8443/8080 open globally. - Merge into a single ingress block with:
matchLabels:kubernetes.io/metadata.name: openshift-monitoring
ports:
- protocol: TCP
port: 8443
Confirm whether port 8080 should remain exposed.
🤖 Prompt for AI Agents
In config/network-policy/allow-network-traffic.yaml around lines 19 to 27, the
NetworkPolicy currently has separate from and ports entries causing ports to be
applied globally and uses the wrong namespace label; merge into a single ingress
rule that uses namespaceSelector.matchLabels: kubernetes.io/metadata.name:
openshift-monitoring and include only the ports list with protocol: TCP and
port: 8443 (remove port 8080), and if 8080 is required re-add it only after
confirmation scoped under the same ingress/namespaceSelector.
|
@siddhibhor-56: all tests passed! Full PR test history. Your PR dashboard. 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Same changes added in #83 hence closing this |
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. 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 kubernetes-sigs/prow repository. |
|
@bharath-b-rh: Closed this PR. In 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 kubernetes-sigs/prow repository. |
4 participants
