OCPBUGS-99270: (Sync 29-07-2026) Use Custom SCC for Speaker - #337
Conversation
Replace usage of the privileged SCC for MetalLB speaker with a purpose-built SCC that grants only the capabilities the speaker actually needs (NET_RAW) and host networking, without granting full privileged access. The operator now creates the metallb-speaker SCC during reconciliation on OpenShift and annotates speaker pods with openshift.io/required-scc to explicitly request it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
Add name-scoped SCC management permissions to operator's ClusterRole in the CSV and update speaker's clusterPermissions to reference the custom metallb-speaker SCC instead of the privileged SCC. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
openshift-ci-robot
added
jira/valid-reference
|
@oribon: This pull request references Jira Issue OCPBUGS-99270, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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. |
WalkthroughThe change adds a named OpenShift SCC for MetalLB speaker pods, injects it and its required annotation into OpenShift Helm output, updates controller and ServiceAccount RBAC, and adds matching overlay and generated manifest changes. ChangesOpenShift speaker SCC integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant envConfig
participant MetalLBChart
participant speakerDaemonSet
participant SpeakerSCC
envConfig->>MetalLBChart: render with IsOpenshift=true
MetalLBChart->>speakerDaemonSet: add required SCC annotation
MetalLBChart->>SpeakerSCC: create metallb-speaker SCC
SpeakerSCC-->>MetalLBChart: return SCC manifest
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: oribon 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
|
@oribon: This pull request references Jira Issue OCPBUGS-99270, which is valid. 3 validation(s) were run on this bug
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: 4
🤖 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 `@controllers/metallb_controller.go`:
- Line 84: The SCC RBAC permissions currently combine create and resource-scoped
patch incorrectly. In controllers/metallb_controller.go at line 84, split them
into an unscoped create rule and a metallb-speaker-scoped patch rule; mirror the
same two-rule permission split in config/rbac/role.yaml at lines 125-133.
In `@pkg/helm/metallb_test.go`:
- Around line 459-467: Update the DaemonSet annotation checks in the test to
track whether the openshift.io/required-scc key exists independently of its
value, and assert that presence matches tt.isOpenshift. When the key is present,
continue validating that its value is metallb-speaker; preserve the existing SCC
assertion.
In `@pkg/helm/metallb.go`:
- Around line 90-104: The OpenShift speaker SCC must allow the FRR container
capabilities it requests. Update openshift.SpeakerSCC in pkg/helm/metallb.go to
include the capabilities retained by requiredDropCapabilities, and update the
expected capability set in pkg/helm/metallb_test.go accordingly; modify both
listed sites to keep the generated SCC and test expectation aligned.
In `@pkg/openshift/scc.go`:
- Line 24: Remove the allowHostPorts entry from the SCC configuration in the
relevant SCC definition. Only retain it if the MetalLB speaker pod template
explicitly declares a hostPort requirement; otherwise leave allowHostNetwork and
the remaining SCC permissions 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 202007ac-a40d-4d64-9124-32e8d1a75f27
📒 Files selected for processing (13)
bin/metallb-operator.yamlbundle/manifests/metallb-operator.clusterserviceversion.yamlconfig/metallb_rbac/metallb-openshift.yamlconfig/metallb_rbac/speaker_role_binding.yamlconfig/rbac/role.yamlcontrollers/metallb_controller.gohack/ocp-kustomize-overlay/rbac.yamlmanifests/stable/metallb-operator.clusterserviceversion.yamlpkg/helm/config.gopkg/helm/metallb.gopkg/helm/metallb_test.gopkg/helm/testdata/ocp-metrics-speaker.goldenpkg/openshift/scc.go
| // +kubebuilder:rbac:groups=admissionregistration.k8s.io,resources=validatingwebhookconfigurations,verbs=create;delete;get;update;patch;list;watch | ||
| // +kubebuilder:rbac:groups="",resources=secrets,verbs=create;delete;get;update;patch;list;watch | ||
| // +kubebuilder:rbac:groups=operator.openshift.io,resources=networks,verbs=get;list;watch;update; | ||
| // +kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,resourceNames=metallb-speaker,verbs=create;patch |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^controllers/metallb_controller\.go$|^config/rbac/role\.yaml$|securitycontextconstraints|rbac|zz_generated|metallb)'
echo
echo "== metallb_controller.go relevant lines =="
if [ -f controllers/metallb_controller.go ]; then
nl -ba controllers/metallb_controller.go | sed -n '60,105p'
fi
echo
echo "== role.yaml relevant lines =="
if [ -f config/rbac/role.yaml ]; then
nl -ba config/rbac/role.yaml | sed -n '110,140p'
fi
echo
echo "== rbac rules =="
rg -n -C3 'securitycontextconstraints|metallb-speaker|Create|Patch|rbac' controllers config/rbac -g '*.go' -g '*.yaml' -g '*.yml' | head -n 200Repository: openshift/metallb-operator
Length of output: 30238
🌐 Web query:
Kubernetes RBAC policy rule resourceNames create verb only patch Subresource authorization
💡 Result:
In Kubernetes RBAC, the use of resourceNames with the create verb depends on whether you are targeting a top-level resource or a subresource [1][2]. 1. Top-Level Resources: You cannot restrict create requests by resourceNames for top-level resources (e.g., pods, secrets) [1][3]. This is because the name of a new object is typically not known to the authorization system at the time the create request is evaluated, as the name is not present in the request's URL [4][3]. Specifying resourceNames in a rule for a create verb on a top-level resource will effectively result in the request being denied, as the authorization check will fail to match the request [5]. 2. Subresources: Unlike top-level resources, it is possible to use resourceNames with the create verb when targeting subresources (e.g., pods/exec, pods/attach, pods/portforward) [1][2]. In these cases, the request is directed at a specific, existing object (identified by the resourceName in the URL), making it possible for the authorization layer to evaluate the policy against that specific name [1][2]. 3. Patch and Subresources: The patch verb functions similarly to other verbs. When used on a standard resource, it can be restricted by resourceNames because the resource name is available in the URL [1]. When using Server-Side Apply (which uses PATCH at the API level), specifying resourceNames allows you to restrict the operation to specific objects [4][3]. In summary, while resourceNames cannot restrict the creation of new top-level objects, it can successfully restrict create operations on subresources and patch operations on resources where the object name is present in the request path [1][2].
Citations:
- 1: https://kubernetes.io/docs/reference/access-authn-authz/rbac/
- 2: [RBAC DOC] Note about resourceNames giving misleading informations kubernetes/website#50454
- 3: Can't create a secret when resourceNames definition is used in role kubernetes/kubernetes#80295
- 4: RBAC + create + resourceName kubernetes/kubernetes#132213
- 5: auth can-i gives wrong answer for create with resource name kubernetes/kubectl#779
🌐 Web query:
Kubernetes RBAC resources field restricts by resource name create patch securitycontextconstraints
💡 Result:
In Kubernetes RBAC, the resourceNames field acts as an optional allow-list that restricts a rule to specific instances of a resource [1][2]. However, its application with certain verbs is fundamentally limited by the structure of Kubernetes API requests [1][3]. 1. The create verb: You cannot use resourceNames to restrict create requests for top-level resources [1][4]. This is because, at the time of authorization, the API server often does not yet know the name of the object being created [1][5][4]. If you include a resourceNames field in a rule that includes the create verb, the request will be denied because the required name matching cannot occur [5][3]. - Note: This limitation does not apply to subresources (e.g., pods/exec, pods/portforward) [1][6]. For subresources, the name of the parent resource is part of the API request path, allowing RBAC to successfully enforce restrictions using resourceNames [1][6]. 2. The patch verb: The resourceNames field is fully supported and effective for the patch verb [1][7]. Because a patch request targets an existing resource via its name in the URL (e.g., /api/v1/namespaces/default/secrets/my-secret), the RBAC system can verify if that specific resource name is present in the allowed list [1][8]. Important practical considerations: - Server-Side Apply (SSA): While a standard create request cannot use resourceNames, it is often possible to achieve a similar outcome by using server-side apply (PATCH requests) to create objects, as these requests include the resource name in the URL [8][5]. - Tooling behavior: Be aware that some diagnostic tools, such as kubectl auth can-i, may not always correctly interpret or report the failure of create requests when a resourceName is specified in the role, potentially leading to confusion [8][3]. In summary, do not use resourceNames when defining roles for the create verb on top-level resources, as it will cause authorization failures [5][3]. It remains a standard and recommended practice for scoping access to specific instances when using verbs like get, update, patch, and delete [1][9].
Citations:
- 1: https://kubernetes.io/docs/reference/access-authn-authz/rbac/
- 2: https://kubernetes.io/docs/reference/kubernetes-api/definitions/resource-rule-v1-authorization/
- 3: auth can-i gives wrong answer for create with resource name kubernetes/kubectl#779
- 4: https://kubernetes.ltd/docs/reference/access-authn-authz/rbac/
- 5: RBAC + create + resourceName kubernetes/kubernetes#132213
- 6: [RBAC DOC] Note about resourceNames giving misleading informations kubernetes/website#50454
- 7: https://github.com/mhausenblas/kubernetes.github.io/blob/master/docs/admin/authorization/rbac.md
- 8: Can't create a secret when resourceNames definition is used in role kubernetes/kubernetes#80295
- 9: https://codingprotocols.com/blog/kubernetes-rbac-advanced-patterns
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== controllers/metallb_controller.go line 84 =="
awk '{printf "%6d\t%s\n", NR, $0}' controllers/metallb_controller.go | sed -n '70,95p'
echo
echo "== config/rbac/role.yaml lines 120-136 =="
awk '{printf "%6d\t%s\n", NR, $0}' config/rbac/role.yaml | sed -n '120,136p'
echo
echo "== SCC create calls/matches =="
python3 - <<'PY'
from pathlib import Path
for p in [Path('controllers/metallb_controller.go'), Path('config/rbac/role.yaml')]:
t = p.read_text()
print(f"\n{p}:")
for i, line in enumerate(t.splitlines(), 1):
if 'securitycontextconstraints' in line or 'metallb-speaker' in line or '.Create(' in line or '\tCreate(' in line:
print(f"{i}: {line}")
PYRepository: openshift/metallb-operator
Length of output: 3077
🌐 Web query:
OpenShift SCC controller create securitycontextconstraints resourceNames create patch Forbidden
💡 Result:
In OpenShift, a 403 Forbidden error when a controller attempts to create or patch a SecurityContextConstraints (SCC) resource—especially when related to resourceNames—is typically caused by one of two primary issues: 1. RBAC Restrictions on Create Requests: In Kubernetes/OpenShift RBAC, you cannot use the resourceNames field to restrict access to the create verb [1][2]. The create verb acts on the collection (e.g., /apis/security.openshift.io/v1/securitycontextconstraints), which does not contain the name of the object being created at the time of authorization [1]. If your controller's Role/ClusterRole includes resourceNames for the create verb, the authorization check will effectively deny the request [2]. To fix this, you must grant the create verb without specifying resourceNames [2]. 2. Insufficient SCC Permissions: Controllers (such as Operators) often need explicit permission to use or modify specific SCCs. - If a controller is trying to create or patch an SCC, ensure its ServiceAccount is bound to a ClusterRole that has full access to the securitycontextconstraints resource [3][4]. - Use of the use verb: If the error involves pods failing to start, the controller or the target pod's ServiceAccount may lack the use verb for the required SCC [3][4]. You can grant this via a RoleBinding or ClusterRoleBinding [3]. - Patch conflicts: Errors indicating that an SCC "cannot be fulfilled... the object has been modified" often occur during concurrent patch operations (e.g., multiple operator instances trying to reconcile the same SCC) [5]. This is a concurrency conflict, not necessarily an RBAC error; ensuring only one controller instance is active is the typical solution [5]. Recommended Troubleshooting Steps: - Check Controller Logs: Identify the specific API request verb (create vs. patch) being denied [6]. - Review RBAC Roles: If the verb is create, remove resourceNames from the rule [2]. If the verb is patch, ensure the controller has update/patch permissions for that specific resource name [1]. - Verify Binding: Ensure the ServiceAccount executing the action is correctly bound to a role that permits the necessary verbs [3][7]. - Resolve Conflicts: If the error is "operation cannot be fulfilled... object has been modified," investigate potential multiple instances of the controller attempting to update the same SCC simultaneously [5].
Citations:
- 1: Can't create a secret when resourceNames definition is used in role kubernetes/kubernetes#80295
- 2: auth can-i gives wrong answer for create with resource name kubernetes/kubectl#779
- 3: https://docs.openshift.com/container-platform/4.11/authentication/managing-security-context-constraints.html
- 4: https://docs.okd.io/4.11/authentication/managing-security-context-constraints.html
- 5: https://kb-ja.netapp.com/Cloud/Astra/Trident/Trident_-_Trident_OpenShift_SCC_Patch_Conflict_Error_because_of_multiple_deployments_of_the_Trident_operator_pod_during_upgrade
- 6: Installation via OpenShift OperatorHub fails NetApp/trident#945
- 7: https://docs.okd.io/4.13/authentication/managing-security-context-constraints.html
Keep create and patch on separate SCC rules
resourceNames scopes create for top-level resources, so this rule denies creating the metallb-speaker SCC. Split the controller generation into an unscoped create rule and a metallb-speaker-scoped patch rule, then mirror that split in config/rbac/role.yaml.
📍 Affects 2 files
controllers/metallb_controller.go#L84-L84(this comment)config/rbac/role.yaml#L125-L133
🤖 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 `@controllers/metallb_controller.go` at line 84, The SCC RBAC permissions
currently combine create and resource-scoped patch incorrectly. In
controllers/metallb_controller.go at line 84, split them into an unscoped create
rule and a metallb-speaker-scoped patch rule; mirror the same two-rule
permission split in config/rbac/role.yaml at lines 125-133.
| if obj.GetKind() == "DaemonSet" && obj.GetName() == "speaker" { | ||
| ann, _, _ := unstructured.NestedStringMap(obj.Object, "spec", "template", "metadata", "annotations") | ||
| if ann["openshift.io/required-scc"] == "metallb-speaker" { | ||
| annotationFound = true | ||
| } | ||
| } | ||
| } | ||
| g.Expect(sccFound).To(Equal(tt.expectSCC)) | ||
| g.Expect(annotationFound).To(Equal(tt.expectSCC)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that the annotation key is absent off OpenShift.
The non-OpenShift case passes when openshift.io/required-scc remains set to a different value (for example, privileged). Assert presence of the key matches tt.isOpenshift, then assert its value when present.
Proposed test adjustment
- if obj.GetKind() == "DaemonSet" && obj.GetName() == "speaker" {
- ann, _, _ := unstructured.NestedStringMap(obj.Object, "spec", "template", "metadata", "annotations")
- if ann["openshift.io/required-scc"] == "metallb-speaker" {
- annotationFound = true
- }
+ if obj.GetKind() == "DaemonSet" && obj.GetName() == "speaker" {
+ ann, _, _ := unstructured.NestedStringMap(obj.Object, "spec", "template", "metadata", "annotations")
+ scc, found := ann["openshift.io/required-scc"]
+ g.Expect(found).To(Equal(tt.isOpenshift))
+ if found {
+ g.Expect(scc).To(Equal("metallb-speaker"))
+ annotationFound = true
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if obj.GetKind() == "DaemonSet" && obj.GetName() == "speaker" { | |
| ann, _, _ := unstructured.NestedStringMap(obj.Object, "spec", "template", "metadata", "annotations") | |
| if ann["openshift.io/required-scc"] == "metallb-speaker" { | |
| annotationFound = true | |
| } | |
| } | |
| } | |
| g.Expect(sccFound).To(Equal(tt.expectSCC)) | |
| g.Expect(annotationFound).To(Equal(tt.expectSCC)) | |
| if obj.GetKind() == "DaemonSet" && obj.GetName() == "speaker" { | |
| ann, _, _ := unstructured.NestedStringMap(obj.Object, "spec", "template", "metadata", "annotations") | |
| scc, found := ann["openshift.io/required-scc"] | |
| g.Expect(found).To(Equal(tt.isOpenshift)) | |
| if found { | |
| g.Expect(scc).To(Equal("metallb-speaker")) | |
| annotationFound = true | |
| } | |
| } | |
| } | |
| g.Expect(sccFound).To(Equal(tt.expectSCC)) | |
| g.Expect(annotationFound).To(Equal(tt.expectSCC)) |
🤖 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 `@pkg/helm/metallb_test.go` around lines 459 - 467, Update the DaemonSet
annotation checks in the test to track whether the openshift.io/required-scc key
exists independently of its value, and assert that presence matches
tt.isOpenshift. When the key is present, continue validating that its value is
metallb-speaker; preserve the existing SCC assertion.
| if isSpeakerDaemonSet(obj) && envConfig.IsOpenshift { | ||
| if err := setRequiredSCCAnnotationForSpeaker(obj); err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
| if isServiceMonitor(obj) && envConfig.IsOpenshift { | ||
| err := setOcpMonitorFields(obj) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
| } | ||
| if envConfig.IsOpenshift { | ||
| objs = append(objs, openshift.SpeakerSCC()) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)pkg/helm/metallb\.(go|test\.go)$|controllers/metallb_controller\.go$|config/rbac/role\.yaml$|openshift' || true
echo "== metallb.go outline =="
ast-grep outline pkg/helm/metallb.go --view expanded || true
echo "== metallb.go relevant =="
sed -n '1,160p' pkg/helm/metallb.go
echo "== metallb_test.go relevant =="
sed -n '420,470p' pkg/helm/metallb_test.go
echo "== search speaker SCC and frr containers/capabilities =="
rg -n "SpeakerSCC|frr|NET_ADMIN|NET_RAW|NET_BIND_SERVICE|SYS_ADMIN|allowedCapabilities|requiredDropCapabilities|automountServiceAccountToken|securityContext|capabilities" -S .Repository: openshift/metallb-operator
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== OpenShift SCC docs not available locally, inspect local docs =="
git ls-files | rg '(^|/)docs/|openshift|metallb' || true
echo "== parse relevant snippets programmatically =="
python3 - <<'PY'
import pathlib, re, yaml
met = pathlib.Path("pkg/helm/metallb.go").read_text()
tests = pathlib.Path("pkg/helm/metallb_test.go").read_text()
for pat in [
r"func\s+SpeakerSCC\(\)\s*\*v1\.SecurityContextConstraints\s*\{.*?(?=\nfunc )",
]:
print("FOUND", pat)
for name in ["SpeakerSCC", "setRequiredSCCAnnotationForSpeaker", "renderFRRContainerCapabilities"]:
print(name, "present", name in met)
# extract frr capability related tests with some context
for m in re.finditer(r'unstructured\.NestedStringSlice\(obj\.Object,\s*"allowedCapabilities"\)|allowedCapabilities|NET_ADMIN|NET_RAW|NET_BIND_SERVICE|SYS_ADMIN', tests):
start=max(0,m.start()-200); end=min(len(tests),m.end()+300)
print(f"== test context at {m.start()} ==")
print(tests[start:end])
# extract function bodies with naive brace matching
def extract_func(name, txt):
i=txt.find(f"func {name}(")
if i<0: return None
j=txt.find("{", i)
bal=0
for k in range(j, len(txt)):
if txt[k]=="{": bal+=1
elif txt[k]=="}":
bal-=1
if bal==0:
return txt[i:k+1]
return None
for name in ["SpeakerSCC", "setRequiredSCCAnnotationForSpeaker"]:
body=extract_func(name, met)
print(f"== {name} body ==")
print(body[:5000] if body else "NONE")
PYRepository: openshift/metallb-operator
Length of output: 12299
🌐 Web query:
OpenShift SCC allowedCapabilities requiredDropCapabilities SecurityContextConstraints additionalCapabilities Pod
💡 Result:
In OpenShift, SecurityContextConstraints (SCCs) control the Linux capabilities of pods using three primary parameters: allowedCapabilities, defaultAddCapabilities, and requiredDropCapabilities [1][2]. 1. allowedCapabilities: This defines the list of capabilities that a pod author is permitted to request for their container [3][4]. If a capability is listed here, the pod manifest may choose to add it [1][2]. An empty list means no additional capabilities can be requested, while using the wildcard '*' allows all capabilities [3][1]. 2. defaultAddCapabilities: This defines the set of capabilities automatically added to a container unless the pod specification explicitly drops them [3][5][4]. 3. requiredDropCapabilities: This defines the list of capabilities that must be stripped from the container and cannot be added by the pod author [3][4]. You can set this to 'ALL' to ensure all capabilities are dropped [1][6]. Crucially, you must not list the same capability in both allowedCapabilities and requiredDropCapabilities, as this configuration is forbidden [3][1][6]. Pod manifest authors can override default behavior within the bounds defined by these SCC parameters by requesting additional capabilities or explicitly dropping them in the pod spec [1][2].
Citations:
- 1: https://docs.okd.io/latest/authentication/managing-security-context-constraints.html
- 2: https://docs.redhat.com/en/documentation/openshift_container_platform/4.21/html/authentication_and_authorization/managing-pod-security-policies
- 3: https://docs.okd.io/latest/rest_api/security_apis/securitycontextconstraints-security-openshift-io-v1.html
- 4: https://docs.redhat.com/en/documentation/openshift_container_platform/4.19/html/security_apis/securitycontextconstraints-security-openshift-io-v1
- 5: https://docs.okd.io/4.22/rest_api/security_apis/securitycontextconstraints-security-openshift-io-v1.html
- 6: https://docs.redhat.com/en/documentation/openshift_container_platform/4.20/html/authentication_and_authorization/managing-pod-security-policies
Allow the FRR container’s requested capabilities in the speaker SCC.
metallb-speaker only allows NET_RAW, while the rendered frr container requests NET_ADMIN, NET_RAW, SYS_ADMIN, and NET_BIND_SERVICE; OpenShift SCCs use allowedCapabilities as the allow-list, so this SCC will reject the Speaker Pod when the pod is selected for admission. Update the expected SCC capability set in pkg/helm/metallb_test.go to include the capabilities still kept by requiredDropCapabilities, or add only the capability(s) actually required to be added.
📍 Affects 2 files
pkg/helm/metallb.go#L90-L104(this comment)pkg/helm/metallb_test.go#L454-L457
🤖 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 `@pkg/helm/metallb.go` around lines 90 - 104, The OpenShift speaker SCC must
allow the FRR container capabilities it requests. Update openshift.SpeakerSCC in
pkg/helm/metallb.go to include the capabilities retained by
requiredDropCapabilities, and update the expected capability set in
pkg/helm/metallb_test.go accordingly; modify both listed sites to keep the
generated SCC and test expectation aligned.
| "allowHostIPC": false, | ||
| "allowHostNetwork": true, | ||
| "allowHostPID": false, | ||
| "allowHostPorts": true, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C4 -g '*.go' -g '*.yaml' -g '*.yml' 'hostPort:'Repository: openshift/metallb-operator
Length of output: 164
🏁 Script executed:
#!/bin/bash
set -u
echo "## files matching scc.go"
fd -a 'scc\.go$' .
echo "## target file outline"
if [ -f pkg/openshift/scc.go ]; then
ast-grep outline pkg/openshift/scc.go --view compact || true
echo "## target file"
cat -n pkg/openshift/scc.go
else
echo "target file not found"
fi
echo "## host networking/port references"
rg -n -C3 'host(network|Port|Ports)|allowHostPorts|allowHostNetwork|NET_RAW|SecurityContextConstraints|speaker.*pod|Pod ' pkg/openshift pkg manifest charts config deploy 2>/dev/null || true
echo "## changed files/stat"
git diff --stat || true
git diff --name-only || trueRepository: openshift/metallb-operator
Length of output: 13962
Remove allowHostPorts unless the MetalLB speaker pod template declares one.
allowHostPorts broadens the SCC independently of allowHostNetwork, and no hostPort use was found in the repository. Keep it only if a speaker container is updated to require host 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 `@pkg/openshift/scc.go` at line 24, Remove the allowHostPorts entry from the
SCC configuration in the relevant SCC definition. Only retain it if the MetalLB
speaker pod template explicitly declares a hostPort requirement; otherwise leave
allowHostNetwork and the remaining SCC permissions unchanged.
|
@oribon: 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. |
|
/verified by ci |
openshift-ci-robot
added
the
verified
|
@oribon: This PR has been marked as verified by 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 |
|
@oribon: Jira Issue OCPBUGS-99270: Some pull requests linked via external trackers have merged: The following pull request, linked via external tracker, has not merged: All associated pull requests must be merged or unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with Jira Issue OCPBUGS-99270 has not been moved to the MODIFIED state. This PR is marked as verified. If the remaining PRs listed above are marked as verified before merging, the issue will automatically be moved to VERIFIED after all of the PRs merge. 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. |
3 participants
Summary by CodeRabbit
New Features
Bug Fixes