Skip to content

Restrict controller-manager Secrets access to Kueue namespace #7188

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
k8s-ci-robot merged 3 commits into from
Oct 16, 2025
Merged

Restrict controller-manager Secrets access to Kueue namespace #7188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7220ac0
Restrict controller-manager Secrets access to Kueue namespace
sbgla-sas Oct 7, 2025
2e16ba2
Add controller-manager secrets role to RBAC Kustomization
sbgla-sas Oct 7, 2025
d42a80e
Add delegated namespaced secrets cache
sbgla-sas Oct 14, 2025
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
19 changes: 19 additions & 0 deletions charts/kueue/templates/rbac/manager_secrets_role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{{/* Code generated by yaml-processor. DO NOT EDIT. */}}

apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
labels:
{{- include "kueue.labels" . | nindent 4 }}
name: '{{ include "kueue.fullname" . }}-manager-secrets-role'
namespace: '{{ .Release.Namespace }}'
rules:
- apiGroups:
- ""
resources:
- secrets
verbs:
- get
- list
- update
- watch
17 changes: 17 additions & 0 deletions charts/kueue/templates/rbac/manager_secrets_role_binding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{{/* Code generated by yaml-processor. DO NOT EDIT. */}}

apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
labels:
{{- include "kueue.labels" . | nindent 4 }}
name: '{{ include "kueue.fullname" . }}-manager-secrets-rolebinding'
namespace: '{{ .Release.Namespace }}'
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: '{{ include "kueue.fullname" . }}-manager-secrets-role'
subjects:
- kind: ServiceAccount
name: '{{ include "kueue.fullname" . }}-controller-manager'
namespace: '{{ .Release.Namespace }}'
9 changes: 0 additions & 9 deletions charts/kueue/templates/rbac/role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,6 @@ rules:
- list
- update
- watch
- apiGroups:
- ""
resources:
- secrets
verbs:
- get
- list
- update
- watch
- apiGroups:
- admissionregistration.k8s.io
resources:
Expand Down
2 changes: 2 additions & 0 deletions config/components/rbac/kustomization.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ resources:
- role_binding.yaml
- leader_election_role.yaml
- leader_election_role_binding.yaml
- manager_secrets_role.yaml
- manager_secrets_role_binding.yaml
# The following RBAC configurations are used to protect
# the metrics endpoint with authn/authz. These configurations
# ensure that only authorized users and service accounts
Expand Down
16 changes: 16 additions & 0 deletions config/components/rbac/manager_secrets_role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# permissions for the manager to access secrets.
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: manager-secrets-role
namespace: system
rules:
- apiGroups:
- ""
resources:
- secrets
verbs:
- get
- list
- update
- watch
13 changes: 13 additions & 0 deletions config/components/rbac/manager_secrets_role_binding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: manager-secrets-rolebinding
namespace: system
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: manager-secrets-role
subjects:
- kind: ServiceAccount
name: controller-manager
namespace: system
9 changes: 0 additions & 9 deletions config/components/rbac/role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,6 @@ rules:
- list
- update
- watch
- apiGroups:
- ""
resources:
- secrets
verbs:
- get
- list
- update
- watch
- apiGroups:
- admissionregistration.k8s.io
resources:
Expand Down
27 changes: 27 additions & 0 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,23 @@ import (
"fmt"
"os"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
ctrl "sigs.k8s.io/controller-runtime"
ctrlcache "sigs.k8s.io/controller-runtime/pkg/cache"
ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/webhook"

configapi "sigs.k8s.io/kueue/apis/config/v1beta1"
)

var (
objectKeySecret = new(corev1.Secret)
)

// fromFile provides an alternative to the deprecated ctrl.ConfigFile().AtPath(path).OfKind(&cfg)
func fromFile(path string, scheme *runtime.Scheme, cfg *configapi.Configuration) error {
content, err := os.ReadFile(path)
Expand All @@ -48,6 +55,8 @@ func fromFile(path string, scheme *runtime.Scheme, cfg *configapi.Configuration)
// addTo provides an alternative to the deprecated o.AndFrom(&cfg)
func addTo(o *ctrl.Options, cfg *configapi.Configuration) {
addLeaderElectionTo(o, cfg)
addCacheByObjectTo(o, cfg)

if o.Metrics.BindAddress == "" && cfg.Metrics.BindAddress != "" {
o.Metrics.BindAddress = cfg.Metrics.BindAddress
}
Expand Down Expand Up @@ -94,6 +103,24 @@ func addTo(o *ctrl.Options, cfg *configapi.Configuration) {
}
}

func addCacheByObjectTo(o *ctrl.Options, cfg *configapi.Configuration) {
Copy link
Contributor

@mimowo mimowo Oct 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

What there is a pre-existing cluster with that configuration? IIUC we should also update kueue/apis/config/v1beta1/defaults.go to make sure it would default on that clusters without manual intervention?

Ah, it seems this would also apply "by default", but seems like obsolete mechanism. IIUC we should migrate to using defaults.go, but maybe this place is not migrated all. Please investigate moving the defautling there.

Copy link
Contributor Author

@sbgla-sas sbgla-sas Oct 15, 2025

Choose a reason for hiding this comment

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

Moving manager cache options to the Kueue defaulter would (I think?) require a Kueue configuration API change to expose all/some of them. There would also still need to be a similar translation between types to provide default/derived values to the controller-runtime manager.

In this case the delegated cache is fully determined by the Kueue namespace, which is covered by the existing defaulter; are there additional cache options you'd like to see exposed more directly?

Copy link
Contributor

@mimowo mimowo Oct 15, 2025

Choose a reason for hiding this comment

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

Ah, ok, no I don't think we need to expose this to users. Let me double check what is the difference here.

if cfg.Namespace == nil {
// Invalid source; noop. This should not be reached
// due to prior defaulting/validation.
return
}

if o.Cache.ByObject == nil {
o.Cache.ByObject = make(map[ctrlclient.Object]ctrlcache.ByObject)
}

o.Cache.ByObject[objectKeySecret] = ctrlcache.ByObject{
Namespaces: map[string]ctrlcache.Config{
*cfg.Namespace: {},
},
}
}

func addLeaderElectionTo(o *ctrl.Options, cfg *configapi.Configuration) {
if cfg.LeaderElection == nil {
// The source does not have any configuration; noop
Expand Down
Loading