Skip to content

[Backport v1.25] Avoid RBAC errors when Operator can't list or watch Secrets#2800

Merged
tbavelier merged 1 commit intov1.25from
backport-2793-to-v1.25
Mar 24, 2026
Merged

[Backport v1.25] Avoid RBAC errors when Operator can't list or watch Secrets#2800
tbavelier merged 1 commit intov1.25from
backport-2793-to-v1.25

Conversation

@dd-octo-sts
Copy link

@dd-octo-sts dd-octo-sts bot commented Mar 23, 2026

Backport 444f938 from #2793.


What does this PR do?

Fixes bug reported in #2791.

  • Don't start secrets watch in helm metadata forwarder if Operator doesn't have list/watch RBAC.
  • Update CredentialManager to avoid unnecessary secrets informer initialization.

Motivation

Additional Notes

Anything else we should know when reviewing?

Minimum Agent Versions

Are there minimum versions of the Datadog Agent and/or Cluster Agent required?

  • Agent: vX.Y.Z
  • Cluster Agent: vX.Y.Z

Describe your test plan

  1. install Operator
❯ helm install operator datadog/datadog-operator \
--set datadogMonitor.enabled=true \
--set datadogAgent.enabled=false \
--set datadogAgentInternal.enabled=false
  1. Update rbac to drop secret watch, list permissions, restart operator pod.
  kubectl get clusterrole operator-datadog-operator -o json \
    | jq '
        .rules |= map(
          if (.resources // [] | index("secrets")) != null
          then .resources |= map(select(. != "secrets"))
          else .
          end
        )
        | .rules += [{"apiGroups":[""],"resources":["secrets"],"verbs":["create","delete","get","patch","update"]}]
      ' \
    | kubectl apply -f -

kubectl rollout restart deployment/operator-datadog-operator
  1. Observer errors
    ..."logger":"klog","msg":"Failed to watch","logger":"UnhandledError","reflector":"pkg/mod/k8s.io/client-go@v0.33.3/tools/cache/reflector.go:285","type":"*v1.Secret","error":"failed to list *v1.Secret: secrets is forbidden: User \"system:serviceaccount:default:operator-datadog-operator\" cannot list resource \"secrets\" in API group \"\" in

  2. Update to fixed image; after restart Operator should log
    {"level":"INFO","ts":"2026-03-20T19:23:55.561Z","logger":"metadata.helm","msg":"No permission to list/watch Secrets, Helm metadata collection from Secrets will be disabled"}

Checklist

  • PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
  • PR has a milestone or the qa/skip-qa label
  • All commits are signed (see: signing commits)

* Avoid RBAC errors when Operator can't list or watch Secrets

* improve comment

(cherry picked from commit 444f938)
@dd-octo-sts dd-octo-sts bot requested a review from a team as a code owner March 23, 2026 18:17
@dd-octo-sts dd-octo-sts bot added the bug Something isn't working label Mar 23, 2026
@dd-octo-sts dd-octo-sts bot requested a review from a team as a code owner March 23, 2026 18:17
@dd-octo-sts dd-octo-sts bot added qa/skip-qa backport label added by backport action bot label added by backport bot team/container-platform bug Something isn't working labels Mar 23, 2026
@dd-octo-sts dd-octo-sts bot added this to the v1.25.0 milestone Mar 23, 2026
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b1718c2201

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +165 to +170
sar := &authorizationv1.SelfSubjectAccessReview{
Spec: authorizationv1.SelfSubjectAccessReviewSpec{
ResourceAttributes: &authorizationv1.ResourceAttributes{
Verb: verb,
Resource: "secrets",
},

Choose a reason for hiding this comment

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

P1 Badge Scope the Secret RBAC check to watched namespaces

SelfSubjectAccessReview is created here without ResourceAttributes.Namespace, so it asks whether the operator may list/watch Secrets cluster-wide. In namespace-scoped installs, cmd/main.go uses config.CacheOptions, and pkg/config/config.go limits DefaultNamespaces from DD_AGENT_WATCH_NAMESPACE/WATCH_NAMESPACE; the Secret informer only needs access in those watched namespaces. A Role/RoleBinding deployment will therefore fail this check, set secretAccessEnabled=false, and stop collecting Helm metadata for Secret-backed releases even though GetInformer(&corev1.Secret{}) would have worked for the configured namespaces.

Useful? React with 👍 / 👎.

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 2.12766% with 46 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.77%. Comparing base (a786298) to head (b1718c2).

Files with missing lines Patch % Lines
pkg/controller/utils/metadata/helm_metadata.go 0.00% 44 Missing ⚠️
cmd/main.go 0.00% 1 Missing ⚠️
pkg/config/creds.go 50.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (2.12%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##            v1.25    #2800      +/-   ##
==========================================
- Coverage   38.79%   38.77%   -0.03%     
==========================================
  Files         309      309              
  Lines       26736    26755      +19     
==========================================
  Hits        10373    10373              
- Misses      15584    15603      +19     
  Partials      779      779              
Flag Coverage Δ
unittests 38.77% <2.12%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cmd/main.go 6.64% <0.00%> (ø)
pkg/config/creds.go 62.06% <50.00%> (ø)
pkg/controller/utils/metadata/helm_metadata.go 26.91% <0.00%> (-1.67%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a786298...b1718c2. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tbavelier tbavelier merged commit 2889a34 into v1.25 Mar 24, 2026
85 checks passed
@tbavelier tbavelier deleted the backport-2793-to-v1.25 branch March 24, 2026 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport label added by backport action bot label added by backport bot bug Something isn't working qa/skip-qa team/container-platform

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants