OCPBUGS-104452: Fix TLS ciphers for MinVersion=1.3 - #296
Conversation
Signed-off-by: Soli0222 <github@str08.net>
Signed-off-by: Soli0222 <github@str08.net>
…-33186 Signed-off-by: funbiscuit <kokurinsv@gmail.com>
Signed-off-by: Yurii Bakurov <45154988+Yurii201811@users.noreply.github.com>
Signed-off-by: funbiscuit <kokurinsv@gmail.com>
Adds a non-blocking GitHub Actions check that warns when a pull request is classified /kind feature but does not touch the website/ documentation, so new features do not ship without their docs counterpart. The check reuses MetalLB's existing /kind convention (classify.yaml), is read-only (fork-safe) and least-privilege, emits a warning annotation plus a step summary, and is wrapped in try/catch so it never fails the build. Fixes metallb#2663 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Per review feedback, fail the job in the missing-docs case instead of only emitting a warning annotation. The job is not a required status check, so PRs remain mergeable, but a failing job is more visible than a warning. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
The unanchored regex matched the first "/kind <type>" substring anywhere in the PR body, including mentions inside prose or code spans (e.g. this PR's own description text), rather than the actual classification line. Anchor to the start of a line so only a real "/kind <type>" line is matched, per the PR template convention. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Per review feedback, gate the job on the classify.yaml-assigned kind/feature label (via a job-level if:) instead of re-deriving the PR kind from the body with a regex. This removes a second, redundant parsing point that was prone to the same class of bug just fixed, and matches the simpler pattern used by openperouter's equivalent check. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Per review feedback, replace the ~30-line actions/github-script block (which paginated the PR files API) with a short bash step that checkouts the PR and greps `git diff --name-only` for website/ changes, mirroring the approach oribon linked from openperouter/openperouter's ci.yaml. Behavior is unchanged: fail (non-blocking, since this isn't a required check) when a kind/feature PR doesn't touch website/, stay green otherwise. Dropped the now unused pull-requests: read permission. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
…container helm/speaker: use exec form for FRR tini command to prevent shell injection Replace the /bin/sh -c wrapper with a direct exec-form command list so tiniPath and dockerStartPath values are never interpreted by a shell. Signed-off-by: Peter Benis <benispeti@gmail.com>
Move the hardcoded securityContext from the FRR container template into values.yaml under speaker.frr.securityContext. The default values remain unchanged. Users can now override or remove the security context via Helm values. Signed-off-by: Peter Benis <benispeti@gmail.com>
document frr securityContext variable Signed-off-by: Peter Benis <benispeti@gmail.com>
Signed-off-by: NPastorale <npastorale@gmail.com>
Signed-off-by: somaz <genius5711@gmail.com>
Signed-off-by: somaz <genius5711@gmail.com>
We were too harsh by denying configurations for suites when tls min version is 1.3, as go itself does not deny them but rather ignores them. Here we align with this approach. Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
Signed-off-by: Ori Braunshtein <obraunsh@redhat.com>
openshift-ci-robot
added
jira/valid-reference
|
@oribon: This pull request references Jira Issue OCPBUGS-104452, which is valid. 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. |
|
[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
WalkthroughThe pull request updates Helm workload and FRR configuration, fixes status reconciliation after concurrent deletion, changes metrics and TLS handling, upgrades Go tooling, and adds documentation workflow and CRD description updates. ChangesHelm configuration
Status reconciliation recovery
Runtime behavior corrections
Build and dependency updates
Documentation and schema maintenance
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 4 warnings)
✅ Passed checks (9 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. 🔧 Trivy (0.72.0)Trivy execution failed: 2026-08-11T10:26:11Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: helm scan error: fs filter error: fs filter error: walk error range error: stat .coderabbit-opengrep-fallback.93fd759e-dc77-4898-a64f-fb1baa677923.yml: no such file or directory: range error: stat .coderabbit-opengrep-fallback.93fd759e-dc77-4898-a64f-fb1baa677923.yml: no such file or directory 🔧 golangci-lint (2.12.2)Command failed 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 |
|
/verified later @ajaggapa |
openshift-ci-robot
added
verified-later
verified
|
@oribon: This PR has been marked to be verified later 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. |
|
@oribon: This pull request references Jira Issue OCPBUGS-104452, 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.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
.github/workflows/docs-reminder.yaml (1)
25-28: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDisable unnecessary checkout credential persistence.
This job only reads Git history. Set
persist-credentials: falseso checkout does not retain the token for later steps. The checkout documentation confirms that this option disables persisted credentials. (github.com) The provided zizmor warning identifies this hardening gap; no artifact is uploaded by this workflow, so treat it as defense in depth.Proposed fix
- name: Checkout uses: actions/checkout@v4 with: fetch-depth: 0 + persist-credentials: false🤖 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 @.github/workflows/docs-reminder.yaml around lines 25 - 28, Update the actions/checkout@v4 step in the docs-reminder workflow to set persist-credentials to false alongside fetch-depth, preventing the checkout token from being retained while preserving full Git history access.Source: Linters/SAST tools
🤖 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 @.github/workflows/docs-reminder.yaml:
- Around line 25-28: Update the Checkout step’s actions/checkout reference from
the mutable v4 tag to a reviewed full commit SHA, and retain an inline comment
identifying the pinned version.
- Around line 21-23: Update the condition in the workflow’s pull request filter
to check github.event.pull_request.user.login against dependabot[bot] instead of
github.actor, while preserving the existing kind/feature label requirement.
In `@charts/metallb/values.schema.json`:
- Line 405: Rename the schema property key in the FRR resource definition from
"resources:" to "resources" so it matches the chart’s speaker.frr.resources
configuration and validates the resource object correctly.
In `@charts/metallb/values.yaml`:
- Around line 357-366: Update the FRR securityContext values to include
capabilities.drop: [ALL], and retain SYS_ADMIN only when required by the
selected FRR image; otherwise remove it. Document the
SYS_ADMIN/allowPrivilegeEscalation exception and validate runAsNonRoot for
custom image overrides, then regenerate the corresponding
charts/metallb/README.md documentation.
In `@configmaptocrs/Dockerfile`:
- Line 3: Pin the Go builder image by digest in the FROM directive for
configmaptocrs/Dockerfile, controller/Dockerfile, and speaker/Dockerfile at
lines 3-3, using the specified docker.io/golang:1.26.5 digest in all three
files.
In `@go.mod`:
- Line 140: Upgrade the google.golang.org/grpc dependency in go.mod from v1.79.3
to v1.82.1 or later, then regenerate go.sum and refresh the vendored
dependencies to match.
In `@internal/k8s/controllers/bgp_status_controller_test.go`:
- Around line 68-72: Handle the DeleteAllOf cleanup errors with Gomega success
assertions instead of discarding them. Update the cleanup closures in
internal/k8s/controllers/bgp_status_controller_test.go:68-72 and
internal/k8s/controllers/layer2_status_controller_test.go:71-75, asserting the
DeleteAllOf call succeeds while preserving the existing namespace and label
filters.
---
Nitpick comments:
In @.github/workflows/docs-reminder.yaml:
- Around line 25-28: Update the actions/checkout@v4 step in the docs-reminder
workflow to set persist-credentials to false alongside fetch-depth, preventing
the checkout token from being retained while preserving full Git history access.
🪄 Autofix
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: 9d9f5d29-20ee-4ff2-b0b7-19cea10d5b74
⛔ Files ignored due to path filters (103)
e2etest/go.workis excluded by!**/*.workgo.sumis excluded by!**/*.sumvendor/cel.dev/expr/BUILD.bazelis excluded by!**/vendor/**,!vendor/**vendor/cel.dev/expr/MODULE.bazelis excluded by!**/vendor/**,!vendor/**vendor/cel.dev/expr/checked.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/cel.dev/expr/eval.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/cel.dev/expr/explain.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/cel.dev/expr/syntax.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/cel.dev/expr/value.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/deviceauth.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/internal/doc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/internal/oauth2.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/internal/token.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/internal/transport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/oauth2.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/pkce.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/token.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/oauth2/transport.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/expr/v1alpha1/checked.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/expr/v1alpha1/eval.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/expr/v1alpha1/explain.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/expr/v1alpha1/syntax.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/expr/v1alpha1/value.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/api/httpbody/httpbody.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/rpc/errdetails/error_details.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/genproto/googleapis/rpc/status/status.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/MAINTAINERS.mdis excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/README.mdis excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/balancer.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/endpointsharding/endpointsharding.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirst.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/pickfirst/pickfirstleaf/pickfirstleaf.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/roundrobin/roundrobin.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer/subconn.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/balancer_wrapper.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/binarylog/grpc_binarylog_v1/binarylog.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/clientconn.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/credentials/credentials.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/credentials/insecure/insecure.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/credentials/tls.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/dialoptions.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/encoding/encoding.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/encoding/gzip/gzip.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/encoding/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/encoding/proto/proto.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/experimental/stats/metricregistry.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/experimental/stats/metrics.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/health/grpc_health_v1/health_grpc.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/interceptor.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/balancer/gracefulswitch/gracefulswitch.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/balancer/weight/weight.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/buffer/unbounded.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/channelz/trace.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/credentials/credentials.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/envconfig/envconfig.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/envconfig/xds.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/experimental.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/grpcsync/callback_serializer.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/grpcsync/event.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/idle/idle.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/resolver/delegatingresolver/delegatingresolver.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/resolver/dns/dns_resolver.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/stats/metrics_recorder_list.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/stats/stats.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/status/status.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/client_stream.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/controlbuf.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/flowcontrol.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/handler_server.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_client.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/http2_server.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/http_util.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/server_stream.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/internal/transport/transport.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/mem/buffer_pool.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/mem/buffer_slice.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/mem/buffers.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/picker_wrapper.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/preloader.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/resolver/resolver.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/resolver_wrapper.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/rpc_util.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/server.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/stats/handlers.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/stats/stats.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/stream.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/grpc/version.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/editionssupport/editions.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc_init.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc_lazy.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/editions.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/genid/api_gen.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/version/version.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/desc.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/desc_init.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/proto.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (36)
.github/workflows/docs-reminder.yamlapi/v1beta2/bgppeer_types.gocharts/metallb/README.mdcharts/metallb/charts/crds/templates/crds.yamlcharts/metallb/templates/controller.yamlcharts/metallb/templates/servicemonitor.yamlcharts/metallb/templates/speaker.yamlcharts/metallb/values.schema.jsoncharts/metallb/values.yamlconfig/crd/bases/metallb.io_bgppeers.yamlconfig/manifests/metallb-frr-k8s-prometheus.yamlconfig/manifests/metallb-frr-k8s.yamlconfig/manifests/metallb-frr-prometheus.yamlconfig/manifests/metallb-frr.yamlconfig/manifests/metallb-native-prometheus.yamlconfig/manifests/metallb-native.yamlconfigmaptocrs/Dockerfilecontroller/Dockerfilecontroller/main.goe2etest/go.modgo.modinternal/bgp/native/messages.gointernal/config/config.gointernal/config/validation.gointernal/k8s/controllers/bgp_status_controller.gointernal/k8s/controllers/bgp_status_controller_test.gointernal/k8s/controllers/layer2_status_controller.gointernal/k8s/controllers/layer2_status_controller_test.gointernal/k8s/k8s.gointernal/k8s/k8s_test.gointernal/k8s/webhooks/webhookv1beta2/bgppeer_webhook.gointernal/tlsconfig/tlsopt.gointernal/tlsconfig/tlsopt_test.gospeaker/Dockerfilespeaker/main.gotasks.py
💤 Files with no reviewable changes (1)
- internal/tlsconfig/tlsopt.go
| if: > | ||
| github.actor != 'dependabot[bot]' && | ||
| contains(github.event.pull_request.labels.*.name, 'kind/feature') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- workflow files ---'
git ls-files '.github/workflows/*'
printf '%s\n' '--- docs-reminder.yaml ---'
cat -n .github/workflows/docs-reminder.yaml
printf '%s\n' '--- label workflow references ---'
rg -n -C 4 'kind/feature|action-add-labels|pull_request_target|pull_request:' .github/workflowsRepository: openshift/metallb
Length of output: 7942
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
p = Path(".github/workflows/docs-reminder.yaml").read_text()
condition = "\n".join(p.splitlines()[20:23])
print("condition:")
print(condition)
print("uses_event_actor:", "github.actor" in condition)
print("uses_pr_author:", "github.event.pull_request.user.login" in condition)
events = re.search(r"types:\s*\[(.*?)\]", p).group(1).replace(" ", "").split(",")
print("workflow_events:", events)
# Model the relevant GitHub event distinction without running repository code.
events_to_qualify = [
{"event": "labeled", "github.actor": "maintainer", "pull_request.user.login": "dependabot[bot]"},
{"event": "reopened", "github.actor": "maintainer", "pull_request.user.login": "dependabot[bot]"},
]
for e in events_to_qualify:
current = e["github.actor"] != "dependabot[bot]"
author_based = e["pull_request.user.login"] != "dependabot[bot]"
print(e["event"], "current_condition_actor_filter=", current,
"author_filter=", author_based)
PYRepository: openshift/metallb
Length of output: 535
Filter Dependabot by pull request author.
github.actor is the account that triggered the workflow, not the pull request author. A maintainer can label or reopen a Dependabot pull request, which makes this condition true. Use github.event.pull_request.user.login != 'dependabot[bot]'.
🤖 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 @.github/workflows/docs-reminder.yaml around lines 21 - 23, Update the
condition in the workflow’s pull request filter to check
github.event.pull_request.user.login against dependabot[bot] instead of
github.actor, while preserving the existing kind/feature label requirement.
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin actions/checkout to a full commit SHA.
actions/checkout@v4 uses a mutable tag. Pin it to a reviewed full commit SHA and retain a version comment.
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 25-28: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/docs-reminder.yaml around lines 25 - 28, Update the
Checkout step’s actions/checkout reference from the mutable v4 tag to a reviewed
full commit SHA, and retain an inline comment identifying the pinned version.
| "metricsPort": { "type": "integer" }, | ||
| "secureMetricsPort": { "type": "integer" }, | ||
| "resources:": { "type": "object" } | ||
| "resources:": { "type": "object" }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- values.yaml ---'
sed -n '335,355p' charts/metallb/values.yaml
printf '%s\n' '--- speaker.yaml ---'
sed -n '438,455p' charts/metallb/templates/speaker.yaml
printf '%s\n' '--- values.schema.json ---'
sed -n '390,415p' charts/metallb/values.schema.json
printf '%s\n' '--- schema resource keys ---'
rg -n '"resources:?"\s*:' charts/metallb/values.schema.json
printf '%s\n' '--- JSON parse and FRR schema shape ---'
python3 - <<'PY'
import json
from pathlib import Path
schema_path = Path("charts/metallb/values.schema.json")
schema = json.loads(schema_path.read_text())
speaker = schema.get("properties", {}).get("speaker", {})
frr = speaker.get("properties", {}).get("frr", {})
print("speaker.frr schema keys:", list(frr.get("properties", {}).keys()))
print("resources schema entry:", frr.get("properties", {}).get("resources"))
PYRepository: openshift/metallb
Length of output: 3130
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json
from pathlib import Path
schema = json.loads(Path("charts/metallb/values.schema.json").read_text())
print("root keys:", list(schema))
print("root properties:", list(schema.get("properties", {})))
def find_frr(node, path="$"):
if isinstance(node, dict):
props = node.get("properties")
if isinstance(props, dict) and "frr" in props:
print("frr parent:", path)
print("parent keys:", list(props))
print("frr schema:", json.dumps(props["frr"], indent=2))
for key, value in node.items():
find_frr(value, f"{path}.{key}")
elif isinstance(node, list):
for index, value in enumerate(node):
find_frr(value, f"{path}[{index}]")
find_frr(schema)
PY
printf '%s\n' '--- schema context around speaker ---'
rg -n -C 12 '"speaker"|"\$ref"|additionalProperties|required' charts/metallb/values.schema.json | head -n 180Repository: openshift/metallb
Length of output: 6713
Rename the FRR resource schema key.
The schema defines speaker.frr["resources:"], but the chart uses speaker.frr.resources. Rename it to "resources" so Helm validates the configured resource object.
🤖 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 `@charts/metallb/values.schema.json` at line 405, Rename the schema property
key in the FRR resource definition from "resources:" to "resources" so it
matches the chart’s speaker.frr.resources configuration and validates the
resource object correctly.
| # -- Security context for the FRR container. | ||
| securityContext: | ||
| readOnlyRootFilesystem: true | ||
| allowPrivilegeEscalation: false | ||
| capabilities: | ||
| add: | ||
| - NET_ADMIN | ||
| - NET_RAW | ||
| - SYS_ADMIN | ||
| - NET_BIND_SERVICE |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- relevant chart files ---'
git ls-files 'charts/metallb/values.yaml' 'charts/metallb/templates/speaker.yaml' 'charts/metallb/README.md' 'charts/metallb/Chart.yaml'
printf '%s\n' '--- values.yaml security-context section ---'
cat -n charts/metallb/values.yaml | sed -n '330,385p'
printf '%s\n' '--- speaker template security-context rendering ---'
cat -n charts/metallb/templates/speaker.yaml | sed -n '390,445p'
printf '%s\n' '--- FRR image and security-context references ---'
rg -n -C 3 'frr|securityContext|runAsNonRoot|SYS_ADMIN|NET_ADMIN|NET_RAW|NET_BIND_SERVICE|allowPrivilegeEscalation|capabilities' charts/metallb
printf '%s\n' '--- README generated values around FRR securityContext ---'
rg -n -C 12 'Security context for the FRR container|NET_ADMIN|SYS_ADMIN|NET_RAW|NET_BIND_SERVICE|readOnlyRootFilesystem' charts/metallb/README.md || true
printf '%s\n' '--- chart metadata and image defaults ---'
cat -n charts/metallb/Chart.yaml | sed -n '1,100p'
rg -n -C 4 'image:|repository:|tag:|digest:' charts/metallb/values.yaml charts/metallb/templatesRepository: openshift/metallb
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- README FRR defaults ---'
sed -n '124,145p' charts/metallb/README.md
printf '%s\n' '--- README security-context rows ---'
rg -n -C 2 'speaker\.frr\.securityContext|readOnlyRootFilesystem|allowPrivilegeEscalation|capabilities|NET_ADMIN|NET_RAW|SYS_ADMIN|NET_BIND_SERVICE' charts/metallb/README.md
printf '%s\n' '--- all chart documentation for FRR capability rationale ---'
rg -n -i -C 3 'sys.admin|net.admin|net.raw|bind.service|capabilit|non.root|privilege' README.md docs charts .github 2>/dev/null | head -n 250 || true
printf '%s\n' '--- local references to the default FRR image ---'
rg -n -C 5 'quay\.io/frrouting/frr|frr:10\.5\.3|speaker\.frr\.securityContext' . --glob '!charts/metallb/README.md' --glob '!vendor/**' | head -n 250Repository: openshift/metallb
Length of output: 25634
🌐 Web query:
Kubernetes container securityContext allowPrivilegeEscalation CAP_SYS_ADMIN always true official documentation
💡 Result:
According to official Kubernetes documentation, the allowPrivilegeEscalation field in a container's securityContext is always treated as true when the container is either run as privileged or has the CAP_SYS_ADMIN capability [1]. In practice, Kubernetes validation logic rejects configurations where allowPrivilegeEscalation is explicitly set to false while the container is configured to be privileged or to include the CAP_SYS_ADMIN capability [2][3]. The setting controls the no_new_privs flag on the container process; when allowPrivilegeEscalation is false, this flag is set, preventing processes from gaining more privileges than their parent process (such as via setuid binaries) [1][2][4]. It is important to note that the Kubernetes validation for this restriction specifically checks for the string "CAP_SYS_ADMIN" [5]. There have been identified inconsistencies where other dangerous capabilities (like CAP_SYS_MODULE or CAP_NET_ADMIN) or the variation "SYS_ADMIN" (without the "CAP_" prefix) may not trigger the same validation rejection, even though they can also enable privilege escalation [5][6][7]. Consequently, while the documentation states that CAP_SYS_ADMIN forces allowPrivilegeEscalation to true, users should be aware that the current implementation may not automatically block all equivalent privilege-escalating configurations [6].
Citations:
- 1: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/
- 2: https://github.com/kubernetes/design-proposals-archive/blob/main/auth/no-new-privs.md
- 3: Default value for Pod.spec.containers.securityContext.allowPrivilegeEscalation kubernetes/website#30104
- 4: https://kubernetes.io/docs/concepts/security/linux-kernel-security-constraints/
- 5: Incorrect validation of
allowPrivilegeEscalationwithSYS_ADMINcapability kubernetes/kubernetes#119568 - 6: allowPrivilegeEscalation:false insufficient for privilege escalation prevention kubernetes/kubernetes#131336
- 7: Inconsistent validation behaviour when adding capabilities kubernetes/kubernetes#115142
🌐 Web query:
FRRouting frr Docker image 10.5.3 required Linux capabilities NET_ADMIN NET_RAW SYS_ADMIN official
💡 Result:
The FRRouting (FRR) Docker image, including version 10.5.3, requires elevated Linux capabilities to function because it interacts directly with the host's networking stack to manage routing protocols [1][2]. While the exact required capabilities can depend on the specific routing daemons (e.g., bgpd, ospfd, zebra) you have enabled [1], it is standard practice and often necessary to provide a specific set of capabilities to the container. The commonly cited requirements for a functional FRR container include [1][3][4]: - NET_ADMIN: Required for modifying network interfaces, routing tables, and firewall rules [1]. - NET_RAW: Required for constructing raw packets (essential for many routing protocols) [1]. - SYS_ADMIN: Frequently required for configuring various kernel-level networking parameters [1]. - NET_BIND_SERVICE: Often required to bind to low-numbered privileged ports [1][3]. The official guidance emphasizes that when running the container, you should grant the full set of capabilities needed by any of the daemons you plan to enable [1]. Many real-world deployments, such as those using FRR in Kubernetes or Docker Compose, explicitly grant these four capabilities to ensure all protocol daemons function correctly [3][4][5][6]. If you are using a security-hardened environment, you may be able to restrict these based on a precise list of active daemons, but granting these is the standard approach for full functionality [1]. Always ensure your host system's kernel settings (e.g., IP forwarding) are configured appropriately to support the routing operations performed by the container [7].
Citations:
- 1: https://hub.docker.com/hardened-images/catalog/dhi/frr/guides
- 2: https://hub.docker.com/r/frrouting/frr
- 3: https://raw.githubusercontent.com/metallb/frr-k8s/v0.0.17/config/all-in-one/frr-k8s.yaml
- 4: SDN-5072: Deploy FRR-K8s cluster-network-operator#2450
- 5: https://github.com/choisungwook/portfolio/blob/master/computer_science/BGP_protocol/docker-compose-second.yml
- 6: https://github.com/annetutil/contribs/blob/main/labs/topologies/lab12_dc_arista_cisco_frr/docker-compose.override.yml
- 7: https://docs.frrouting.org/en/stable-10.3/installation.html
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- FRR container and init-container sections ---'
sed -n '421,540p' charts/metallb/templates/speaker.yaml
printf '%s\n' '--- generated legacy FRR manifest security contexts and resources ---'
sed -n '2470,2675p' config/manifests/metallb-frr-prometheus.yaml | \
rg -n -C 5 'name: frr$|name: reloader|name: frr-metrics|securityContext:|capabilities:|resources:|readinessProbe:|livenessProbe:|startupProbe:|runAsNonRoot'
printf '%s\n' '--- capability and non-root settings in chart and legacy manifests ---'
rg -n -C 2 'drop:|add:|SYS_ADMIN|NET_ADMIN|NET_RAW|NET_BIND_SERVICE|runAsNonRoot|resources: \{\}|readinessProbe:' \
charts/metallb/templates/speaker.yaml charts/metallb/values.yaml \
config/frr/speaker-patch.yaml config/manifests/metallb-frr-prometheus.yaml | head -n 300
printf '%s\n' '--- upstream references available in repository metadata ---'
rg -n -i 'frrouting/frr|dockerfile|capabilities|SYS_ADMIN' .github docs config charts tasks.py \
--glob '!config/manifests/*.yaml' | head -n 200 || trueRepository: openshift/metallb
Length of output: 17772
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- FRR upstream repository metadata ---'
curl -fsSL 'https://api.github.com/repos/FRRouting/frr/contents/docker' |
jq -r '.[] | [.name, .download_url] | `@tsv`' | head -n 80
printf '%s\n' '--- FRR repository references to container capabilities ---'
curl -fsSL 'https://api.github.com/search/code?q=repo%3AFRRouting%2Ffrr+SYS_ADMIN' |
jq -r '.message // empty, (.items[]?.html_url // empty)' || true
printf '%s\n' '--- MetalLB frr-k8s configuration capability references ---'
curl -fsSL 'https://raw.githubusercontent.com/metallb/frr-k8s/main/config/all-in-one/frr-k8s.yaml' |
rg -n -C 8 'capabilities:|SYS_ADMIN|NET_ADMIN|NET_RAW|NET_BIND_SERVICE|runAsNonRoot'Repository: openshift/metallb
Length of output: 2137
🌐 Web query:
site:github.com/FRRouting/frr Dockerfile 10.5.3 SYS_ADMIN NET_ADMIN NET_RAW
💡 Result:
The references to SYS_ADMIN, NET_ADMIN, and NET_RAW in the context of FRRouting (FRR) version 10.5.3 relate to Linux capabilities required for the proper operation of the FRR routing suite within Docker containers [1][2][3][4]. FRR services such as zebra, bgpd, ospfd, and watchfrr perform tasks that require elevated system privileges, including managing network namespaces (for VRF support) and manipulating network interfaces [3]. Specifically: - CAP_SYS_ADMIN: Required by components like ospfd to perform operations such as switching network namespaces [3]. - CAP_NET_ADMIN: Required for managing network configurations, routing tables, and interface settings [1][3]. - CAP_NET_RAW: Required for crafting and accessing raw network packets [1][3]. In Docker environments, these capabilities are not granted by default for security reasons. Users running FRR containers often encounter permission errors if these capabilities are not explicitly provided at runtime [1][3]. To resolve these errors, operators must either run the container with the --privileged flag (which grants all capabilities) or, more securely, use --cap-add to grant only the specific capabilities required (e.g., --cap-add=NET_ADMIN --cap-add=NET_RAW --cap-add=SYS_ADMIN) [1][3]. Alternatively, some users may configure file-level capabilities using setcap on the specific FRR binaries within the container image, although this requires careful management of container security policies [1][2].
Citations:
- 1: docker: permission deny when running container for Ubuntu-20 and Ubuntu-22 FRRouting/frr#16710
- 2: Why offical docker image not setting required file capabilities? FRRouting/frr#9968
- 3: ospfd requires cap_sys_admin FRRouting/frr#8681
- 4: https://github.com/FRRouting/frr/releases/tag/frr-10.5.3
🌐 Web query:
site:github.com/metallb/metallb "speaker.frr.securityContext" "SYS_ADMIN"
💡 Result:
In the context of the MetalLB project and its FRRouting (FRR) integration, speaker.frr.securityContext refers to a configuration setting in the MetalLB Helm chart that defines the Kubernetes security context for the FRR sidecar container [1]. Historically, the FRR container in MetalLB required elevated Linux capabilities, including NET_ADMIN, NET_RAW, SYS_ADMIN, and NET_BIND_SERVICE, to perform networking operations such as BGP peering and ARP/NDP responses [1][2]. The addition of the speaker.frr.securityContext setting allowed operators to make these security settings configurable, enabling them to harden the container or adjust for custom images [1]. It is important to note: 1. Security Best Practices: While these capabilities were previously standard for the FRR sidecar, the project has historically moved to remove unnecessary elevated privileges (such as SYS_ADMIN) whenever possible to reduce the risk of container breakouts [3][4]. 2. Deprecation Warning: MetalLB has deprecated the FRR mode in favor of the frr-k8s mode [5]. Operators are encouraged to migrate to frr-k8s, which is the current default BGP backend [5]. 3. Current Implementation: In modern versions of the MetalLB chart, this configuration allows users to define the specific security context applied to the FRR container, maintaining the defaults that match required capabilities while providing flexibility for specific deployment environments [1].
Citations:
- 1: feat(chart): add configurable paths for frr container metallb/metallb#3096
- 2: https://github.com/metallb/metallb/blob/main/charts/metallb/templates/speaker.yaml
- 3: Test if we need more than CAP_NET_RAW metallb/metallb#762
- 4: Remove NET_ADMIN and SYS_ADMIN capabilities metallb/metallb#764
- 5: https://github.com/metallb/metallb/blob/main/charts/metallb/values.yaml
Drop all capabilities and document FRR security requirements.
The template renders speaker.frr.securityContext directly. Add capabilities.drop: [ALL]; the current default and generated README omit it.
Retain SYS_ADMIN only when the selected FRR image requires it. Kubernetes documents CAP_SYS_ADMIN as forcing effective allowPrivilegeEscalation: true. Document this exception or remove the capability. Validate runAsNonRoot for custom image overrides. Regenerate charts/metallb/README.md.
🤖 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 `@charts/metallb/values.yaml` around lines 357 - 366, Update the FRR
securityContext values to include capabilities.drop: [ALL], and retain SYS_ADMIN
only when required by the selected FRR image; otherwise remove it. Document the
SYS_ADMIN/allowPrivilegeEscalation exception and validate runAsNonRoot for
custom image overrides, then regenerate the corresponding
charts/metallb/README.md documentation.
Sources: Path instructions, MCP tools
| # syntax=docker/dockerfile:1.2 | ||
|
|
||
| FROM --platform=$BUILDPLATFORM docker.io/golang:1.25.11 AS builder | ||
| FROM --platform=$BUILDPLATFORM docker.io/golang:1.26.5 AS builder |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
docker buildx imagetools inspect docker.io/golang:1.26.5Repository: openshift/metallb
Length of output: 199
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- Dockerfile references ---'
for file in configmaptocrs/Dockerfile controller/Dockerfile speaker/Dockerfile; do
printf '%s\n' "$file"
sed -n '1,12p' "$file"
done
printf '%s\n' '--- Registry manifest resolution ---'
token="$(
curl -fsSL 'https://auth.docker.io/token?service=registry.docker.io&scope=repository:library/golang:pull' |
jq -r '.token'
)"
curl -fsSLI \
-H "Authorization: Bearer $token" \
-H 'Accept: application/vnd.oci.image.index.v1+json, application/vnd.docker.distribution.manifest.list.v2+json' \
'https://registry-1.docker.io/v2/library/golang/manifests/1.26.5' |
awk 'BEGIN{IGNORECASE=1} /^docker-content-digest:|^content-type:/ {print}'Repository: openshift/metallb
Length of output: 1469
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
token="$(
curl -fsSL 'https://auth.docker.io/token?service=registry.docker.io&scope=repository:library/golang:pull' |
jq -r '.token'
)"
digest='sha256:7caba5286b4c3613a337b709c573047d8ae62ee76106647313b61e72b99f20af'
manifest="$(
curl -fsSL \
-H "Authorization: Bearer $token" \
-H 'Accept: application/vnd.oci.image.index.v1+json, application/vnd.docker.distribution.manifest.list.v2+json' \
"https://registry-1.docker.io/v2/library/golang/manifests/$digest"
)"
printf '%s\n' "$manifest" | jq -c '{
schemaVersion,
mediaType,
digest: "'"$digest"'",
platforms: ([.manifests[]? | {
os: .platform.os,
architecture: .platform.architecture,
variant: (.platform.variant // null)
}] // [])
}'Repository: openshift/metallb
Length of output: 1206
Pin all Go builder images by digest.
Use docker.io/golang:1.26.5@sha256:7caba5286b4c3613a337b709c573047d8ae62ee76106647313b61e72b99f20af in all three Dockerfiles.
📍 Affects 3 files
configmaptocrs/Dockerfile#L3-L3(this comment)controller/Dockerfile#L3-L3speaker/Dockerfile#L3-L3
🤖 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 `@configmaptocrs/Dockerfile` at line 3, Pin the Go builder image by digest in
the FROM directive for configmaptocrs/Dockerfile, controller/Dockerfile, and
speaker/Dockerfile at lines 3-3, using the specified docker.io/golang:1.26.5
digest in all three files.
Source: Path instructions
| google.golang.org/protobuf v1.36.7 // indirect | ||
| google.golang.org/genproto/googleapis/api v0.0.0-20251202230838-ff82c1b0f217 // indirect | ||
| google.golang.org/genproto/googleapis/rpc v0.0.0-20251202230838-ff82c1b0f217 // indirect | ||
| google.golang.org/grpc v1.79.3 // indirect |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
go mod why -m google.golang.org/grpc
go list -deps ./... | rg 'google.golang.org/grpc/(internal/transport|internal/xds/rbac)' || true
curl -fsS -X POST 'https://api.osv.dev/v1/query' \
-H 'Content-Type: application/json' \
-d '{"package":{"ecosystem":"Go","name":"google.golang.org/grpc"},"version":"v1.79.3"}'Repository: openshift/metallb
Length of output: 9809
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- go.mod context ---'
sed -n '125,148p' go.mod
printf '%s\n' '--- executable package declarations ---'
rg -n --glob '*.go' '^package main$' .
printf '%s\n' '--- grpc imports ---'
rg -n --glob '*.go' 'google\.golang\.org/grpc' .
printf '%s\n' '--- module references and build metadata ---'
rg -n 'go build|go install|cmd/|frr-tools|grpc' Makefile* Dockerfile* .github scripts hack deploy config 2>/dev/null || trueRepository: openshift/metallb
Length of output: 50373
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import json
import subprocess
from collections import defaultdict, deque
targets = ["./controller", "./speaker", "./frr-tools/metrics", "./frr-tools/cp-tool"]
proc = subprocess.Popen(
["go", "list", "-mod=vendor", "-json", "-deps", *targets],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
)
decoder = json.JSONDecoder()
buf = ""
packages = {}
while True:
chunk = proc.stdout.read(65536)
if not chunk:
break
buf += chunk
while buf:
try:
obj, end = decoder.raw_decode(buf)
except json.JSONDecodeError:
break
buf = buf[end:].lstrip()
if "ImportPath" in obj:
packages[obj["ImportPath"]] = obj
stderr = proc.stderr.read()
status = proc.wait()
if status:
print(stderr)
raise SystemExit(status)
reverse = defaultdict(list)
for path, obj in packages.items():
for dep in obj.get("Imports", []):
reverse[dep].append(path)
print("--- vulnerable package presence ---")
for path in (
"google.golang.org/grpc",
"google.golang.org/grpc/internal/transport",
"google.golang.org/grpc/internal/xds/rbac",
):
print(path, "present" if path in packages else "absent")
print("--- target paths to grpc transport ---")
for target in targets:
start = next((p for p in packages if p == target[2:] or p.endswith("/" + target[2:])), None)
if not start:
print(target, "not listed")
continue
queue = deque([(start, [start])])
seen = {start}
found = None
while queue:
current, path = queue.popleft()
if current == "google.golang.org/grpc/internal/transport":
found = path
break
for parent in reverse[current]:
if parent not in seen:
seen.add(parent)
queue.append((parent, path + [parent]))
print(target, " -> ".join(found) if found else "no path")
print("--- grpc version in vendor/modules.txt ---")
with open("vendor/modules.txt") as f:
for line in f:
if line.startswith("# google.golang.org/grpc "):
print(line)
PYRepository: openshift/metallb
Length of output: 531
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import json
import subprocess
from collections import deque
targets = ["controller", "speaker", "frr-tools/metrics", "frr-tools/cp-tool"]
proc = subprocess.Popen(
["go", "list", "-mod=vendor", "-json", "-deps", "./..."],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True,
)
decoder = json.JSONDecoder()
buf = ""
packages = {}
while True:
chunk = proc.stdout.read(65536)
if not chunk:
break
buf += chunk
while buf:
try:
obj, end = decoder.raw_decode(buf)
except json.JSONDecodeError:
break
buf = buf[end:].lstrip()
if "ImportPath" in obj:
packages[obj["ImportPath"]] = obj
stderr = proc.stderr.read()
status = proc.wait()
if status:
print(stderr)
raise SystemExit(status)
for target in targets:
start = next(
(path for path in packages if path == target or path.endswith("/" + target)),
None,
)
if not start:
print(f"{target}: package not listed")
continue
queue = deque([(start, [start])])
seen = {start}
paths = []
while queue:
current, path = queue.popleft()
if current in {
"google.golang.org/grpc/internal/transport",
"google.golang.org/grpc/internal/xds/rbac",
}:
paths.append(path)
continue
for dep in packages.get(current, {}).get("Imports", []):
if dep in packages and dep not in seen:
seen.add(dep)
queue.append((dep, path + [dep]))
print(f"--- {start} ---")
if paths:
for path in paths:
print(" -> ".join(path))
else:
print("no path to affected grpc packages")
PYRepository: openshift/metallb
Length of output: 1188
Upgrade google.golang.org/grpc before merging.
GO-2026-6061/GHSA-hrxh-6v49-42gf affects v1.79.3 and is fixed in v1.82.1. The speaker binary links google.golang.org/grpc/internal/transport. Upgrade to v1.82.1 or later, then regenerate go.sum and refresh the vendored dependencies.
🧰 Tools
🪛 OSV Scanner (2.4.0)
[HIGH] 140-140: google.golang.org/grpc 1.79.3: Vulnerabilities in the xDS RBAC authorization engine and the HTTP/2 transport server implementation in google.golang.org/grpc
(GO-2026-6061)
[HIGH] 140-140: google.golang.org/grpc 1.79.3: gRPC-Go: xDS RBAC and HTTP/2 Vulnerabilities
🤖 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 `@go.mod` at line 140, Upgrade the google.golang.org/grpc dependency in go.mod
from v1.79.3 to v1.82.1 or later, then regenerate go.sum and refresh the
vendored dependencies to match.
Sources: Path instructions, Linters/SAST tools
| cleanup := func() { | ||
| _ = k8sClient.DeleteAllOf(ctx, &v1beta1.ServiceBGPStatus{}, | ||
| client.InNamespace(speakerNamespace), | ||
| client.MatchingLabels{LabelServiceName: raceSvc}) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle errors from test cleanup.
A failed DeleteAllOf can leave status objects in the test namespace. The next test can then observe stale state. Assert that both cleanup calls succeed.
internal/k8s/controllers/bgp_status_controller_test.go#L68-L72: replace the discardedDeleteAllOferror with a Gomega success assertion.internal/k8s/controllers/layer2_status_controller_test.go#L71-L75: replace the discardedDeleteAllOferror with a Gomega success assertion.
As per path instructions, **/*.go requires: “Never ignore error returns.”
📍 Affects 2 files
internal/k8s/controllers/bgp_status_controller_test.go#L68-L72(this comment)internal/k8s/controllers/layer2_status_controller_test.go#L71-L75
🤖 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 `@internal/k8s/controllers/bgp_status_controller_test.go` around lines 68 - 72,
Handle the DeleteAllOf cleanup errors with Gomega success assertions instead of
discarding them. Update the cleanup closures in
internal/k8s/controllers/bgp_status_controller_test.go:68-72 and
internal/k8s/controllers/layer2_status_controller_test.go:71-75, asserting the
DeleteAllOf call succeeds while preserving the existing namespace and label
filters.
Source: Path instructions
|
@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. |
|
/lgtm |
|
@oribon: Jira Issue OCPBUGS-104452: 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-104452 has not been moved to the MODIFIED state. This PR is marked as verified-later. Jira issue(s) in the title of this PR will require post-merge verification. After testing, it must be manually moved to the 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. |
10 participants
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
0disables metrics.