ACM-31164: Test and validate network policies for Image Based components - #847
ACM-31164: Test and validate network policies for Image Based components#847shay23bra wants to merge 1 commit into
Conversation
|
@shay23bra: This pull request references ACM-31164 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 task to target either version "5.0." or "openshift-5.0.", but it targets "ACM 5.0.0" instead. 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. |
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: shay23bra 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds and registers a Kubernetes NetworkPolicy for the image-based install operator, defining ingress and egress access for DNS, monitoring, API, HTTPS, and selected ports. Bundles the policy and updates the ClusterServiceVersion creation timestamp. ChangesOperator network policy
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)bundle/manifests/image-based-install-operator-networkpolicy.yamlTraceback (most recent call last): bundle/manifests/image-based-install-operator.clusterserviceversion.yamlTraceback (most recent call last): config/manager/image-based-install-operator-networkpolicy.yamlTraceback (most recent call last):
Comment |
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 `@config/manager/image-based-install-operator-networkpolicy.yaml`:
- Around line 17-18: Update the ingress rule in the network policy to add
explicit port restrictions, allowing same-namespace pods to reach only the
documented operator service ports and excluding the manager health endpoint on
8081. Preserve the existing podSelector scope while configuring the allowed
ports in the policy’s ports section.
- Around line 35-38: Restrict the TCP/9443 ingress rule in the image-based
install operator network policy to kube-apiserver traffic by adding the
platform-appropriate API-server source CIDR or selector under from. Preserve
port 9443 and the existing webhook ingress intent while preventing access from
all other sources.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 11b550e5-21c1-43b0-8b94-c68da4b7a283
📒 Files selected for processing (2)
config/manager/image-based-install-operator-networkpolicy.yamlconfig/manager/kustomization.yaml
| - from: | ||
| - podSelector: {} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict same-namespace ingress to the intended ports.
This rule allows every pod in the namespace to reach every port on the operator, including the manager health endpoint on 8081. Add explicit port rules so the policy actually limits ingress to the documented service ports.
🤖 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 `@config/manager/image-based-install-operator-networkpolicy.yaml` around lines
17 - 18, Update the ingress rule in the network policy to add explicit port
restrictions, allowing same-namespace pods to reach only the documented operator
service ports and excluding the manager health endpoint on 8081. Preserve the
existing podSelector scope while configuring the allowed ports in the policy’s
ports section.
| # Webhook calls from kube-apiserver | ||
| - ports: | ||
| - protocol: TCP | ||
| port: 9443 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Scope webhook ingress to the API-server source.
A rule with ports but no from permits TCP/9443 from all sources, not just the kube-apiserver described by the comment. Restrict it using the platform-appropriate API-server source CIDR or selector.
🤖 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 `@config/manager/image-based-install-operator-networkpolicy.yaml` around lines
35 - 38, Restrict the TCP/9443 ingress rule in the image-based install operator
network policy to kube-apiserver traffic by adding the platform-appropriate
API-server source CIDR or selector under from. Preserve port 9443 and the
existing webhook ingress intent while preventing access from all other sources.
|
@shay23bra A few things:
|
|
Good catches, thanks!
|
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 `@config/manager/image-based-install-operator-networkpolicy.yaml`:
- Around line 46-54: Restrict the TCP/6443 egress rule in the image-based
install operator NetworkPolicy by removing the unrestricted IPv6 ::/0 entry and
replacing it with the applicable Kubernetes and spoke API-server IPv6 CIDRs or
the approved egress path. If dynamic addresses require broad access, document
the security trade-off and add coverage proving the permitted destinations
remain constrained.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ecde0169-7142-4ccd-8431-85041cf96e24
📒 Files selected for processing (2)
bundle/manifests/image-based-install-operator.clusterserviceversion.yamlconfig/manager/image-based-install-operator-networkpolicy.yaml
|
/retest |
bfd7e38 to
165cff5
Compare
Add per-component NetworkPolicy restricting ingress and egress traffic for the image-based-install-operator pod, following the ACM 5.0 DDR pattern. Ingress restricted to known ports (8080 metrics, 9443 webhook, 8000 image server). Egress restricted to DNS, K8s API, spoke clusters, and HTTPS. IPv4 and IPv6 supported.
165cff5 to
1a23edb
Compare
|
/retest |
|
/retest-required |
|
@shay23bra: The following tests failed, say
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. |
Summary
NetworkPolicyfor theimage-based-install-operatorpod, following the ACM 5.0 DDR pattern (Create-Once + Operand Ownership)NetworkPolicy Details
Ingress:
Egress:
Test plan
Test results (kind + Calico)
Note: metrics port 8080 binds to localhost in the manager — reachable via ServiceMonitor/Service on OCP, not via pod IP directly.
Related
Summary by CodeRabbit