diff --git a/cmd/main.go b/cmd/main.go index 767d203f..c05624e0 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -45,8 +45,12 @@ func main() { var metricsAddr string var enableLeaderElection bool var probeAddr string + var watchNamespaces string flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") + flag.StringVar(&watchNamespaces, "watch-namespaces", "", + "Comma-separated list of namespaces the controller watches. "+ + "If empty, the controller watches all namespaces.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") @@ -56,10 +60,19 @@ func main() { opts.BindFlags(flag.CommandLine) flag.Parse() + if watchNamespaces == "" { + watchNamespaces = os.Getenv("WATCH_NAMESPACES") + } + namespaces := controller.ParseWatchNamespaces(watchNamespaces) + //ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) ctrl.SetLogger(zap.New(zap.JSONEncoder())) - cacheOptions, err := controller.NewCacheOptions() + if len(namespaces) > 0 { + setupLog.Info("running controller in namespace-scoped mode", "namespaces", namespaces) + } + + cacheOptions, err := controller.NewCacheOptions(namespaces) if err != nil { setupLog.Error(err, "unable to build manager cache options") os.Exit(1) @@ -86,6 +99,7 @@ func main() { // after the manager stops then its usage might be unsafe. // LeaderElectionReleaseOnCancel: true, }) + if err != nil { setupLog.Error(err, "unable to start manager") os.Exit(1) diff --git a/helm/temporal-worker-controller/templates/_helpers.tpl b/helm/temporal-worker-controller/templates/_helpers.tpl index 5b143384..41fb9871 100644 --- a/helm/temporal-worker-controller/templates/_helpers.tpl +++ b/helm/temporal-worker-controller/templates/_helpers.tpl @@ -18,3 +18,19 @@ Used for matchLabels (Deployments, Services, affinities, etc.) app.kubernetes.io/name: temporal-worker-controller app.kubernetes.io/instance: {{ .Release.Name }} {{- end }} + +{{/* +Namespace selector restricting admission webhooks to the watched namespaces. +Rendered only when rbac.restrictWatchNamespaces is set; keys off the +kubernetes.io/metadata.name label the API server sets on every namespace. +*/}} +{{- define "temporal-worker-controller.webhookNamespaceSelector" -}} +namespaceSelector: + matchExpressions: + - key: kubernetes.io/metadata.name + operator: In + values: + {{- range .Values.rbac.restrictWatchNamespaces }} + - {{ . | quote }} + {{- end }} +{{- end }} diff --git a/helm/temporal-worker-controller/templates/manager.yaml b/helm/temporal-worker-controller/templates/manager.yaml index a6e66c82..06bac819 100644 --- a/helm/temporal-worker-controller/templates/manager.yaml +++ b/helm/temporal-worker-controller/templates/manager.yaml @@ -98,6 +98,10 @@ spec: {{- end }} - name: ALLOWED_KINDS value: {{ join "," $allKinds | quote }} + {{- with .Values.rbac.restrictWatchNamespaces }} + - name: WATCH_NAMESPACES + value: {{ join "," . | quote }} + {{- end }} args: - --leader-elect {{- if .Values.metrics.enabled }} diff --git a/helm/temporal-worker-controller/templates/rbac.yaml b/helm/temporal-worker-controller/templates/rbac.yaml index 83614c3f..cf3e9ba2 100644 --- a/helm/temporal-worker-controller/templates/rbac.yaml +++ b/helm/temporal-worker-controller/templates/rbac.yaml @@ -174,6 +174,70 @@ rules: - update {{- end }} --- +{{- if .Values.rbac.restrictWatchNamespaces }} +# Namespace-scoped mode: the manager role is bound only within the watched +# namespaces (RoleBindings below). The two grants that must stay cluster-wide are +# split into a minimal ClusterRole: namespaces "get" (restricted to the release +# namespace, for the controller identity-suffix lookup) and subjectaccessreviews +# "create" (the always-on validating webhook). +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app.kubernetes.io/component: rbac + {{- include "temporal-worker-controller.labels" . | nindent 4 }} + name: {{ .Release.Name }}-{{ .Release.Namespace }}-manager-cluster-role +rules: + - apiGroups: + - "" + resources: + - namespaces + resourceNames: + - {{ .Release.Namespace }} + verbs: + - get + - apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + app.kubernetes.io/component: rbac + {{- include "temporal-worker-controller.labels" . | nindent 4 }} + name: {{ .Release.Name }}-{{ .Release.Namespace }}-manager-cluster-rolebinding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: {{ .Release.Name }}-{{ .Release.Namespace }}-manager-cluster-role +subjects: + - kind: ServiceAccount + name: {{ .Values.serviceAccount.name | default (printf "%s-service-account" .Release.Name) }} + namespace: {{ .Release.Namespace }} +{{- range .Values.rbac.restrictWatchNamespaces }} +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + labels: + app.kubernetes.io/component: rbac + {{- include "temporal-worker-controller.labels" $ | nindent 4 }} + name: {{ $.Release.Name }}-{{ $.Release.Namespace }}-manager-rolebinding + namespace: {{ . }} +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: {{ $.Release.Name }}-{{ $.Release.Namespace }}-manager-role +subjects: + - kind: ServiceAccount + name: {{ $.Values.serviceAccount.name | default (printf "%s-service-account" $.Release.Name) }} + namespace: {{ $.Release.Namespace }} +{{- end }} +{{- else }} apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: @@ -189,6 +253,7 @@ subjects: - kind: ServiceAccount name: {{ .Values.serviceAccount.name | default (printf "%s-service-account" .Release.Name) }} namespace: {{ .Release.Namespace }} +{{- end }} --- # permissions for end users to edit connections (new name). apiVersion: rbac.authorization.k8s.io/v1 diff --git a/helm/temporal-worker-controller/templates/webhook.yaml b/helm/temporal-worker-controller/templates/webhook.yaml index 9acd8b74..c159d5c1 100644 --- a/helm/temporal-worker-controller/templates/webhook.yaml +++ b/helm/temporal-worker-controller/templates/webhook.yaml @@ -48,6 +48,9 @@ webhooks: operations: ["CREATE", "UPDATE", "DELETE"] resources: ["workerresourcetemplates"] sideEffects: None + {{- if .Values.rbac.restrictWatchNamespaces }} + {{- include "temporal-worker-controller.webhookNamespaceSelector" . | nindent 4 }} + {{- end }} {{- if .Values.webhook.enabled }} --- # ValidatingWebhookConfiguration for WorkerDeployment. @@ -80,4 +83,7 @@ webhooks: operations: ["CREATE", "UPDATE"] resources: ["workerdeployments"] sideEffects: None + {{- if .Values.rbac.restrictWatchNamespaces }} + {{- include "temporal-worker-controller.webhookNamespaceSelector" . | nindent 4 }} + {{- end }} {{- end }} diff --git a/helm/temporal-worker-controller/values.schema.json b/helm/temporal-worker-controller/values.schema.json index fab85d20..30001aff 100644 --- a/helm/temporal-worker-controller/values.schema.json +++ b/helm/temporal-worker-controller/values.schema.json @@ -109,6 +109,13 @@ "create": { "type": "boolean", "description": "Whether to create RBAC resources" + }, + "restrictWatchNamespaces": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Namespaces the controller watches and is granted RBAC in. Empty (the default) watches all namespaces with cluster-wide RBAC; when set, the controller is scoped to these namespaces via per-namespace RoleBindings plus a minimal cluster-scoped role." } } }, diff --git a/helm/temporal-worker-controller/values.yaml b/helm/temporal-worker-controller/values.yaml index 22466977..d4246c31 100644 --- a/helm/temporal-worker-controller/values.yaml +++ b/helm/temporal-worker-controller/values.yaml @@ -40,6 +40,13 @@ priorityClassName: "" rbac: # Specifies whether RBAC resources should be created create: true + # restrictWatchNamespaces keeps the namespaces the controller watches and the + # namespaces its RBAC grants access to in sync. Empty (the default) watches all + # namespaces with cluster-wide RBAC. When set, the controller watches only these + # namespaces (via WATCH_NAMESPACES) and is granted the manager role through a + # RoleBinding in each listed namespace, plus a minimal cluster-scoped role for the + # two grants that cannot be namespaced (namespaces get, subjectaccessreviews create). + restrictWatchNamespaces: [] serviceAccount: # Specifies whether a ServiceAccount should be created diff --git a/internal/controller/cache.go b/internal/controller/cache.go index e0b5730b..bf0a9b1d 100644 --- a/internal/controller/cache.go +++ b/internal/controller/cache.go @@ -5,6 +5,8 @@ package controller import ( + "strings" + "github.com/temporalio/temporal-worker-controller/internal/k8s" appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/labels" @@ -19,17 +21,48 @@ import ( // but controller-runtime still lists, watches, and retains cached Deployment // objects before those events reach the controller. Restricting the manager cache // prevents unrelated cluster Deployments from growing the controller's memory use. -func NewCacheOptions() (cache.Options, error) { +// +// When watchNamespaces is non-empty the cache (and therefore the controller's +// watches) is restricted to those namespaces; empty means all namespaces. +func NewCacheOptions(watchNamespaces []string) (cache.Options, error) { deploymentLabelReq, err := labels.NewRequirement(k8s.WorkerDeploymentNameLabel, selection.Exists, nil) if err != nil { return cache.Options{}, err } - return cache.Options{ + opts := cache.Options{ ByObject: map[client.Object]cache.ByObject{ &appsv1.Deployment{}: { Label: labels.NewSelector().Add(*deploymentLabelReq), }, }, - }, nil + } + + if len(watchNamespaces) > 0 { + defaultNamespaces := make(map[string]cache.Config, len(watchNamespaces)) + for _, ns := range watchNamespaces { + defaultNamespaces[ns] = cache.Config{} + } + opts.DefaultNamespaces = defaultNamespaces + } + + return opts, nil +} + +// ParseWatchNamespaces splits a comma-separated namespace list (from the +// --watch-namespaces flag or WATCH_NAMESPACES env var) into a slice, trimming +// whitespace and dropping empty entries. An empty input returns nil, which +// NewCacheOptions treats as "watch all namespaces". +func ParseWatchNamespaces(raw string) []string { + if raw == "" { + return nil + } + parts := strings.Split(raw, ",") + namespaces := make([]string, 0, len(parts)) + for _, p := range parts { + if ns := strings.TrimSpace(p); ns != "" { + namespaces = append(namespaces, ns) + } + } + return namespaces } diff --git a/internal/controller/cache_test.go b/internal/controller/cache_test.go index df709dd2..8474c291 100644 --- a/internal/controller/cache_test.go +++ b/internal/controller/cache_test.go @@ -5,6 +5,7 @@ package controller import ( + "reflect" "testing" "github.com/temporalio/temporal-worker-controller/internal/k8s" @@ -13,7 +14,7 @@ import ( ) func TestNewCacheOptionsScopesDeploymentsByWorkerLabel(t *testing.T) { - opts, err := NewCacheOptions() + opts, err := NewCacheOptions(nil) if err != nil { t.Fatalf("NewCacheOptions returned error: %v", err) } @@ -40,3 +41,53 @@ func TestNewCacheOptionsScopesDeploymentsByWorkerLabel(t *testing.T) { t.Fatal("expected selector not to match unlabeled Deployment") } } + +func TestNewCacheOptionsScopesToWatchNamespaces(t *testing.T) { + opts, err := NewCacheOptions([]string{"ns-a", "ns-b"}) + if err != nil { + t.Fatalf("NewCacheOptions returned error: %v", err) + } + + if len(opts.DefaultNamespaces) != 2 { + t.Fatalf("expected 2 default namespaces, got %d", len(opts.DefaultNamespaces)) + } + for _, ns := range []string{"ns-a", "ns-b"} { + if _, ok := opts.DefaultNamespaces[ns]; !ok { + t.Fatalf("expected namespace %q in DefaultNamespaces", ns) + } + } +} + +func TestNewCacheOptionsWatchesAllNamespacesWhenEmpty(t *testing.T) { + opts, err := NewCacheOptions(nil) + if err != nil { + t.Fatalf("NewCacheOptions returned error: %v", err) + } + + if len(opts.DefaultNamespaces) != 0 { + t.Fatalf("expected no default namespaces, got %d", len(opts.DefaultNamespaces)) + } +} + +func TestParseWatchNamespaces(t *testing.T) { + tests := []struct { + name string + raw string + want []string + }{ + {name: "empty returns nil", raw: "", want: nil}, + {name: "single namespace", raw: "ns-a", want: []string{"ns-a"}}, + {name: "comma separated", raw: "ns-a,ns-b", want: []string{"ns-a", "ns-b"}}, + {name: "trims whitespace and drops empties", raw: " ns-a , , ns-b ,", want: []string{"ns-a", "ns-b"}}, + {name: "separators only returns empty slice", raw: " , , ", want: []string{}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ParseWatchNamespaces(tt.raw) + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("ParseWatchNamespaces(%q) = %#v, want %#v", tt.raw, got, tt.want) + } + }) + } +}