CNTRLPLANE-2905: add network policies#414
Conversation
|
@dusk125: This pull request references CNTRLPLANE-2905 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 epic to target the "4.22.0" version, but no target version was set. 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. |
openshift-ci-robot
added
the
jira/valid-reference
|
/retest-required |
1 similar comment
|
/retest-required |
manifests/0000_25_openshift-controller-manager-operator_01_network-policy-operator.yaml
Outdated
Show resolved
Hide resolved
manifests/0000_25_openshift-controller-manager_01_network-policy-controller-manager.yaml
Outdated
Show resolved
Hide resolved
...s/0000_25_openshift-route-controller-manager_01_network-policy-route-controller-manager.yaml
Outdated
Show resolved
Hide resolved
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds NetworkPolicy YAMLs: namespace-wide default-deny and specific allow policies (ingress TCP 8443, unrestricted egress) for controller-manager, route-controller-manager, and the controller-manager-operator; and registers four new controller-manager assets in the operator static resource list. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
|
@dusk125: This pull request references CNTRLPLANE-2905 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 epic to target the "4.22.0" version, but no target version was set. 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bindata/assets/openshift-controller-manager/networkpolicy-allow.yaml`:
- Around line 8-24: The NetworkPolicy named allow-controller-manager currently
permits ingress on port 8443 but lacks a source restriction; update the
spec.ingress of the allow-controller-manager NetworkPolicy to include a from
clause that limits traffic to the openshift-monitoring namespace (e.g., add a
from: - namespaceSelector with an appropriate label selector matching the
monitoring namespace) so only pods from openshift-monitoring can reach port 8443
on pods selected by spec.podSelector (app: openshift-controller-manager-a,
controller-manager: "true").
In
`@bindata/assets/openshift-controller-manager/route-controller-manager-networkpolicy-allow.yaml`:
- Around line 8-24: The NetworkPolicy named allow-route-controller-manager in
namespace openshift-route-controller-manager currently permits ingress to pods
matching labels app: route-controller-manager and route-controller-manager:
"true" on TCP port 8443 from any source; update the spec.ingress entry to
include a from block that restricts sources to the openshift-monitoring
namespace (use namespaceSelector with matchLabels or metadata.name selector for
the monitoring namespace) so Prometheus alone can scrape metrics, or if open
access is intended, change the comment above to say it allows ingress from any
source rather than from openshift-monitoring.
In
`@manifests/0000_25_openshift-controller-manager-operator_01_network-policy-operator.yaml`:
- Around line 8-26: The doc/comment claims ingress is limited "from
openshift-monitoring" but the NetworkPolicy allow-operator (namespace
openshift-controller-manager-operator, podSelector app:
openshift-controller-manager-operator) currently has no from block and therefore
allows all sources; fix by adding a from block to the spec.ingress that
restricts traffic to the openshift-monitoring namespace (e.g., add a from: -
namespaceSelector: matchLabels: kubernetes.io/metadata.name:
openshift-monitoring) so the port: 8443 rule truly only allows Prometheus from
that namespace, or alternatively update the comment to accurately state that the
rule allows all sources—choose one and make the change in the NetworkPolicy
manifest.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2eea60ce-ba75-46c4-9a24-3663c5f3bbdd
📒 Files selected for processing (7)
bindata/assets/openshift-controller-manager/networkpolicy-allow.yamlbindata/assets/openshift-controller-manager/networkpolicy-default-deny.yamlbindata/assets/openshift-controller-manager/route-controller-manager-networkpolicy-allow.yamlbindata/assets/openshift-controller-manager/route-controller-manager-networkpolicy-default-deny.yamlmanifests/0000_25_openshift-controller-manager-operator_01_network-policy-default-deny.yamlmanifests/0000_25_openshift-controller-manager-operator_01_network-policy-operator.yamlpkg/operator/starter.go
bindata/assets/openshift-controller-manager/route-controller-manager-networkpolicy-allow.yaml
Show resolved
Hide resolved
manifests/0000_25_openshift-controller-manager-operator_01_network-policy-operator.yaml
Show resolved
Hide resolved
|
/payload 4.22 nightly blocking |
|
@dusk125: trigger 14 job(s) of type blocking for the nightly release of OCP 4.22
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/a53ecd80-1cb9-11f1-955c-2264a6e7c378-0 trigger 65 job(s) of type informing for the nightly release of OCP 4.22
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/a53ecd80-1cb9-11f1-955c-2264a6e7c378-1 |
manifests/0000_25_openshift-controller-manager_01_network-policy-controller-manager.yaml
Outdated
Show resolved
Hide resolved
|
@dusk125: This pull request references CNTRLPLANE-2905 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 epic to target the "4.22.0" version, but no target version was set. 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. |
|
/label tide/merge-method-squash |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
bindata/assets/openshift-controller-manager/route-controller-manager-networkpolicy-allow.yaml (1)
8-23:⚠️ Potential issue | 🟠 MajorRestrict metrics ingress or fix the comment.
The comment says port
8443is only opened toopenshift-monitoring, but this rule has nofromselector, so it allows ingress on8443from all sources. That leaves the metrics endpoint broader than intended.🔒 Proposed fix
ingress: - - ports: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: openshift-monitoring + ports: - protocol: TCP port: 8443If open access is intentional, update Lines 9-10 to say that explicitly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindata/assets/openshift-controller-manager/route-controller-manager-networkpolicy-allow.yaml` around lines 8 - 23, The NetworkPolicy named allow-route-controller-manager in namespace openshift-route-controller-manager currently allows ingress to pods with label route-controller-manager on TCP port 8443 from any source, contradicting the comment that it should only allow openshift-monitoring; either restrict the rule by adding a from clause that selects the openshift-monitoring namespace (e.g., namespaceSelector matching metadata.name: openshift-monitoring or a suitable podSelector) to limit ingress to Prometheus, or if open access was intended, update the comment to state that port 8443 is intentionally open to all sources.bindata/assets/openshift-controller-manager/networkpolicy-allow.yaml (1)
8-23:⚠️ Potential issue | 🟠 MajorRestrict metrics ingress or fix the comment.
Same issue here: Lines 9-10 say metrics ingress is from
openshift-monitoring, but the rule has nofromclause, so port8443is reachable from all sources.🔒 Proposed fix
ingress: - - ports: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: openshift-monitoring + ports: - protocol: TCP port: 8443If cluster-wide access is intended, please make the comment match the actual policy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindata/assets/openshift-controller-manager/networkpolicy-allow.yaml` around lines 8 - 23, The NetworkPolicy named allow-controller-manager in namespace openshift-controller-manager currently exposes port 8443 to all sources (spec.ingress has no from clause) while the comment says it should allow ingress only from the openshift-monitoring namespace; either update the comment to reflect cluster-wide access or restrict the rule by adding a from clause under spec.ingress that limits traffic to pods in the openshift-monitoring namespace (e.g., a namespaceSelector matching metadata.name: openshift-monitoring or a podSelector as appropriate) so the policy behavior matches the documented intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@bindata/assets/openshift-controller-manager/networkpolicy-allow.yaml`:
- Around line 8-23: The NetworkPolicy named allow-controller-manager in
namespace openshift-controller-manager currently exposes port 8443 to all
sources (spec.ingress has no from clause) while the comment says it should allow
ingress only from the openshift-monitoring namespace; either update the comment
to reflect cluster-wide access or restrict the rule by adding a from clause
under spec.ingress that limits traffic to pods in the openshift-monitoring
namespace (e.g., a namespaceSelector matching metadata.name:
openshift-monitoring or a podSelector as appropriate) so the policy behavior
matches the documented intent.
In
`@bindata/assets/openshift-controller-manager/route-controller-manager-networkpolicy-allow.yaml`:
- Around line 8-23: The NetworkPolicy named allow-route-controller-manager in
namespace openshift-route-controller-manager currently allows ingress to pods
with label route-controller-manager on TCP port 8443 from any source,
contradicting the comment that it should only allow openshift-monitoring; either
restrict the rule by adding a from clause that selects the openshift-monitoring
namespace (e.g., namespaceSelector matching metadata.name: openshift-monitoring
or a suitable podSelector) to limit ingress to Prometheus, or if open access was
intended, update the comment to state that port 8443 is intentionally open to
all sources.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9b517a00-bb3a-41e6-87b8-534a9184167e
📒 Files selected for processing (2)
bindata/assets/openshift-controller-manager/networkpolicy-allow.yamlbindata/assets/openshift-controller-manager/route-controller-manager-networkpolicy-allow.yaml
openshift-ci
bot
added
the
tide/merge-method-squash
|
/lgtm |
|
@dusk125: This pull request references CNTRLPLANE-2905 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 epic to target the "4.22.0" version, but no target version was set. 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dusk125, liouk 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 |
|
@dusk125: 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. |
3 participants
Adds NetworkPolicy resources for both operator and operand namespaces