Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
levan-m merged 2 commits into from
Mar 23, 2026
+74 −41
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0843336
Avoid RBAC errors when Operator can't list or watch Secrets
levan-m Mar 20, 2026
2403c14
improve comment
levan-m Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions cmd/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,10 @@ func run(opts *options) error {

}

// Client is needed when Creds should be resolved from DDA so cached client is fine
credsManager := config.NewCredentialManagerWithDecryptor(mgr.GetClient(), secrets.NewSecretBackend())
// Get call on a cached client initializes informer which requires list and watch permissions.
// If RBAC restricts list and watch permissions, the informer will log errors and may cause crash loops.
// Reader interface as returned from mgr.GetAPIReader() reads directly from API server bypassing cache and informer initialization.
credsManager := config.NewCredentialManagerWithDecryptor(mgr.GetAPIReader(), secrets.NewSecretBackend())
creds, err := credsManager.GetCredentials()

if opts.secretRefreshInterval > 0 && opts.secretBackendCommand == "" {
Expand Down
6 changes: 3 additions & 3 deletions pkg/config/creds.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type Creds struct {

// CredentialManager provides the credentials from the operator configuration.
type CredentialManager struct {
client client.Client
client client.Reader
secretBackend secrets.Decryptor
creds Creds
credsMutex sync.Mutex
Expand All @@ -61,7 +61,7 @@ func (cm *CredentialManager) RegisterCallback(cb CredentialChangeCallback) {
}

// NewCredentialManager returns a CredentialManager.
func NewCredentialManagerWithDecryptor(client client.Client, decryptor secrets.Decryptor) *CredentialManager {
func NewCredentialManagerWithDecryptor(client client.Reader, decryptor secrets.Decryptor) *CredentialManager {
return &CredentialManager{
client: client,
secretBackend: decryptor,
Expand All @@ -79,7 +79,7 @@ func NewCredentialManagerWithDecryptor(client client.Client, decryptor secrets.D

// TODO deprecate in favor of NewCredentialManagerWithDecryptor
// NewCredentialManager returns a CredentialManager.
func NewCredentialManager(client client.Client) *CredentialManager {
func NewCredentialManager(client client.Reader) *CredentialManager {
return &CredentialManager{
client: client,
secretBackend: secrets.NewSecretBackend(),
Expand Down
103 changes: 67 additions & 36 deletions pkg/controller/utils/metadata/helm_metadata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"github.com/go-logr/logr"
"gopkg.in/yaml.v2"
authorizationv1 "k8s.io/api/authorization/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
Expand Down Expand Up @@ -65,6 +66,10 @@ type HelmMetadataForwarder struct {
// Key: "namespace/releaseName"
// Value: *ReleaseEntry
releaseSnapshots sync.Map

// secretAccessEnabled tracks whether the operator has permission to read Secrets.
// Set once in Start() and used to skip the Secret path in processKey.
secretAccessEnabled bool
}

// ReleaseEntry wraps a ReleaseSnapshot with a mutex for safe concurrent access
Expand Down Expand Up @@ -155,6 +160,28 @@ func NewHelmMetadataForwarderWithManager(logger logr.Logger, mgr manager.Manager
}
}

// canListWatchSecrets checks if the operator has permission to list and watch Secrets
func (hmf *HelmMetadataForwarder) canListWatchSecrets(ctx context.Context) bool {
for _, verb := range []string{"list", "watch"} {
sar := &authorizationv1.SelfSubjectAccessReview{
Spec: authorizationv1.SelfSubjectAccessReviewSpec{
ResourceAttributes: &authorizationv1.ResourceAttributes{
Verb: verb,
Resource: "secrets",
},
Comment on lines +168 to +171
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 20, 2026

Choose a reason for hiding this comment

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

P2 Badge Scope Secret RBAC check to watched namespaces

canListWatchSecrets builds a SelfSubjectAccessReview without resourceAttributes.namespace, which Kubernetes interprets as an all-namespaces check for namespaced resources. In namespace-scoped deployments (for example when WATCH_NAMESPACE/DD_AGENT_WATCH_NAMESPACE limits cache scope and RBAC is granted per namespace), this returns denied even though list/watch is allowed in the watched namespace(s), so secretAccessEnabled is set to false and Secret-backed Helm releases are never processed. This is a functional regression for namespace-scoped RBAC setups.

Useful? React with 👍 / 👎.

},
}
if err := hmf.mgr.GetClient().Create(ctx, sar); err != nil {
hmf.logger.V(1).Info("Failed to check Secret RBAC permission", "verb", verb, "error", err)
return false
}
if !sar.Status.Allowed {
return false
}
}
return true
}

// Start implements manager.Runnable interface
// It is called by the manager after the cache is synced but we don't need to initialize resources at start.
// Cache sends synthetic 'Add' events to the newly registered handler, see
Expand All @@ -166,12 +193,6 @@ func (hmf *HelmMetadataForwarder) Start(ctx context.Context) error {
hmf.logger.Info("Unable to get ConfigMap informer, Helm metadata collection will be disabled", "error", err)
return nil
}
secretInformer, err := hmf.mgr.GetCache().GetInformer(ctx, &corev1.Secret{})
if err != nil {
hmf.logger.Info("Unable to get Secret informer, Helm metadata collection will be disabled", "error", err)
return nil
}

_, err = cmInformer.AddEventHandler(toolscache.FilteringResourceEventHandler{
FilterFunc: func(obj any) bool {
cm, ok := obj.(*corev1.ConfigMap)
Expand Down Expand Up @@ -200,30 +221,38 @@ func (hmf *HelmMetadataForwarder) Start(ctx context.Context) error {
return nil
}

_, err = secretInformer.AddEventHandler(toolscache.FilteringResourceEventHandler{
FilterFunc: func(obj any) bool {
secret, ok := obj.(*corev1.Secret)
return ok &&
secret.Labels["owner"] == "helm" &&
strings.HasPrefix(secret.Name, releasePrefix)
},
Handler: toolscache.ResourceEventHandlerFuncs{
AddFunc: func(obj any) {
if key, keyErr := toolscache.MetaNamespaceKeyFunc(obj); keyErr == nil {
hmf.queue.Add(key)
}
},
DeleteFunc: func(obj any) {
if key, keyErr := toolscache.DeletionHandlingMetaNamespaceKeyFunc(obj); keyErr == nil {
hmf.queue.Add(deletePrefix + key)
}
},
},
})

if err != nil {
hmf.logger.Info("Unable to add Secret event handler, Helm metadata collection will be disabled", "error", err)
return nil
hmf.secretAccessEnabled = hmf.canListWatchSecrets(ctx)
if hmf.secretAccessEnabled {
secretInformer, secretErr := hmf.mgr.GetCache().GetInformer(ctx, &corev1.Secret{})
if secretErr != nil {
hmf.logger.Info("Unable to get Secret informer, Helm metadata collection from Secrets will be disabled", "error", secretErr)
} else {
_, secretErr = secretInformer.AddEventHandler(toolscache.FilteringResourceEventHandler{
FilterFunc: func(obj any) bool {
secret, ok := obj.(*corev1.Secret)
return ok &&
secret.Labels["owner"] == "helm" &&
strings.HasPrefix(secret.Name, releasePrefix)
},
Handler: toolscache.ResourceEventHandlerFuncs{
AddFunc: func(obj any) {
if key, keyErr := toolscache.MetaNamespaceKeyFunc(obj); keyErr == nil {
hmf.queue.Add(key)
}
},
DeleteFunc: func(obj any) {
if key, keyErr := toolscache.DeletionHandlingMetaNamespaceKeyFunc(obj); keyErr == nil {
hmf.queue.Add(deletePrefix + key)
}
},
},
})
if secretErr != nil {
hmf.logger.Info("Unable to add Secret event handler, Helm metadata collection from Secrets will be disabled", "error", secretErr)
}
}
} else {
hmf.logger.Info("No permission to list/watch Secrets, Helm metadata collection from Secrets will be disabled")
}

// Cache is already synced by the manager before Start() is called
Expand Down Expand Up @@ -301,12 +330,14 @@ func (hmf *HelmMetadataForwarder) processKey(ctx context.Context, key string) er
return nil
}

// Try as Secret
secret := &corev1.Secret{}
err = hmf.k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, secret)
if err == nil && secret.Labels["owner"] == "helm" {
hmf.handleHelmResource(ctx, secret.Name, secret.Namespace, string(secret.UID), secret.Data["release"])
return nil
// Try as Secret (only if we have permission, to avoid lazily registering a Secret informer)
if hmf.secretAccessEnabled {
secret := &corev1.Secret{}
err = hmf.k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, secret)
if err == nil && secret.Labels["owner"] == "helm" {
hmf.handleHelmResource(ctx, secret.Name, secret.Namespace, string(secret.UID), secret.Data["release"])
return nil
}
}

// If not found, likely a race condition with deletion - ignore it
Expand Down
Loading