fix(hypershift-kubevirt): grant networkpolicies RBAC to external infra SA#79081
Conversation
WalkthroughThe external-infra service account's Role is extended to allow full access to ChangesExternal-Infra RBAC Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/pj-rehearse pull-ci-openshift-cluster-api-provider-kubevirt-release-4.22-e2e-hypershift-kubevirt |
|
@orenc1: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
…a SA The hypershift-operator now creates a virt-launcher NetworkPolicy on the external infrastructure cluster to enforce network isolation for KubeVirt guest VMs (openshift/hypershift#8056, OCPBUGS-78575). A new ValidKubeVirtInfraNetworkPolicyRBAC condition was added that requires the external infra service account to have networking.k8s.io/networkpolicies permissions and cluster-scoped read access to networks.config.openshift.io for CIDR-based egress rules. The CI step that provisions the restricted infra kubeconfig was not updated to include these permissions, so the e2e-hypershift-kubevirt job fails with forbidden errors when the operator attempts to manage the virt-launcher NetworkPolicy and read cluster network configuration. Add networking.k8s.io/networkpolicies with full verbs to the kv-external-infra-role, and create a ClusterRole+ClusterRoleBinding granting get access to networks.config.openshift.io so the operator can build CIDR-based egress restrictions for full tenant isolation. Signed-off-by: Oren Cohen <ocohen@redhat.com> Assisted-by: Claude <noreply@anthropic.com>
cf5a16d to
c777e86
Compare
|
[REHEARSALNOTIFIER]
A total of 61 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse pull-ci-openshift-cluster-api-provider-kubevirt-release-4.22-e2e-hypershift-kubevirt |
|
@orenc1: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
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
`@ci-operator/step-registry/hypershift/kubevirt/install/create-external-infra-kubeconfig/hypershift-kubevirt-install-create-external-infra-kubeconfig-commands.sh`:
- Around line 140-152: The ClusterRoleBinding uses a fixed cluster-scoped name
(kv-external-infra-network-reader-binding) which causes different runs with
different EXTERNAL_INFRA_NS to clobber each other; update the ClusterRoleBinding
metadata.name to include the EXTERNAL_INFRA_NS (for example append or
interpolate ${EXTERNAL_INFRA_NS}) so the binding becomes unique per external
infra namespace, and ensure any references to that binding name (if any
elsewhere in the script) are updated accordingly; keep the subjects
(ServiceAccount name ${SA_NAME} and namespace ${EXTERNAL_INFRA_NS}) and roleRef
unchanged.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 76aa6fdf-3f7c-4f92-ae7a-1b3ba3144b58
📒 Files selected for processing (1)
ci-operator/step-registry/hypershift/kubevirt/install/create-external-infra-kubeconfig/hypershift-kubevirt-install-create-external-infra-kubeconfig-commands.sh
| oc apply -f - <<EOF | ||
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| kind: ClusterRoleBinding | ||
| metadata: | ||
| name: kv-external-infra-network-reader-binding | ||
| subjects: | ||
| - kind: ServiceAccount | ||
| name: ${SA_NAME} | ||
| namespace: ${EXTERNAL_INFRA_NS} | ||
| roleRef: | ||
| apiGroup: rbac.authorization.k8s.io | ||
| kind: ClusterRole | ||
| name: kv-external-infra-network-reader |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where this step is referenced and whether EXTERNAL_INFRA_NS is overridden.
rg -n -C3 'hypershift-kubevirt-install-create-external-infra-kubeconfig|EXTERNAL_INFRA_NS' ci-operator
# Confirm the binding name is currently fixed.
rg -n -C2 'kv-external-infra-network-reader-binding' ci-operator/step-registry/hypershift/kubevirt/install/create-external-infra-kubeconfig/hypershift-kubevirt-install-create-external-infra-kubeconfig-commands.shRepository: openshift/release
Length of output: 16988
🏁 Script executed:
# Check if there are any other chains or workflows that might use this step with different EXTERNAL_INFRA_NS
rg -n "hypershift-kubevirt-install-create-external-infra-kubeconfig" ci-operator --type yaml
# Look for any test configurations or multi-run scenarios
fd -e yaml -e yml ci-operator | xargs grep -l "EXTERNAL_INFRA_NS" | head -20Repository: openshift/release
Length of output: 715
🏁 Script executed:
# Find all jobs/tests that reference the chain containing this step
rg -n "hypershift-kubevirt-run-e2e-external" ci-operator --type yaml -A 5 -B 5
# Check if EXTERNAL_INFRA_NS is ever overridden in any job configs
rg -n "EXTERNAL_INFRA_NS" ci-operator --type yaml | grep -v "default:" | head -30Repository: openshift/release
Length of output: 9375
🏁 Script executed:
# Search for any job configs that might override EXTERNAL_INFRA_NS
fd -e yaml ci-operator/config | xargs grep -l "EXTERNAL_INFRA_NS" 2>/dev/null | head -20
# If found, show the context
fd -e yaml ci-operator/config | xargs grep -B5 -A5 "EXTERNAL_INFRA_NS" 2>/dev/null | head -50Repository: openshift/release
Length of output: 817
🏁 Script executed:
# Search in the correct directory
find ci-operator/config -name "*.yaml" -exec grep -l "EXTERNAL_INFRA_NS" {} \; 2>/dev/null | head -20
# Show the context
find ci-operator/config -name "*.yaml" -exec grep -B3 -A3 "EXTERNAL_INFRA_NS" {} + 2>/dev/null | head -80Repository: openshift/release
Length of output: 43
🏁 Script executed:
# Check if there are any comments or documentation about reusing the same infra cluster
rg -i "shared.*infra|reuse.*infra|multi.*run" ci-operator/step-registry/hypershift/kubevirt --type yaml --type sh -B2 -A2
# Look at the actual script to understand the scope better
cat ci-operator/step-registry/hypershift/kubevirt/install/create-external-infra-kubeconfig/hypershift-kubevirt-install-create-external-infra-kubeconfig-commands.shRepository: openshift/release
Length of output: 3751
Include EXTERNAL_INFRA_NS in the ClusterRoleBinding name to prevent cross-run clobbering.
The ClusterRoleBinding kv-external-infra-network-reader-binding is cluster-scoped with a fixed name. When this step runs multiple times against the same infra cluster with different EXTERNAL_INFRA_NS values, the second run overwrites the first run's binding, breaking its permissions.
Proposed change
+CRB_NAME="kv-external-infra-network-reader-binding-${EXTERNAL_INFRA_NS}"
oc apply -f - <<EOF
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
- name: kv-external-infra-network-reader-binding
+ name: ${CRB_NAME}
subjects:
- kind: ServiceAccount
name: ${SA_NAME}
namespace: ${EXTERNAL_INFRA_NS}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: kv-external-infra-network-reader
EOF🤖 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
`@ci-operator/step-registry/hypershift/kubevirt/install/create-external-infra-kubeconfig/hypershift-kubevirt-install-create-external-infra-kubeconfig-commands.sh`
around lines 140 - 152, The ClusterRoleBinding uses a fixed cluster-scoped name
(kv-external-infra-network-reader-binding) which causes different runs with
different EXTERNAL_INFRA_NS to clobber each other; update the ClusterRoleBinding
metadata.name to include the EXTERNAL_INFRA_NS (for example append or
interpolate ${EXTERNAL_INFRA_NS}) so the binding becomes unique per external
infra namespace, and ensure any references to that binding name (if any
elsewhere in the script) are updated accordingly; keep the subjects
(ServiceAccount name ${SA_NAME} and namespace ${EXTERNAL_INFRA_NS}) and roleRef
unchanged.
|
/pj-rehearse pull-ci-openshift-cluster-api-provider-kubevirt-release-4.22-e2e-hypershift-kubevirt |
|
@orenc1: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@orenc1: 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. |
|
/pj-rehearse ack |
|
@orenc1: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: orenc1, qinqon 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 |
…a SA (openshift#79081) The hypershift-operator now creates a virt-launcher NetworkPolicy on the external infrastructure cluster to enforce network isolation for KubeVirt guest VMs (openshift/hypershift#8056, OCPBUGS-78575). A new ValidKubeVirtInfraNetworkPolicyRBAC condition was added that requires the external infra service account to have networking.k8s.io/networkpolicies permissions and cluster-scoped read access to networks.config.openshift.io for CIDR-based egress rules. The CI step that provisions the restricted infra kubeconfig was not updated to include these permissions, so the e2e-hypershift-kubevirt job fails with forbidden errors when the operator attempts to manage the virt-launcher NetworkPolicy and read cluster network configuration. Add networking.k8s.io/networkpolicies with full verbs to the kv-external-infra-role, and create a ClusterRole+ClusterRoleBinding granting get access to networks.config.openshift.io so the operator can build CIDR-based egress restrictions for full tenant isolation. Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Oren Cohen <ocohen@redhat.com>
The hypershift-operator now creates a virt-launcher NetworkPolicy on the external infrastructure cluster to enforce network isolation for KubeVirt guest VMs (openshift/hypershift#8056, OCPBUGS-78575). A new ValidKubeVirtInfraNetworkPolicyRBAC condition was added that requires the external infra service account to have networking.k8s.io/networkpolicies permissions.
The CI step that provisions the restricted infra kubeconfig was not updated to include this permission, so the e2e-hypershift-kubevirt job fails with:
Add networking.k8s.io/networkpolicies with full verbs to the kv-external-infra-role so the operator can manage the virt-launcher NetworkPolicy on the infra cluster.
KubeVirt external infrastructure RBAC update (openshift/release CI)
What changed: The CI step that creates the external infra kubeconfig for hypershift-kubevirt (ci-operator/step-registry/hypershift/kubevirt/install/create-external-infra-kubeconfig/hypershift-kubevirt-install-create-external-infra-kubeconfig-commands.sh) now grants the kv-external-infra service account additional RBAC:
Why it matters: The hypershift operator was updated to enforce network isolation for KubeVirt VMs by creating virt-launcher NetworkPolicy objects on the external infra cluster. A new ValidKubeVirtInfraNetworkPolicyRBAC validation requires the external infra service account to have networkpolicy permissions; the previous CI-provisioned restricted kubeconfig lacked them, causing e2e-hypershift-kubevirt runs to fail with forbidden errors.
Impact: Aligns the CI-provisioned infra kubeconfig's permissions with operator runtime needs and fixes the e2e-hypershift-kubevirt test failures by enabling the operator to manage NetworkPolicies and read cluster network configuration.