Skip to content

Migrate from kube-rbac-proxy to Controller-Runtime’s feature WithAuthenticationAndAuthorization - #1352

Open
Siddharth-Kannan wants to merge 8 commits into
mainfrom
VAULT-30534-vso-migrate-off-gcr-io-kubebuilder-kube-rbac-proxy
Open

Siddharth-Kannan wants to merge 8 commits into
mainfrom
VAULT-30534-vso-migrate-off-gcr-io-kubebuilder-kube-rbac-proxy

Conversation

@Siddharth-Kannan

@Siddharth-Kannan Siddharth-Kannan commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Images provided under gcr.io/kubebuilder/ are unavailable since March 18, 2025.This migration from kube-rbac-proxy drops the sidecar and protects /metrics in-process with controller-runtime's filters.WithAuthenticationAndAuthorization, which runs the same TokenReview/SubjectAccessReview checks. The manager now serves /metrics over HTTPS on 8443 itself.

  • --metrics-secure and --enable-http2 flags (both default false); HTTP/2 off by default on the metrics and webhook servers.
  • The named port had to move onto the manager. The Service uses targetPort: https, a name the sidecar owned. Miss it and the Service resolves to no endpoints — metrics break silently.

Testing:

  • Chart renderingdeployment.bats 89/89; confirmed the rendered Deployment has exactly one
    container carrying --metrics-secure=true and containerPort: 8443 named https, and that
    the metrics Service still resolves via targetPort: https
  • Authn/authz (envtest) — no token → 401, malformed token → 500, valid token without the
    reader role → 403, token with the role → 200
  • TLS/HTTP2 — all four --metrics-secure × --enable-http2 combinations, including confirming
    hostile h2c clients are rejected and plain HTTP against the TLS port fails
  • Integration suite — full run on kind via both the kustomize and Helm paths, and on OpenShift
    amd64/s390x.
  • Live scrapeService endpoints resolve to …:8443, metrics returned over HTTP/1.1; on
    OpenShift the cluster's own prometheus-k8s ServiceAccount scrapes successfully

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.

  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.

  • If applicable, I've documented the impact of any changes to security controls.

    Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

@Siddharth-Kannan
Siddharth-Kannan marked this pull request as ready for review September 13, 2026 11:58
@Siddharth-Kannan
Siddharth-Kannan requested review from a team as code owners September 13, 2026 11:59

@siyer-corp siyer-corp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Provided some initial comments.

Comment thread chart/values.yaml
Comment thread test/unit/deployment.bats
Comment thread config/default-openshift/manager_ubi_auth_proxy_patch.yaml Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Secure metrics flags are lost in persistence Kustomize overlays, and migration guidance contains contradictory instructions.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 2 Low severity

Open (3)
What changed in this PR

Migrates metrics authentication from kube-rbac-proxy to controller-runtime’s in-process authentication/authorization filter.

Changes:

  • Adds secure metrics and HTTP/2 configuration to the manager.
  • Removes sidecar images, versions, Helm values, and OpenShift patches.
  • Updates manifests, RBAC, tests, dependencies, and documentation.
File Description
main.go Adds secure metrics, authentication, TLS, and HTTP/2 handling.
chart/​templates/​deployment.yaml Removes the sidecar and exposes the manager’s HTTPS metrics port.
config/​default/​manager_auth_proxy_patch.yaml Configures secure metrics for Kustomize deployments.
config/​default-openshift/​* Removes the OpenShift-specific proxy patch.
config/​rbac/​* Documents metrics authentication RBAC resources.
chart/​values.yaml Removes proxy configuration values.
test/​unit/​deployment.bats Updates deployment tests for the single manager container.
test/​chart/​chart_test.go Preserves proxy overrides for legacy chart upgrades.
scripts/​check-versions.sh Removes obsolete proxy version checks.
Makefile Removes obsolete proxy variables.
go.mod, go.sum Adds controller-runtime dependencies.
docs/​helm.mdx Updates generated Helm documentation.
hack/​helm-reference-gen/​fixtures/​* Updates documentation fixtures.
CHANGELOG.md Documents the breaking migration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread config/default/manager_auth_proxy_patch.yaml
Comment thread CHANGELOG.md Outdated
Comment thread config/rbac/kustomization.yaml Outdated
…cement to appending, and corrected disable protection instruction.
…c-proxy' of github.com:hashicorp/vault-secrets-operator into VAULT-30534-vso-migrate-off-gcr-io-kubebuilder-kube-rbac-proxy
args:
- --health-probe-bind-address=:8081
- --metrics-bind-address=127.0.0.1:8080
- --metrics-bind-address=:8443

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can users override reserved metrics flags (like --metrics-secure or --metrics-bind-address) via extraArgs? If so, does this allow them to expose unauthenticated metrics?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants