-
Notifications
You must be signed in to change notification settings - Fork 23
Adding network policies to kmm including to kmm bundle #1652
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| apiVersion: networking.k8s.io/v1 | ||
| kind: NetworkPolicy | ||
| metadata: | ||
| name: controller | ||
| namespace: system | ||
| spec: | ||
| podSelector: | ||
| matchLabels: | ||
| control-plane: controller | ||
| policyTypes: | ||
| - Egress | ||
| - Ingress | ||
| ingress: | ||
| - ports: | ||
| - protocol: TCP # metrics port | ||
| port: 8443 | ||
| - protocol: TCP | ||
| port: 8081 # Healthz | ||
| egress: | ||
| - to: | ||
| - namespaceSelector: # DNS | ||
| matchLabels: | ||
| kubernetes.io/metadata.name: openshift-dns | ||
| podSelector: | ||
| matchLabels: | ||
| dns.operator.openshift.io/daemonset-dns: default | ||
| ports: | ||
| - protocol: UDP # DNS | ||
| port: 53 | ||
| - protocol: TCP # DNS | ||
| port: 53 | ||
| - ports: # kube api server | ||
| - protocol: TCP | ||
| port: 6443 | ||
| - protocol: TCP | ||
| port: 443 | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,22 @@ | ||||||||||||||||||||||||||||||||
| apiVersion: networking.k8s.io/v1 | ||||||||||||||||||||||||||||||||
| kind: NetworkPolicy | ||||||||||||||||||||||||||||||||
| metadata: | ||||||||||||||||||||||||||||||||
| name: webhook | ||||||||||||||||||||||||||||||||
| namespace: system | ||||||||||||||||||||||||||||||||
| spec: | ||||||||||||||||||||||||||||||||
| podSelector: | ||||||||||||||||||||||||||||||||
| matchLabels: | ||||||||||||||||||||||||||||||||
| control-plane: webhook-server | ||||||||||||||||||||||||||||||||
| policyTypes: | ||||||||||||||||||||||||||||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||
| - Egress | ||||||||||||||||||||||||||||||||
| - Ingress | ||||||||||||||||||||||||||||||||
| ingress: | ||||||||||||||||||||||||||||||||
| - ports: | ||||||||||||||||||||||||||||||||
| - protocol: TCP | ||||||||||||||||||||||||||||||||
| port: 9443 | ||||||||||||||||||||||||||||||||
|
Comment on lines
+13
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Ingress currently allows from anywhere; restrict to API server With an ingress rule that only lists ports and no from peers, traffic on 9443 is allowed from any source. For admission webhooks we should scope ingress to the API server namespaces to reduce blast radius. Example tightening (covers both vanilla and OpenShift): ingress:
- - ports:
- - protocol: TCP
- port: 9443
+ - from:
+ - namespaceSelector:
+ matchLabels:
+ kubernetes.io/metadata.name: kube-system
+ - namespaceSelector:
+ matchLabels:
+ kubernetes.io/metadata.name: openshift-kube-apiserver
+ ports:
+ - protocol: TCP
+ port: 9443If you prefer stricter scoping, we can further constrain with podSelector labels on the apiserver pods in those namespaces. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| egress: | ||||||||||||||||||||||||||||||||
| - ports: # kube api server port | ||||||||||||||||||||||||||||||||
| - protocol: TCP | ||||||||||||||||||||||||||||||||
| port: 6443 | ||||||||||||||||||||||||||||||||
| - protocol: TCP | ||||||||||||||||||||||||||||||||
| port: 443 | ||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,36 @@ | ||||||||||||||||
| apiVersion: networking.k8s.io/v1 | ||||||||||||||||
| kind: NetworkPolicy | ||||||||||||||||
| metadata: | ||||||||||||||||
| name: controller | ||||||||||||||||
| namespace: system | ||||||||||||||||
| spec: | ||||||||||||||||
|
Comment on lines
+3
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainBundle manifest should not hardcode OLM typically applies bundle objects into the operator’s install namespace. Shipping a hardcoded namespace can cause install-time failures or resources landing in the wrong namespace. Recommend dropping the field in the bundled file (or have the bundle generator strip it). metadata:
name: controller
- namespace: system🏁 Script executed: #!/usr/bin/env bash
# Locate the NetworkPolicy manifest and print its metadata section
manifest="bundle/manifests/kmm-operator-controller_networking.k8s.io_v1_networkpolicy.yaml"
if [ -f "$manifest" ]; then
echo "Showing lines 1-20 of $manifest:"
sed -n '1,20p' "$manifest"
else
echo "$manifest not found."
fiLength of output: 871 Remove hardcoded namespace from NetworkPolicy manifest The bundle’s NetworkPolicy manifest explicitly sets Please update the file as follows: • File: Suggested diff: metadata:
name: controller
- namespace: system📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| podSelector: | ||||||||||||||||
| matchLabels: | ||||||||||||||||
| control-plane: controller | ||||||||||||||||
| policyTypes: | ||||||||||||||||
| - Egress | ||||||||||||||||
| - Ingress | ||||||||||||||||
| ingress: | ||||||||||||||||
| - ports: | ||||||||||||||||
| - protocol: TCP # metrics port | ||||||||||||||||
| port: 8443 | ||||||||||||||||
| - protocol: TCP | ||||||||||||||||
| port: 8081 # Healthz | ||||||||||||||||
| egress: | ||||||||||||||||
| - to: | ||||||||||||||||
| - namespaceSelector: # DNS | ||||||||||||||||
| matchLabels: | ||||||||||||||||
| kubernetes.io/metadata.name: openshift-dns | ||||||||||||||||
| podSelector: | ||||||||||||||||
| matchLabels: | ||||||||||||||||
| dns.operator.openshift.io/daemonset-dns: default | ||||||||||||||||
| ports: | ||||||||||||||||
| - protocol: UDP # DNS | ||||||||||||||||
| port: 53 | ||||||||||||||||
| - protocol: TCP # DNS | ||||||||||||||||
| port: 53 | ||||||||||||||||
| - ports: # kube api server | ||||||||||||||||
| - protocol: TCP | ||||||||||||||||
| port: 6443 | ||||||||||||||||
| - protocol: TCP | ||||||||||||||||
| port: 443 | ||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,22 @@ | ||||
| apiVersion: networking.k8s.io/v1 | ||||
| kind: NetworkPolicy | ||||
| metadata: | ||||
| name: webhook | ||||
| namespace: system | ||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Hard-coded namespace "system" in a bundle may not match install namespace. OLM typically installs namespaced resources into the operator’s target namespace. A hard-coded "system" can cause install failures or drift if the operator is installed into openshift-kmm (as your default overlay suggests). Two safe options:
Proposed change (if you choose to drop explicit namespace in source before bundling): - namespace: systemIf you’d rather rewrite at bundle time, adjust hack/generate-bundle to strip or replace the namespace on copied NetworkPolicy YAMLs. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||
| spec: | ||||
| podSelector: | ||||
| matchLabels: | ||||
| control-plane: webhook-server | ||||
| policyTypes: | ||||
| - Egress | ||||
| - Ingress | ||||
| ingress: | ||||
| - ports: | ||||
| - protocol: TCP | ||||
| port: 9443 | ||||
| egress: | ||||
| - ports: # kube api server port | ||||
| - protocol: TCP | ||||
| port: 6443 | ||||
| - protocol: TCP | ||||
| port: 443 | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,8 +10,10 @@ generatorOptions: | |
|
|
||
| configMapGenerator: | ||
| - name: kmm-kmod-dockerfile | ||
| namespace: openshift-kmm | ||
| files: [dockerfile=Dockerfile] | ||
|
|
||
| secretGenerator: | ||
| - name: build-secret | ||
| namespace: openshift-kmm | ||
| literals: [ci-build-secret=super-secret-value] | ||
|
Comment on lines
11
to
19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add namespace resource or document pre-requisite namespace creation. The 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,10 +6,10 @@ POD_NAME='' | |||||
|
|
||||||
| wait_for_pod_and_print_logs () { | ||||||
| # we can't exec a command nor get the logs on a pod that isn't `Running` yet. | ||||||
| oc wait "pod/${POD_NAME}" --for jsonpath='{.status.phase}'=Running --timeout=60s | ||||||
| oc wait -n openshift-kmm"pod/${POD_NAME}" --for jsonpath='{.status.phase}'=Running --timeout=60s | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix syntax error in the oc wait command. There's a missing space between - oc wait -n openshift-kmm"pod/${POD_NAME}" --for jsonpath='{.status.phase}'=Running --timeout=60s
+ oc wait -n openshift-kmm "pod/${POD_NAME}" --for jsonpath='{.status.phase}'=Running --timeout=60s📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| echo "Print pod ${POD_NAME} logs..." | ||||||
| oc logs "pod/${POD_NAME}" -f | ||||||
| oc logs -n openshift-kmm "pod/${POD_NAME}" -f | ||||||
| } | ||||||
|
|
||||||
| check_module_not_loaded () { | ||||||
|
|
@@ -46,14 +46,14 @@ timeout 1m bash -c 'until oc apply -k ci/e2e; do sleep 3; done' | |||||
| # Wait for the build pod to be created. `kubectl wait` doesn't support such option, | ||||||
| # see https://github.com/kubernetes/kubernetes/issues/83242. | ||||||
| echo "Waiting for the build pod to be created..." | ||||||
| timeout 1m bash -c 'until oc get pods -o json | jq -er ".items[].metadata.name | select(.? | match(\"build\"))"; do sleep 1; done' | ||||||
| POD_NAME=$(oc get pods -o json | jq -r '.items[].metadata.name | select(.? | match("build"))') | ||||||
| timeout 1m bash -c 'until oc get pods -n openshift-kmm -o json | jq -er ".items[].metadata.name | select(.? | match(\"build\"))"; do sleep 1; done' | ||||||
| POD_NAME=$(oc get pods -n openshift-kmm -o json | jq -r '.items[].metadata.name | select(.? | match("build"))') | ||||||
|
|
||||||
| wait_for_pod_and_print_logs | ||||||
|
|
||||||
| echo "Waiting for the signing pod to be created..." | ||||||
| timeout 1m bash -c 'until oc get pods -o json | jq -er ".items[].metadata.name | select(.? | match(\"sign\"))"; do sleep 1; done' | ||||||
| POD_NAME=$(oc get pods -o json | jq -r '.items[].metadata.name | select(.? | match("sign"))') | ||||||
| timeout 1m bash -c 'until oc get pods -n openshift-kmm -o json | jq -er ".items[].metadata.name | select(.? | match(\"sign\"))"; do sleep 1; done' | ||||||
| POD_NAME=$(oc get pods -n openshift-kmm -o json | jq -r '.items[].metadata.name | select(.? | match("sign"))') | ||||||
|
|
||||||
| wait_for_pod_and_print_logs | ||||||
|
|
||||||
|
|
@@ -66,7 +66,7 @@ oc debug "node/${NODE}" -- chroot host/ lsmod | grep kmm_ci_b | |||||
| check_module_not_loaded "dummy" | ||||||
|
|
||||||
| echo "Remove the Module..." | ||||||
| oc delete modules.kmm.sigs.x-k8s.io/kmm-ci --wait=false | ||||||
| oc delete modules.kmm.sigs.x-k8s.io/kmm-ci --wait=false -n openshift-kmm | ||||||
|
|
||||||
| echo "Check that the module gets unloaded from the node..." | ||||||
| timeout 1m bash -c 'until ! oc debug node/${NODE} -- chroot host/ lsmod | grep kmm_ci_a; do sleep 3; done' | ||||||
|
|
@@ -75,4 +75,4 @@ echo "Check that the dependent module is also unloaded from the node..." | |||||
| check_module_not_loaded "kmm_ci_b" | ||||||
|
|
||||||
| echo "Wait for the Module to be deleted..." | ||||||
| oc wait --for delete modules.kmm.sigs.x-k8s.io/kmm-ci | ||||||
| oc wait --for delete modules.kmm.sigs.x-k8s.io/kmm-ci -n openshift-kmm | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: rh-ecosystem-edge/kernel-module-management
Length of output: 452
🏁 Script executed:
Repository: rh-ecosystem-edge/kernel-module-management
Length of output: 1169
🏁 Script executed:
Repository: rh-ecosystem-edge/kernel-module-management
Length of output: 315
🏁 Script executed:
Repository: rh-ecosystem-edge/kernel-module-management
Length of output: 125
🏁 Script executed:
Repository: rh-ecosystem-edge/kernel-module-management
Length of output: 1548
Restrict egress to API servers with a destination selector.
The egress rule for Kubernetes API servers (lines 32-36) lacks a
toselector, permitting egress to any destination on TCP ports 6443 and 443. In a default-deny context, this rule should restrict traffic to specific API server targets. Add a destination selector to limit traffic appropriately.Consider applying this change:
- ports: # kube api server - protocol: TCP port: 6443 - protocol: TCP port: 443 + to: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: default + podSelector: + matchLabels: + component: kube-apiserver🤖 Prompt for AI Agents