CNTRLPLANE-2699: adding network policies for operator and operands#645
CNTRLPLANE-2699: adding network policies for operator and operands#645dusk125 wants to merge 4 commits intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@dusk125: This pull request references CNTRLPLANE-2699 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.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
bot
added
the
do-not-merge/work-in-progress
dusk125
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds Kubernetes NetworkPolicy manifests (allow and default-deny) for openshift-apiserver and openshift-apiserver-operator, and registers the two new bindata asset files in the embedded assets and starter 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
|
@dusk125: This pull request references CNTRLPLANE-2699 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.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: 1
🧹 Nitpick comments (1)
manifests/11_networkpolicies.yaml (1)
23-26: Scope metrics ingress more tightly.With no
fromclause, every pod in the cluster can reach port 8443. If this path is only for Prometheus scraping, restricting sources to the monitoring namespace keeps the policy aligned with least privilege.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@manifests/11_networkpolicies.yaml` around lines 23 - 26, The NetworkPolicy's ingress rule currently allows all sources to reach port 8443 (ingress -> ports -> port: 8443); restrict it by adding a from clause that limits traffic to the monitoring namespace (e.g., ingress -> from -> - namespaceSelector with a label match for your monitoring namespace such as matchLabels: {name: monitoring} or kubernetes.io/metadata.name: monitoring) so only pods in that namespace (used by Prometheus) can scrape on port 8443.
🤖 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/v3.11.0/openshift-apiserver/networkpolicy-allow.yaml`:
- Around line 27-28: The current egress entry "egress: - {}" in the
NetworkPolicy for openshift-apiserver removes egress isolation; remove that
unconditional allow and replace it with explicit egress rules limited to only
required destinations/ports (e.g., DNS to cluster DNS service on UDP/TCP 53,
kube-apiserver service on TCP 443, etcd endpoints on TCP 2379/2380) so the
companion default-deny can effectively restrict outbound traffic; update the
resource containing the "egress" key and its empty-item entry so it enumerates
only the minimal allowed to/from selectors instead of "- {}".
---
Nitpick comments:
In `@manifests/11_networkpolicies.yaml`:
- Around line 23-26: The NetworkPolicy's ingress rule currently allows all
sources to reach port 8443 (ingress -> ports -> port: 8443); restrict it by
adding a from clause that limits traffic to the monitoring namespace (e.g.,
ingress -> from -> - namespaceSelector with a label match for your monitoring
namespace such as matchLabels: {name: monitoring} or
kubernetes.io/metadata.name: monitoring) so only pods in that namespace (used by
Prometheus) can scrape on port 8443.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a89c62b7-8295-437f-9879-7dc149b3036e
📒 Files selected for processing (5)
bindata/v3.11.0/openshift-apiserver/networkpolicy-allow.yamlbindata/v3.11.0/openshift-apiserver/networkpolicy-default-deny.yamlmanifests/11_networkpolicies.yamlmanifests/11_networkpolicies_default-deny.yamlpkg/operator/starter.go
| egress: | ||
| - {} |
There was a problem hiding this comment.
egress: - {} removes meaningful egress isolation.
This rule allows every destination and port for the selected pods, so the companion default-deny no longer constrains outbound traffic from openshift-apiserver. If the goal is to harden the operand namespace, replace the unconditional allow with only the required DNS / kube-apiserver / etcd paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bindata/v3.11.0/openshift-apiserver/networkpolicy-allow.yaml` around lines 27
- 28, The current egress entry "egress: - {}" in the NetworkPolicy for
openshift-apiserver removes egress isolation; remove that unconditional allow
and replace it with explicit egress rules limited to only required
destinations/ports (e.g., DNS to cluster DNS service on UDP/TCP 53,
kube-apiserver service on TCP 443, etcd endpoints on TCP 2379/2380) so the
companion default-deny can effectively restrict outbound traffic; update the
resource containing the "egress" key and its empty-item entry so it enumerates
only the minimal allowed to/from selectors instead of "- {}".
There was a problem hiding this comment.
Network policies do not allow us to select endpoints on the host network (which the api server is running on). So we would either need to generate a network policy dynamically that contain the endpoints of the kubernetes service and regenerate it when they change, or we allow any egress (since the port is changeable).
Our guidance for this feature was do to the latter.
There was a problem hiding this comment.
@dusk125, understood — since kube-apiserver and etcd run on the host network, NetworkPolicy pod/namespace selectors can't target them, making static egress restrictions impractical without a dynamic controller. Allowing all egress is the correct approach here. Thanks for the explanation!
📝 Learnings were identified and not saved (knowledge base disabled). Enable
|
@dusk125: This pull request references CNTRLPLANE-2699 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.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. |
|
/retest-required |
|
@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. |
|
/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/6202c4c0-1d6f-11f1-9148-00284363ce4b-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/6202c4c0-1d6f-11f1-9148-00284363ce4b-1 |
|
/label tide/merge-method-squash |
openshift-ci
bot
added
the
tide/merge-method-squash
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dusk125, liouk, sanchezl 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
4 participants
Adds NetworkPolicy resources for both operator and operand namespaces:
Summary by CodeRabbit