From 79cea7590a763dc9409641c139604885215c490f Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 25 Sep 2026 13:27:40 +0000 Subject: [PATCH] feat: run the standalone aggregated API server as its own ServiceAccount Add config/apiserver-standalone/: the coder-k8s-apiserver ServiceAccount, a Role with get and update on coder-k8s-apiserver-tls only, the delegated authentication and authorization bindings, the APIService caBundle binding, and the empty placeholder Secret the server fills. No create on Secrets: namespace-wide create would let this identity mint tokens for other ServiceAccounts in the namespace. Standalone mode needs nothing from manager-role; dist/install.yaml is unchanged. An envtest runs the server's clients as that ServiceAccount against a real kube-apiserver with only these manifests: it fills, renews and adopts after a real conflict, patches the APIService, serves an authorized request through delegation, and gets 403 for every other Secret access, 422 for a type change and 404 for an update of the absent Secret. The docs move Option B to the new identity, describe the apply order and the upgrade from coder-k8s, and split CA replacement and corrupt Secret recovery by mode. The corrupt-Secret error now says to re-apply the placeholder when the identity may not create Secrets. Refs #143 Change-Id: I3c24ee1c1ed3d55427d76106a3226ebf2d404af8 Signed-off-by: Thomas Kosiewski --- .../apiservice-cabundle-binding.yaml | 13 + .../auth-delegator-binding.yaml | 12 + .../authentication-reader-binding.yaml | 13 + .../apiserver-standalone/serviceaccount.yaml | 8 + .../apiserver-standalone/serving-ca-role.yaml | 31 ++ .../serving-ca-secret.yaml | 13 + docs/explanation/architecture.md | 1 + docs/how-to/deploy-aggregated-apiserver.md | 69 +++- docs/how-to/troubleshooting.md | 10 +- .../aggregated/servingcert/servingcert.go | 2 +- .../servingcert/servingcert_test.go | 3 +- .../standalone_rbac_envtest_test.go | 339 ++++++++++++++++++ 12 files changed, 499 insertions(+), 15 deletions(-) create mode 100644 config/apiserver-standalone/apiservice-cabundle-binding.yaml create mode 100644 config/apiserver-standalone/auth-delegator-binding.yaml create mode 100644 config/apiserver-standalone/authentication-reader-binding.yaml create mode 100644 config/apiserver-standalone/serviceaccount.yaml create mode 100644 config/apiserver-standalone/serving-ca-role.yaml create mode 100644 config/apiserver-standalone/serving-ca-secret.yaml create mode 100644 internal/app/apiserverapp/standalone_rbac_envtest_test.go diff --git a/config/apiserver-standalone/apiservice-cabundle-binding.yaml b/config/apiserver-standalone/apiservice-cabundle-binding.yaml new file mode 100644 index 00000000..23768650 --- /dev/null +++ b/config/apiserver-standalone/apiservice-cabundle-binding.yaml @@ -0,0 +1,13 @@ +# Binds the ClusterRole from config/rbac/apiservice-cabundle-role.yaml; apply that file too. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: coder-k8s-apiserver-apiservice-cabundle +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: coder-k8s-apiservice-cabundle +subjects: + - kind: ServiceAccount + name: coder-k8s-apiserver + namespace: coder-system diff --git a/config/apiserver-standalone/auth-delegator-binding.yaml b/config/apiserver-standalone/auth-delegator-binding.yaml new file mode 100644 index 00000000..bdf9722c --- /dev/null +++ b/config/apiserver-standalone/auth-delegator-binding.yaml @@ -0,0 +1,12 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: coder-k8s-apiserver-auth-delegator +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: system:auth-delegator +subjects: + - kind: ServiceAccount + name: coder-k8s-apiserver + namespace: coder-system diff --git a/config/apiserver-standalone/authentication-reader-binding.yaml b/config/apiserver-standalone/authentication-reader-binding.yaml new file mode 100644 index 00000000..1aecfbcc --- /dev/null +++ b/config/apiserver-standalone/authentication-reader-binding.yaml @@ -0,0 +1,13 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: coder-k8s-apiserver-authentication-reader + namespace: kube-system +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: extension-apiserver-authentication-reader +subjects: + - kind: ServiceAccount + name: coder-k8s-apiserver + namespace: coder-system diff --git a/config/apiserver-standalone/serviceaccount.yaml b/config/apiserver-standalone/serviceaccount.yaml new file mode 100644 index 00000000..da57aadc --- /dev/null +++ b/config/apiserver-standalone/serviceaccount.yaml @@ -0,0 +1,8 @@ +# Identity for a standalone aggregated API server (--app=aggregated-apiserver). It gets only what +# that mode uses: its own serving-CA Secret, delegated authentication and authorization, and its +# own APIService. Not part of dist/install.yaml, which installs the controller only. +apiVersion: v1 +kind: ServiceAccount +metadata: + name: coder-k8s-apiserver + namespace: coder-system diff --git a/config/apiserver-standalone/serving-ca-role.yaml b/config/apiserver-standalone/serving-ca-role.yaml new file mode 100644 index 00000000..3b61e501 --- /dev/null +++ b/config/apiserver-standalone/serving-ca-role.yaml @@ -0,0 +1,31 @@ +# Read, fill and renew the serving-CA Secret, and nothing else. No create: see +# serving-ca-secret.yaml. +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: coder-k8s-apiserver-serving-ca + namespace: coder-system +rules: + - apiGroups: + - "" + resources: + - secrets + resourceNames: + - coder-k8s-apiserver-tls + verbs: + - get + - update +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: coder-k8s-apiserver-serving-ca + namespace: coder-system +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: coder-k8s-apiserver-serving-ca +subjects: + - kind: ServiceAccount + name: coder-k8s-apiserver + namespace: coder-system diff --git a/config/apiserver-standalone/serving-ca-secret.yaml b/config/apiserver-standalone/serving-ca-secret.yaml new file mode 100644 index 00000000..b4eb05ec --- /dev/null +++ b/config/apiserver-standalone/serving-ca-secret.yaml @@ -0,0 +1,13 @@ +# Empty placeholder for the serving CA. The server fills it on first start, so its identity needs +# no create on Secrets: namespace-wide create would let this identity mint tokens for other +# ServiceAccounts in the namespace. Applying this file again keeps the filled data. +apiVersion: v1 +kind: Secret +metadata: + name: coder-k8s-apiserver-tls + namespace: coder-system + labels: + app.kubernetes.io/name: coder-k8s + app.kubernetes.io/component: aggregated-apiserver-serving-ca + app.kubernetes.io/managed-by: coder-k8s +type: coder.com/aggregated-apiserver-serving-ca diff --git a/docs/explanation/architecture.md b/docs/explanation/architecture.md index 982e9a62..d079ad5f 100644 --- a/docs/explanation/architecture.md +++ b/docs/explanation/architecture.md @@ -64,5 +64,6 @@ graph TD | --- | --- | | `config/crd/bases/` | Generated CRDs for `CoderControlPlane`, `CoderProvisioner`, `CoderWorkspaceProxy` | | `config/rbac/` | ServiceAccount, `manager-role`, and bindings (including auth-delegator) | +| `config/apiserver-standalone/` | ServiceAccount `coder-k8s-apiserver`, its bindings, and the serving-CA placeholder Secret for standalone `--app=aggregated-apiserver` | | `deploy/deployment.yaml` | The `coder-k8s` Deployment (defaults to `--app=all`) | | `deploy/apiserver-service.yaml`, `deploy/apiserver-apiservice.yaml` | Expose the aggregated API | diff --git a/docs/how-to/deploy-aggregated-apiserver.md b/docs/how-to/deploy-aggregated-apiserver.md index dc399a9f..eea5184d 100644 --- a/docs/how-to/deploy-aggregated-apiserver.md +++ b/docs/how-to/deploy-aggregated-apiserver.md @@ -4,23 +4,23 @@ Serve `CoderWorkspace` and `CoderTemplate` (`aggregation.coder.com/v1alpha1`) th Commands run from a clone of this repository. -## 1. Apply RBAC and register the API +## 1. Register the API ```bash kubectl create namespace coder-system -kubectl apply -f config/rbac/ kubectl apply -f deploy/apiserver-service.yaml -f deploy/apiserver-apiservice.yaml ``` -`config/rbac/` includes two bindings the aggregated API server needs to check callers: `auth-delegator-binding.yaml` (create TokenReviews and SubjectAccessReviews) and `authentication-reader-binding.yaml` (read `kube-system/extension-apiserver-authentication`; the default `manager-role` also grants cluster-wide ConfigMap reads). Both name the `coder-k8s` ServiceAccount in `coder-system`; edit them if you install elsewhere. The server fails closed without these permissions: without read access to that ConfigMap it does not start, and without permission to create SubjectAccessReviews it answers every request with an error (members of `system:masters` excepted). +Each option in step 2 applies its own RBAC. Both include two bindings the aggregated API server needs to check callers: `auth-delegator-binding.yaml` (create TokenReviews and SubjectAccessReviews) and `authentication-reader-binding.yaml` (read `kube-system/extension-apiserver-authentication`). They name the ServiceAccount in `coder-system`; edit them if you install elsewhere. The server fails closed without these permissions: without read access to that ConfigMap it does not start, and without permission to create SubjectAccessReviews it answers every request with an error (members of `system:masters` excepted). ## 2. Deploy ### Option A: all-in-one (recommended) -The default `--app=all` already includes the aggregated API server. It finds its Coder backend automatically from an eligible `CoderControlPlane`. +The default `--app=all` already includes the aggregated API server. It finds its Coder backend automatically from an eligible `CoderControlPlane`. It runs as the `coder-k8s` ServiceAccount with `manager-role`, which the controller needs. ```bash +kubectl apply -f config/rbac/ kubectl apply -f deploy/deployment.yaml ``` @@ -28,7 +28,21 @@ kubectl apply -f deploy/deployment.yaml Run only the aggregated API server (`--app=aggregated-apiserver`) and point it at a Coder instance yourself. -Save the Coder session token in a file, for example `./coder-session-token`. Store it in a Secret, deploy, then set the backend: +It runs as its own ServiceAccount, `coder-k8s-apiserver`, and needs nothing from `manager-role`. `config/apiserver-standalone/` grants it only: + +- `get` and `update` on the `coder-k8s-apiserver-tls` Secret, which the directory ships as an empty placeholder that the server fills. There is no `create`: namespace-wide `create` would let this identity mint tokens for other ServiceAccounts in the namespace. +- Creating TokenReviews and SubjectAccessReviews, and reading `kube-system/extension-apiserver-authentication`. +- Its own APIService, through the ClusterRole in `config/rbac/apiservice-cabundle-role.yaml` (that file's binding for `coder-k8s` is unused here). + +Apply the placeholder and the RBAC before the Deployment: + +```bash +kubectl apply -f config/apiserver-standalone/ -f config/rbac/apiservice-cabundle-role.yaml +``` + +If the pod starts before the placeholder exists, it exits because it may not create the Secret. Apply the placeholder; the pod recovers on its next restart (Kubernetes restarts it with a back-off of up to 5 minutes). + +Save the Coder session token in a file, for example `./coder-session-token`. Store it in a Secret, deploy, then set the ServiceAccount and the backend: ```bash kubectl -n coder-system create secret generic coder-k8s-session-token \ @@ -37,6 +51,10 @@ kubectl -n coder-system create secret generic coder-k8s-session-token \ kubectl apply -f deploy/deployment.yaml kubectl -n coder-system patch deployment coder-k8s --type=json -p '[{ + "op": "add", + "path": "/spec/template/spec/serviceAccountName", + "value": "coder-k8s-apiserver" +}, { "op": "add", "path": "/spec/template/spec/containers/0/env", "value": [{ @@ -69,6 +87,28 @@ kubectl -n coder-system patch deployment coder-k8s --type=strategic -p '{ }' ``` +#### Move a standalone server from `coder-k8s` to its own ServiceAccount + +Earlier versions of this guide ran the standalone server as `coder-k8s`. To move it: + +1. Apply the standalone RBAC and placeholder. Applying the placeholder over the existing Secret keeps its data and CA. + + ```bash + kubectl apply -f config/apiserver-standalone/ -f config/rbac/apiservice-cabundle-role.yaml + ``` + +2. Switch the ServiceAccount. The new pods reuse the existing Secret and CA, so the APIService `caBundle` does not change. + + ```bash + kubectl -n coder-system patch deployment coder-k8s --type=json \ + -p '[{"op": "replace", "path": "/spec/template/spec/serviceAccountName", "value": "coder-k8s-apiserver"}]' + kubectl -n coder-system rollout status deployment/coder-k8s + ``` + +3. If nothing else runs as `coder-k8s` in this cluster, delete the `config/rbac/` bindings for it. + +A tool that replaces the whole Secret from the manifest (for example a GitOps sync that replaces instead of applying) clears its data. The server then generates a new CA, so treat that as a [CA replacement](#replace-the-ca) and restart every replica. + ## How callers are checked The aggregated API server authenticates and authorizes every request with the Kubernetes API: @@ -116,11 +156,11 @@ 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.`, `coder-k8s-apiserver..svc`, and `coder-k8s-apiserver..svc.cluster.local`. -- The server creates the Secret on first start and reuses it afterwards. With several replicas, they all use the same Secret. +- In `--app=all`, the server creates the Secret on first start. In standalone mode, `config/apiserver-standalone/` ships it as an empty placeholder that the server fills. Afterwards the server reuses it. 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 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. +- 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 (in standalone mode, re-apply the placeholder after deleting 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" @@ -135,13 +175,14 @@ These identities can read the key: - Any subject allowed to `get`, `list`, or `watch` Secrets in the server's namespace (`coder-system` by default). - Any subject allowed to read Secrets cluster-wide. These are easy to miss: cluster administrators, and GitOps, backup, or monitoring tools with cluster-wide Secret access. - The `coder-k8s` ServiceAccount. Its `manager-role` allows every verb on Secrets in every namespace, because the controller manages Secrets for each `CoderControlPlane`. +- The `coder-k8s-apiserver` ServiceAccount, for this one Secret only. By deployment: | Deployment | Runs as | Can read the CA key | | --- | --- | --- | | `--app=all` ([Option A](#option-a-all-in-one-recommended)) | `coder-k8s` | Yes. The process also keeps every Secret in the cluster in its cache, because the controllers watch Secrets. | -| Standalone `--app=aggregated-apiserver` ([Option B](#option-b-standalone)) | `coder-k8s`, as this guide deploys it | Yes, through `manager-role`, although this mode reads no Secret except its own. | +| Standalone `--app=aggregated-apiserver` ([Option B](#option-b-standalone)) | `coder-k8s-apiserver`, with `config/apiserver-standalone/` | Yes, and no other Secret: it may `get` and `update` only `coder-k8s-apiserver-tls`, and may not list, watch, or create Secrets. `coder-k8s` can still read the key if it exists in the cluster. | | Controller only (`dist/install.yaml`) | `coder-k8s` | Only if an aggregated API server has created the Secret somewhere in the cluster; this bundle never creates it. | For `--app=all`, a narrower Role would not change this: one process runs both the controller and the aggregated API server under one ServiceAccount, and the controller needs cluster-wide Secret access. The same access also covers the operator token Secrets, which give owner rights in Coder and are more sensitive than the CA key. Limit who can read Secrets in `coder-system` and cluster-wide, and [replace the CA](#replace-the-ca) if the Secret may have been exposed. @@ -157,7 +198,9 @@ Replacing the CA takes a restart of every replica, and requests through kube-api base64 -d | openssl x509 -noout -fingerprint -sha256 ``` -2. Delete the Secret and restart every replica. Do both: a running replica keeps serving the old certificate until it restarts. +2. Remove the old CA and restart every replica. Do both: a running replica keeps serving the old certificate until it restarts. + + With `--app=all`, delete the Secret; the server creates a new one: ```bash kubectl -n coder-system delete secret coder-k8s-apiserver-tls @@ -165,6 +208,14 @@ Replacing the CA takes a restart of every replica, and requests through kube-api kubectl -n coder-system rollout status deployment/coder-k8s ``` + In standalone mode, the server may not create the Secret. Clear its data instead, which turns it back into the empty placeholder (or delete it and re-apply `config/apiserver-standalone/serving-ca-secret.yaml`): + + ```bash + kubectl -n coder-system patch secret coder-k8s-apiserver-tls --type=json -p '[{"op": "remove", "path": "/data"}]' + kubectl -n coder-system rollout restart deployment/coder-k8s + kubectl -n coder-system rollout status deployment/coder-k8s + ``` + The first new replica generates a new CA and sets the APIService `caBundle` to it. Requests that kube-apiserver sends to an old replica fail verification until that replica is gone; that is the `503` window. 3. Check the result. The fingerprint differs from step 1, the APIService `caBundle` equals the Secret's `ca.crt` (the two commands print the same value), and a request through kube-apiserver succeeds: diff --git a/docs/how-to/troubleshooting.md b/docs/how-to/troubleshooting.md index 0ac134ce..dde0e8b5 100644 --- a/docs/how-to/troubleshooting.md +++ b/docs/how-to/troubleshooting.md @@ -101,7 +101,7 @@ If they differ, find the cause: --as=system:serviceaccount:coder-system:coder-k8s ``` - Apply the RBAC (it must name the ServiceAccount the pod runs as); the server retries within about a minute: + In standalone mode, use `--as=system:serviceaccount:coder-system:coder-k8s-apiserver`. Apply the RBAC (it must name the ServiceAccount the pod runs as; standalone mode also needs `config/apiserver-standalone/apiservice-cabundle-binding.yaml`); the server retries within about a minute: ```bash kubectl apply -f config/rbac/apiservice-cabundle-role.yaml @@ -129,7 +129,7 @@ See [Replace the CA](deploy-aggregated-apiserver.md#replace-the-ca) for the full The aggregated API server checks every caller with the Kubernetes API and refuses to start without it. -- `... configmaps "extension-apiserver-authentication" is forbidden`: apply `config/rbac/authentication-reader-binding.yaml`. It must name the ServiceAccount the pod runs as (for example after installing into another namespace). +- `... configmaps "extension-apiserver-authentication" is forbidden`: apply `config/rbac/authentication-reader-binding.yaml` (`config/apiserver-standalone/authentication-reader-binding.yaml` in standalone mode). It must name the ServiceAccount the pod runs as (for example after installing into another namespace). - `no Kubernetes configuration for delegated authentication and authorization` (outside a cluster): set `KUBECONFIG` to one kubeconfig file, or create `~/.kube/config`. - `load kubeconfig ...` or `invalid kubeconfig ...`: the file named by `KUBECONFIG` is missing or incomplete. The server does not fall back to another configuration. @@ -142,17 +142,19 @@ kubectl -n coder-system delete secret coder-k8s-apiserver-tls kubectl -n coder-system rollout restart deployment/coder-k8s ``` +In standalone mode the server may not create the Secret: after deleting it, re-apply the placeholder with `kubectl apply -f config/apiserver-standalone/serving-ca-secret.yaml` before the restart. + 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). +- `create secret …`: the Secret does not exist and the ServiceAccount may not create Secrets. In standalone mode with `coder-k8s-apiserver`, this is expected until the placeholder exists: apply `config/apiserver-standalone/serving-ca-secret.yaml`, and the pod recovers on its next restart. Otherwise 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` - **`401`:** the request has no valid credential. Requests sent straight to port `6443` need a Kubernetes bearer token; anonymous requests only reach `/healthz`, `/livez`, and `/readyz`. Use `kubectl`, which goes through kube-apiserver. - **`403`:** the caller lacks RBAC for `aggregation.coder.com` in that namespace. Check with `kubectl auth can-i list codertemplates.aggregation.coder.com -n --as=`. Before you grant it, note that this RBAC is owner-equivalent inside Coder (see [How callers are checked](deploy-aggregated-apiserver.md#how-callers-are-checked)). -- **`500` mentioning `subjectaccessreviews`:** the server's ServiceAccount cannot create SubjectAccessReviews. Apply `config/rbac/auth-delegator-binding.yaml`. +- **`500` mentioning `subjectaccessreviews`:** the server's ServiceAccount cannot create SubjectAccessReviews. Apply `config/rbac/auth-delegator-binding.yaml` (`config/apiserver-standalone/auth-delegator-binding.yaml` in standalone mode). ## Aggregated reads return `ServiceUnavailable` diff --git a/internal/aggregated/servingcert/servingcert.go b/internal/aggregated/servingcert/servingcert.go index 4b255457..d2ba3df9 100644 --- a/internal/aggregated/servingcert/servingcert.go +++ b/internal/aggregated/servingcert/servingcert.go @@ -92,7 +92,7 @@ type CorruptSecretError struct { } func (e *CorruptSecretError) Error() string { - return fmt.Sprintf("secret %s/%s: %s; fix it, or delete it so coder-k8s generates a new CA (clients that trust the old CA must then be updated)", + return fmt.Sprintf("secret %s/%s: %s; fix it, or delete it so coder-k8s generates a new CA (re-apply the empty placeholder if this identity may not create Secrets); clients that trust the old CA must then be updated", e.Namespace, SecretName, e.Problem) } diff --git a/internal/aggregated/servingcert/servingcert_test.go b/internal/aggregated/servingcert/servingcert_test.go index e503b8ab..93d3948b 100644 --- a/internal/aggregated/servingcert/servingcert_test.go +++ b/internal/aggregated/servingcert/servingcert_test.go @@ -472,7 +472,8 @@ func TestEnsureRejectsCorruptSecret(t *testing.T) { if !strings.Contains(err.Error(), tt.want) { t.Fatalf("error %q does not name the problem %q", err, tt.want) } - if !strings.Contains(err.Error(), "secret coder-system/coder-k8s-apiserver-tls") || !strings.Contains(err.Error(), "delete it") { + if !strings.Contains(err.Error(), "secret coder-system/coder-k8s-apiserver-tls") || !strings.Contains(err.Error(), "delete it") || + !strings.Contains(err.Error(), "re-apply the empty placeholder if this identity may not create Secrets") { t.Fatalf("error must name the Secret and the remedy: %q", err) } assertNoWrites(t, client) diff --git a/internal/app/apiserverapp/standalone_rbac_envtest_test.go b/internal/app/apiserverapp/standalone_rbac_envtest_test.go new file mode 100644 index 00000000..fcdffb55 --- /dev/null +++ b/internal/app/apiserverapp/standalone_rbac_envtest_test.go @@ -0,0 +1,339 @@ +package apiserverapp + +import ( + "bytes" + "context" + "encoding/base64" + "errors" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + + appsv1 "k8s.io/api/apps/v1" + authenticationv1 "k8s.io/api/authentication/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + utilyaml "k8s.io/apimachinery/pkg/util/yaml" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + + "github.com/coder/coder-k8s/internal/aggregated/apiservicetrust" + "github.com/coder/coder-k8s/internal/aggregated/servingcert" +) + +const ( + standaloneManifests = "../../../config/apiserver-standalone" + cabundleRoleManifest = "../../../config/rbac/apiservice-cabundle-role.yaml" + standaloneNS = "coder-system" + standaloneUser = "system:serviceaccount:coder-system:coder-k8s-apiserver" +) + +var manifestGVRs = map[string]schema.GroupVersionResource{ + "ServiceAccount": {Version: "v1", Resource: "serviceaccounts"}, + "Secret": {Version: "v1", Resource: "secrets"}, + "Role": {Group: rbacv1.GroupName, Version: "v1", Resource: "roles"}, + "RoleBinding": {Group: rbacv1.GroupName, Version: "v1", Resource: "rolebindings"}, + "ClusterRole": {Group: rbacv1.GroupName, Version: "v1", Resource: "clusterroles"}, + "ClusterRoleBinding": {Group: rbacv1.GroupName, Version: "v1", Resource: "clusterrolebindings"}, +} + +// applyShippedManifests creates every object in the files exactly as shipped, so the test checks +// the RBAC users apply rather than a copy. +func applyShippedManifests(t *testing.T, dyn dynamic.Interface, paths ...string) { + t.Helper() + for _, path := range paths { + data, err := os.ReadFile(path) //nolint:gosec // G304: repository manifests. + if err != nil { + t.Fatal(err) + } + decoder := utilyaml.NewYAMLOrJSONDecoder(bytes.NewReader(data), 4096) + for { + var obj map[string]any + if err := decoder.Decode(&obj); errors.Is(err, io.EOF) { + break + } else if err != nil { + t.Fatalf("%s: %v", path, err) + } + if len(obj) == 0 { + continue + } + u := &unstructured.Unstructured{Object: obj} + gvr, ok := manifestGVRs[u.GetKind()] + if !ok { + t.Fatalf("%s: unexpected kind %q", path, u.GetKind()) + } + var resource dynamic.ResourceInterface = dyn.Resource(gvr) + if u.GetNamespace() != "" { + resource = dyn.Resource(gvr).Namespace(u.GetNamespace()) + } + mustCreate(t, func() error { _, err := resource.Create(t.Context(), u, metav1.CreateOptions{}); return err }) + } + } +} + +// impersonatingKubeconfig writes a kubeconfig that acts as user through the envtest admin, the +// same file shape the server reads from KUBECONFIG. +func impersonatingKubeconfig(t *testing.T, user string) (string, *rest.Config) { + t.Helper() + admin := envtestEnv.Config + cfg := clientcmdapi.NewConfig() + cfg.Clusters["envtest"] = &clientcmdapi.Cluster{Server: admin.Host, CertificateAuthorityData: admin.CAData} + cfg.AuthInfos["sa"] = &clientcmdapi.AuthInfo{ClientCertificateData: admin.CertData, ClientKeyData: admin.KeyData, Impersonate: user} + cfg.Contexts["sa"] = &clientcmdapi.Context{Cluster: "envtest", AuthInfo: "sa"} + cfg.CurrentContext = "sa" + path := filepath.Join(t.TempDir(), "sa.kubeconfig") + if err := clientcmd.WriteToFile(*cfg, path); err != nil { + t.Fatal(err) + } + restCfg, err := clientcmd.BuildConfigFromFlags("", path) + if err != nil { + t.Fatal(err) + } + return path, restCfg +} + +type roundTripperFunc func(*http.Request) (*http.Response, error) + +func (f roundTripperFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +// TestEnvtestStandaloneRunsWithOnlyItsOwnRBAC runs the standalone server's Kubernetes clients as +// the coder-k8s-apiserver ServiceAccount with only config/apiserver-standalone/ (and the APIService +// ClusterRole) bound: no manager-role. +func TestEnvtestStandaloneRunsWithOnlyItsOwnRBAC(t *testing.T) { + startSharedEnvtest(t) + ctx := t.Context() + adminDyn := dynamic.NewForConfigOrDie(envtestEnv.Config) + secrets := envtestAdmin.CoreV1().Secrets(standaloneNS) + + mustCreate(t, func() error { + _, err := envtestAdmin.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: standaloneNS}}, metav1.CreateOptions{}) + return err + }) + mustCreate(t, func() error { + _, err := secrets.Create(ctx, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "unrelated"}, StringData: map[string]string{"k": "v"}}, metav1.CreateOptions{}) + return err + }) + apiService := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "apiregistration.k8s.io/v1", "kind": "APIService", + "metadata": map[string]any{"name": apiservicetrust.APIServiceName}, + "spec": map[string]any{ + "group": "aggregation.coder.com", "version": "v1alpha1", "insecureSkipTLSVerify": true, + "groupPriorityMinimum": int64(1000), "versionPriority": int64(15), + "service": map[string]any{"name": "coder-k8s-apiserver", "namespace": standaloneNS}, + }, + }} + mustCreate(t, func() error { + _, err := adminDyn.Resource(apiservicetrust.APIServiceGVR).Create(ctx, apiService, metav1.CreateOptions{}) + return err + }) + t.Cleanup(func() { + _ = adminDyn.Resource(apiservicetrust.APIServiceGVR).Delete(t.Context(), apiservicetrust.APIServiceName, metav1.DeleteOptions{}) + }) + files, err := filepath.Glob(filepath.Join(standaloneManifests, "*.yaml")) + if err != nil || len(files) == 0 { + t.Fatalf("no manifests in %s: %v", standaloneManifests, err) + } + applyShippedManifests(t, adminDyn, append(files, cabundleRoleManifest)...) + + kubeconfig, saCfg := impersonatingKubeconfig(t, standaloneUser) + sa := kubernetes.NewForConfigOrDie(saCfg) + eventuallyNoError(t, "RBAC to propagate", func() error { + _, err := sa.CoreV1().Secrets(standaloneNS).Get(ctx, servingcert.SecretName, metav1.GetOptions{}) + return err + }) + + // Nothing beyond the one Secret and what delegation needs; in particular nothing from manager-role. + for name, call := range map[string]func() error{ + "get another Secret": func() error { + _, err := sa.CoreV1().Secrets(standaloneNS).Get(ctx, "unrelated", metav1.GetOptions{}) + return err + }, + "list Secrets": func() error { _, err := sa.CoreV1().Secrets(standaloneNS).List(ctx, metav1.ListOptions{}); return err }, + "watch Secrets": func() error { _, err := sa.CoreV1().Secrets(standaloneNS).Watch(ctx, metav1.ListOptions{}); return err }, + "list Secrets cluster-wide": func() error { _, err := sa.CoreV1().Secrets("").List(ctx, metav1.ListOptions{}); return err }, + "get a Secret in another ns": func() error { + _, err := sa.CoreV1().Secrets("test-ns").Get(ctx, "any", metav1.GetOptions{}) + return err + }, + "create a Secret": func() error { + _, err := sa.CoreV1().Secrets(standaloneNS).Create(ctx, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "minted"}}, metav1.CreateOptions{}) + return err + }, + "delete the serving-CA Secret": func() error { + return sa.CoreV1().Secrets(standaloneNS).Delete(ctx, servingcert.SecretName, metav1.DeleteOptions{}) + }, + "patch the serving-CA Secret": func() error { + _, err := sa.CoreV1().Secrets(standaloneNS).Patch(ctx, servingcert.SecretName, types.MergePatchType, []byte(`{}`), metav1.PatchOptions{}) + return err + }, + "list ConfigMaps (manager-role)": func() error { + _, err := sa.CoreV1().ConfigMaps(standaloneNS).List(ctx, metav1.ListOptions{}) + return err + }, + "create a Deployment (manager-role)": func() error { + _, err := sa.AppsV1().Deployments(standaloneNS).Create(ctx, &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "x"}}, metav1.CreateOptions{}) + return err + }, + } { + if err := call(); !apierrors.IsForbidden(err) { + t.Errorf("%s: want 403 Forbidden, got %v", name, err) + } + } + + // Production wiring: fill the shipped placeholder, then keep the APIService caBundle in sync. + nsFile := filepath.Join(t.TempDir(), "namespace") + mustWrite(t, nsFile, standaloneNS) + tlsSetup, err := newManagedTLS(kubeconfig, nsFile) + if err != nil || tlsSetup == nil { + t.Fatalf("newManagedTLS: %v", err) + } + filled, err := tlsSetup.manager.Ensure(ctx) + if err != nil { + t.Fatalf("fill the placeholder as the ServiceAccount: %v", err) + } + stored, err := secrets.Get(ctx, servingcert.SecretName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + if string(stored.Data[servingcert.CACertKey]) != string(filled.CACertPEM) { + t.Fatal("the filled CA must be stored in the Secret") + } + syncCtx, cancel := context.WithCancel(ctx) + done := make(chan struct{}) + tlsSetup.manager.AddListener(tlsSetup.sync) + go func() { tlsSetup.sync.Run(syncCtx); close(done) }() + t.Cleanup(func() { cancel(); <-done }) + wantBundle := base64.StdEncoding.EncodeToString(filled.CACertPEM) + eventuallyNoError(t, "caBundle patched by the ServiceAccount", func() error { + u, err := adminDyn.Resource(apiservicetrust.APIServiceGVR).Get(ctx, apiservicetrust.APIServiceName, metav1.GetOptions{}) + if err != nil { + return err + } + if got, _, _ := unstructured.NestedString(u.Object, "spec", "caBundle"); got != wantBundle { + return errors.New("caBundle not set yet") + } + return nil + }) + + // Renewal: SANs for another namespace force a new serving certificate with the same CA. + copied, err := servingcert.Generate("another-namespace", time.Now()) + if err != nil { + t.Fatal(err) + } + stored.Data = copied.Data() + if _, err := secrets.Update(ctx, stored, metav1.UpdateOptions{}); err != nil { + t.Fatal(err) + } + renewed, err := tlsSetup.manager.Ensure(ctx) + if err != nil { + t.Fatalf("renew as the ServiceAccount: %v", err) + } + if string(renewed.CACertPEM) != string(copied.CACertPEM) || renewed.Cert.DNSNames[0] != servingcert.DNSNames(standaloneNS)[0] { + t.Fatal("renewal must keep the CA and issue a certificate for the server's namespace") + } + + // Conflict: another replica fills the placeholder between this server's get and update. + stored, err = secrets.Get(ctx, servingcert.SecretName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + stored.Data = nil + if _, err := secrets.Update(ctx, stored, metav1.UpdateOptions{}); err != nil { + t.Fatal(err) + } + theirs, err := servingcert.Generate(standaloneNS, time.Now()) + if err != nil { + t.Fatal(err) + } + var raced atomic.Bool + racyCfg := rest.CopyConfig(saCfg) + racyCfg.Wrap(func(next http.RoundTripper) http.RoundTripper { + return roundTripperFunc(func(r *http.Request) (*http.Response, error) { + if r.Method == http.MethodPut && strings.HasSuffix(r.URL.Path, "/secrets/"+servingcert.SecretName) && raced.CompareAndSwap(false, true) { + current, err := secrets.Get(r.Context(), servingcert.SecretName, metav1.GetOptions{}) + if err == nil { + current.Data = theirs.Data() + _, err = secrets.Update(r.Context(), current, metav1.UpdateOptions{}) + } + if err != nil { + return nil, err + } + } + return next.RoundTrip(r) + }) + }) + racer, err := servingcert.NewManager(kubernetes.NewForConfigOrDie(racyCfg), standaloneNS) + if err != nil { + t.Fatal(err) + } + adopted, err := racer.Ensure(ctx) + if err != nil { + t.Fatalf("fill after a conflict: %v", err) + } + if !raced.Load() || string(adopted.CACertPEM) != string(theirs.CACertPEM) { + t.Fatal("after the real 409 Conflict the other replica's CA must be adopted") + } + + // Delegated authentication and authorization: startup reads + // kube-system/extension-apiserver-authentication, and requests create SubjectAccessReviews. + if _, err := sa.AuthenticationV1().TokenReviews().Create(ctx, &authenticationv1.TokenReview{Spec: authenticationv1.TokenReviewSpec{Token: "not-a-token"}}, metav1.CreateOptions{}); err != nil { + t.Fatalf("create TokenReview as the ServiceAccount: %v", err) + } + server := startEnvtestAuthServer(t, kubeconfig) + grantTemplateReader(t, "test-ns", rbacv1.Subject{Kind: rbacv1.UserKind, APIGroup: rbacv1.GroupName, Name: "standalone-alice"}, "standalone-alice-reader") + frontProxy := envtestFrontProxyCA.clientCert(t, frontProxyName) + eventuallyNoError(t, "an authorized request through the standalone identity", func() error { + if status, body := server.do(t, &frontProxy, http.MethodGet, templatesTestNS, remoteUser("standalone-alice"), ""); status != http.StatusOK { + return errors.New(body) + } + return nil + }) + + // The Secret's type is immutable, and update cannot recreate a deleted Secret. + current, err := sa.CoreV1().Secrets(standaloneNS).Get(ctx, servingcert.SecretName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + current.Type = corev1.SecretTypeServiceAccountToken + if _, err := sa.CoreV1().Secrets(standaloneNS).Update(ctx, current, metav1.UpdateOptions{}); !apierrors.IsInvalid(err) { + t.Fatalf("changing the type must be rejected with 422 Invalid, got %v", err) + } + if err := secrets.Delete(ctx, servingcert.SecretName, metav1.DeleteOptions{}); err != nil { + t.Fatal(err) + } + absent := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: servingcert.SecretName, Namespace: standaloneNS}, Type: servingcert.SecretType} + if _, err := sa.CoreV1().Secrets(standaloneNS).Update(ctx, absent, metav1.UpdateOptions{}); !apierrors.IsNotFound(err) { + t.Fatalf("update of the absent Secret must return 404 NotFound, got %v", err) + } + if _, err := tlsSetup.manager.Ensure(ctx); !apierrors.IsForbidden(err) || !strings.Contains(err.Error(), "placeholder") { + t.Fatalf("without the placeholder, startup must fail with a Forbidden create that names the placeholder, got %v", err) + } +} + +func eventuallyNoError(t *testing.T, what string, fn func() error) { + t.Helper() + deadline := time.Now().Add(20 * time.Second) + for { + err := fn() + if err == nil { + return + } + if time.Now().After(deadline) { + t.Fatalf("timed out waiting for %s: %v", what, err) + } + time.Sleep(200 * time.Millisecond) + } +}