From eb2dc9d7354b5cca05e9b2842bacc6ccaf61d182 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Thu, 29 Oct 2026 15:30:46 +0000 Subject: [PATCH 1/2] feat: share a resource in named namespaces alone -shared-watch-namespaced-resources (chart: sharedWatches.namespacedResources) shares a resource in the namespaces named for it, for an identity granted it there with a Role, never a ClusterRole. A stream of it in any other namespace, or across every namespace, is the user's own and never reaches the shared identity. For RoleBindings: an application can watch the grants in its own namespace without the shared identity reading every grant in the cluster. Co-Authored-By: Claude Opus 5.5 --- charts/krm-foyer/README.md | 5 +- charts/krm-foyer/templates/deployment.yaml | 15 ++- .../krm-foyer/templates/shared-watches.yaml | 61 +++++++++--- charts/krm-foyer/values.schema.json | 15 +++ charts/krm-foyer/values.yaml | 12 ++- cmd/krm-foyer/chart_test.go | 91 +++++++++++++++++- cmd/krm-foyer/main.go | 30 ++++-- cmd/krm-foyer/main_test.go | 23 ++++- docs/design.md | 6 +- docs/watches.md | 36 ++++++- internal/stream/shared.go | 89 +++++++++++++++++- internal/stream/shared_test.go | 93 +++++++++++++++++++ 12 files changed, 439 insertions(+), 37 deletions(-) diff --git a/charts/krm-foyer/README.md b/charts/krm-foyer/README.md index d2a2878..9bb2d76 100644 --- a/charts/krm-foyer/README.md +++ b/charts/krm-foyer/README.md @@ -2,8 +2,9 @@ Installs [krm-foyer](../../README.md): one Deployment with one replica, a Service for the origin, a Service for metrics, the pod's service account, and a NetworkPolicy that admits traffic to krm-foyer's ports alone. -With `sharedWatches.resources` set, it also creates the -[shared-watch identity](../../docs/watches.md) and its grants. +With `sharedWatches.resources` or `sharedWatches.namespacedResources` set, it also +creates the [shared-watch identity](../../docs/watches.md) and its grants: a ClusterRole +for the first, a Role in each named namespace for the second. ```bash kubectl create namespace krm-foyer diff --git a/charts/krm-foyer/templates/deployment.yaml b/charts/krm-foyer/templates/deployment.yaml index b7bb9f6..9ed7424 100644 --- a/charts/krm-foyer/templates/deployment.yaml +++ b/charts/krm-foyer/templates/deployment.yaml @@ -1,6 +1,6 @@ {{- $tls := ne .Values.tls.secretName "" }} {{- $scheme := ternary "https" "http" $tls }} -{{- $shared := gt (len .Values.sharedWatches.resources) 0 }} +{{- $shared := or .Values.sharedWatches.resources .Values.sharedWatches.namespacedResources }} {{- $login := include "krm-foyer.login" . }} apiVersion: apps/v1 kind: Deployment @@ -88,7 +88,18 @@ spec: {{- . | trim | nindent 12 }} {{- end }} {{- if $shared }} - - {{ printf "-shared-watch-resources=%s" (join "," .Values.sharedWatches.resources) | quote }} + {{- with .Values.sharedWatches.resources }} + - {{ printf "-shared-watch-resources=%s" (join "," .) | quote }} + {{- end }} + {{- with .Values.sharedWatches.namespacedResources }} + {{- $pairs := list }} + {{- range $namespace, $resources := . }} + {{- range $resources }} + {{- $pairs = append $pairs (printf "%s/%s" $namespace .) }} + {{- end }} + {{- end }} + - {{ printf "-shared-watch-namespaced-resources=%s" (join "," $pairs) | quote }} + {{- end }} - -shared-watch-token-file=/etc/krm-foyer/shared-watch/token {{- with include "krm-foyer.flags" (list .Values.sharedWatches.tuning (list (list "recheckInterval" "shared-watch-recheck-interval") diff --git a/charts/krm-foyer/templates/shared-watches.yaml b/charts/krm-foyer/templates/shared-watches.yaml index a12df75..051b1e8 100644 --- a/charts/krm-foyer/templates/shared-watches.yaml +++ b/charts/krm-foyer/templates/shared-watches.yaml @@ -1,16 +1,18 @@ -{{- if and .Values.sharedWatches.resources .Values.sharedWatches.serviceAccount.create }} +{{- if and (or .Values.sharedWatches.resources .Values.sharedWatches.namespacedResources) .Values.sharedWatches.serviceAccount.create }} {{- $name := include "krm-foyer.sharedAccountName" . }} +{{- $labels := include "krm-foyer.labels" . }} # The shared-watch identity (docs/watches.md): it may list and watch the shared -# resources, and ask the API server whether a user may (create subjectaccessreviews), -# and nothing else. Not system:auth-delegator, which would also let it review tokens. -# Its binding has a new name: a binding's roleRef cannot change, so an upgrade from -# the auth-delegator binding replaces it rather than editing it. +# resources, each where it is shared, and ask the API server whether a user may +# (create subjectaccessreviews), and nothing else. Not system:auth-delegator, which +# would also let it review tokens. Its binding has a new name: a binding's roleRef +# cannot change, so an upgrade from the auth-delegator binding replaces it rather +# than editing it. apiVersion: v1 kind: ServiceAccount metadata: name: {{ $name }} labels: - {{- include "krm-foyer.labels" . | nindent 4 }} + {{- $labels | nindent 4 }} automountServiceAccountToken: false --- # Its token, for the pod to mount. Kubernetes fills this Secret in. @@ -19,17 +21,18 @@ kind: Secret metadata: name: {{ $name }}-token labels: - {{- include "krm-foyer.labels" . | nindent 4 }} + {{- $labels | nindent 4 }} annotations: kubernetes.io/service-account.name: {{ $name }} type: kubernetes.io/service-account-token +{{- if .Values.sharedWatches.resources }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: name: {{ $name }}-watches labels: - {{- include "krm-foyer.labels" . | nindent 4 }} + {{- $labels | nindent 4 }} rules: {{- range .Values.sharedWatches.resources }} {{- $parts := splitn "." 2 . }} @@ -43,7 +46,7 @@ kind: ClusterRoleBinding metadata: name: {{ $name }}-watches labels: - {{- include "krm-foyer.labels" . | nindent 4 }} + {{- $labels | nindent 4 }} roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole @@ -52,13 +55,49 @@ subjects: - kind: ServiceAccount name: {{ $name }} namespace: {{ .Release.Namespace }} +{{- end }} +{{- /* A Role in each namespace a namespaced resource is shared in, never a + ClusterRole: these are what the identity must not read cluster-wide. */}} +{{- range $namespace, $resources := .Values.sharedWatches.namespacedResources }} +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: {{ $name }}-watches + namespace: {{ $namespace }} + labels: + {{- $labels | nindent 4 }} +rules: + {{- range $resources }} + {{- $parts := splitn "." 2 . }} + - apiGroups: [{{ $parts._1 | default "" | quote }}] + resources: [{{ $parts._0 | quote }}] + verbs: [list, watch] + {{- end }} +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: {{ $name }}-watches + namespace: {{ $namespace }} + labels: + {{- $labels | nindent 4 }} +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: {{ $name }}-watches +subjects: + - kind: ServiceAccount + name: {{ $name }} + namespace: {{ $.Release.Namespace }} +{{- end }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: name: {{ $name }}-subject-access-reviews labels: - {{- include "krm-foyer.labels" . | nindent 4 }} + {{- $labels | nindent 4 }} rules: - apiGroups: [authorization.k8s.io] resources: [subjectaccessreviews] @@ -69,7 +108,7 @@ kind: ClusterRoleBinding metadata: name: {{ $name }}-subject-access-reviews labels: - {{- include "krm-foyer.labels" . | nindent 4 }} + {{- $labels | nindent 4 }} roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole diff --git a/charts/krm-foyer/values.schema.json b/charts/krm-foyer/values.schema.json index 3c7d958..7b7a943 100644 --- a/charts/krm-foyer/values.schema.json +++ b/charts/krm-foyer/values.schema.json @@ -177,6 +177,21 @@ "description": "A resource as kubectl names it: plural, then the group if it has one." } }, + "namespacedResources": { + "type": "object", + "description": "Resources shared in the namespace each key names, and nowhere else.", + "propertyNames": { "pattern": "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$", "maxLength": 63 }, + "additionalProperties": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "pattern": "^[a-z0-9-]+(\\.[a-z0-9]([-a-z0-9.]*[a-z0-9])?)?$", + "description": "A resource as kubectl names it: plural, then the group if it has one." + } + } + }, "serviceAccount": { "type": "object", "additionalProperties": false, diff --git a/charts/krm-foyer/values.yaml b/charts/krm-foyer/values.yaml index db9b76e..07334b6 100644 --- a/charts/krm-foyer/values.yaml +++ b/charts/krm-foyer/values.yaml @@ -92,10 +92,16 @@ bounds: {} # them: notes.hello.krm-foyer.example, configmaps. Empty opens every watch as its user. sharedWatches: resources: [] + # Resources shared in some namespaces only, by namespace: {voter: + # [rolebindings.rbac.authorization.k8s.io]}. The identity may list and watch them in + # those namespaces alone (a Role in each, not a ClusterRole), and a stream of one + # anywhere else is opened as its user. For what the identity must not read + # cluster-wide, such as RBAC. + namespacedResources: {} # The identity shared watches are opened with. Created, it may list and watch the - # resources above and create subjectaccessreviews, and nothing else; its token is a - # Secret named -token. Not created, the account named, and a token Secret named - # -token, must already exist. + # resources above, each where it is shared, and create subjectaccessreviews, and + # nothing else; its token is a Secret named -token. Not created, the account + # named, and a token Secret named -token, must already exist. serviceAccount: create: true # Empty is -shared. diff --git a/cmd/krm-foyer/chart_test.go b/cmd/krm-foyer/chart_test.go index 02c7c75..fc2e90c 100644 --- a/cmd/krm-foyer/chart_test.go +++ b/cmd/krm-foyer/chart_test.go @@ -70,6 +70,7 @@ type rendered struct { secrets []corev1.Secret services []corev1.Service clusterRoles []rbacv1.ClusterRole + roles []rbacv1.Role bindings []rbacv1.ClusterRoleBinding roleBindings []rbacv1.RoleBinding networkPolicies []networkingv1.NetworkPolicy @@ -104,6 +105,8 @@ func render(t *testing.T, valueFiles []string, set ...string) rendered { target = appendNew(&r.services) case "ClusterRole": target = appendNew(&r.clusterRoles) + case "Role": + target = appendNew(&r.roles) case "ClusterRoleBinding": target = appendNew(&r.bindings) case "RoleBinding": @@ -314,6 +317,9 @@ func TestChartRefusesValues(t *testing.T) { "a bound of zero": {[]string{"bounds.maxStreams=0"}, "/bounds/maxStreams"}, "a resource with a wildcard": {[]string{"sharedWatches.resources={*}"}, "/sharedWatches/resources/0"}, "a resource with a subresource": {[]string{"sharedWatches.resources={pods/log}"}, "/sharedWatches/resources/0"}, + "a namespace that is no label": {[]string{"sharedWatches.namespacedResources.Voter={configmaps}"}, "invalid propertyName 'Voter'"}, + "a namespace with no resource": {[]string{"sharedWatches.namespacedResources.voter=null"}, "/sharedWatches/namespacedResources/voter"}, + "a namespaced subresource": {[]string{"sharedWatches.namespacedResources.voter={pods/log}"}, "/sharedWatches/namespacedResources/voter/0"}, "two replicas": {[]string{"replicaCount=2"}, "replicaCount"}, "plain HTTP open to anyone": {[]string{"networkPolicy.from=null"}, "networkPolicy.from must name the ingress"}, "an account named by no one": {[]string{"serviceAccount.create=false"}, "/serviceAccount"}, @@ -354,6 +360,8 @@ func TestChartGivesThePodsAccountNothing(t *testing.T) { }{ "defaults": {nil, ""}, "shared watches": {[]string{"sharedWatches.resources={notes.hello.krm-foyer.example,configmaps}"}, "krm-foyer-shared-token"}, + "namespaced shared watches": {[]string{"sharedWatches.resources=null", + "sharedWatches.namespacedResources.voter={rolebindings.rbac.authorization.k8s.io}"}, "krm-foyer-shared-token"}, "an account it brings": {[]string{"serviceAccount.create=false", "serviceAccount.name=app", "sharedWatches.resources={configmaps}"}, "krm-foyer-shared-token"}, "a shared account it brings": {[]string{"sharedWatches.resources={configmaps}", @@ -393,9 +401,6 @@ func TestChartGivesThePodsAccountNothing(t *testing.T) { t.Fatalf("Secret %s holds the pod's own token", s.Name) } } - if len(r.roleBindings) > 0 { - t.Fatal("the chart binds a Role; check it does not grant the pod's account anything, then allow it here") - } for _, b := range r.bindings { for _, s := range b.Subjects { if s.Name == account { @@ -403,6 +408,13 @@ func TestChartGivesThePodsAccountNothing(t *testing.T) { } } } + for _, b := range r.roleBindings { + for _, s := range b.Subjects { + if s.Name == account { + t.Fatalf("%s in %s binds the pod's account to %s", b.Name, b.Namespace, b.RoleRef.Name) + } + } + } }) } } @@ -448,6 +460,79 @@ func TestChartGrantsTheSharedIdentityOnlyWhatItNeeds(t *testing.T) { } } +// A resource shared in some namespaces is granted there alone: a Role and a +// RoleBinding in each, and no ClusterRole that lets the identity list or watch it +// anywhere else. RoleBindings are why this exists: a ClusterRole for them would let +// the identity read every grant in the cluster. krm-foyer is told the same +// namespaces, so it sends no stream elsewhere to the identity. +func TestChartGrantsANamespacedResourceInItsNamespacesAlone(t *testing.T) { + r := render(t, nil, "sharedWatches.resources={configmaps}", + "sharedWatches.namespacedResources.voter={rolebindings.rbac.authorization.k8s.io}", + "sharedWatches.namespacedResources.team={rolebindings.rbac.authorization.k8s.io,notes.hello.krm-foyer.example}") + for _, cr := range r.clusterRoles { + for _, rule := range cr.Rules { + if slices.Contains(rule.Resources, "rolebindings") || slices.Contains(rule.Resources, "notes") { + t.Fatalf("ClusterRole %s grants %+v cluster-wide", cr.Name, rule) + } + } + } + want := map[string][]rbacv1.PolicyRule{ + "team": { + {APIGroups: []string{"rbac.authorization.k8s.io"}, Resources: []string{"rolebindings"}, Verbs: []string{"list", "watch"}}, + {APIGroups: []string{"hello.krm-foyer.example"}, Resources: []string{"notes"}, Verbs: []string{"list", "watch"}}, + }, + "voter": { + {APIGroups: []string{"rbac.authorization.k8s.io"}, Resources: []string{"rolebindings"}, Verbs: []string{"list", "watch"}}, + }, + } + if len(r.roles) != len(want) { + t.Fatalf("%d Roles, want %d", len(r.roles), len(want)) + } + for _, role := range r.roles { + rules, ok := want[role.Namespace] + if !ok || len(role.Rules) != len(rules) { + t.Fatalf("Role %s in %q has %+v", role.Name, role.Namespace, role.Rules) + } + for i := range rules { + got := role.Rules[i] + if !slices.Equal(got.APIGroups, rules[i].APIGroups) || !slices.Equal(got.Resources, rules[i].Resources) || + !slices.Equal(got.Verbs, rules[i].Verbs) || len(got.ResourceNames) > 0 || len(got.NonResourceURLs) > 0 { + t.Fatalf("Role in %s, rule %d is %+v, want %+v", role.Namespace, i, got, rules[i]) + } + } + } + bound := map[string]bool{} + for _, b := range r.roleBindings { + if len(b.Subjects) != 1 || b.Subjects[0].Name != "krm-foyer-shared" || b.Subjects[0].Namespace != "krm-foyer" || + b.RoleRef.Kind != "Role" || b.RoleRef.Name != "krm-foyer-shared-watches" { + t.Fatalf("RoleBinding %s in %s binds %+v to %+v", b.Name, b.Namespace, b.Subjects, b.RoleRef) + } + bound[b.Namespace] = true + } + if !bound["voter"] || !bound["team"] || len(bound) != 2 { + t.Fatalf("RoleBindings in %v", bound) + } + + sh := parseRendered(t, r).login.shared + if len(sh.Resources) != 1 || len(sh.NamespacedResources) != 2 { + t.Fatalf("shared %+v", sh) + } + namespaces := map[string][]string{} + for _, nr := range sh.NamespacedResources { + namespaces[nr.Resource] = nr.Namespaces + } + if !slices.Equal(namespaces["rolebindings"], []string{"team", "voter"}) || !slices.Equal(namespaces["notes"], []string{"team"}) { + t.Fatalf("krm-foyer is told %v", namespaces) + } + + // Without a cluster-wide resource there is no cluster-wide watch grant at all. + only := render(t, nil, "sharedWatches.resources=null", + "sharedWatches.namespacedResources.voter={rolebindings.rbac.authorization.k8s.io}") + if len(only.clusterRoles) != 1 || only.clusterRoles[0].Name != "krm-foyer-shared-subject-access-reviews" { + t.Fatalf("ClusterRoles %+v, want the reviews one alone", only.clusterRoles) + } +} + // With plain HTTP, the hop from the ingress carries session cookies, so only the // ingress may reach the origin port (docs/ingress.md); the chart refuses to render // without it being named. With TLS the origin is open, and metrics are open unless diff --git a/cmd/krm-foyer/main.go b/cmd/krm-foyer/main.go index f2f0da6..17d716a 100644 --- a/cmd/krm-foyer/main.go +++ b/cmd/krm-foyer/main.go @@ -92,6 +92,7 @@ func parseConfig(args []string, readFile func(string) ([]byte, error), output io perSecond float64 maxBytes int64 sharedResources, sharedToken string + sharedNamespaced string recheck, decisionTTL time.Duration streamWrites time.Duration sharedQPS float64 @@ -137,6 +138,10 @@ func parseConfig(args []string, readFile func(string) ([]byte, error), output io fs.StringVar(&sharedResources, "shared-watch-resources", "", "resources whose streams share one watch per scope, as kubectl names them, comma-separated "+ "(notes.hello.krm-foyer.example,configmaps); needs -shared-watch-token-file. See docs/watches.md") + fs.StringVar(&sharedNamespaced, "shared-watch-namespaced-resources", "", + "resources shared in some namespaces only, as namespace/resource, comma-separated "+ + "(voter/rolebindings.rbac.authorization.k8s.io); in any other namespace a stream is the user's own. "+ + "Needs -shared-watch-token-file. See docs/watches.md") fs.StringVar(&sharedToken, "shared-watch-token-file", "", "the token of the identity shared watches are opened with: a service account that may list and watch "+ "the shared resources and create subjectaccessreviews, and nothing else") @@ -171,7 +176,7 @@ func parseConfig(args []string, readFile func(string) ([]byte, error), output io } switch { case len(missing) == len(required): - if issuerCAFile != "" || apiServerCA != "" || loginFile != "" || sharedResources != "" || sharedToken != "" { + if issuerCAFile != "" || apiServerCA != "" || loginFile != "" || sharedResources != "" || sharedNamespaced != "" || sharedToken != "" { return config{}, errors.New("-oidc-ca-file, -kubernetes-ca-file, -login-config-file and the -shared-watch flags need the sign-in flags too") } return cfg, nil // no sign-in and no API proxy: the start page and probes only @@ -279,16 +284,27 @@ func parseConfig(args []string, readFile func(string) ([]byte, error), output io } // Shared watches, for the resources named: off unless both flags are given. - if (sharedResources == "") != (sharedToken == "") { - return config{}, errors.New("-shared-watch-resources and -shared-watch-token-file go together") + if (sharedResources == "" && sharedNamespaced == "") != (sharedToken == "") { + return config{}, errors.New("-shared-watch-resources or -shared-watch-namespaced-resources, and -shared-watch-token-file, go together") } - if sharedResources != "" { + if sharedToken != "" { resources, err := stream.ParseResources(sharedResources) if err != nil { return config{}, fmt.Errorf("-shared-watch-resources: %w", err) } - if len(resources) == 0 { - return config{}, errors.New("-shared-watch-resources names no resource") + namespaced, err := stream.ParseNamespacedResources(sharedNamespaced) + if err != nil { + return config{}, fmt.Errorf("-shared-watch-namespaced-resources: %w", err) + } + if len(resources)+len(namespaced) == 0 { + return config{}, errors.New("the -shared-watch flags name no resource") + } + for _, nr := range namespaced { + for _, gr := range resources { + if gr.Group == nr.Group && gr.Resource == nr.Resource { + return config{}, fmt.Errorf("%s is in -shared-watch-resources and -shared-watch-namespaced-resources: name it once", nr.Resource) + } + } } if sharedQPS <= 0 { return config{}, fmt.Errorf("-shared-watch-qps must be positive, got %v", sharedQPS) @@ -297,7 +313,7 @@ func parseConfig(args []string, readFile func(string) ([]byte, error), output io return config{}, fmt.Errorf("-shared-watch-recheck-interval and -shared-watch-decision-ttl must be positive, "+ "the lifetime no longer than the interval; got %v and %v", recheck, decisionTTL) } - login.shared = &stream.SharedConfig{TokenFile: sharedToken, Resources: resources, + login.shared = &stream.SharedConfig{TokenFile: sharedToken, Resources: resources, NamespacedResources: namespaced, RecheckInterval: recheck, DecisionTTL: decisionTTL, QPS: float32(sharedQPS)} } cfg.login = login diff --git a/cmd/krm-foyer/main_test.go b/cmd/krm-foyer/main_test.go index fc35c1a..102b13e 100644 --- a/cmd/krm-foyer/main_test.go +++ b/cmd/krm-foyer/main_test.go @@ -9,6 +9,7 @@ import ( "net" "net/http" "net/http/httptest" + "slices" "strings" "sync" "sync/atomic" @@ -103,6 +104,21 @@ func TestParseConfigSharedWatches(t *testing.T) { sh.RecheckInterval != 30*time.Second || sh.DecisionTTL != 10*time.Second || sh.QPS != 100 { t.Fatalf("%+v", sh) } + + // Namespaced alone is enough, and gathers a resource's namespaces. + cfg, err = parseConfig(append(loginArgs, "-shared-watch-namespaced-resources", + "voter/rolebindings.rbac.authorization.k8s.io,team/rolebindings.rbac.authorization.k8s.io", + "-shared-watch-token-file", "/token"), secret, io.Discard) + if err != nil { + t.Fatal(err) + } + sh = cfg.login.shared + if sh == nil || len(sh.Resources) != 0 || len(sh.NamespacedResources) != 1 || + sh.NamespacedResources[0].Group != "rbac.authorization.k8s.io" || sh.NamespacedResources[0].Resource != "rolebindings" || + sh.NamespacedResources[0].AllowAllNamespaces || + !slices.Equal(sh.NamespacedResources[0].Namespaces, []string{"voter", "team"}) { + t.Fatalf("%+v", sh) + } } // Sign-in and the proxy come together or not at all, and nothing half-configured @@ -160,7 +176,12 @@ func TestParseConfigRefuses(t *testing.T) { "shared without token": {append(loginArgs, "-shared-watch-resources", "configmaps"), secret, "go together"}, "shared token alone": {append(loginArgs, "-shared-watch-token-file", "/token"), secret, "go together"}, "shared resource bad": {append(loginArgs, "-shared-watch-resources", "apps/deployments", "-shared-watch-token-file", "/token"), secret, "-shared-watch-resources"}, - "shared names none": {append(loginArgs, "-shared-watch-resources", ",", "-shared-watch-token-file", "/token"), secret, "names no resource"}, + "shared names none": {append(loginArgs, "-shared-watch-resources", ",", "-shared-watch-token-file", "/token"), secret, "name no resource"}, + "namespaced without token": {append(loginArgs, "-shared-watch-namespaced-resources", "voter/configmaps"), secret, "go together"}, + "namespaced bad": {append(loginArgs, "-shared-watch-namespaced-resources", "configmaps", "-shared-watch-token-file", "/token"), secret, "-shared-watch-namespaced-resources"}, + "shared both ways": {append(loginArgs, "-shared-watch-resources", "configmaps", "-shared-watch-namespaced-resources", "voter/configmaps", + "-shared-watch-token-file", "/token"), secret, "name it once"}, + "namespaced without login": {[]string{"-shared-watch-namespaced-resources", "voter/configmaps"}, nil, "need the sign-in flags"}, "decisions outlive check": {append(loginArgs, "-shared-watch-resources", "configmaps", "-shared-watch-token-file", "/token", "-shared-watch-decision-ttl", "1m"), secret, "-shared-watch-decision-ttl"}, "no stream write timeout": {append(loginArgs, "-stream-write-timeout", "0s"), secret, "-stream-write-timeout"}, diff --git a/docs/design.md b/docs/design.md index db96a7f..a8f57dd 100644 --- a/docs/design.md +++ b/docs/design.md @@ -698,7 +698,11 @@ additional confidentiality boundary in krm-foyer. ### Shared watches Which resources use a shared watch is configuration for efficiency, not for access -(`-shared-watch-resources`; [how to choose and set up](watches.md)). Every stream of one +(`-shared-watch-resources`; [how to choose and set up](watches.md)). A resource may +instead be shared in named namespaces alone (`-shared-watch-namespaced-resources`), for +an identity granted it there and nowhere else: a stream of it in any other namespace, +or in every namespace at once, is opened as its user, as for a resource not shared, and +never reaches the shared identity. Every stream of one scope of such a resource reads from one watch at the API server, opened with a narrowly scoped identity given by token file, never implicitly the pod's service account. That identity reads more than any one user, so the API server is asked about each user: diff --git a/docs/watches.md b/docs/watches.md index 3cf6406..4dc72bd 100644 --- a/docs/watches.md +++ b/docs/watches.md @@ -132,12 +132,39 @@ Shared watches are off until two flags are given: Resources are named as kubectl names them: `resource.group`, or `resource` for the core group. Every other resource is streamed per-user, as before. +### Shared in some namespaces only + +Some resources the identity must not read cluster-wide. RoleBindings are the example: +an application can watch the grants in its own namespace to learn when to ask "what may +I do?" again, but a ClusterRole for them would let the shared identity read every grant +in the cluster. Name those with the namespaces they are shared in: + +```text +-shared-watch-namespaced-resources=voter/rolebindings.rbac.authorization.k8s.io +``` + +A stream of such a resource in a named namespace reads the shared watch, checked per +user exactly as below. In any other namespace, and across every namespace at once, it +is the user's own stream, opened with their token: krm-foyer never sends it to the +shared identity, which has no grant there. A resource is named in one flag or the +other, not both. With the chart: + +```yaml +sharedWatches: + namespacedResources: + voter: [rolebindings.rbac.authorization.k8s.io] +``` + +The chart grants these with a Role and RoleBinding in each namespace, never a +ClusterRole. + ### The identity The token file holds a service account's token. That account needs: -- `list` and `watch` on each shared resource, wherever users will watch it: usually a - ClusterRole bound with a ClusterRoleBinding; +- `list` and `watch` on each shared resource, wherever users will watch it: a + ClusterRole bound with a ClusterRoleBinding, or for a namespaced resource a Role and + RoleBinding in each of its namespaces; - `create` on `subjectaccessreviews` in `authorization.k8s.io`; - nothing else. @@ -271,7 +298,10 @@ wait for. One could watch RBAC's Roles and Bindings and recheck when they change also have no RBAC event, but require fresh identity resolution and a new token; repeating a SubjectAccessReview alone does not pick them up. - **The shared-watch identity would read every Role and Binding** in the cluster: the - whole access policy, a broader grant than it has now. + whole access policy, a broader grant than it has now. A host that needs RBAC events + for one namespace can share RoleBindings there alone (see + [Shared in some namespaces only](#shared-in-some-namespaces-only)); krm-foyer itself + does not watch them. - **krm-stream rechecks on its own timer,** per stream; a host cannot ask it to recheck now. diff --git a/internal/stream/shared.go b/internal/stream/shared.go index 4a72cd6..f946e78 100644 --- a/internal/stream/shared.go +++ b/internal/stream/shared.go @@ -18,6 +18,7 @@ import ( authorizationv1 "k8s.io/api/authorization/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -43,6 +44,10 @@ type SharedConfig struct { // Resources are the resources whose streams share watches. Any other resource // is streamed with the user's own token, as without sharing. Resources []gateway.GroupResource + // NamespacedResources are shared in the namespaces named for them alone, for an + // identity granted them there and nowhere else. A stream of one in any other + // namespace, or in every namespace, is the user's own, as without sharing. + NamespacedResources []NamespacedResource // RecheckInterval is how often an open shared stream is authorized again. Zero // means DefaultRecheckInterval. RecheckInterval time.Duration @@ -102,18 +107,87 @@ func sharedResource(group, resource string) gateway.GroupResource { return gateway.GroupResource{Group: group, Resource: resource, Scope: gateway.ResourceScopeNamespaced, AllowAllNamespaces: true} } +// NamespacedResource is a resource shared in some namespaces only: RoleBindings in +// the one namespace an application's grants live in, say, where a ClusterRole +// would let the shared identity read every RoleBinding in the cluster. +type NamespacedResource struct { + gateway.GroupResource + Namespaces []string +} + +// ParseNamespacedResources reads namespace/resource pairs, comma-separated, the +// resource as kubectl writes it: voter/rolebindings.rbac.authorization.k8s.io. The +// namespaces of one resource are gathered, so it is listed once. +func ParseNamespacedResources(list string) ([]NamespacedResource, error) { + var out []NamespacedResource + at := map[gateway.GroupResource]int{} + for _, item := range strings.Split(list, ",") { + item = strings.TrimSpace(item) + if item == "" { + continue + } + namespace, name, ok := strings.Cut(item, "/") + if !ok || len(validation.IsDNS1123Label(namespace)) > 0 { + return nil, fmt.Errorf("%q is not a namespace and a resource, such as voter/rolebindings.rbac.authorization.k8s.io", item) + } + parsed, err := ParseResources(name) + if err != nil { + return nil, err + } + if len(parsed) != 1 { + return nil, fmt.Errorf("%q names no resource", item) + } + // Never in every namespace: the identity is granted these namespaces alone. + gr := parsed[0] + gr.AllowAllNamespaces = false + i, seen := at[gr] + if !seen { + i = len(out) + at[gr] = i + out = append(out, NamespacedResource{GroupResource: gr}) + } + if !slices.Contains(out[i].Namespaces, namespace) { + out[i].Namespaces = append(out[i].Namespaces, namespace) + } + } + return out, nil +} + // shared is what shared streams need, made once for the life of the process. type shared struct { backend *gateway.SharedBackend resources []gateway.GroupResource + // confined holds the namespaces a namespaced resource is shared in; a resource + // without an entry is shared in every namespace. + confined map[gateway.GroupResource][]string authorize gateway.Authorizer interval time.Duration } func newShared(cfg SharedConfig, s *Streams) (*shared, error) { - if cfg.TokenFile == "" || len(cfg.Resources) == 0 { + if cfg.TokenFile == "" || len(cfg.Resources)+len(cfg.NamespacedResources) == 0 { return nil, errors.New("shared watches need a token file and at least one resource") } + resources := slices.Clone(cfg.Resources) + confined := map[gateway.GroupResource][]string{} + for _, nr := range cfg.NamespacedResources { + // Shared everywhere and in some namespaces is a contradiction, and the wider + // reading would serve namespaces the identity was never meant to cover. + if slices.ContainsFunc(cfg.Resources, func(gr gateway.GroupResource) bool { + return gr.Group == nr.Group && gr.Resource == nr.Resource + }) { + return nil, fmt.Errorf("%s is shared in every namespace and in some: name it once", nr.Resource) + } + if len(nr.Namespaces) == 0 { + return nil, fmt.Errorf("%s is shared in no namespace", nr.Resource) + } + // Never in every namespace, whatever the caller built: the identity is granted + // these namespaces alone. + gr := nr.GroupResource + gr.AllowAllNamespaces = false + resources = append(resources, gr) + confined[gr] = slices.Clone(nr.Namespaces) + } interval, ttl := cfg.RecheckInterval, cfg.DecisionTTL if interval == 0 { interval = DefaultRecheckInterval @@ -170,7 +244,8 @@ func newShared(cfg SharedConfig, s *Streams) (*shared, error) { backend: gateway.NewSharedBackend(upstream, gateway.SharedOptions{ Observer: observer{metrics: m}, }), - resources: slices.Clone(cfg.Resources), + resources: resources, + confined: confined, authorize: &decisions{ check: kube.SubjectAccessReviewAuthorizer(clientset, subjectOf), ttl: ttl, now: time.Now, metrics: m, entries: map[string]*decision{}, @@ -197,13 +272,19 @@ func (t sharedTransport) RoundTrip(r *http.Request) (*http.Response, error) { return t.next.RoundTrip(r) } -// serves reports whether streams of scope share a watch. +// serves reports whether streams of scope share a watch. A namespaced resource's do +// in its namespaces alone; elsewhere, and in every namespace at once, the stream is +// the user's own, since the shared identity may not watch it there. func (sh *shared) serves(scope gateway.Scope) bool { if sh == nil { return false } return slices.ContainsFunc(sh.resources, func(gr gateway.GroupResource) bool { - return gr.Group == scope.Group && gr.Resource == scope.Resource + if gr.Group != scope.Group || gr.Resource != scope.Resource { + return false + } + namespaces, isConfined := sh.confined[gr] + return !isConfined || slices.Contains(namespaces, scope.Namespace) }) } diff --git a/internal/stream/shared_test.go b/internal/stream/shared_test.go index 6f3ec22..68571d2 100644 --- a/internal/stream/shared_test.go +++ b/internal/stream/shared_test.go @@ -697,6 +697,99 @@ func TestParseResources(t *testing.T) { } } +// Namespaced resources are configured as namespace/resource, the namespaces of one +// resource gathered, and never admitted in every namespace. +func TestParseNamespacedResources(t *testing.T) { + got, err := ParseNamespacedResources(" hello/notes.hello.krm-foyer.example, team/configmaps,team/notes.hello.krm-foyer.example,,hello/notes.hello.krm-foyer.example") + if err != nil { + t.Fatal(err) + } + notes := sharedResource("hello.krm-foyer.example", "notes") + notes.AllowAllNamespaces = false + cms := sharedResource("", "configmaps") + cms.AllowAllNamespaces = false + want := []NamespacedResource{{notes, []string{"hello", "team"}}, {cms, []string{"team"}}} + if fmt.Sprint(got) != fmt.Sprint(want) { + t.Errorf("got %v, want %v", got, want) + } + for _, bad := range []string{"notes.hello.krm-foyer.example", "Hello/notes", "/notes", "hello/", "hello/a/b", "hello.example/notes", "hello/Notes"} { + if _, err := ParseNamespacedResources(bad); err == nil { + t.Errorf("%q was taken", bad) + } + } + file := filepath.Join(t.TempDir(), "token") + if err := os.WriteFile(file, []byte(sharedToken+"\n"), 0o600); err != nil { + t.Fatal(err) + } + everywhere, err := ParseResources("notes.hello.krm-foyer.example") + if err != nil { + t.Fatal(err) + } + for name, cfg := range map[string]SharedConfig{ + "shared everywhere and in some namespaces": {TokenFile: file, Resources: everywhere, NamespacedResources: got}, + "shared in no namespace": {TokenFile: file, NamespacedResources: []NamespacedResource{{GroupResource: cms}}}, + } { + if _, err := New(Config{Server: mustURL(t, "https://api.example"), Gate: mustGate(t), Shared: &cfg}); err == nil { + t.Errorf("%s was taken", name) + } + } +} + +// A resource shared in some namespaces is shared there alone. The identity is +// granted those namespaces and no others, so a stream anywhere else, and one of every +// namespace at once, must never reach the shared watch: it is the user's own, opened +// with the user's token, with no review sent about it. +func TestANamespacedResourceIsSharedInItsNamespacesAlone(t *testing.T) { + api := newSharedAPI(t) + namespaced, err := ParseNamespacedResources("hello/notes.hello.krm-foyer.example") + if err != nil { + t.Fatal(err) + } + m := metrics.New() + f := sharedFoyer(t, api, byHeader{}, m, time.Minute, func(c *Config) { + c.Shared.Resources = nil + c.Shared.NamespacedResources = namespaced + }) + elsewhere := strings.Replace(notes, "namespace=hello", "namespace=elsewhere", 1) + everywhere := strings.Replace(notes, "&namespace=hello", "", 1) + for _, target := range []string{notes, notes, elsewhere, everywhere} { + f.open(t.Context(), t, target, as(userToken)).until(t, "synced") + } + + for _, r := range api.received() { + if !strings.HasSuffix(r.URL.Path, "/notes") { + continue + } + auth := r.Header.Get("Authorization") + switch r.URL.Path { + case "/apis/hello.krm-foyer.example/v1/namespaces/hello/notes": + if auth != "Bearer "+sharedToken { + t.Errorf("%s %s was sent with %q, want the shared token", r.Method, r.URL.Path, auth) + } + case "/apis/hello.krm-foyer.example/v1/namespaces/elsewhere/notes", "/apis/hello.krm-foyer.example/v1/notes": + if auth != "Bearer "+userToken { + t.Errorf("%s %s was sent with %q, want the user's token: the identity may not watch there", r.Method, r.URL.Path, auth) + } + default: + t.Errorf("unexpected %s %s", r.Method, r.URL.Path) + } + } + for _, spec := range api.subjectReviews() { + if attrs := spec.ResourceAttributes; attrs == nil || attrs.Namespace != "hello" { + t.Errorf("a review about %+v; want reviews of the shared namespace alone", attrs) + } + } + for series, want := range map[string]string{ + `krm_foyer_upstream_watches_open{identity="shared"}`: "1", + "krm_foyer_shared_subscriptions_open": "2", + userWatches: "2", + } { + if got := gauge(t, m, series); got != want { + t.Errorf("%s %s, want %s", series, got, want) + } + } +} + // A decision is reused for the very question it answered, for its lifetime, and // one asked twice at once is asked once. An error is not kept. func TestDecisionsAreReusedForTheSameQuestionOnly(t *testing.T) { From 5e4fef4920aee6c25a6f1cf51f6ca67f3ae91367 Mon Sep 17 00:00:00 2001 From: Simon Koudijs Date: Thu, 29 Oct 2026 15:37:54 +0000 Subject: [PATCH 2/2] test: prove a namespaced shared resource shows nothing of other namespaces The e2e fixture shares RoleBindings in foyer-shared alone. One spec opens them there, in another namespace, and across every namespace, as a user allowed in the shared namespace only and as one allowed in the other. It checks the streams' contents, the watch metrics, SubjectAccessReviews of the shared identity, and the audit log: the identity read RoleBindings in foyer-shared and nowhere else. Mutation-checked: with serves() ignoring the namespace, it fails. Co-Authored-By: Claude Opus 5.5 --- test/e2e/cluster/deploy-foyer.sh | 8 ++ test/e2e/cluster/foyer-values.yaml | 4 + test/e2e/fixture_test.go | 4 + test/e2e/foyer_shared_test.go | 143 +++++++++++++++++++++++++++++ 4 files changed, 159 insertions(+) diff --git a/test/e2e/cluster/deploy-foyer.sh b/test/e2e/cluster/deploy-foyer.sh index b1ee88e..b32b5af 100755 --- a/test/e2e/cluster/deploy-foyer.sh +++ b/test/e2e/cluster/deploy-foyer.sh @@ -62,6 +62,13 @@ apiVersion: v1 kind: Namespace metadata: name: krm-foyer +--- +# Where RoleBindings are shared (foyer-values.yaml): the chart puts the shared +# identity's Role here, so it must exist first. +apiVersion: v1 +kind: Namespace +metadata: + name: foyer-shared EOF # A fixture from before the chart has the same objects, applied with kubectl, which Helm # refuses to take over. Remove them once. @@ -120,5 +127,6 @@ FOYER_NAMESPACE=krm-foyer FOYER_IMAGE=$tag FOYER_SERVICE_ACCOUNT=system:serviceaccount:krm-foyer:krm-foyer FOYER_SHARED_ACCOUNT=system:serviceaccount:krm-foyer:krm-foyer-shared +FOYER_SHARED_NAMESPACE=foyer-shared EOF echo "krm-foyer ready at $FOYER_URL ($server_ip:$NODE_PORT)" diff --git a/test/e2e/cluster/foyer-values.yaml b/test/e2e/cluster/foyer-values.yaml index ba2054b..737f618 100644 --- a/test/e2e/cluster/foyer-values.yaml +++ b/test/e2e/cluster/foyer-values.yaml @@ -21,8 +21,12 @@ login: sessionKeys: secretName: krm-foyer-session-keys # Notes share one watch per scope: the hello example's, and the rehearsal's. +# RoleBindings are shared in foyer-shared alone (deploy-foyer.sh creates it): the +# identity is granted them there with a Role, and nowhere else. sharedWatches: resources: [notes.hello.krm-foyer.example] + namespacedResources: + foyer-shared: [rolebindings.rbac.authorization.k8s.io] # The bait (foyer-bait.yaml): the pod's account is cluster-admin, and its token is # mounted, so a fallback to it would turn a 401 or a 403 into a 200: loud, where a # fallback to a weak account would be silent. The suite checks the bait is in place diff --git a/test/e2e/fixture_test.go b/test/e2e/fixture_test.go index 67795bb..8102a5b 100644 --- a/test/e2e/fixture_test.go +++ b/test/e2e/fixture_test.go @@ -56,6 +56,9 @@ type fixture struct { // sharedAccount is the identity shared watches are opened with: narrow, and not // the bait. sharedAccount string + // sharedNamespace is the one namespace RoleBindings are shared in, and the only + // one the shared identity may read them in. + sharedNamespace string // briefTransport reaches the brief krm-foyer, whose sessions end within a minute, // instead of the main one. briefTransport http.RoundTripper @@ -177,6 +180,7 @@ func loadFixture() *fixture { foyerNamespace: env["FOYER_NAMESPACE"], foyerAccount: env["FOYER_SERVICE_ACCOUNT"], sharedAccount: env["FOYER_SHARED_ACCOUNT"], + sharedNamespace: env["FOYER_SHARED_NAMESPACE"], briefTransport: transportTo(env["FOYER_BRIEF_ADDR"]), // The front door: Traefik, through the port-forward on this container's // loopback (port-forward.sh), under the same name as krm-foyer, as a browser diff --git a/test/e2e/foyer_shared_test.go b/test/e2e/foyer_shared_test.go index 11d90c7..8edd22f 100644 --- a/test/e2e/foyer_shared_test.go +++ b/test/e2e/foyer_shared_test.go @@ -4,7 +4,11 @@ package e2e import ( "context" + "fmt" "net/http" + "net/url" + "os" + "path/filepath" "strings" "time" @@ -145,6 +149,103 @@ var _ = Describe("krm-foyer's shared watches", Label("foyer"), func() { Expect(fx.metric(sharedWatches)).To(Equal(before + 1)) }) + // RoleBindings are shared in one namespace alone (foyer-values.yaml), with a Role + // there: a ClusterRole for them would let the identity read every grant in the + // cluster. What must hold is that nothing of another namespace comes out: not + // through the shared watch, which may not read it, and not by the shared identity + // asking on anyone's behalf. Elsewhere a stream is the user's own, and the API + // server answers it as the user. + It("shares a namespaced resource in its namespace alone, and shows nothing of any other", func(ctx SpecContext) { + shared, other := fx.sharedNamespace, fx.namespace() + id := randomID() + visible, secret := "e2e-visible-"+id, "e2e-secret-"+id + fx.kubectl("-n", shared, "create", "rolebinding", visible, "--clusterrole=view", "--user=nobody-"+id) + DeferCleanup(func() { fx.kubectl("-n", shared, "delete", "rolebinding", visible, "--ignore-not-found") }) + fx.kubectl("-n", other, "create", "rolebinding", secret, "--clusterrole=view", "--user=nobody-"+id) + DeferCleanup(fx.grant(shared, aliceK8sName, "rolebindings.rbac.authorization.k8s.io", "get", "list", "watch")) + fx.grant(other, bobK8sName, "rolebindings.rbac.authorization.k8s.io", "get", "list", "watch") // not alice + alice, bob := signIn(ctx, alice), signIn(ctx, bob) + beforeShared, beforeUser := fx.metric(sharedWatches), fx.metric(userWatches) + + By("the shared namespace: served from the shared watch, with that namespace's grants alone") + inShared := alice.open(ctx, roleBindingStream(shared)) + Expect(inShared.resp.StatusCode).To(Equal(http.StatusOK)) + names := map[string]bool{} + for _, e := range inShared.until("synced") { + names[e.Object.Metadata.Name] = true + } + Expect(names).To(HaveKey(visible)) + Expect(names).NotTo(HaveKey(secret)) + Expect(fx.metric(sharedWatches)).To(Equal(beforeShared + 1)) + Expect(fx.metric(userWatches)).To(Equal(beforeUser)) + + By("another namespace, which alice may not read: refused as alice, with nothing of it") + elsewhere := alice.open(ctx, roleBindingStream(other)) + e := elsewhere.krmEvent() + Expect(e.Type).To(Equal("error")) + Expect(e.Code).To(Equal("FORBIDDEN")) + Expect(e.Terminal).To(BeTrue()) + _, err := elsewhere.end(10 * time.Second) + Expect(err).NotTo(HaveOccurred()) + Expect(elsewhere.read.String()).NotTo(ContainSubstring(secret)) + Expect(elsewhere.read.String()).NotTo(ContainSubstring(fx.sharedAccount)) + expectAskedOnlyAs(ctx, elsewhere, aliceK8sName, "/apis/rbac.authorization.k8s.io/v1/namespaces/"+other+"/rolebindings") + + By("every namespace at once: refused as alice, with nothing of any namespace") + everywhere := alice.open(ctx, roleBindingStream("")) + e = everywhere.krmEvent() + Expect(e.Type).To(Equal("error")) + Expect(e.Code).To(Equal("FORBIDDEN")) + Expect(e.Terminal).To(BeTrue()) + _, err = everywhere.end(10 * time.Second) + Expect(err).NotTo(HaveOccurred()) + Expect(everywhere.read.String()).NotTo(ContainSubstring(secret)) + Expect(everywhere.read.String()).NotTo(ContainSubstring(visible)) + expectAskedOnlyAs(ctx, everywhere, aliceK8sName, "/apis/rbac.authorization.k8s.io/v1/rolebindings") + + By("another namespace, which bob may read: his own watch, with that namespace's grants alone") + bobs := bob.open(ctx, roleBindingStream(other)) + names = map[string]bool{} + for _, e := range bobs.until("synced") { + names[e.Object.Metadata.Name] = true + } + Expect(names).To(HaveKey(secret)) + Expect(names).NotTo(HaveKey(visible)) + Expect(fx.metric(sharedWatches)).To(Equal(beforeShared+1), "bob's stream must not open a shared watch") + Expect(fx.metric(userWatches)).To(Equal(beforeUser + 1)) + _ = bobs.resp.Body.Close() + _, _ = bobs.end(10 * time.Second) + expectAskedOnlyAs(ctx, bobs, bobK8sName, "/apis/rbac.authorization.k8s.io/v1/namespaces/"+other+"/rolebindings") + + By("the API server: the shared identity may not read RoleBindings outside its namespace") + for _, ns := range []string{other, ""} { + for _, verb := range []string{"list", "watch"} { + Expect(fx.accessReview(fx.sharedAccount, verb, "rbac.authorization.k8s.io", "rolebindings", ns)). + To(BeFalse(), "%s may %s rolebindings in %q", fx.sharedAccount, verb, ns) + } + } + Expect(fx.accessReview(fx.sharedAccount, "watch", "rbac.authorization.k8s.io", "rolebindings", shared)).To(BeTrue()) + + By("the audit log: the shared identity read RoleBindings in the shared namespace, and nowhere else") + _ = inShared.resp.Body.Close() + _, _ = inShared.end(10 * time.Second) + eventually(ctx, func() float64 { return fx.metric(sharedWatches) }).Should(Equal(beforeShared)) + var reads []auditEvent + eventually(ctx, func() []auditEvent { + reads = nil + for _, ev := range fx.audited(fx.sharedAccount) { + if ev.User.Username == fx.sharedAccount && strings.Contains(ev.RequestURI, "/rolebindings") { + reads = append(reads, ev) + } + } + return reads + }).ShouldNot(BeEmpty()) + for _, ev := range reads { + Expect(ev.RequestURI).To(HavePrefix("/apis/rbac.authorization.k8s.io/v1/namespaces/"+shared+"/rolebindings"), + "the shared identity sent %s %s", ev.Verb, ev.RequestURI) + } + }) + It("ends a user's shared stream at the next recheck once RBAC no longer allows it, and only theirs", func(ctx SpecContext) { ns := fx.namespace() fx.grant(ns, aliceK8sName, "notes.hello.krm-foyer.example", "get", "list", "watch") @@ -182,3 +283,45 @@ var _ = Describe("krm-foyer's shared watches", Label("foyer"), func() { Expect(changed[len(changed)-1].Object.Spec).To(HaveKeyWithValue("text", "after bob's grant")) }) }) + +// roleBindingStream is the /stream/v1 target for the RoleBindings of namespace ns, or +// of every namespace when ns is empty. They are shared in fx.sharedNamespace alone. +func roleBindingStream(ns string) string { + q := url.Values{"group": {"rbac.authorization.k8s.io"}, "version": {"v1"}, "resource": {"rolebindings"}} + if ns != "" { + q.Set("namespace", ns) + } + return fx.foyerURL + "/stream/v1?" + q.Encode() +} + +// expectAskedOnlyAs checks, in the audit log, that every request s led to was sent as +// username, impersonating nobody, and that one of them was for a path under prefix: +// the stream was the user's own, and the shared identity was never asked. +func expectAskedOnlyAs(ctx SpecContext, s *stream, username, prefix string) { + GinkgoHelper() + var events []auditEvent + eventually(ctx, func() []auditEvent { events = fx.audited(s.Marker); return events }).ShouldNot(BeEmpty()) + asked := false + for _, ev := range events { + Expect(ev.User.Username).To(Equal(username), "%s %s", ev.Verb, ev.RequestURI) + Expect(ev.ImpersonatedUser).To(BeNil()) + asked = asked || strings.HasPrefix(ev.RequestURI, prefix) + } + Expect(asked).To(BeTrue(), "no request under %s in %+v", prefix, events) +} + +// accessReview asks the API server, as the fixture's admin, whether a service account +// may verb resource in ns, or in every namespace when ns is empty. A review names its +// groups itself, so the account's are given as the API server would resolve them. +func (f *fixture) accessReview(account, verb, group, resource, ns string) bool { + GinkgoHelper() + parts := strings.Split(account, ":") // system:serviceaccount:: + Expect(parts).To(HaveLen(4)) + review := fmt.Sprintf(`{"apiVersion":"authorization.k8s.io/v1","kind":"SubjectAccessReview","spec":{`+ + `"user":%q,"groups":["system:serviceaccounts","system:serviceaccounts:%s","system:authenticated"],`+ + `"resourceAttributes":{"verb":%q,"group":%q,"resource":%q,"namespace":%q}}}`, + account, parts[2], verb, group, resource, ns) + file := filepath.Join(GinkgoT().TempDir(), "review.json") + Expect(os.WriteFile(file, []byte(review), 0o600)).To(Succeed()) + return f.kubectl("create", "-f", file, "-o", "jsonpath={.status.allowed}") == "true" +}