The Kubernetes credential provider reloads its namespace policy without a restart

cmd/credential-provider/kubernetes-secrets/kubeprovider.go

@@ -133,6 +133,10 @@ func NewServer(client kubernetes.Interface, nsAuth *NamespaceAuthorizer) *Server
 // when authorization is disabled.
 func (s *Server) Grants() map[string][]string { return s.nsAuth.Grants() }
 
+// PolicyError reports why the policy file failed to load while an older
+// policy stays in force, for /statusz.
+func (s *Server) PolicyError() error { return s.nsAuth.LoadError() }
+
 // FetchSecret resolves one ate-secret:// URI to its Secret value.
 func (s *Server) FetchSecret(ctx context.Context, req *credproviderpb.FetchSecretRequest) (*credproviderpb.FetchSecretResponse, error) {
 	ref, err := ParseURI(req.GetUri())

cmd/credential-provider/kubernetes-secrets/kubeprovider_test.go

@@ -16,7 +16,9 @@ package main
 
 import (
 	"context"
+	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -456,6 +458,105 @@ func TestLoadNamespaceAuthorizerRejectsUnknownFields(t *testing.T) {
 	}
 }
 
+// The provider rereads its policy when the file changes, and keeps the last
+// good policy while the file fails to load.
+func TestLoadNamespaceAuthorizerReloads(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "policy.yaml")
+	dataLink := filepath.Join(dir, "..data")
+	// path resolves through the ..data symlink, and each step swaps that
+	// symlink to a new directory, the way the kubelet updates a ConfigMap
+	// volume.
+	if err := os.Symlink(filepath.Join("..data", "policy.yaml"), path); err != nil {
+		t.Fatalf("link policy: %v", err)
+	}
+	version := 0
+	replace := func(t *testing.T, content string) {
+		t.Helper()
+		version++
+		target := fmt.Sprintf("..v%d", version)
+		if err := os.Mkdir(filepath.Join(dir, target), 0o700); err != nil {
+			t.Fatalf("create policy dir: %v", err)
+		}
+		if err := os.WriteFile(filepath.Join(dir, target, "policy.yaml"), []byte(content), 0o600); err != nil {
+			t.Fatalf("write policy: %v", err)
+		}
+		tmp := filepath.Join(dir, "..data_tmp")
+		if err := os.Symlink(target, tmp); err != nil {
+			t.Fatalf("link policy dir: %v", err)
+		}
+		if err := os.Rename(tmp, dataLink); err != nil {
+			t.Fatalf("swap policy dir: %v", err)
+		}
+	}
+	replace(t, "policies:\n- atespace: team-a\n  allowedNamespaces: [ns1]\n")
+	authz, err := LoadNamespaceAuthorizer(path)
+	if err != nil {
+		t.Fatalf("LoadNamespaceAuthorizer: %v", err)
+	}
+
+	steps := []struct {
+		name string
+		// content replaces the file; empty removes it.
+		content     string
+		wantAllowed []string
+		wantRefused []string
+		wantErr     bool
+	}{
+		{
+			name:        "grant moves to another namespace",
+			content:     "policies:\n- atespace: team-a\n  allowedNamespaces: [ns2]\n",
+			wantAllowed: []string{"ns2"},
+			wantRefused: []string{"ns1"},
+		},
+		{
+			name:        "malformed file keeps the previous policy",
+			content:     "policies:\n- atespace: team-a\n  allowedNamespace: [ns3]\n",
+			wantAllowed: []string{"ns2"},
+			wantRefused: []string{"ns3"},
+			wantErr:     true,
+		},
+		{
+			name:        "fixed file loads",
+			content:     "policies:\n- atespace: team-a\n  allowedNamespaces: [ns3]\n",
+			wantAllowed: []string{"ns3"},
+			wantRefused: []string{"ns2"},
+		},
+		{
+			name:        "missing file keeps the previous policy",
+			wantAllowed: []string{"ns3"},
+			wantErr:     true,
+		},
+		{
+			name:        "file comes back",
+			content:     "policies: []\n",
+			wantRefused: []string{"ns3"},
+		},
+	}
+	for _, step := range steps {
+		if step.content == "" {
+			if err := os.Remove(dataLink); err != nil {
+				t.Fatalf("%s: remove policy: %v", step.name, err)
+			}
+		} else {
+			replace(t, step.content)
+		}
+		for _, ns := range step.wantAllowed {
+			if !authz.Allowed("team-a", ns) || !authz.AllowedSecret("team-a", ns, nil) {
+				t.Errorf("%s: team-a/%s refused, want allowed", step.name, ns)
+			}
+		}
+		for _, ns := range step.wantRefused {
+			if authz.Allowed("team-a", ns) || authz.AllowedSecret("team-a", ns, nil) {
+				t.Errorf("%s: team-a/%s allowed, want refused", step.name, ns)
+			}
+		}
+		if err := authz.LoadError(); (err != nil) != step.wantErr {
+			t.Errorf("%s: LoadError() = %v, want error %v", step.name, err, step.wantErr)
+		}
+	}
+}
+
 // Each policy is its own grant and they are OR-ed. Merging them into one would
 // let a narrow policy take away what a broad one granted, and would AND the
 // selectors of separate policies together.

