feat: fill an empty placeholder serving-CA Secret instead of creating it

docs/how-to/deploy-aggregated-apiserver.md

@@ -117,9 +117,10 @@ These resources are backed by Coder, not etcd, so some Kubernetes behavior diffe
 In a cluster, the aggregated API server serves a certificate signed by its own CA. Both live in the Secret `coder-k8s-apiserver-tls` in the server's namespace (type `coder.com/aggregated-apiserver-serving-ca`, label `app.kubernetes.io/component: aggregated-apiserver-serving-ca`). The certificate is valid for `coder-k8s-apiserver`, `coder-k8s-apiserver.<namespace>`, `coder-k8s-apiserver.<namespace>.svc`, and `coder-k8s-apiserver.<namespace>.svc.cluster.local`.
 
 - The server creates the Secret on first start and reuses it afterwards. With several replicas, they all use the same Secret.
+- If the Secret already exists as an empty placeholder (type `coder.com/aggregated-apiserver-serving-ca`, no `data` keys at all, and not `immutable`), the server fills it with a new CA instead of creating it, so it does not need `create` on Secrets. If the Secret does not exist and the server may not create Secrets, it does not start, and the log says to create the placeholder.
 - The serving certificate is valid for 1 year. The server checks it at startup and every 12 hours, and renews it with the same CA when less than a third of its lifetime is left. The new certificate is served without a restart.
 - The CA is valid for 10 years. To replace it earlier (for example after the Secret was exposed), see [Replace the CA](#replace-the-ca).
-- If the Secret exists but is unusable (a missing key, unparsable PEM, a key that does not match its certificate, a serving certificate not signed by the CA, or an expired CA), the server does not start and the log names the field. Fix the Secret or delete it.
+- If the Secret exists but is unusable and is not an empty placeholder (a missing key, unparsable PEM, a key that does not match its certificate, a serving certificate not signed by the CA, or an expired CA), the server does not start and the log names the field. Fix the Secret or delete it.
 - Outside a cluster (for example `go run`), the server serves a self-signed certificate for `localhost` instead.
 
 !!! warning "The Secret holds the CA private key"

docs/how-to/troubleshooting.md

@@ -142,7 +142,11 @@ kubectl -n coder-system delete secret coder-k8s-apiserver-tls
 kubectl -n coder-system rollout restart deployment/coder-k8s
 ```
 
-A read or create error instead of a field name means the ServiceAccount cannot get or create Secrets in its namespace.
+An error without a field name means the ServiceAccount is missing a permission on this Secret. The message says which:
+
+- `get secret …`: it may not read the Secret. Grant `get` on `coder-k8s-apiserver-tls`.
+- `create secret …`: the Secret does not exist and the ServiceAccount may not create Secrets. Grant `create`, or create the empty placeholder that the message describes (the server fills it).
+- `fill placeholder secret …` or `update secret …`: the ServiceAccount may not update the Secret, to fill a placeholder or to renew the serving certificate. Grant `update` on `coder-k8s-apiserver-tls`.
 
 ## Aggregated requests fail with `401 Unauthorized` or `403 Forbidden`
 

internal/aggregated/servingcert/manager.go

@@ -53,6 +53,9 @@ func NewManager(client kubernetes.Interface, namespace string) (*Manager, error)
 
 // Ensure loads, creates, or renews the Secret and makes it the served certificate. Invalid
 // trust material returns a *CorruptSecretError and is never overwritten.
+//
+// An existing placeholder (see IsPlaceholder) is filled with a new CA instead of being created,
+// so an identity that may only get and update this one Secret can still bootstrap it.
 func (m *Manager) Ensure(ctx context.Context) (*Bundle, error) {
 	if ctx == nil {
 		return nil, fmt.Errorf("assertion failed: context must not be nil")
@@ -75,6 +78,11 @@ func (m *Manager) Ensure(ctx context.Context) (*Bundle, error) {
 			if apierrors.IsAlreadyExists(err) {
 				continue // Another replica created it first; adopt theirs.
 			}
+			if apierrors.IsForbidden(err) {
+				return nil, fmt.Errorf("create secret %s/%s: %w; if this identity may not create Secrets, "+
+					"create an empty placeholder Secret %q of type %q (no data) and the server fills it",
+					m.namespace, SecretName, err, SecretName, SecretType)
+			}
 			if err != nil {
 				return nil, fmt.Errorf("create secret %s/%s: %w", m.namespace, SecretName, err)
 			}
@@ -84,6 +92,40 @@ func (m *Manager) Ensure(ctx context.Context) (*Bundle, error) {
 			return nil, fmt.Errorf("get secret %s/%s: %w", m.namespace, SecretName, err)
 		}
 
+		if secret.Type == SecretType && len(secret.Data) == 0 && isImmutable(secret) {
+			// Kubernetes forbids data changes on an immutable Secret, so it can never be filled.
+			return nil, corrupt(m.namespace, "empty placeholder is immutable and cannot be filled; recreate it without immutable: true")
+		}
+		if IsPlaceholder(secret) {
+			bundle, genErr := Generate(m.namespace, now)
+			if genErr != nil {
+				return nil, genErr
+			}
+			filled := secret.DeepCopy()
+			filled.Data = bundle.Data()
+			// Mark it like a created Secret, keeping any labels the placeholder already has.
+			if filled.Labels == nil {
+				filled.Labels = map[string]string{}
+			}
+			for k, v := range SecretLabels {
+				filled.Labels[k] = v
+			}
+			// Without a resourceVersion the update would be unconditional and could replace a CA
+			// that another replica wrote in the meantime.
+			if filled.ResourceVersion == "" {
+				return nil, fmt.Errorf("assertion failed: placeholder secret %s/%s has no resourceVersion", m.namespace, SecretName)
+			}
+			_, err = secrets.Update(ctx, filled, metav1.UpdateOptions{})
+			if apierrors.IsConflict(err) {
+				continue // Another replica filled it first; re-read and adopt theirs.
+			}
+			if err != nil {
+				return nil, fmt.Errorf("fill placeholder secret %s/%s: %w", m.namespace, SecretName, err)
+			}
+			log.Info("Created aggregated API server CA and serving certificate in the placeholder Secret", "namespace", m.namespace, "secret", SecretName)
+			return bundle, m.serve(bundle)
+		}
+
 		bundle, err := Parse(secret, m.namespace, now)
 		if err != nil {
 			return nil, err

internal/aggregated/servingcert/servingcert.go

@@ -177,6 +177,16 @@ func (b *Bundle) issueServingCert(namespace string, now time.Time) error {
 	return nil
 }
 
+// IsPlaceholder reports whether secret is an empty placeholder the server may fill: exactly the
+// managed type and zero data keys. Anything else is parsed, and invalid material stays corrupt.
+func IsPlaceholder(secret *corev1.Secret) bool {
+	return secret != nil && secret.Type == SecretType && len(secret.Data) == 0 && !isImmutable(secret)
+}
+
+func isImmutable(secret *corev1.Secret) bool {
+	return secret.Immutable != nil && *secret.Immutable
+}
+
 // Parse validates the Secret's trust material. Each failure names the field that is wrong.
 func Parse(secret *corev1.Secret, namespace string, now time.Time) (*Bundle, error) {
 	if secret == nil {

internal/aggregated/servingcert/servingcert_test.go

@@ -115,6 +115,158 @@ func TestEnsureCreatesMarkedSecretAndServesIt(t *testing.T) {
 	if listener.count.Load() != 1 {
 		t.Fatalf("listener notified %d times, want 1", listener.count.Load())
 	}
+	// The create path (used by --app=all, where the Secret does not exist yet) stays get + create.
+	if got := verbs(client); !slices.Equal(got, []string{"get", "create", "get"}) {
+		t.Fatalf("secret calls = %v, want [get create get] (the last get is this test's read)", got)
+	}
+}
+
+func TestEnsureCreateForbiddenNamesPlaceholder(t *testing.T) {
+	client := fake.NewClientset()
+	client.PrependReactor("create", "secrets", func(k8stesting.Action) (bool, runtime.Object, error) {
+		return true, nil, apierrors.NewForbidden(schema.GroupResource{Resource: "secrets"}, "", errors.New("no create"))
+	})
+	m := newTestManager(t, client, time.Now())
+	_, err := m.Ensure(t.Context())
+	if err == nil {
+		t.Fatal("expected an error when create is forbidden")
+	}
+	var corruptErr *CorruptSecretError
+	if errors.As(err, &corruptErr) || !apierrors.IsForbidden(err) {
+		t.Fatalf("expected a wrapped Forbidden error, got %v", err)
+	}
+	if !strings.Contains(err.Error(), "empty placeholder Secret") || !strings.Contains(err.Error(), string(SecretType)) {
+		t.Fatalf("error must tell how to provide a placeholder: %q", err)
+	}
+	if got := verbs(client); !slices.Equal(got, []string{"get", "create"}) {
+		t.Fatalf("secret calls = %v, want [get create]", got)
+	}
+}
+
+func TestEnsureFillsPlaceholder(t *testing.T) {
+	for name, data := range map[string]map[string][]byte{"nil data": nil, "empty data": {}} {
+		t.Run(name, func(t *testing.T) {
+			client := fake.NewClientset(placeholder(corev1.Secret{Type: SecretType, Data: data}))
+			var updatedRV string
+			client.PrependReactor("update", "secrets", func(a k8stesting.Action) (bool, runtime.Object, error) {
+				updatedRV = a.(k8stesting.UpdateAction).GetObject().(*corev1.Secret).ResourceVersion
+				return false, nil, nil // let the tracker store it
+			})
+			m := newTestManager(t, client, time.Now())
+			listener := &countingListener{}
+			m.AddListener(listener)
+
+			b, err := m.Ensure(t.Context())
+			if err != nil {
+				t.Fatal(err)
+			}
+			if got := verbs(client); !slices.Equal(got, []string{"get", "update"}) {
+				t.Fatalf("secret calls = %v, want [get update] (never create)", got)
+			}
+			if updatedRV != "7" {
+				t.Fatalf("update resourceVersion = %q, want the fetched %q", updatedRV, "7")
+			}
+			stored, err := client.CoreV1().Secrets(testNS).Get(t.Context(), SecretName, metav1.GetOptions{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			if stored.Type != SecretType || stored.Labels["kept"] != "yes" {
+				t.Fatalf("fill must keep the placeholder's type and metadata: type %q, labels %v", stored.Type, stored.Labels)
+			}
+			for k, v := range SecretLabels {
+				if stored.Labels[k] != v {
+					t.Fatalf("filled Secret must carry managed label %s=%s like a created one; labels %v", k, v, stored.Labels)
+				}
+			}
+			if _, err := Parse(stored, testNS, time.Now()); err != nil {
+				t.Fatalf("filled Secret must be valid: %v", err)
+			}
+			cert, _ := m.CurrentCertKeyContent()
+			if string(cert) != string(stored.Data[CertKey]) || string(m.CABundle()) != string(b.CACertPEM) {
+				t.Fatal("served certificate and CA must be the ones written to the placeholder")
+			}
+			if listener.count.Load() != 1 {
+				t.Fatalf("listener notified %d times, want 1", listener.count.Load())
+			}
+		})
+	}
+}
+
+func TestEnsureRefusesPlaceholderWithoutResourceVersion(t *testing.T) {
+	secret := placeholder(corev1.Secret{Type: SecretType})
+	secret.ResourceVersion = ""
+	client := fake.NewClientset(secret)
+	m := newTestManager(t, client, time.Now())
+	if _, err := m.Ensure(t.Context()); err == nil || !strings.Contains(err.Error(), "assertion failed") {
+		t.Fatalf("expected an assertion failure, got %v", err)
+	}
+	assertNoWrites(t, client)
+}
+
+func TestEnsureAdoptsOtherReplicasPlaceholderFill(t *testing.T) {
+	now := time.Now()
+	theirs, err := Generate(testNS, now)
+	if err != nil {
+		t.Fatal(err)
+	}
+	client := fake.NewClientset(placeholder(corev1.Secret{Type: SecretType}))
+	var updates atomic.Int32
+	client.PrependReactor("update", "secrets", func(k8stesting.Action) (bool, runtime.Object, error) {
+		updates.Add(1)
+		if err := client.Tracker().Update(schema.GroupVersionResource{Version: "v1", Resource: "secrets"}, secretFor(theirs), testNS); err != nil {
+			t.Fatal(err)
+		}
+		return true, nil, apierrors.NewConflict(schema.GroupResource{Resource: "secrets"}, SecretName, errors.New("modified"))
+	})
+	m := newTestManager(t, client, now)
+	if _, err := m.Ensure(t.Context()); err != nil {
+		t.Fatal(err)
+	}
+	if string(m.CABundle()) != string(theirs.CACertPEM) {
+		t.Fatal("after a conflict the other replica's CA must be adopted")
+	}
+	if updates.Load() != 1 {
+		t.Fatalf("updates = %d, want 1", updates.Load())
+	}
+}
+
+// Only the exact placeholder (managed type, zero data keys) is filled. Everything else is corrupt
+// and left alone.
+func TestEnsureRejectsNonPlaceholderSecrets(t *testing.T) {
+	now := time.Now()
+	good, err := Generate(testNS, now.Add(-time.Hour))
+	if err != nil {
+		t.Fatal(err)
+	}
+	immutable := true
+	tests := []struct {
+		name   string
+		secret corev1.Secret
+		want   string
+	}{
+		{"immutable empty placeholder", corev1.Secret{Type: SecretType, Immutable: &immutable}, "placeholder is immutable"},
+		{"opaque with no data", corev1.Secret{Type: corev1.SecretTypeOpaque}, `type is "Opaque"`},
+		{"no type with no data", corev1.Secret{}, `type is ""`},
+		{"TLS with no data", corev1.Secret{Type: corev1.SecretTypeTLS, Data: map[string][]byte{}}, `type is "kubernetes.io/tls"`},
+		{"one empty key", corev1.Secret{Type: SecretType, Data: map[string][]byte{CACertKey: {}}}, `data["ca.crt"] is missing or empty`},
+		{"only ca.crt", corev1.Secret{Type: SecretType, Data: map[string][]byte{CACertKey: good.CACertPEM}}, `data["ca.key"] is missing or empty`},
+		{"unrelated key", corev1.Secret{Type: SecretType, Data: map[string][]byte{"note": []byte("x")}}, `data["ca.crt"] is missing or empty`},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			client := fake.NewClientset(placeholder(tt.secret))
+			m := newTestManager(t, client, now)
+			_, err := m.Ensure(t.Context())
+			var corruptErr *CorruptSecretError
+			if !errors.As(err, &corruptErr) || !strings.Contains(err.Error(), tt.want) {
+				t.Fatalf("expected *CorruptSecretError naming %q, got %v", tt.want, err)
+			}
+			assertNoWrites(t, client)
+			if cert, _ := m.CurrentCertKeyContent(); cert != nil {
+				t.Fatal("nothing may be served from a Secret that is not a placeholder")
+			}
+		})
+	}
 }
 
 func TestEnsureAdoptsValidSecretUnchanged(t *testing.T) {
@@ -428,6 +580,23 @@ func secretFor(b *Bundle) *corev1.Secret {
 	}
 }
 
+// placeholder returns s as the managed Secret with resourceVersion "7" and a marker label.
+func placeholder(s corev1.Secret) *corev1.Secret {
+	s.ObjectMeta = metav1.ObjectMeta{Name: SecretName, Namespace: testNS, ResourceVersion: "7", Labels: map[string]string{"kept": "yes"}}
+	return &s
+}
+
+// verbs lists the verbs of every recorded action on Secrets, in order.
+func verbs(client *fake.Clientset) []string {
+	var out []string
+	for _, a := range client.Actions() {
+		if a.GetResource().Resource == "secrets" {
+			out = append(out, a.GetVerb())
+		}
+	}
+	return out
+}
+
 func assertNoWrites(t *testing.T, client *fake.Clientset) {
 	t.Helper()
 	for _, a := range client.Actions() {
@@ -472,3 +641,13 @@ func selfSignedCA(t *testing.T, notBefore, notAfter time.Time) ([]byte, []byte)
 type countingListener struct{ count atomic.Int32 }
 
 func (l *countingListener) Enqueue() { l.count.Add(1) }
+
+func TestIsPlaceholderExcludesImmutable(t *testing.T) {
+	immutable, mutable := true, false
+	if IsPlaceholder(&corev1.Secret{Type: SecretType, Immutable: &immutable}) {
+		t.Fatal("an immutable Secret can never be filled, so it is not a placeholder")
+	}
+	if !IsPlaceholder(&corev1.Secret{Type: SecretType, Immutable: &mutable}) {
+		t.Fatal("immutable: false is still a placeholder")
+	}
+}