cmd/credential-provider/kubernetes-secrets/main.go

@@ -110,7 +110,6 @@ func run(ctx context.Context) error {
 	if err != nil {
 		return fmt.Errorf("namespace policy: %w", err)
 	}
-	slog.InfoContext(ctx, "loaded namespace authorization policy", slog.String("file", *nsPolicyFile))
 
 	creds, err := buildServerCreds(ctx)
 	if err != nil {

cmd/credential-provider/kubernetes-secrets/nsauthz.go

@@ -16,8 +16,10 @@ package main
 
 import (
 	"fmt"
+	"log/slog"
 	"os"
 	"sort"
+	"sync"
 
 	"k8s.io/apimachinery/pkg/util/validation"
 
@@ -69,8 +71,20 @@ type grant struct {
 
 // NamespaceAuthorizer decides whether an atespace may resolve secrets in a given
 // Kubernetes namespace. It is default-deny: an atespace absent from the mapping
-// can resolve nothing.
+// can resolve nothing. One loaded from a file rereads the file when it changes.
 type NamespaceAuthorizer struct {
+	// path is the policy file; empty when the authorizer was built from an
+	// already parsed policy and never reloads.
+	path string
+
+	mu sync.Mutex
+	// fi is the stat of path taken before its last parse, successful or not,
+	// so a file that fails to parse is read and logged once rather than per
+	// request. Nil when path could not be stat'ed, so the next call retries.
+	fi os.FileInfo
+	// loadErr is why the file as it is now failed to load; allowed then holds
+	// the last policy that loaded.
+	loadErr error
 	// allowed maps atespace -> namespace -> the grants that apply there. Each
 	// policy contributes its own grant and they are OR-ed: a Secret is admitted
 	// when any one of them admits it. Within a grant the criteria are AND-ed,
@@ -79,8 +93,19 @@ type NamespaceAuthorizer struct {
 }
 
 // LoadNamespaceAuthorizer reads the YAML policy file at path and builds an
-// authorizer, so a malformed file fails startup rather than the first request.
+// authorizer that rereads the file whenever it changes. A malformed file fails
+// here, at startup; once running, a file that fails to load leaves the last
+// good policy in force.
 func LoadNamespaceAuthorizer(path string) (*NamespaceAuthorizer, error) {
+	a := &NamespaceAuthorizer{path: path}
+	if _, err := a.policy(); err != nil {
+		return nil, err
+	}
+	return a, nil
+}
+
+// readNamespacePolicy parses the YAML policy file at path.
+func readNamespacePolicy(path string) (*NamespaceAuthorizer, error) {
 	data, err := os.ReadFile(path)
 	if err != nil {
 		return nil, fmt.Errorf("reading namespace policy file %q: %w", path, err)
@@ -133,13 +158,63 @@ func newNamespaceAuthorizer(file namespacePolicyFile) (*NamespaceAuthorizer, err
 	return &NamespaceAuthorizer{allowed: allowed}, nil
 }
 
+// policy returns the grants in force, first rereading the policy file if its
+// stat no longer matches the last one parsed. The kubelet updates a ConfigMap
+// volume by swapping in a new file, so the stat changes even when the size
+// does not. The error is why the file as it is now failed to load.
+func (a *NamespaceAuthorizer) policy() (map[string]map[string][]grant, error) {
+	a.mu.Lock()
+	defer a.mu.Unlock()
+	if a.path == "" {
+		return a.allowed, nil
+	}
+	fi, err := os.Stat(a.path)
+	if err != nil {
+		a.fi = nil
+		return a.allowed, a.loadFailed(fmt.Errorf("reading namespace policy file %q: %w", a.path, err))
+	}
+	if a.fi != nil && os.SameFile(a.fi, fi) && fi.ModTime().Equal(a.fi.ModTime()) && fi.Size() == a.fi.Size() {
+		return a.allowed, a.loadErr
+	}
+	a.fi = fi
+	next, err := readNamespacePolicy(a.path)
+	if err != nil {
+		return a.allowed, a.loadFailed(err)
+	}
+	a.allowed, a.loadErr = next.allowed, nil
+	slog.Info("loaded namespace authorization policy", slog.String("file", a.path))
+	return a.allowed, nil
+}
+
+// loadFailed records err as the load error and returns it. It logs err unless
+// it repeats the last one or no policy has loaded yet, when
+// LoadNamespaceAuthorizer returns it instead. The caller holds a.mu.
+func (a *NamespaceAuthorizer) loadFailed(err error) error {
+	if a.allowed != nil && (a.loadErr == nil || a.loadErr.Error() != err.Error()) {
+		slog.Error("namespace policy file failed to load, keeping the last policy that loaded", slog.String("file", a.path), slog.Any("err", err))
+	}
+	a.loadErr = err
+	return err
+}
+
+// LoadError reports why the policy file failed to load, or nil when the
+// policy in force is the file's current content.
+func (a *NamespaceAuthorizer) LoadError() error {
+	if a == nil {
+		return nil
+	}
+	_, err := a.policy()
+	return err
+}
+
 // Grants returns the loaded policy as atespace → sorted namespaces
 func (a *NamespaceAuthorizer) Grants() map[string][]string {
 	if a == nil {
 		return nil
 	}
-	out := make(map[string][]string, len(a.allowed))
-	for atespace, namespaces := range a.allowed {
+	allowed, _ := a.policy()
+	out := make(map[string][]string, len(allowed))
+	for atespace, namespaces := range allowed {
 		list := make([]string, 0, len(namespaces))
 		for ns := range namespaces {
 			list = append(list, ns)
@@ -154,7 +229,8 @@ func (a *NamespaceAuthorizer) Grants() map[string][]string {
 // deny: an atespace absent from the mapping, or a namespace not in its list, is
 // refused.
 func (a *NamespaceAuthorizer) Allowed(atespace, namespace string) bool {
-	set, ok := a.allowed[atespace]
+	allowed, _ := a.policy()
+	set, ok := allowed[atespace]
 	if !ok {
 		return false
 	}
@@ -164,9 +240,11 @@ func (a *NamespaceAuthorizer) Allowed(atespace, namespace string) bool {
 // AllowedSecret reports whether atespace may resolve a Secret with these
 // labels in namespace. A grant without a label selector admits every Secret,
 // including one that does not exist: pass nil labels to ask that question
-// before reading anything from Kubernetes.
+// before reading anything from Kubernetes. The namespace grant is checked again
+// here, so a request that straddles a reload is settled by the newer policy.
 func (a *NamespaceAuthorizer) AllowedSecret(atespace, namespace string, labels map[string]string) bool {
-	for _, g := range a.allowed[atespace][namespace] {
+	allowed, _ := a.policy()
+	for _, g := range allowed[atespace][namespace] {
 		if len(g.labels) == 0 {
 			return true
 		}

cmd/credential-provider/kubernetes-secrets/statusz.go

@@ -15,9 +15,9 @@
 // The provider's /statusz page, mirroring the atenet router's: one handler
 // serving an HTML dashboard by default and JSON under ?format=json (or
 // Accept: application/json). It shows the provider's configuration as the
-// running process holds it — notably the authorization policy loaded at
-// startup, which the ConfigMap on disk may have since drifted from. Names
-// and configuration only, never secret values.
+// running process holds it — notably the authorization policy in force, which
+// stays at the last version that loaded while the file on disk fails to parse.
+// Names and configuration only, never secret values.
 package main
 
 import (
@@ -41,6 +41,7 @@ type statusContext struct {
 	Flags        map[string]string   `json:"flags"`
 	InjectorSAN  string              `json:"injector_san"`
 	PolicyGrants map[string][]string `json:"policy_grants"`
+	PolicyError  string              `json:"policy_error,omitempty"`
 }
 
 // newStatuszHandler serves /statusz over the provider's state.
@@ -57,6 +58,9 @@ func newStatuszHandler(srv *Server) http.HandlerFunc {
 			InjectorSAN:  *injectorIdentity,
 			PolicyGrants: srv.Grants(),
 		}
+		if err := srv.PolicyError(); err != nil {
+			data.PolicyError = err.Error()
+		}
 
 		if strings.Contains(req.Header.Get("Accept"), "application/json") || req.URL.Query().Get("format") == "json" {
 			w.Header().Set("Content-Type", "application/json")
@@ -88,7 +92,8 @@ provider: {{.ProviderName}}<br>
 required caller SAN: {{.InjectorSAN}}<br>
 args: {{.Args}}</p>
 
-<h2>Namespace policy (as loaded at startup)</h2>
+<h2>Namespace policy</h2>
+{{if .PolicyError}}<p><b>policy file failed to load, previous policy in force:</b> {{.PolicyError}}</p>{{end}}
 {{if .PolicyGrants}}
 <table>
 <tr><th>atespace</th><th>allowed namespaces</th></tr>

docs/egress-credential-injection.md

@@ -244,12 +244,10 @@ kubectl -n ate-system edit configmap k8s-credential-provider-namespace-policy
       - ns1
 ```
 
-The provider loads the policy **once at startup** and does not yet reload it,
-so after editing, restart the provider:
-
-```bash
-kubectl -n ate-system rollout restart deployment/k8s-credential-provider
-```
+The provider rereads the policy when the kubelet updates the mounted ConfigMap,
+which can take a minute or so; no restart is needed. If the edited policy fails
+to parse, the provider logs the error and keeps enforcing the previous one;
+`/statusz` on the provider shows the error and the grants in force.
 
 For a first smoke test, grant your actor's atespace `ns1` as above and apply a
 sample secret that the URI in the sample policy resolves to:

internal/e2e/credprovider.go

@@ -49,8 +49,9 @@ const (
 // ConfigureCredentialProvider points the install's k8s-credential-provider at
 // the credinject fixture: the Secret behind CredentialInjectionURI and the
 // authorization policy that lets the probe atespaces resolve it. The provider
-// Deployment is restarted after the policy ConfigMap is applied because it
-// reads the policy once at startup. When the test passes the fixture is
+// Deployment is restarted after the policy ConfigMap is applied, which is
+// quicker than waiting for the kubelet to update the mounted policy the
+// provider rereads. When the test passes the fixture is
 // removed, the shipped policy put back, and the provider restarted again so
 // it drops the fixture's grants.
 //

manifests/egress-credential-injection/namespace-policy.yaml

@@ -16,10 +16,7 @@
 # atespace may only resolve Secrets whose URI namespace is listed for it here.
 # Enforcement is default-deny — an atespace absent from this file resolves
 # nothing. The install creates this ConfigMap only when it is absent, granting
-# nothing; the provider needs it to start.
-#
-# TODO: k8s-credential-provider loads this once at startup, so editing this ConfigMap
-# requires restarting the k8s-credential-provider Deployment. Make it reload dynamically.
+# nothing; the provider needs it to start, and rereads it when it changes.
 apiVersion: v1
 kind: ConfigMap
 metadata: