From 4ce89ac70608c30b44d478025d2aeba4d42b6009 Mon Sep 17 00:00:00 2001 From: Wen Huang <50309350+wenhug@users.noreply.github.com> Date: Tue, 25 Aug 2026 18:54:35 +0000 Subject: [PATCH 1/3] Restrict bootstrap token RBAC --- docs/usages/aks-flex-config.md | 50 +- docs/usages/joining-nodes.md | 2 +- hack/e2e/lib/node-join-kubeadm.sh | 35 +- scripts/aks-flex-config | 493 +++++++- scripts/aks_flex_config_test.go | 1729 +++++++++++++++++++++++++++++ 5 files changed, 2236 insertions(+), 73 deletions(-) create mode 100644 scripts/aks_flex_config_test.go diff --git a/docs/usages/aks-flex-config.md b/docs/usages/aks-flex-config.md index 8448dfbd..f61b70e0 100644 --- a/docs/usages/aks-flex-config.md +++ b/docs/usages/aks-flex-config.md @@ -9,7 +9,7 @@ The helper does not install anything on the target host. It uses Azure CLI and, - Azure CLI authenticated to the subscription that contains the AKS cluster. - `python3` on the workstation. - `kubectl` on the workstation for `setup-node-rbac` and `--bootstrap-token` config generation. -- Permission to run `az aks get-credentials --admin` and create Kubernetes `ClusterRoleBinding` and bootstrap token `Secret` objects. +- Permission to run `az aks get-credentials --admin`, create Kubernetes `ClusterRoleBinding` and bootstrap token `Secret` objects, and remove the obsolete `aks-flex-node-role` binding when present. ## Save The Helper @@ -46,7 +46,53 @@ Run this once per cluster for bootstrap-token joins: --subscription "$SUBSCRIPTION_ID" ``` -This applies the bootstrap-related `ClusterRoleBinding` objects for the `system:bootstrappers:aks-flex-node` group. +This applies only the CSR creation and approval `ClusterRoleBinding` objects for the `system:bootstrappers:aks-flex-node` group. If any binding still grants that group the obsolete `system:node` role, the command stops after applying the safe bindings and explains how to migrate. It does not silently remove the binding because older and development-mode agents may still use their bootstrap token after joining. + +`v0.1.1` introduced a separate daemon client certificate, but the version alone does not prove that certificate was issued successfully. Upgrade every bootstrap-token agent to `v0.1.1` or later (preferably the latest release), and on every host verify that the certificate exists, is unexpired, and the agent remains healthy after a restart: + +```bash +sudo test -s /etc/aks-flex-node/daemon-credentials/daemon-controller-current.pem +sudo openssl x509 \ + -in /etc/aks-flex-node/daemon-credentials/daemon-controller-current.pem \ + -noout -subject -enddate -checkend 0 +sudo systemctl restart aks-flex-node-agent.service +sudo systemctl is-active aks-flex-node-agent.service +``` + +Then explicitly remove the obsolete binding: + +```bash +./aks-flex-config setup-node-rbac \ + --resource-group "$RESOURCE_GROUP" \ + --cluster-name "$CLUSTER_NAME" \ + --subscription "$SUBSCRIPTION_ID" \ + --remove-legacy-node-role-binding +``` + +This migration is idempotent. It automatically deletes only the canonical `aks-flex-node-role` object created by older helpers. If another binding grants the same unsafe edge, or that object has extra subjects, the helper refuses to guess and identifies the objects for manual review. Bootstrap-token config generation refuses to create a token while any such binding exists, rather than either issuing an over-privileged token or unexpectedly breaking an old daemon. + +To verify no binding still grants the bootstrap group `system:node`, run: + +```bash +kubectl get clusterrolebinding -o json | jq -r ' + .items[] + | select(.roleRef.kind == "ClusterRole" and .roleRef.name == "system:node") + | .metadata.name as $binding + | .subjects[]? + | select(.kind == "Group" and .name == "system:bootstrappers:aks-flex-node") + | $binding' +``` + +The expected result is no output. The canonical `aks-flex-node-role` object is +deleted; a safe, repurposed object with that name is preserved. Once certificate +issuance has been verified, both the kubelet and long-running Flex daemon use +issued client certificates, so removing the unsafe binding does not interrupt +joined nodes. New and in-progress joins retain the CSR permissions installed +above. + +Do not roll back a migrated host to an older or development-mode agent that still uses the bootstrap token for ordinary Kubernetes API requests. After this binding is removed, those requests correctly receive `403 Forbidden`. Restore a supported certificate-using agent instead of restoring the broad binding. + +Finally, delete bootstrap-token Secrets that are no longer needed. In particular, tokens made by helpers before `v0.1.1` had no expiration. Removing the broad binding limits them to bootstrap permissions, but does not revoke them; do not delete a token that is still being used by an in-progress join. ## Generate Node Config diff --git a/docs/usages/joining-nodes.md b/docs/usages/joining-nodes.md index 427bde56..15551bc6 100644 --- a/docs/usages/joining-nodes.md +++ b/docs/usages/joining-nodes.md @@ -16,7 +16,7 @@ Bootstrap token mode is the recommended quickstart path. It uses Kubernetes TLS High-level flow: -1. Run [`scripts/aks-flex-config setup-node-rbac`](../../scripts/aks-flex-config) to setup required node bootstrap RBAC permissions. +1. Run [`scripts/aks-flex-config setup-node-rbac`](../../scripts/aks-flex-config) to set up the least-privilege node bootstrap RBAC permissions. Clusters configured by an older helper require the explicit compatibility migration documented in the helper guide before another token can be generated. 2. Run `scripts/aks-flex-config generate-node-config --bootstrap-token` to create a bootstrap token, fetch AKS cluster metadata, and render the host config. 3. Copy the generated config to `/etc/aks-flex-node/config.json` on the target host. 4. Run `aks-flex-node preflight --config /etc/aks-flex-node/config.json` to validate host, cluster, rootfs, and artifact prerequisites without mutating the node. diff --git a/hack/e2e/lib/node-join-kubeadm.sh b/hack/e2e/lib/node-join-kubeadm.sh index ef9addf2..443516f1 100644 --- a/hack/e2e/lib/node-join-kubeadm.sh +++ b/hack/e2e/lib/node-join-kubeadm.sh @@ -36,10 +36,10 @@ _kubeadm_ensure_rbac() { # - ClusterRoleBindings for CSR creation and auto-approval # - Roles/RoleBindings granting bootstrappers read access to kubeadm config # and kubelet config (required by kubeadm join's preflight phase) - # - ClusterRole/ClusterRoleBinding for bootstrappers to GET nodes + # - ClusterRole/ClusterRoleBinding for kubeadm's bootstrap group to GET nodes # - ConfigMaps: cluster-info (kube-public), kubeadm-config and # kubelet-config (kube-system) consumed by kubeadm join - kubectl apply -f - < "${config_file}" < None: @@ -65,13 +84,428 @@ def setup_node_rbac(args: argparse.Namespace) -> None: require_command("kubectl") load_admin_kubeconfig(args) - log_info("applying bootstrap token RBAC bindings") - run(["kubectl", "apply", "-f", "-"], input_text=RBAC_MANIFEST) + bindings = reconcile_bootstrap_rbac_bindings() + if args.remove_legacy_node_role_binding: + remove_legacy_node_role_binding(bindings) + else: + require_legacy_node_role_binding_absent(bindings) + + +def cluster_role_bindings() -> list[dict[str, object]]: + raw = run(["kubectl", "get", "clusterrolebindings", "-o", "json"], capture=True) + try: + payload = json.loads(raw) + except json.JSONDecodeError as err: + raise SystemExit(f"ERROR: could not parse ClusterRoleBinding inventory: {err}") from err + + if not isinstance(payload, dict): + raise SystemExit("ERROR: ClusterRoleBinding inventory is not a JSON object") + items = payload.get("items") + if not isinstance(items, list): + raise SystemExit("ERROR: ClusterRoleBinding inventory does not contain an items list") + if not all(isinstance(item, dict) for item in items): + raise SystemExit("ERROR: ClusterRoleBinding inventory contains a malformed item") + return items + + +def expected_role_ref(role_name: str) -> dict[str, str]: + return {"apiGroup": RBAC_API_GROUP, "kind": "ClusterRole", "name": role_name} + + +def bootstrap_group_subject() -> dict[str, str]: + return { + "apiGroup": RBAC_API_GROUP, + "kind": "Group", + "name": FLEX_NODE_BOOTSTRAP_GROUP, + } + + +def binding_has_bootstrap_group(name: str, binding: dict[str, object]) -> bool: + subjects = binding.get("subjects", []) + if subjects is None: + subjects = [] + if not isinstance(subjects, list): + raise SystemExit( + f"ERROR: managed ClusterRoleBinding {name!r} subjects is not a list" + ) + return any( + isinstance(subject, dict) + and subject.get("apiGroup") == RBAC_API_GROUP + and subject.get("kind") == "Group" + and subject.get("name") == FLEX_NODE_BOOTSTRAP_GROUP + and not subject.get("namespace") + for subject in subjects + ) + + +def managed_binding_inventory(bindings: list[dict[str, object]]) -> dict[str, dict[str, object]]: + expected_names = {name for name, _ in MANAGED_BOOTSTRAP_BINDINGS} + managed: dict[str, dict[str, object]] = {} + for binding in bindings: + metadata = binding.get("metadata") + name = metadata.get("name") if isinstance(metadata, dict) else None + if name not in expected_names: + continue + if name in managed: + raise SystemExit(f"ERROR: duplicate managed ClusterRoleBinding {name!r} in inventory") + managed[name] = binding + return managed + + +def validate_managed_binding( + name: str, + role_name: str, + binding: dict[str, object], + *, + require_subject: bool, +) -> bool: + role_ref = binding.get("roleRef") + wanted_role_ref = expected_role_ref(role_name) + if role_ref != wanted_role_ref: + raise SystemExit( + f"ERROR: refusing to modify managed ClusterRoleBinding {name!r}: roleRef is " + f"{role_ref!r}, expected {wanted_role_ref!r}. The roleRef is immutable and replacing " + "this object could discard operator-managed subjects or metadata. Review it manually." + ) + + has_subject = binding_has_bootstrap_group(name, binding) + if require_subject and not has_subject: + raise SystemExit( + f"ERROR: managed ClusterRoleBinding {name!r} does not contain the required " + f"bootstrap group {FLEX_NODE_BOOTSTRAP_GROUP!r} after reconciliation" + ) + return has_subject + + +def desired_managed_binding(name: str, role_name: str) -> dict[str, object]: + return { + "apiVersion": f"{RBAC_API_GROUP}/v1", + "kind": "ClusterRoleBinding", + "metadata": {"name": name}, + "roleRef": expected_role_ref(role_name), + "subjects": [bootstrap_group_subject()], + } + + +def reconcile_bootstrap_rbac_bindings() -> list[dict[str, object]]: + before = cluster_role_bindings() + managed = managed_binding_inventory(before) + actions: list[tuple[str, str, dict[str, object]]] = [] + + # Validate every managed name before making any change. A roleRef is + # immutable, so replacing a customized binding would lose operator-managed + # subjects and metadata. + for name, role_name in MANAGED_BOOTSTRAP_BINDINGS: + binding = managed.get(name) + if binding is None: + actions.append(("create", name, desired_managed_binding(name, role_name))) + continue + + has_subject = validate_managed_binding(name, role_name, binding, require_subject=False) + if has_subject: + continue + + metadata = binding.get("metadata") + annotations = metadata.get("annotations", {}) if isinstance(metadata, dict) else {} + if not isinstance(annotations, dict): + raise SystemExit(f"ERROR: managed ClusterRoleBinding {name!r} annotations is not an object") + if str(annotations.get(RBAC_AUTOUPDATE_ANNOTATION, "")).lower() == "false": + raise SystemExit( + f"ERROR: managed ClusterRoleBinding {name!r} disables RBAC autoupdate but is missing " + f"the required bootstrap group {FLEX_NODE_BOOTSTRAP_GROUP!r}. Add the subject " + "manually or remove the autoupdate=false annotation." + ) + resource_version = metadata.get("resourceVersion") if isinstance(metadata, dict) else None + if not isinstance(resource_version, str) or not resource_version: + raise SystemExit( + f"ERROR: refusing to update managed ClusterRoleBinding {name!r} without a " + "resourceVersion concurrency precondition" + ) + + updated = dict(binding) + subjects = binding.get("subjects", []) + if subjects is None: + subjects = [] + updated["apiVersion"] = f"{RBAC_API_GROUP}/v1" + updated["kind"] = "ClusterRoleBinding" + updated["subjects"] = [*subjects, bootstrap_group_subject()] + actions.append(("replace", name, updated)) + + for operation, name, binding in actions: + log_info(f"{operation} managed bootstrap RBAC binding {name}") + run(["kubectl", operation, "-f", "-"], input_text=json.dumps(binding), capture=True) + + after = cluster_role_bindings() + reconciled = managed_binding_inventory(after) + for name, role_name in MANAGED_BOOTSTRAP_BINDINGS: + binding = reconciled.get(name) + if binding is None: + raise SystemExit(f"ERROR: managed ClusterRoleBinding {name!r} is absent after reconciliation") + validate_managed_binding(name, role_name, binding, require_subject=True) + return after + + +def unsafe_node_role_bindings(bindings: list[dict[str, object]] | None = None) -> list[dict[str, object]]: + if bindings is None: + bindings = cluster_role_bindings() + + unsafe = [] + for item in bindings: + if not isinstance(item, dict): + continue + role_ref = item.get("roleRef") + subjects = item.get("subjects") + if not isinstance(role_ref, dict) or not isinstance(subjects, list): + continue + if ( + role_ref.get("apiGroup") != "rbac.authorization.k8s.io" + or role_ref.get("kind") != "ClusterRole" + or role_ref.get("name") != "system:node" + ): + continue + if any( + isinstance(subject, dict) + and subject.get("apiGroup") == "rbac.authorization.k8s.io" + and subject.get("kind") == "Group" + and subject.get("name") == FLEX_NODE_BOOTSTRAP_GROUP + for subject in subjects + ): + unsafe.append(item) + return unsafe + + +def unsafe_binding_names(bindings: list[dict[str, object]]) -> str: + names = [] + for binding in bindings: + metadata = binding.get("metadata") + name = metadata.get("name") if isinstance(metadata, dict) else None + names.append(name if isinstance(name, str) and name else "") + return ", ".join(sorted(names)) + + +def is_canonical_legacy_node_role_binding(binding: dict[str, object]) -> bool: + metadata = binding.get("metadata") + subjects = binding.get("subjects") + if not isinstance(metadata, dict) or metadata.get("name") != LEGACY_NODE_ROLE_BINDING: + return False + return subjects == [ + { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "Group", + "name": FLEX_NODE_BOOTSTRAP_GROUP, + } + ] + + +def require_legacy_node_role_binding_absent(bindings: list[dict[str, object]] | None = None) -> None: + bindings = unsafe_node_role_bindings(bindings) + if not bindings: + return + raise SystemExit( + "ERROR: ClusterRoleBinding(s) " + f"{unsafe_binding_names(bindings)} still grant {FLEX_NODE_BOOTSTRAP_GROUP!r} the " + "system:node role. Some older or development agents may still depend on that access. " + "First upgrade them to a release with daemon client certificates (v0.1.1 or later), " + "verify /etc/aks-flex-node/daemon-credentials/daemon-controller-current.pem exists " + "and the agent remains healthy after restart, then rerun setup-node-rbac with " + "--remove-legacy-node-role-binding." + ) + + +class UnixSocketHTTPConnection(http.client.HTTPConnection): + def __init__(self, socket_path: str, *, timeout: float) -> None: + super().__init__("localhost", timeout=timeout) + self.socket_path = socket_path + + def connect(self) -> None: + connection = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + connection.settimeout(self.timeout) + try: + connection.connect(self.socket_path) + except OSError: + connection.close() + raise + self.sock = connection + + +def delete_cluster_role_binding_with_preconditions(name: str, uid: str, resource_version: str) -> None: + # kubectl delete does not expose UID/resourceVersion preconditions, and its + # --raw mode sends no request body. A proxy on a mode-0600 Unix socket lets + # us submit the Kubernetes DeleteOptions body without reimplementing + # kubeconfig authentication, exposing admin credentials on a localhost TCP + # port, or silently deleting a concurrently replaced object. + with tempfile.TemporaryDirectory(prefix="aks-flex-config-") as proxy_dir: + socket_path = os.path.join(proxy_dir, "kubectl-proxy.sock") + proxy = subprocess.Popen( + [ + "kubectl", + "proxy", + f"--unix-socket={socket_path}", + r"--accept-hosts=^localhost$", + ], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + start_new_session=True, + ) + try: + if proxy.stdout is None or proxy.stderr is None: + raise SystemExit("ERROR: failed to capture kubectl proxy startup output") + + startup_output = {"stdout": bytearray(), "stderr": bytearray()} + startup_pattern = re.compile( + rb"(?:^|\n)Starting to serve on " + re.escape(os.fsencode(socket_path)) + rb"\r?\n" + ) + ready = False + deadline = time.monotonic() + 10 + with selectors.DefaultSelector() as selector: + selector.register(proxy.stdout, selectors.EVENT_READ, "stdout") + selector.register(proxy.stderr, selectors.EVENT_READ, "stderr") + while time.monotonic() < deadline: + events = selector.select(max(0, deadline - time.monotonic())) + if not events: + break + for key, _ in events: + chunk = os.read(key.fd, 4096) + if not chunk: + selector.unregister(key.fileobj) + continue + stream_output = startup_output[key.data] + stream_output.extend(chunk) + if len(stream_output) > 65536: + del stream_output[:-65536] + if key.data == "stdout" and startup_pattern.search(stream_output): + ready = True + break + if ready: + break + if proxy.poll() is not None and not selector.get_map(): + break + + if not ready: + detail = b"\n".join( + output.strip() for output in startup_output.values() if output.strip() + ).decode(errors="replace") + if proxy.poll() is None: + raise SystemExit( + "ERROR: timed out starting kubectl proxy for conditional RBAC deletion" + f"{': ' + detail if detail else ''}" + ) + raise SystemExit(f"ERROR: kubectl proxy failed to start: {detail or 'unknown error'}") + + try: + socket_info = os.stat(socket_path, follow_symlinks=False) + if not stat.S_ISSOCK(socket_info.st_mode): + raise SystemExit("ERROR: kubectl proxy did not create a Unix socket") + os.chmod(socket_path, 0o600) + socket_mode = stat.S_IMODE(os.stat(socket_path, follow_symlinks=False).st_mode) + except OSError as err: + raise SystemExit(f"ERROR: could not secure kubectl proxy Unix socket: {err}") from err + if socket_mode != 0o600: + raise SystemExit( + f"ERROR: kubectl proxy Unix socket mode is {socket_mode:o}, expected 600" + ) + + body = json.dumps( + { + "apiVersion": "v1", + "kind": "DeleteOptions", + "preconditions": {"uid": uid, "resourceVersion": resource_version}, + } + ).encode() + resource_path = f"/apis/{RBAC_API_GROUP}/v1/clusterrolebindings/{quote(name, safe='')}" + connection = UnixSocketHTTPConnection(socket_path, timeout=30) + try: + connection.request( + "DELETE", + resource_path, + body=body, + headers={"Content-Type": "application/json"}, + ) + response = connection.getresponse() + detail = response.read().decode(errors="replace").strip() + if response.status == 409: + raise SystemExit( + f"ERROR: refusing to delete ClusterRoleBinding {name!r}: it changed after inspection" + ) + if not 200 <= response.status < 300: + raise SystemExit( + f"ERROR: conditional deletion of ClusterRoleBinding {name!r} returned " + f"HTTP {response.status}: {detail}" + ) + except (OSError, http.client.HTTPException) as err: + raise SystemExit( + f"ERROR: conditional deletion of ClusterRoleBinding {name!r} failed: {err}" + ) from err + finally: + connection.close() + finally: + try: + os.killpg(proxy.pid, signal.SIGTERM) + except ProcessLookupError: + pass + try: + proxy.wait(timeout=5) + except subprocess.TimeoutExpired: + pass + try: + # The kubectl process may have exited while an exec credential + # plugin kept the process group and pipe descriptors alive. + os.killpg(proxy.pid, signal.SIGKILL) + except ProcessLookupError: + pass + try: + proxy.wait(timeout=5) + except subprocess.TimeoutExpired: + log_error(f"kubectl proxy process {proxy.pid} did not exit after SIGKILL") + if proxy.stdout is not None: + proxy.stdout.close() + if proxy.stderr is not None: + proxy.stderr.close() + + +def remove_legacy_node_role_binding(bindings: list[dict[str, object]] | None = None) -> None: + bindings = unsafe_node_role_bindings(bindings) + if not bindings: + return + if len(bindings) != 1 or not is_canonical_legacy_node_role_binding(bindings[0]): + raise SystemExit( + "ERROR: refusing automatic removal because the unsafe ClusterRoleBinding set is " + f"not the canonical {LEGACY_NODE_ROLE_BINDING!r} object: " + f"{unsafe_binding_names(bindings)}. Review and remove only the bootstrap-group " + "subjects manually." + ) + + metadata = bindings[0].get("metadata") + uid = metadata.get("uid") if isinstance(metadata, dict) else None + resource_version = metadata.get("resourceVersion") if isinstance(metadata, dict) else None + if not isinstance(uid, str) or not uid or not isinstance(resource_version, str) or not resource_version: + raise SystemExit( + "ERROR: refusing automatic removal because the canonical legacy " + "ClusterRoleBinding has no UID/resourceVersion preconditions." + ) + + # Delete only the object version that was inspected above. If another + # actor replaces or edits it between inventory and deletion, the API server + # rejects the request instead of deleting an unreviewed object. + log_info("removing legacy bootstrap node role binding") + delete_cluster_role_binding_with_preconditions(LEGACY_NODE_ROLE_BINDING, uid, resource_version) + remaining = unsafe_node_role_bindings() + if remaining: + raise SystemExit( + "ERROR: unsafe bootstrap node role binding remains after deletion: " + f"{unsafe_binding_names(remaining)}" + ) def generate_bootstrap_token(args: argparse.Namespace) -> str: require_command("kubectl") + # Do not silently break old agents by deleting their binding while rendering + # a config, and never mint another broadly privileged token. Migration is an + # explicit setup-node-rbac action after existing agents have been upgraded. + require_legacy_node_role_binding_absent() + log_info("creating bootstrap token") + token_id = secrets.token_hex(3) token_secret = secrets.token_hex(8) token = f"{token_id}.{token_secret}" @@ -177,7 +611,6 @@ def render_config(args: argparse.Namespace, mode: str, metadata: dict[str, str]) if mode == "bootstrap-token": load_admin_kubeconfig(args) - log_info("creating bootstrap token") token = generate_bootstrap_token(args) server_url = run( ["kubectl", "config", "view", "--minify", "-o", "jsonpath={.clusters[0].cluster.server}"], @@ -258,8 +691,16 @@ def build_parser() -> argparse.ArgumentParser: subparser.add_argument("--cluster-name", required=True) subparser.add_argument("--subscription") - rbac = subparsers.add_parser("setup-node-rbac", help="Apply node bootstrap RBAC bindings.") + rbac = subparsers.add_parser("setup-node-rbac", help="Reconcile node bootstrap RBAC bindings.") add_cluster_args(rbac) + rbac.add_argument( + "--remove-legacy-node-role-binding", + action="store_true", + help=( + "Remove the canonical obsolete aks-flex-node-role binding after existing agents " + "have issued daemon certificates and remain healthy after restart." + ), + ) rbac.set_defaults(func=setup_node_rbac) generate = subparsers.add_parser("generate-node-config", help="Render a Flex Node config.") @@ -282,48 +723,6 @@ def build_parser() -> argparse.ArgumentParser: return parser -RBAC_MANIFEST = """ -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: aks-flex-node-bootstrapper -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: system:node-bootstrapper -subjects: -- apiGroup: rbac.authorization.k8s.io - kind: Group - name: system:bootstrappers:aks-flex-node ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: aks-flex-node-auto-approve-csr -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: system:certificates.k8s.io:certificatesigningrequests:nodeclient -subjects: -- apiGroup: rbac.authorization.k8s.io - kind: Group - name: system:bootstrappers:aks-flex-node ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRoleBinding -metadata: - name: aks-flex-node-role -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: system:node -subjects: -- apiGroup: rbac.authorization.k8s.io - kind: Group - name: system:bootstrappers:aks-flex-node -""".lstrip() - - def main() -> None: parser = build_parser() args = parser.parse_args() diff --git a/scripts/aks_flex_config_test.go b/scripts/aks_flex_config_test.go new file mode 100644 index 00000000..c0226ee2 --- /dev/null +++ b/scripts/aks_flex_config_test.go @@ -0,0 +1,1729 @@ +package scripts + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "io/fs" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + "testing" + + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/yaml" +) + +const ( + bootstrapGroup = "system:bootstrappers:aks-flex-node" + bootstrapBindingName = "aks-flex-node-bootstrapper" + bootstrapRole = "system:node-bootstrapper" + approvalBindingName = "aks-flex-node-auto-approve-csr" + approvalRole = "system:certificates.k8s.io:certificatesigningrequests:nodeclient" + legacyBindingName = "aks-flex-node-role" + legacyNodeRole = "system:node" + fakeDeleteFailure = 37 + fakeGetFailure = 39 + fakeCommandLogEnv = "AKS_FLEX_CONFIG_TEST_COMMAND_LOG" + fakeManifestEnv = "AKS_FLEX_CONFIG_TEST_MANIFEST" + fakeManagedStateEnv = "AKS_FLEX_CONFIG_TEST_MANAGED_STATE" + fakeDeleteOptsEnv = "AKS_FLEX_CONFIG_TEST_DELETE_OPTIONS" + fakeLegacyStateEnv = "AKS_FLEX_CONFIG_TEST_LEGACY_STATE" + fakeDeleteExitEnv = "AKS_FLEX_CONFIG_TEST_DELETE_EXIT" + fakeDeleteKeepsEnv = "AKS_FLEX_CONFIG_TEST_DELETE_KEEPS_STATE" + fakeConcurrentEnv = "AKS_FLEX_CONFIG_TEST_CONCURRENT_REPLACE" + fakeProxyStartupEnv = "AKS_FLEX_CONFIG_TEST_PROXY_STARTUP" + fakeConcurrentManagedEnv = "AKS_FLEX_CONFIG_TEST_CONCURRENT_MANAGED_REPLACE" + fakeManagedPostconditionEnv = "AKS_FLEX_CONFIG_TEST_MANAGED_POSTCONDITION" + fakeSkipManagedMutationEnv = "AKS_FLEX_CONFIG_TEST_SKIP_MANAGED_MUTATION" + fakeGetExitEnv = "AKS_FLEX_CONFIG_TEST_GET_EXIT" + fakeApplyCountEnv = "AKS_FLEX_CONFIG_TEST_APPLY_COUNT" + fakeApplyFailAtEnv = "AKS_FLEX_CONFIG_TEST_APPLY_FAIL_AT" +) + +type commandCall struct { + name string + args []string +} + +type configScriptHarness struct { + pythonPath string + scriptPath string + fakeBinDir string + commandLogPath string + manifestPath string + managedState string + deleteOptsPath string + configPath string + legacyState string + deleteExitCode int + deleteKeeps bool + concurrentSwap bool + proxyStartup string + poisonHTTPProxy bool + concurrentManaged string + managedPostcondition string + skipManagedMutation bool + getExitCode int + applyCountPath string + applyFailAt int +} + +func TestSetupNodeRBACManifestUsesOnlyBootstrapPermissions(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + output, err := harness.runSetupNodeRBAC(false) + if err != nil { + t.Fatalf("setup-node-rbac failed: %v\n%s", err, output) + } + + bindings := readRBACManifest(t, harness.manifestPath) + expectedRoles := map[string]string{ + "aks-flex-node-bootstrapper": "system:node-bootstrapper", + "aks-flex-node-auto-approve-csr": "system:certificates.k8s.io:certificatesigningrequests:nodeclient", + } + if len(bindings) != len(expectedRoles) { + t.Fatalf("RBAC manifest has %d bindings, want %d: %v", len(bindings), len(expectedRoles), bindingNames(bindings)) + } + + seen := make(map[string]struct{}, len(bindings)) + for _, binding := range bindings { + if binding.APIVersion != "rbac.authorization.k8s.io/v1" || binding.Kind != "ClusterRoleBinding" { + t.Errorf("binding %q has apiVersion/kind %q/%q, want rbac.authorization.k8s.io/v1/ClusterRoleBinding", binding.Name, binding.APIVersion, binding.Kind) + } + if binding.Namespace != "" { + t.Errorf("binding %q unexpectedly has namespace %q", binding.Name, binding.Namespace) + } + if _, duplicate := seen[binding.Name]; duplicate { + t.Errorf("binding %q appears more than once", binding.Name) + } + seen[binding.Name] = struct{}{} + + wantRole, expected := expectedRoles[binding.Name] + if !expected { + t.Errorf("unexpected ClusterRoleBinding %q", binding.Name) + continue + } + if binding.RoleRef.APIGroup != rbacv1.GroupName || binding.RoleRef.Kind != "ClusterRole" || binding.RoleRef.Name != wantRole { + t.Errorf("binding %q roleRef = %#v, want ClusterRole %q in %q", binding.Name, binding.RoleRef, wantRole, rbacv1.GroupName) + } + if len(binding.Subjects) != 1 { + t.Errorf("binding %q has %d subjects, want exactly one", binding.Name, len(binding.Subjects)) + continue + } + subject := binding.Subjects[0] + if subject.APIGroup != rbacv1.GroupName || subject.Kind != "Group" || subject.Name != bootstrapGroup || subject.Namespace != "" { + t.Errorf("binding %q subject = %#v, want bootstrapper group %q", binding.Name, subject, bootstrapGroup) + } + if binding.RoleRef.Name == legacyNodeRole { + t.Errorf("bootstrap group must not be bound to broad legacy role %q", legacyNodeRole) + } + } + + if _, found := seen[legacyBindingName]; found { + t.Errorf("RBAC manifest still contains legacy binding %q", legacyBindingName) + } + + calls := readCommandCalls(t, harness.commandLogPath) + applyIndexes, _ := kubectlOperationIndexes(calls) + if len(applyIndexes) != 2 { + t.Fatalf("RBAC reconciliation count = %d, want 2; calls: %s", len(applyIndexes), formatCalls(calls)) + } + for _, index := range applyIndexes { + assertSafeManagedRBACMutation(t, calls[index]) + } +} + +func TestSetupNodeRBACRejectsManagedRoleRefDriftBeforeMutation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + bindingName string + }{ + {name: "bootstrapper binding", bindingName: bootstrapBindingName}, + {name: "approval binding", bindingName: approvalBindingName}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + bootstrapper := managedBindingFixture(bootstrapBindingName, bootstrapRole, true) + approver := managedBindingFixture(approvalBindingName, approvalRole, true) + if test.bindingName == bootstrapBindingName { + bootstrapper.RoleRef.Name = "view" + bootstrapper.Subjects = append(bootstrapper.Subjects, rbacv1.Subject{ + APIGroup: rbacv1.GroupName, + Kind: "Group", + Name: "operator-bootstrapper-group", + }) + } else { + approver.RoleRef.Name = "view" + approver.Subjects = append(approver.Subjects, rbacv1.Subject{ + APIGroup: rbacv1.GroupName, + Kind: "Group", + Name: "operator-approver-group", + }) + } + writeManagedState(t, harness.managedState, bootstrapper, approver) + before := readFile(t, harness.managedState) + + output, err := harness.runSetupNodeRBAC(false) + if err == nil { + t.Fatalf("setup-node-rbac replaced a customized roleRef\n%s", output) + } + if !strings.Contains(output, test.bindingName) || + !strings.Contains(output, "roleRef is immutable") || + !strings.Contains(output, "Review it manually") { + t.Fatalf("failure did not identify the safe manual remediation:\n%s", output) + } + if after := readFile(t, harness.managedState); after != before { + t.Fatalf("managed bindings changed despite preflight failure\nbefore: %s\nafter: %s", before, after) + } + + calls := readCommandCalls(t, harness.commandLogPath) + mutations, deletes := kubectlOperationIndexes(calls) + if len(mutations) != 0 || len(deletes) != 0 { + t.Fatalf("roleRef preflight performed a mutation: %s", formatCalls(calls)) + } + }) + } +} + +func TestSetupNodeRBACIdentifiesManagedBindingWithMalformedSubjects(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + bindingName string + }{ + {name: "bootstrapper binding", bindingName: bootstrapBindingName}, + {name: "approval binding", bindingName: approvalBindingName}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + bindings := []map[string]any{ + { + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRoleBinding", + "metadata": map[string]any{"name": bootstrapBindingName, "resourceVersion": "7"}, + "roleRef": map[string]any{ + "apiGroup": rbacv1.GroupName, + "kind": "ClusterRole", + "name": bootstrapRole, + }, + "subjects": []any{map[string]any{ + "apiGroup": rbacv1.GroupName, + "kind": "Group", + "name": bootstrapGroup, + }}, + }, + { + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRoleBinding", + "metadata": map[string]any{"name": approvalBindingName, "resourceVersion": "7"}, + "roleRef": map[string]any{ + "apiGroup": rbacv1.GroupName, + "kind": "ClusterRole", + "name": approvalRole, + }, + "subjects": []any{map[string]any{ + "apiGroup": rbacv1.GroupName, + "kind": "Group", + "name": bootstrapGroup, + }}, + }, + } + malformedIndex := 0 + if test.bindingName == approvalBindingName { + malformedIndex = 1 + } + bindings[malformedIndex]["subjects"] = "not-a-list" + data, err := json.Marshal(bindings) + if err != nil { + t.Fatalf("marshal malformed managed bindings: %v", err) + } + if err := os.WriteFile(harness.managedState, append(data, '\n'), 0o600); err != nil { + t.Fatalf("write malformed managed bindings: %v", err) + } + + output, err := harness.runSetupNodeRBAC(false) + if err == nil { + t.Fatalf("setup-node-rbac accepted malformed subjects\n%s", output) + } + if !strings.Contains(output, test.bindingName) || + !strings.Contains(output, "subjects is not a list") { + t.Fatalf("failure did not identify the malformed binding:\n%s", output) + } + + mutations, deletes := kubectlOperationIndexes(readCommandCalls(t, harness.commandLogPath)) + if len(mutations) != 0 || len(deletes) != 0 { + t.Fatalf("malformed-subject preflight performed a mutation") + } + }) + } +} + +func TestSetupNodeRBACPreservesManagedCustomizations(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + bootstrapper := managedBindingFixture(bootstrapBindingName, bootstrapRole, true) + bootstrapper.Labels = map[string]string{"owner": "operator"} + bootstrapper.Annotations = map[string]string{"example.test/note": "keep"} + bootstrapper.Subjects = append(bootstrapper.Subjects, rbacv1.Subject{ + APIGroup: rbacv1.GroupName, + Kind: "Group", + Name: "operator-group", + }) + approver := managedBindingFixture(approvalBindingName, approvalRole, true) + approver.Annotations = map[string]string{"rbac.authorization.kubernetes.io/autoupdate": "false"} + writeManagedState(t, harness.managedState, bootstrapper, approver) + before := readFile(t, harness.managedState) + + output, err := harness.runSetupNodeRBAC(false) + if err != nil { + t.Fatalf("setup-node-rbac rejected valid customized bindings: %v\n%s", err, output) + } + if after := readFile(t, harness.managedState); after != before { + t.Fatalf("already-correct managed bindings changed\nbefore: %s\nafter: %s", before, after) + } + mutations, deletes := kubectlOperationIndexes(readCommandCalls(t, harness.commandLogPath)) + if len(mutations) != 0 || len(deletes) != 0 { + t.Fatalf("already-correct managed bindings were mutated") + } +} + +func TestSetupNodeRBACAddsSubjectWithOptimisticReplace(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + bootstrapper := managedBindingFixture(bootstrapBindingName, bootstrapRole, false) + bootstrapper.Labels = map[string]string{"owner": "operator"} + bootstrapper.Subjects = []rbacv1.Subject{{ + APIGroup: rbacv1.GroupName, + Kind: "Group", + Name: "operator-group", + }} + approver := managedBindingFixture(approvalBindingName, approvalRole, true) + writeManagedState(t, harness.managedState, bootstrapper, approver) + + output, err := harness.runSetupNodeRBAC(false) + if err != nil { + t.Fatalf("setup-node-rbac failed to add the required subject: %v\n%s", err, output) + } + mutations, deletes := kubectlOperationIndexes(readCommandCalls(t, harness.commandLogPath)) + if len(mutations) != 1 || len(deletes) != 0 { + t.Fatalf("managed reconciliation mutation/delete counts = %d/%d, want 1/0", len(mutations), len(deletes)) + } + if call := readCommandCalls(t, harness.commandLogPath)[mutations[0]]; len(call.args) == 0 || call.args[0] != "replace" { + t.Fatalf("managed binding update = %#v, want optimistic kubectl replace", call) + } + + updated := findManagedBinding(t, readManagedState(t, harness.managedState), bootstrapBindingName) + if updated.Labels["owner"] != "operator" { + t.Fatalf("operator metadata was not preserved: %#v", updated.Labels) + } + if !hasSubject(updated.Subjects, "operator-group") || !hasSubject(updated.Subjects, bootstrapGroup) { + t.Fatalf("operator and required subjects were not both preserved: %#v", updated.Subjects) + } + if updated.ResourceVersion != "8" { + t.Fatalf("resourceVersion = %q, want optimistic replacement of version 7", updated.ResourceVersion) + } +} + +func TestSetupNodeRBACRejectsMissingManagedResourceVersion(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + bindingName string + }{ + {name: "bootstrapper binding", bindingName: bootstrapBindingName}, + {name: "approval binding", bindingName: approvalBindingName}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + bootstrapper := managedBindingFixture(bootstrapBindingName, bootstrapRole, true) + approver := managedBindingFixture(approvalBindingName, approvalRole, true) + if test.bindingName == bootstrapBindingName { + bootstrapper.ResourceVersion = "" + bootstrapper.Subjects = nil + } else { + approver.ResourceVersion = "" + approver.Subjects = nil + } + writeManagedState(t, harness.managedState, bootstrapper, approver) + before := readFile(t, harness.managedState) + + output, err := harness.runSetupNodeRBAC(false) + if err == nil { + t.Fatalf("setup-node-rbac replaced a binding without a resourceVersion\n%s", output) + } + if !strings.Contains(output, test.bindingName) || !strings.Contains(output, "resourceVersion") { + t.Fatalf("failure did not identify the missing concurrency precondition:\n%s", output) + } + if after := readFile(t, harness.managedState); after != before { + t.Fatalf("managed bindings changed despite a missing resourceVersion\nbefore: %s\nafter: %s", before, after) + } + + mutations, deletes := kubectlOperationIndexes(readCommandCalls(t, harness.commandLogPath)) + if len(mutations) != 0 || len(deletes) != 0 { + t.Fatalf("missing resourceVersion preflight performed a mutation") + } + }) + } +} + +func TestSetupNodeRBACRespectsAutoupdateFalse(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + includeBootstrap bool + wantFailure bool + }{ + {name: "missing required subject", wantFailure: true}, + {name: "required subject already present", includeBootstrap: true}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + bootstrapper := managedBindingFixture(bootstrapBindingName, bootstrapRole, test.includeBootstrap) + bootstrapper.Annotations = map[string]string{ + "rbac.authorization.kubernetes.io/autoupdate": "false", + } + approver := managedBindingFixture(approvalBindingName, approvalRole, true) + writeManagedState(t, harness.managedState, bootstrapper, approver) + + output, err := harness.runSetupNodeRBAC(false) + if test.wantFailure && err == nil { + t.Fatalf("setup-node-rbac changed a protected binding\n%s", output) + } + if !test.wantFailure && err != nil { + t.Fatalf("setup-node-rbac rejected a complete protected binding: %v\n%s", err, output) + } + if test.wantFailure && (!strings.Contains(output, "autoupdate") || !strings.Contains(output, "manually")) { + t.Fatalf("failure did not explain protected-binding remediation:\n%s", output) + } + mutations, deletes := kubectlOperationIndexes(readCommandCalls(t, harness.commandLogPath)) + if len(mutations) != 0 || len(deletes) != 0 { + t.Fatalf("protected binding was mutated") + } + }) + } +} + +func TestSetupNodeRBACPreservesConcurrentManagedReplacement(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + bootstrapper := managedBindingFixture(bootstrapBindingName, bootstrapRole, false) + approver := managedBindingFixture(approvalBindingName, approvalRole, true) + writeManagedState(t, harness.managedState, bootstrapper, approver) + harness.concurrentManaged = bootstrapBindingName + + output, err := harness.runSetupNodeRBAC(false) + if err == nil { + t.Fatalf("setup-node-rbac overwrote a concurrently replaced binding\n%s", output) + } + replacement := findManagedBinding(t, readManagedState(t, harness.managedState), bootstrapBindingName) + if replacement.UID != types.UID("operator-replacement") || + replacement.RoleRef.Name != "view" || + !hasSubject(replacement.Subjects, "operator-group") { + t.Fatalf("concurrent operator replacement was not preserved: %#v", replacement) + } +} + +func TestSetupNodeRBACPreservesConcurrentManagedCreate(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + harness.concurrentManaged = bootstrapBindingName + + output, err := harness.runSetupNodeRBAC(false) + if err == nil { + t.Fatalf("setup-node-rbac overwrote a concurrently created binding\n%s", output) + } + replacement := findManagedBinding(t, readManagedState(t, harness.managedState), bootstrapBindingName) + if replacement.UID != types.UID("operator-replacement") || + replacement.RoleRef.Name != "view" || + !hasSubject(replacement.Subjects, "operator-group") { + t.Fatalf("concurrent operator creation was not preserved: %#v", replacement) + } + + mutations, deletes := kubectlOperationIndexes(readCommandCalls(t, harness.commandLogPath)) + if len(mutations) != 1 || len(deletes) != 0 { + t.Fatalf("concurrent create mutation/delete attempts = %d/%d, want 1/0", len(mutations), len(deletes)) + } +} + +func TestSetupNodeRBACChecksManagedPostconditions(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + configure func(*configScriptHarness) + wantSubstring string + }{ + { + name: "binding absent", + configure: func(harness *configScriptHarness) { + harness.skipManagedMutation = true + }, + wantSubstring: "absent after reconciliation", + }, + { + name: "wrong roleRef", + configure: func(harness *configScriptHarness) { + harness.managedPostcondition = bootstrapBindingName + ":wrong-role-ref" + }, + wantSubstring: "roleRef is immutable", + }, + { + name: "required subject missing", + configure: func(harness *configScriptHarness) { + harness.managedPostcondition = bootstrapBindingName + ":missing-subject" + }, + wantSubstring: "does not contain the required bootstrap group", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + test.configure(harness) + output, err := harness.runSetupNodeRBAC(false) + if err == nil { + t.Fatalf("setup-node-rbac succeeded despite a missing postcondition\n%s", output) + } + if !strings.Contains(output, test.wantSubstring) { + t.Fatalf("postcondition failure was not actionable:\n%s", output) + } + }) + } +} + +func TestSetupNodeRBACPreservesLegacyBindingWithoutExplicitMigration(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, true, 0) + output, err := harness.runSetupNodeRBAC(false) + if err == nil { + t.Fatalf("setup-node-rbac succeeded without explicit migration while legacy binding exists\n%s", output) + } + if !strings.Contains(output, "--remove-legacy-node-role-binding") || + !strings.Contains(output, "v0.1.1") || + !strings.Contains(output, "/etc/aks-flex-node/daemon-credentials/daemon-controller-current.pem") { + t.Fatalf("setup-node-rbac did not explain the compatible migration path:\n%s", output) + } + + state, readErr := os.ReadFile(harness.legacyState) + if readErr != nil { + t.Fatalf("read fake legacy state: %v", readErr) + } + if got := strings.TrimSpace(string(state)); got != "present" { + t.Fatalf("legacy binding state = %q, want present until migration is acknowledged", got) + } + + calls := readCommandCalls(t, harness.commandLogPath) + applyIndexes, deleteIndexes := kubectlOperationIndexes(calls) + if len(applyIndexes) != 2 || len(deleteIndexes) != 0 { + t.Fatalf("kubectl mutation/delete counts = %d/%d, want 2/0; calls: %s", len(applyIndexes), len(deleteIndexes), formatCalls(calls)) + } + if getIndexes := kubectlIndexes(calls, "get"); len(getIndexes) != 2 || getIndexes[0] >= applyIndexes[0] || applyIndexes[1] >= getIndexes[1] { + t.Fatalf("safe RBAC must be preflighted and verified before checking legacy migration: %s", formatCalls(calls)) + } + for _, index := range applyIndexes { + assertSafeManagedRBACMutation(t, calls[index]) + } +} + +func TestSetupNodeRBACMigratesLegacyBindingIdempotently(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, true, 0) + for run := 1; run <= 2; run++ { + output, err := harness.runSetupNodeRBAC(true) + if err != nil { + t.Fatalf("setup-node-rbac run %d failed: %v\n%s", run, err, output) + } + } + + state, err := os.ReadFile(harness.legacyState) + if err != nil { + t.Fatalf("read fake legacy state: %v", err) + } + if got := strings.TrimSpace(string(state)); got != "absent" { + t.Fatalf("legacy binding state = %q, want absent", got) + } + + calls := readCommandCalls(t, harness.commandLogPath) + applyIndexes, deleteIndexes := kubectlOperationIndexes(calls) + if len(applyIndexes) != 2 || len(deleteIndexes) != 1 { + t.Fatalf("kubectl apply/delete counts = %d/%d, want 2/1; calls: %s", len(applyIndexes), len(deleteIndexes), formatCalls(calls)) + } + proxyIndexes := kubectlIndexes(calls, "proxy") + if len(proxyIndexes) != 1 { + t.Fatalf("conditional deletion started %d kubectl proxies, want 1: %s", len(proxyIndexes), formatCalls(calls)) + } + unixSocketArgs := 0 + for _, arg := range calls[proxyIndexes[0]].args { + if strings.HasPrefix(arg, "--unix-socket=") { + unixSocketArgs++ + } + if strings.HasPrefix(arg, "--port=") || strings.HasPrefix(arg, "--address=") { + t.Errorf("conditional deletion exposed a TCP listener: %q", arg) + } + } + if unixSocketArgs != 1 { + t.Fatalf("conditional deletion must use one private Unix socket: %s", formatCalls(calls)) + } + if applyIndexes[0] >= deleteIndexes[0] { + t.Errorf("legacy binding was deleted before safe RBAC was applied: %s", formatCalls(calls)) + } + assertLegacyDeleteCall(t, calls[deleteIndexes[0]]) + assertLegacyDeletePreconditions(t, harness.deleteOptsPath, "legacy-uid", "7") + if getIndexes := kubectlIndexes(calls, "get"); len(getIndexes) != 5 || getIndexes[1] >= deleteIndexes[0] || deleteIndexes[0] >= getIndexes[2] { + t.Fatalf("migration must inventory before and verify after deletion, then stay idempotent: %s", formatCalls(calls)) + } +} + +func TestSetupNodeRBACRejectsConcurrentLegacyBindingReplacement(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, true, 0) + harness.concurrentSwap = true + output, err := harness.runSetupNodeRBAC(true) + if err == nil { + t.Fatalf("setup-node-rbac deleted a concurrently replaced binding\n%s", output) + } + + state, readErr := os.ReadFile(harness.legacyState) + if readErr != nil { + t.Fatalf("read fake legacy state: %v", readErr) + } + if got := strings.TrimSpace(string(state)); got != "customized" { + t.Fatalf("legacy binding state = %q, want concurrently replaced object preserved", got) + } + assertLegacyDeletePreconditions(t, harness.deleteOptsPath, "legacy-uid", "7") +} + +func TestSetupNodeRBACFailsWhenLegacyBindingDeleteFails(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, true, fakeDeleteFailure) + output, err := harness.runSetupNodeRBAC(true) + if err == nil { + t.Fatalf("setup-node-rbac succeeded when legacy binding deletion failed\n%s", output) + } + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("setup-node-rbac error = %T %v, want *exec.ExitError", err, err) + } + if got := exitErr.ExitCode(); got == 0 { + t.Fatalf("setup-node-rbac exit code = %d, want nonzero\n%s", got, output) + } + if !strings.Contains(output, "HTTP 500") { + t.Fatalf("setup-node-rbac did not report the API deletion failure:\n%s", output) + } + + state, readErr := os.ReadFile(harness.legacyState) + if readErr != nil { + t.Fatalf("read fake legacy state: %v", readErr) + } + if got := strings.TrimSpace(string(state)); got != "present" { + t.Fatalf("legacy binding state = %q after failed deletion, want present", got) + } + + calls := readCommandCalls(t, harness.commandLogPath) + applyIndexes, deleteIndexes := kubectlOperationIndexes(calls) + if len(applyIndexes) != 2 || len(deleteIndexes) != 1 { + t.Fatalf("kubectl mutation/delete counts = %d/%d, want 2/1; calls: %s", len(applyIndexes), len(deleteIndexes), formatCalls(calls)) + } + if applyIndexes[0] >= deleteIndexes[0] { + t.Errorf("legacy delete failure occurred before safe RBAC was applied; calls: %s", formatCalls(calls)) + } + assertLegacyDeleteCall(t, calls[deleteIndexes[0]]) +} + +func TestSetupNodeRBACFailsWhenLegacyBindingRemainsAfterDelete(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, true, 0) + harness.deleteKeeps = true + output, err := harness.runSetupNodeRBAC(true) + if err == nil { + t.Fatalf("setup-node-rbac succeeded while unsafe binding remained\n%s", output) + } + if !strings.Contains(output, "remains after deletion") { + t.Fatalf("failure did not report the failed postcondition:\n%s", output) + } + + calls := readCommandCalls(t, harness.commandLogPath) + _, deleteIndexes := kubectlOperationIndexes(calls) + if len(deleteIndexes) != 1 || len(kubectlIndexes(calls, "get")) != 3 { + t.Fatalf("migration did not inventory, delete, and verify: %s", formatCalls(calls)) + } +} + +func TestSetupNodeRBACReportsKubectlProxyStartupFailure(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, true, 0) + harness.proxyStartup = "exit-before-ready" + output, err := harness.runSetupNodeRBAC(true) + if err == nil { + t.Fatalf("setup-node-rbac succeeded when kubectl proxy exited before readiness\n%s", output) + } + if !strings.Contains(output, "injected proxy startup failure") { + t.Fatalf("setup-node-rbac did not surface kubectl proxy's startup error:\n%s", output) + } + if got := strings.TrimSpace(readFile(t, harness.legacyState)); got != "present" { + t.Fatalf("legacy binding state = %q after proxy startup failure, want present", got) + } +} + +func TestSetupNodeRBACHandlesFragmentedKubectlProxyReadiness(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, true, 0) + harness.proxyStartup = "fragmented-ready" + output, err := harness.runSetupNodeRBAC(true) + if err != nil { + t.Fatalf("setup-node-rbac did not handle fragmented kubectl proxy output: %v\n%s", err, output) + } + if got := strings.TrimSpace(readFile(t, harness.legacyState)); got != "absent" { + t.Fatalf("legacy binding state = %q after fragmented readiness output, want absent", got) + } +} + +func TestSetupNodeRBACBypassesEnvironmentProxyForUnixSocketDeletion(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, true, 0) + harness.poisonHTTPProxy = true + output, err := harness.runSetupNodeRBAC(true) + if err != nil { + t.Fatalf("setup-node-rbac sent its Unix-socket deletion through the environment proxy: %v\n%s", err, output) + } + if got := strings.TrimSpace(readFile(t, harness.legacyState)); got != "absent" { + t.Fatalf("legacy binding state = %q after Unix-socket deletion, want absent", got) + } +} + +func TestSetupNodeRBACRefusesAmbiguousUnsafeBindings(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + state string + }{ + {name: "canonical name with additional subject", state: "customized"}, + {name: "unexpected binding name", state: "renamed"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + if err := os.WriteFile(harness.legacyState, []byte(test.state+"\n"), 0o600); err != nil { + t.Fatalf("write fake legacy state: %v", err) + } + output, err := harness.runSetupNodeRBAC(true) + if err == nil { + t.Fatalf("setup-node-rbac removed an ambiguous binding\n%s", output) + } + if !strings.Contains(output, "refusing automatic removal") || !strings.Contains(output, "manually") { + t.Fatalf("failure did not explain manual remediation:\n%s", output) + } + + calls := readCommandCalls(t, harness.commandLogPath) + _, deleteIndexes := kubectlOperationIndexes(calls) + if len(deleteIndexes) != 0 { + t.Fatalf("ambiguous binding was deleted: %s", formatCalls(calls)) + } + }) + } +} + +func TestSetupNodeRBACPreservesSameNamedNonLegacyBinding(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + if err := os.WriteFile(harness.legacyState, []byte("safe-customized\n"), 0o600); err != nil { + t.Fatalf("write fake legacy state: %v", err) + } + output, err := harness.runSetupNodeRBAC(true) + if err != nil { + t.Fatalf("setup-node-rbac rejected a non-legacy same-named binding: %v\n%s", err, output) + } + + state, readErr := os.ReadFile(harness.legacyState) + if readErr != nil { + t.Fatalf("read fake legacy state: %v", readErr) + } + if got := strings.TrimSpace(string(state)); got != "safe-customized" { + t.Fatalf("same-named non-legacy binding state = %q, want preserved", got) + } + if _, deleteIndexes := kubectlOperationIndexes(readCommandCalls(t, harness.commandLogPath)); len(deleteIndexes) != 0 { + t.Fatal("same-named non-legacy binding was deleted") + } +} + +func TestGenerateBootstrapTokenRequiresCompletedLegacyMigration(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + legacyPresent bool + getExitCode int + wantExitCode int + }{ + {name: "migration already complete"}, + {name: "legacy binding present", legacyPresent: true, wantExitCode: 1}, + {name: "legacy state cannot be read", getExitCode: fakeGetFailure, wantExitCode: fakeGetFailure}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, test.legacyPresent, 0) + harness.getExitCode = test.getExitCode + output, err := harness.runGenerateNodeConfig() + if test.wantExitCode != 0 { + if err == nil { + t.Fatalf("generate-node-config succeeded before legacy migration completed\n%s", output) + } + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) || exitErr.ExitCode() != test.wantExitCode { + t.Fatalf("generate-node-config error = %v, want exit code %d\n%s", err, test.wantExitCode, output) + } + } else if err != nil { + t.Fatalf("generate-node-config failed: %v\n%s", err, output) + } + + calls := readCommandCalls(t, harness.commandLogPath) + applyIndexes, deleteIndexes := kubectlOperationIndexes(calls) + getIndexes := kubectlIndexes(calls, "get") + if len(getIndexes) != 1 || len(deleteIndexes) != 0 { + t.Fatalf("kubectl get/delete counts = %d/%d, want 1/0; calls: %s", len(getIndexes), len(deleteIndexes), formatCalls(calls)) + } + if test.wantExitCode != 0 { + if len(applyIndexes) != 0 { + t.Fatalf("token Secret was applied before legacy migration completed; calls: %s", formatCalls(calls)) + } + if _, statErr := os.Stat(harness.manifestPath); !errors.Is(statErr, os.ErrNotExist) { + t.Fatalf("token manifest exists after cleanup failure: %v", statErr) + } + if test.legacyPresent && !strings.Contains(output, "--remove-legacy-node-role-binding") { + t.Fatalf("failure did not explain explicit migration path:\n%s", output) + } + return + } + + if len(applyIndexes) != 1 || getIndexes[0] >= applyIndexes[0] { + t.Fatalf("legacy-state check must precede the single token apply; calls: %s", formatCalls(calls)) + } + manifest, readErr := os.ReadFile(harness.manifestPath) + if readErr != nil { + t.Fatalf("read token manifest: %v", readErr) + } + if !strings.Contains(string(manifest), "kind: Secret") || !strings.Contains(string(manifest), "type: bootstrap.kubernetes.io/token") { + t.Fatalf("applied manifest is not a bootstrap token Secret:\n%s", manifest) + } + }) + } +} + +func TestKubeadmRBACReconciliation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + applyFailAt int + wantApplies int + wantFailure bool + }{ + {name: "succeeds", wantApplies: 2}, + {name: "initial RBAC apply fails", applyFailAt: 1, wantApplies: 1, wantFailure: true}, + {name: "ConfigMap apply fails", applyFailAt: 2, wantApplies: 2, wantFailure: true}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, true, 0) + harness.applyFailAt = test.applyFailAt + output, err := harness.runKubeadmEnsureRBAC() + if test.wantFailure && err == nil { + t.Fatalf("kubeadm RBAC reconciliation succeeded despite injected failure\n%s", output) + } + if test.wantFailure { + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) || exitErr.ExitCode() != 1 { + t.Fatalf("kubeadm RBAC reconciliation error = %v, want exit code 1\n%s", err, output) + } + } else if err != nil { + t.Fatalf("kubeadm RBAC reconciliation failed: %v\n%s", err, output) + } + + calls := readCommandCalls(t, harness.commandLogPath) + applyIndexes, deleteIndexes := kubectlOperationIndexes(calls) + if len(applyIndexes) != test.wantApplies || len(deleteIndexes) != 0 { + t.Fatalf( + "kubectl apply/delete counts = %d/%d, want %d/%d; calls: %s", + len(applyIndexes), len(deleteIndexes), test.wantApplies, 0, formatCalls(calls), + ) + } + }) + } +} + +func TestRepositoryDoesNotBindBootstrapGroupsToLegacyNodeRole(t *testing.T) { + t.Parallel() + + workingDir, err := os.Getwd() + if err != nil { + t.Fatalf("get working directory: %v", err) + } + repositoryRoot := filepath.Dir(workingDir) + sourceRoots := []string{"cmd", "hack", "pkg", "scripts"} + bindingPattern := regexp.MustCompile(`(?m)^[ \t]*kind:[ \t]*ClusterRoleBinding[ \t]*$`) + legacyRolePattern := regexp.MustCompile(`(?m)^[ \t]*name:[ \t]*system:node[ \t]*$`) + bootstrapGroupPattern := regexp.MustCompile(`(?m)^[ \t]*name:[ \t]*system:bootstrappers:[^ \t\n]+[ \t]*$`) + documentSeparator := regexp.MustCompile(`(?m)^[ \t]*---[ \t]*$`) + + for _, sourceRoot := range sourceRoots { + root := filepath.Join(repositoryRoot, sourceRoot) + walkErr := filepath.WalkDir(root, func(path string, entry fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if entry.IsDir() { + return nil + } + extension := filepath.Ext(path) + if entry.Name() != "aks-flex-config" && extension != ".go" && extension != ".py" && extension != ".sh" && extension != ".yaml" && extension != ".yml" { + return nil + } + contents, readErr := os.ReadFile(path) + if readErr != nil { + return fmt.Errorf("read %s: %w", path, readErr) + } + normalized := strings.ReplaceAll(string(contents), "\r\n", "\n") + for documentIndex, document := range documentSeparator.Split(normalized, -1) { + if bindingPattern.MatchString(document) && legacyRolePattern.MatchString(document) && bootstrapGroupPattern.MatchString(document) { + relativePath, relErr := filepath.Rel(repositoryRoot, path) + if relErr != nil { + relativePath = path + } + t.Errorf("%s YAML document %d binds a bootstrap group to the broad legacy %q role", relativePath, documentIndex+1, legacyNodeRole) + } + } + return nil + }) + if walkErr != nil { + t.Fatalf("scan %s for unsafe bootstrap RBAC: %v", sourceRoot, walkErr) + } + } +} + +func newConfigScriptHarness(t *testing.T, legacyPresent bool, deleteExitCode int) *configScriptHarness { + t.Helper() + + pythonPath, err := exec.LookPath("python3") + if err != nil { + t.Fatal("python3 is required to test scripts/aks-flex-config") + } + workingDir, err := os.Getwd() + if err != nil { + t.Fatalf("get working directory: %v", err) + } + scriptPath := filepath.Join(workingDir, "aks-flex-config") + if _, err := os.Stat(scriptPath); err != nil { + t.Fatalf("stat aks-flex-config: %v", err) + } + + tempDir := t.TempDir() + fakeBinDir := filepath.Join(tempDir, "bin") + if err := os.Mkdir(fakeBinDir, 0o700); err != nil { + t.Fatalf("create fake bin directory: %v", err) + } + writeExecutable(t, filepath.Join(fakeBinDir, "az"), fakeAZScript) + writeExecutable(t, filepath.Join(fakeBinDir, "kubectl"), fakeKubectlScript) + + legacyState := filepath.Join(tempDir, "legacy-state") + state := "absent\n" + if legacyPresent { + state = "present\n" + } + if err := os.WriteFile(legacyState, []byte(state), 0o600); err != nil { + t.Fatalf("write fake legacy state: %v", err) + } + managedState := filepath.Join(tempDir, "managed-state.json") + if err := os.WriteFile(managedState, []byte("[]\n"), 0o600); err != nil { + t.Fatalf("write fake managed binding state: %v", err) + } + + return &configScriptHarness{ + pythonPath: pythonPath, + scriptPath: scriptPath, + fakeBinDir: fakeBinDir, + commandLogPath: filepath.Join(tempDir, "commands.log"), + manifestPath: filepath.Join(tempDir, "rbac.yaml"), + managedState: managedState, + deleteOptsPath: filepath.Join(tempDir, "delete-options.json"), + configPath: filepath.Join(tempDir, "config.json"), + legacyState: legacyState, + deleteExitCode: deleteExitCode, + applyCountPath: filepath.Join(tempDir, "apply-count"), + } +} + +func (h *configScriptHarness) runSetupNodeRBAC(removeLegacy bool) (string, error) { + args := []string{ + h.scriptPath, + "setup-node-rbac", + "--resource-group", "test-rg", + "--cluster-name", "test-cluster", + "--subscription", "test-subscription", + } + if removeLegacy { + args = append(args, "--remove-legacy-node-role-binding") + } + cmd := exec.Command(h.pythonPath, args...) + cmd.Env = append(os.Environ(), + "PATH="+h.fakeBinDir+string(os.PathListSeparator)+os.Getenv("PATH"), + "KUBECONFIG="+filepath.Join(filepath.Dir(h.fakeBinDir), "kubeconfig"), + "PYTHONDONTWRITEBYTECODE=1", + fakeCommandLogEnv+"="+h.commandLogPath, + fakeManifestEnv+"="+h.manifestPath, + fakeManagedStateEnv+"="+h.managedState, + fakeDeleteOptsEnv+"="+h.deleteOptsPath, + fakeLegacyStateEnv+"="+h.legacyState, + fmt.Sprintf("%s=%d", fakeDeleteExitEnv, h.deleteExitCode), + fmt.Sprintf("%s=%t", fakeDeleteKeepsEnv, h.deleteKeeps), + fmt.Sprintf("%s=%t", fakeConcurrentEnv, h.concurrentSwap), + fakeProxyStartupEnv+"="+h.proxyStartup, + fakeConcurrentManagedEnv+"="+h.concurrentManaged, + fakeManagedPostconditionEnv+"="+h.managedPostcondition, + fmt.Sprintf("%s=%t", fakeSkipManagedMutationEnv, h.skipManagedMutation), + fmt.Sprintf("%s=%d", fakeGetExitEnv, h.getExitCode), + fakeApplyCountEnv+"="+h.applyCountPath, + fmt.Sprintf("%s=%d", fakeApplyFailAtEnv, h.applyFailAt), + ) + if h.poisonHTTPProxy { + cmd.Env = append(cmd.Env, + "HTTP_PROXY=http://127.0.0.1:1", + "http_proxy=http://127.0.0.1:1", + "HTTPS_PROXY=http://127.0.0.1:1", + "https_proxy=http://127.0.0.1:1", + "ALL_PROXY=http://127.0.0.1:1", + "all_proxy=http://127.0.0.1:1", + "NO_PROXY=", + "no_proxy=", + ) + } + output, err := cmd.CombinedOutput() + return string(output), err +} + +func (h *configScriptHarness) runGenerateNodeConfig() (string, error) { + cmd := exec.Command( + h.pythonPath, + h.scriptPath, + "generate-node-config", + "--resource-group", "test-rg", + "--cluster-name", "test-cluster", + "--subscription", "test-subscription", + "--bootstrap-token", + "--output", h.configPath, + ) + cmd.Env = append(os.Environ(), + "PATH="+h.fakeBinDir+string(os.PathListSeparator)+os.Getenv("PATH"), + "KUBECONFIG="+filepath.Join(filepath.Dir(h.fakeBinDir), "kubeconfig"), + "PYTHONDONTWRITEBYTECODE=1", + fakeCommandLogEnv+"="+h.commandLogPath, + fakeManifestEnv+"="+h.manifestPath, + fakeManagedStateEnv+"="+h.managedState, + fakeDeleteOptsEnv+"="+h.deleteOptsPath, + fakeLegacyStateEnv+"="+h.legacyState, + fmt.Sprintf("%s=%d", fakeDeleteExitEnv, h.deleteExitCode), + fmt.Sprintf("%s=%t", fakeDeleteKeepsEnv, h.deleteKeeps), + fmt.Sprintf("%s=%t", fakeConcurrentEnv, h.concurrentSwap), + fakeConcurrentManagedEnv+"="+h.concurrentManaged, + fmt.Sprintf("%s=%t", fakeSkipManagedMutationEnv, h.skipManagedMutation), + fmt.Sprintf("%s=%d", fakeGetExitEnv, h.getExitCode), + fakeApplyCountEnv+"="+h.applyCountPath, + fmt.Sprintf("%s=%d", fakeApplyFailAtEnv, h.applyFailAt), + ) + output, err := cmd.CombinedOutput() + return string(output), err +} + +func (h *configScriptHarness) runKubeadmEnsureRBAC() (string, error) { + workingDir, err := os.Getwd() + if err != nil { + return "", fmt.Errorf("get working directory: %w", err) + } + kubeadmScript := filepath.Join(filepath.Dir(workingDir), "hack", "e2e", "lib", "node-join-kubeadm.sh") + cmd := exec.Command( + "bash", + "-c", + `source "$1"; with_cluster_lock _kubeadm_ensure_rbac "https://test-cluster.example.test:443" "dGVzdC1jYQ=="`, + "bash", + kubeadmScript, + ) + cmd.Env = append(os.Environ(), + "PATH="+h.fakeBinDir+string(os.PathListSeparator)+os.Getenv("PATH"), + "E2E_WORK_DIR="+filepath.Join(filepath.Dir(h.fakeBinDir), "e2e-work"), + "E2E_KUBERNETES_VERSION=1.35.0", + fakeCommandLogEnv+"="+h.commandLogPath, + fakeManifestEnv+"="+h.manifestPath, + fakeManagedStateEnv+"="+h.managedState, + fakeDeleteOptsEnv+"="+h.deleteOptsPath, + fakeLegacyStateEnv+"="+h.legacyState, + fmt.Sprintf("%s=%d", fakeDeleteExitEnv, h.deleteExitCode), + fmt.Sprintf("%s=%t", fakeDeleteKeepsEnv, h.deleteKeeps), + fmt.Sprintf("%s=%t", fakeConcurrentEnv, h.concurrentSwap), + fakeConcurrentManagedEnv+"="+h.concurrentManaged, + fmt.Sprintf("%s=%t", fakeSkipManagedMutationEnv, h.skipManagedMutation), + fmt.Sprintf("%s=%d", fakeGetExitEnv, h.getExitCode), + fakeApplyCountEnv+"="+h.applyCountPath, + fmt.Sprintf("%s=%d", fakeApplyFailAtEnv, h.applyFailAt), + ) + output, err := cmd.CombinedOutput() + return string(output), err +} + +func managedBindingFixture(name, role string, includeBootstrap bool) rbacv1.ClusterRoleBinding { + subjects := []rbacv1.Subject{} + if includeBootstrap { + subjects = append(subjects, rbacv1.Subject{ + APIGroup: rbacv1.GroupName, + Kind: "Group", + Name: bootstrapGroup, + }) + } + return rbacv1.ClusterRoleBinding{ + TypeMeta: metav1.TypeMeta{ + APIVersion: rbacv1.SchemeGroupVersion.String(), + Kind: "ClusterRoleBinding", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + UID: types.UID(name + "-uid"), + ResourceVersion: "7", + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: role, + }, + Subjects: subjects, + } +} + +func writeManagedState(t *testing.T, path string, bindings ...rbacv1.ClusterRoleBinding) { + t.Helper() + data, err := json.Marshal(bindings) + if err != nil { + t.Fatalf("marshal managed binding state: %v", err) + } + if err := os.WriteFile(path, append(data, '\n'), 0o600); err != nil { + t.Fatalf("write managed binding state: %v", err) + } +} + +func readManagedState(t *testing.T, path string) []rbacv1.ClusterRoleBinding { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read managed binding state: %v", err) + } + var bindings []rbacv1.ClusterRoleBinding + if err := json.Unmarshal(data, &bindings); err != nil { + t.Fatalf("decode managed binding state: %v", err) + } + return bindings +} + +func findManagedBinding(t *testing.T, bindings []rbacv1.ClusterRoleBinding, name string) rbacv1.ClusterRoleBinding { + t.Helper() + for _, binding := range bindings { + if binding.Name == name { + return binding + } + } + t.Fatalf("managed binding %q not found", name) + return rbacv1.ClusterRoleBinding{} +} + +func hasSubject(subjects []rbacv1.Subject, name string) bool { + for _, subject := range subjects { + if subject.APIGroup == rbacv1.GroupName && subject.Kind == "Group" && subject.Name == name { + return true + } + } + return false +} + +func readFile(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + return string(data) +} + +func readRBACManifest(t *testing.T, path string) []rbacv1.ClusterRoleBinding { + t.Helper() + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read captured RBAC manifest: %v", err) + } + decoder := yaml.NewYAMLOrJSONDecoder(bytes.NewReader(data), 4096) + var bindings []rbacv1.ClusterRoleBinding + for { + var binding rbacv1.ClusterRoleBinding + err := decoder.Decode(&binding) + if errors.Is(err, io.EOF) { + break + } + if err != nil { + t.Fatalf("decode captured RBAC manifest: %v", err) + } + if binding.APIVersion == "" && binding.Kind == "" && binding.Name == "" { + continue + } + bindings = append(bindings, binding) + } + return bindings +} + +func bindingNames(bindings []rbacv1.ClusterRoleBinding) []string { + names := make([]string, 0, len(bindings)) + for _, binding := range bindings { + names = append(names, binding.Name) + } + return names +} + +func readCommandCalls(t *testing.T, path string) []commandCall { + t.Helper() + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read fake command log: %v", err) + } + lines := strings.Split(strings.TrimSpace(string(data)), "\n") + calls := make([]commandCall, 0, len(lines)) + for _, line := range lines { + if line == "" { + continue + } + fields := strings.Split(line, "\t") + calls = append(calls, commandCall{name: fields[0], args: fields[1:]}) + } + return calls +} + +func kubectlOperationIndexes(calls []commandCall) (apply []int, delete []int) { + for i, call := range calls { + if call.name == "http-delete" { + delete = append(delete, i) + continue + } + if call.name != "kubectl" || len(call.args) == 0 { + continue + } + switch call.args[0] { + case "apply", "create", "replace": + apply = append(apply, i) + case "auth": + if len(call.args) > 1 && call.args[1] == "reconcile" { + apply = append(apply, i) + } + case "delete": + delete = append(delete, i) + } + } + return apply, delete +} + +func kubectlIndexes(calls []commandCall, operation string) []int { + var indexes []int + for i, call := range calls { + if call.name == "kubectl" && len(call.args) > 0 && call.args[0] == operation { + indexes = append(indexes, i) + } + } + return indexes +} + +func assertLegacyDeleteCall(t *testing.T, call commandCall) { + t.Helper() + + if call.name != "http-delete" || len(call.args) != 1 { + t.Fatalf("migration call = %#v, want conditional Kubernetes HTTP DELETE", call) + } + wantPath := "/apis/rbac.authorization.k8s.io/v1/clusterrolebindings/" + legacyBindingName + if call.args[0] != wantPath { + t.Errorf("migration delete path = %q, want %q", call.args[0], wantPath) + } +} + +func slicesContain(args []string, want string) bool { + for _, arg := range args { + if arg == want { + return true + } + } + return false +} + +func assertLegacyDeletePreconditions(t *testing.T, path, wantUID, wantResourceVersion string) { + t.Helper() + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read delete options: %v", err) + } + var options struct { + APIVersion string `json:"apiVersion"` + Kind string `json:"kind"` + Preconditions struct { + UID string `json:"uid"` + ResourceVersion string `json:"resourceVersion"` + } `json:"preconditions"` + } + if err := json.Unmarshal(data, &options); err != nil { + t.Fatalf("decode delete options: %v", err) + } + if options.APIVersion != "v1" || options.Kind != "DeleteOptions" || + options.Preconditions.UID != wantUID || options.Preconditions.ResourceVersion != wantResourceVersion { + t.Fatalf("delete options = %#v, want UID %q and resourceVersion %q", options, wantUID, wantResourceVersion) + } +} + +func assertSafeManagedRBACMutation(t *testing.T, call commandCall) { + t.Helper() + if call.name != "kubectl" || len(call.args) != 3 || (call.args[0] != "create" && call.args[0] != "replace") { + t.Fatalf("RBAC mutation = %#v, want kubectl create/replace -f -", call) + } + if !slicesContain(call.args, "-f") || !slicesContain(call.args, "-") || slicesContain(call.args, "--force") { + t.Errorf("RBAC mutation must be non-destructive and read stdin: %q", call.args) + } +} + +func formatCalls(calls []commandCall) string { + formatted := make([]string, 0, len(calls)) + for _, call := range calls { + formatted = append(formatted, strings.Join(append([]string{call.name}, call.args...), " ")) + } + return strings.Join(formatted, "; ") +} + +func writeExecutable(t *testing.T, path, contents string) { + t.Helper() + if err := os.WriteFile(path, []byte(contents), 0o700); err != nil { + t.Fatalf("write fake executable %s: %v", path, err) + } +} + +const fakeAZScript = `#!/bin/sh +set -eu +{ + printf 'az' + for arg in "$@"; do + printf '\t%s' "$arg" + done + printf '\n' +} >> "${AKS_FLEX_CONFIG_TEST_COMMAND_LOG:?}" + +case " $* " in +*" account show "*" --query id "*) printf 'test-subscription\n' ;; +*" account show "*" --query tenantId "*) printf 'test-tenant\n' ;; +*" aks show "*" --query id "*) printf '/subscriptions/test/resourceGroups/test-rg/providers/Microsoft.ContainerService/managedClusters/test-cluster\n' ;; +*" aks show "*" --query location "*) printf 'test-region\n' ;; +*" currentKubernetesVersion "*) printf '1.35.0\n' ;; +*" networkProfile.dnsServiceIp "*) printf '10.0.0.10\n' ;; +esac +` + +const fakeKubectlScript = `#!/bin/sh +set -eu +{ + printf 'kubectl' + for arg in "$@"; do + printf '\t%s' "$arg" + done + printf '\n' +} >> "${AKS_FLEX_CONFIG_TEST_COMMAND_LOG:?}" + +case "${1:-}" in +proxy) + socket_path="" + for arg in "$@"; do + case "$arg" in + --unix-socket=*) socket_path="${arg#--unix-socket=}" ;; + --port=*|--address=*) exit 52 ;; + esac + done + if [ -z "$socket_path" ]; then + exit 50 + fi + if [ "${AKS_FLEX_CONFIG_TEST_PROXY_STARTUP:-}" = "exit-before-ready" ]; then + printf '%s\n' 'injected proxy startup failure' >&2 + exit 51 + fi + exec python3 -u - "$socket_path" "${AKS_FLEX_CONFIG_TEST_COMMAND_LOG:?}" "${AKS_FLEX_CONFIG_TEST_DELETE_OPTIONS:?}" "${AKS_FLEX_CONFIG_TEST_LEGACY_STATE:?}" "${AKS_FLEX_CONFIG_TEST_DELETE_EXIT:-0}" "${AKS_FLEX_CONFIG_TEST_DELETE_KEEPS_STATE:-false}" "${AKS_FLEX_CONFIG_TEST_CONCURRENT_REPLACE:-false}" "${AKS_FLEX_CONFIG_TEST_PROXY_STARTUP:-}" <<'PY' +import http.server +import json +import os +import socketserver +import stat +import sys +import time + +socket_path, log_path, options_path, state_path, delete_exit, keep_state, concurrent, startup_mode = sys.argv[1:] + +class Handler(http.server.BaseHTTPRequestHandler): + def do_DELETE(self): + socket_mode = stat.S_IMODE(os.stat(socket_path, follow_symlinks=False).st_mode) + directory_mode = stat.S_IMODE(os.stat(os.path.dirname(socket_path)).st_mode) + if socket_mode != 0o600 or directory_mode != 0o700: + self.respond(500, {"message": f"insecure socket modes {socket_mode:o}/{directory_mode:o}"}) + return + length = int(self.headers.get("Content-Length", "0")) + body = self.rfile.read(length) + with open(log_path, "a", encoding="utf-8") as stream: + stream.write("http-delete\t" + self.path + "\n") + with open(options_path, "wb") as stream: + stream.write(body) + + if int(delete_exit) != 0: + self.respond(500, {"message": "injected delete failure"}) + return + if concurrent == "true": + with open(state_path, "w", encoding="utf-8") as stream: + stream.write("customized\n") + self.respond(409, {"message": "object changed"}) + return + if self.path != "/apis/rbac.authorization.k8s.io/v1/clusterrolebindings/aks-flex-node-role": + self.respond(404, {"message": "unexpected resource"}) + return + + try: + options = json.loads(body) + except json.JSONDecodeError: + self.respond(400, {"message": "missing DeleteOptions"}) + return + preconditions = options.get("preconditions", {}) + with open(state_path, encoding="utf-8") as stream: + state = stream.read().strip() + if state != "present": + self.respond(404, {"message": "not found"}) + return + if preconditions != {"uid": "legacy-uid", "resourceVersion": "7"}: + self.respond(409, {"message": "precondition failed"}) + return + if keep_state != "true": + with open(state_path, "w", encoding="utf-8") as stream: + stream.write("absent\n") + self.respond(200, {"status": "Success"}) + + def respond(self, status, payload): + encoded = json.dumps(payload).encode() + self.send_response(status) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(encoded))) + self.end_headers() + self.wfile.write(encoded) + + def log_message(self, *_): + pass + +class UnixHTTPServer(socketserver.ThreadingMixIn, socketserver.UnixStreamServer): + daemon_threads = True + +os.umask(0o077) +server = UnixHTTPServer(socket_path, Handler) +sys.stderr.write("fake proxy diagnostic before readiness\n") +sys.stderr.flush() +readiness = f"Starting to serve on {socket_path}\n" +if startup_mode == "fragmented-ready": + readiness_prefix = "Starting to " + for fragment in ( + "fake stdout warning before readiness\n" + readiness_prefix, + readiness[len(readiness_prefix):-1], + "\n", + ): + sys.stdout.write(fragment) + sys.stdout.flush() + time.sleep(0.01) +else: + # Keep both lines in one write to catch implementations that mix an OS + # selector with a buffered text reader and strand the readiness line. + sys.stdout.write("fake stdout warning before readiness\n" + readiness) + sys.stdout.flush() +server.serve_forever() +PY + ;; +create|replace) + apply_count=0 + if [ -f "${AKS_FLEX_CONFIG_TEST_APPLY_COUNT:?}" ]; then + apply_count=$(cat "$AKS_FLEX_CONFIG_TEST_APPLY_COUNT") + fi + apply_count=$((apply_count + 1)) + printf '%s\n' "$apply_count" > "$AKS_FLEX_CONFIG_TEST_APPLY_COUNT" + if [ "${AKS_FLEX_CONFIG_TEST_APPLY_FAIL_AT:-0}" -eq "$apply_count" ]; then + exit 38 + fi + + input="${AKS_FLEX_CONFIG_TEST_MANIFEST:?}.input.$$" + cat > "$input" + if [ -s "${AKS_FLEX_CONFIG_TEST_MANIFEST}" ]; then + printf '%s\n' '---' >> "${AKS_FLEX_CONFIG_TEST_MANIFEST}" + fi + cat "$input" >> "${AKS_FLEX_CONFIG_TEST_MANIFEST}" + printf '\n' >> "${AKS_FLEX_CONFIG_TEST_MANIFEST}" + + status=0 + python3 - "$1" "$input" "${AKS_FLEX_CONFIG_TEST_MANAGED_STATE:?}" "${AKS_FLEX_CONFIG_TEST_CONCURRENT_MANAGED_REPLACE:-}" "${AKS_FLEX_CONFIG_TEST_SKIP_MANAGED_MUTATION:-false}" "${AKS_FLEX_CONFIG_TEST_MANAGED_POSTCONDITION:-}" <<'PY' || status=$? +import json +import sys + +operation, input_path, state_path, concurrent_name, skip_mutation, postcondition = sys.argv[1:] +with open(input_path, encoding="utf-8") as stream: + incoming = json.load(stream) +with open(state_path, encoding="utf-8") as stream: + items = json.load(stream) + +name = incoming.get("metadata", {}).get("name") +matches = [index for index, item in enumerate(items) if item.get("metadata", {}).get("name") == name] +if operation == "create": + if matches: + raise SystemExit(48) + if concurrent_name == name: + replacement = { + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRoleBinding", + "metadata": { + "name": name, + "resourceVersion": "concurrent", + "uid": "operator-replacement", + "labels": {"owner": "operator"}, + }, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "view", + }, + "subjects": [ + { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "Group", + "name": "operator-group", + } + ], + } + items.append(replacement) + with open(state_path, "w", encoding="utf-8") as stream: + json.dump(items, stream) + raise SystemExit(48) + incoming.setdefault("metadata", {})["resourceVersion"] = "1" + incoming["metadata"]["uid"] = f"{name}-uid" + if skip_mutation != "true": + items.append(incoming) +elif operation == "replace": + if len(matches) != 1: + raise SystemExit(49) + index = matches[0] + current = items[index] + if concurrent_name == name: + replacement = { + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRoleBinding", + "metadata": { + "name": name, + "resourceVersion": "concurrent", + "uid": "operator-replacement", + "labels": {"owner": "operator"}, + }, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "view", + }, + "subjects": [ + { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "Group", + "name": "operator-group", + } + ], + } + items[index] = replacement + with open(state_path, "w", encoding="utf-8") as stream: + json.dump(items, stream) + raise SystemExit(41) + if incoming.get("metadata", {}).get("resourceVersion") != current.get("metadata", {}).get("resourceVersion"): + raise SystemExit(41) + incoming["metadata"]["resourceVersion"] = str(int(current["metadata"]["resourceVersion"]) + 1) + if skip_mutation != "true": + items[index] = incoming +else: + raise SystemExit(47) + +if skip_mutation != "true" and postcondition: + postcondition_name, separator, mutation = postcondition.partition(":") + if not separator: + raise SystemExit(50) + if postcondition_name == name: + target = next(item for item in items if item.get("metadata", {}).get("name") == name) + if mutation == "wrong-role-ref": + target["roleRef"]["name"] = "view" + elif mutation == "missing-subject": + target["subjects"] = [ + subject for subject in target.get("subjects", []) + if subject.get("name") != "system:bootstrappers:aks-flex-node" + ] + else: + raise SystemExit(51) + +with open(state_path, "w", encoding="utf-8") as stream: + json.dump(items, stream) +PY + rm -f "$input" + exit "$status" + ;; +apply) + apply_count=0 + if [ -f "${AKS_FLEX_CONFIG_TEST_APPLY_COUNT:?}" ]; then + apply_count=$(cat "$AKS_FLEX_CONFIG_TEST_APPLY_COUNT") + fi + apply_count=$((apply_count + 1)) + printf '%s\n' "$apply_count" > "$AKS_FLEX_CONFIG_TEST_APPLY_COUNT" + if [ "${AKS_FLEX_CONFIG_TEST_APPLY_FAIL_AT:-0}" -eq "$apply_count" ]; then + exit 38 + fi + cat > "${AKS_FLEX_CONFIG_TEST_MANIFEST:?}" + ;; +get) + get_exit="${AKS_FLEX_CONFIG_TEST_GET_EXIT:-0}" + if [ "$get_exit" -ne 0 ]; then + exit "$get_exit" + fi + + case " $* " in + *" clusterrolebindings "*) ;; + *) exit 46 ;; + esac + + python3 - "${AKS_FLEX_CONFIG_TEST_MANAGED_STATE:?}" "${AKS_FLEX_CONFIG_TEST_LEGACY_STATE:?}" <<'PY' +import json +import sys + +with open(sys.argv[1], encoding="utf-8") as stream: + items = json.load(stream) +with open(sys.argv[2], encoding="utf-8") as stream: + state = stream.read().strip() + +subject = { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "Group", + "name": "system:bootstrappers:aks-flex-node", +} +if state == "present": + items.append({ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRoleBinding", + "metadata": {"name": "aks-flex-node-role", "uid": "legacy-uid", "resourceVersion": "7"}, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "system:node", + }, + "subjects": [subject], + }) +elif state == "customized": + items.append({ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRoleBinding", + "metadata": {"name": "aks-flex-node-role", "uid": "replacement-uid", "resourceVersion": "8"}, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "system:node", + }, + "subjects": [subject, { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "Group", + "name": "another-group", + }], + }) +elif state == "renamed": + items.append({ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRoleBinding", + "metadata": {"name": "custom-bootstrap-node-role", "uid": "renamed-uid", "resourceVersion": "9"}, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "system:node", + }, + "subjects": [subject], + }) +elif state == "safe-customized": + items.append({ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRoleBinding", + "metadata": {"name": "aks-flex-node-role"}, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "view", + }, + "subjects": [{ + "apiGroup": "rbac.authorization.k8s.io", + "kind": "Group", + "name": "readers", + }], + }) + +print(json.dumps({"items": items})) +PY + ;; +config) + case " $* " in + *"certificate-authority-data"*) printf 'dGVzdC1jYQ==\n' ;; + *"cluster.server"*) printf 'https://test-cluster.example.test:443\n' ;; + *) exit 45 ;; + esac + ;; +*) + exit 42 + ;; +esac +` From 3a9bdfbb2b1631d7d6f9ec20c2fd76ee033571b0 Mon Sep 17 00:00:00 2001 From: Wen Huang <50309350+wenhug@users.noreply.github.com> Date: Wed, 26 Aug 2026 18:12:35 +0000 Subject: [PATCH 2/3] fix: harden legacy RBAC migration guard --- docs/usages/aks-flex-config.md | 165 ++++++++++++++++++++++++++++---- scripts/aks-flex-config | 76 +++++++++++---- scripts/aks_flex_config_test.go | 127 ++++++++++++++++++++++-- 3 files changed, 320 insertions(+), 48 deletions(-) diff --git a/docs/usages/aks-flex-config.md b/docs/usages/aks-flex-config.md index f61b70e0..d1f406c3 100644 --- a/docs/usages/aks-flex-config.md +++ b/docs/usages/aks-flex-config.md @@ -48,17 +48,118 @@ Run this once per cluster for bootstrap-token joins: This applies only the CSR creation and approval `ClusterRoleBinding` objects for the `system:bootstrappers:aks-flex-node` group. If any binding still grants that group the obsolete `system:node` role, the command stops after applying the safe bindings and explains how to migrate. It does not silently remove the binding because older and development-mode agents may still use their bootstrap token after joining. -`v0.1.1` introduced a separate daemon client certificate, but the version alone does not prove that certificate was issued successfully. Upgrade every bootstrap-token agent to `v0.1.1` or later (preferably the latest release), and on every host verify that the certificate exists, is unexpired, and the agent remains healthy after a restart: +`v0.1.1` introduced a separate daemon client certificate. Before removing the +legacy binding, upgrade every bootstrap-token agent to `v0.1.1` or later +(preferably the latest release). Then run the following on every host. Set +`EXPECTED_VERSION` to the exact release you deployed; the check fails if the +live process is not that binary, restarts during the stability window, has the +wrong certificate identity, or cannot read its exact Node with that certificate. +The host needs `curl`, `jq`, and `openssl`. + +Run this as root so the protected config and private key never need broader +permissions. The command extracts only the cluster CA, not the bootstrap token, +into a root-only temporary directory. It removes the file immediately after the +probe, and also on any earlier exit: ```bash -sudo test -s /etc/aks-flex-node/daemon-credentials/daemon-controller-current.pem -sudo openssl x509 \ - -in /etc/aks-flex-node/daemon-credentials/daemon-controller-current.pem \ - -noout -subject -enddate -checkend 0 -sudo systemctl restart aks-flex-node-agent.service -sudo systemctl is-active aks-flex-node-agent.service +sudo bash <<'EOF' +set -eu +umask 077 + +EXPECTED_VERSION="v0.1.1" # Change to the exact v0.1.1-or-later release deployed. +SERVICE="aks-flex-node-agent.service" +CONFIG="/etc/aks-flex-node/config.json" +CERT="/etc/aks-flex-node/daemon-credentials/daemon-controller-current.pem" +CURRENT_LINK="/usr/local/lib/aks-flex-node/aks-flex-node-current" +DIRECT_BINARY="/usr/local/bin/aks-flex-node" + +systemctl restart "$SERVICE" +systemctl is-active --quiet "$SERVICE" + +PID_BEFORE="$(systemctl show --property MainPID --value "$SERVICE")" +test "$PID_BEFORE" -gt 0 +LIVE_EXE="$(readlink -f "/proc/$PID_BEFORE/exe")" +if [ -e "$CURRENT_LINK" ]; then + INSTALLED_EXE="$(readlink -f "$CURRENT_LINK")" +else + INSTALLED_EXE="$(readlink -f "$DIRECT_BINARY")" +fi +test "$LIVE_EXE" = "$INSTALLED_EXE" + +LIVE_VERSION="$("/proc/$PID_BEFORE/exe" version | awk -F ': ' '$1 == "Version" {print $2; exit}')" +printf 'live binary: %s\nlive version: %s\n' "$LIVE_EXE" "$LIVE_VERSION" +test "$LIVE_VERSION" = "$EXPECTED_VERSION" + +NODE_NAME="$(jq -er '(.agent.nodeName // "") | gsub("^\\s+|\\s+$"; "")' "$CONFIG")" +if [ -z "$NODE_NAME" ]; then + NODE_NAME="$(hostname | tr '[:upper:]' '[:lower:]')" +fi + +test -s "$CERT" +openssl x509 -in "$CERT" -noout -enddate -checkend 0 +SUBJECT="$(openssl x509 -in "$CERT" -noout -subject -nameopt RFC2253)" +SUBJECT="${SUBJECT#subject=}" +# RFC2253 uses ',' between RDNs and '+' inside Go's multi-valued O RDN. +ACTUAL_ATTRIBUTES="$(printf '%s\n' "$SUBJECT" | tr ',+' '\n' | LC_ALL=C sort)" +EXPECTED_ATTRIBUTES="$(printf '%s\n' \ + "CN=system:node:$NODE_NAME" \ + 'O=system:nodes' \ + 'O=aks-flex-node-daemons' | LC_ALL=C sort)" +printf 'daemon certificate subject: %s\n' "$SUBJECT" +test "$ACTUAL_ATTRIBUTES" = "$EXPECTED_ATTRIBUTES" + +API_SERVER="$(jq -er ' + (.node.kubelet.clusterFQDN // .node.kubelet.serverURL) + | strings + | gsub("^\\s+|\\s+$"; "") + | select(length > 0) +' "$CONFIG")" +case "$API_SERVER" in + https://*) ;; + *://*) printf 'unsupported API server URL: %s\n' "$API_SERVER" >&2; exit 1 ;; + *:*) API_SERVER="https://$API_SERVER" ;; + *) API_SERVER="https://$API_SERVER:443" ;; +esac + +CHECK_DIR="$(mktemp -d /run/aks-flex-node-rbac-check.XXXXXX)" +CA_FILE="$CHECK_DIR/cluster-ca.pem" +cleanup() { + rm -f -- "$CA_FILE" + rmdir -- "$CHECK_DIR" +} +trap cleanup EXIT +jq -er '.node.kubelet.caCertData | strings | select(length > 0)' "$CONFIG" \ + | base64 --decode >"$CA_FILE" +chmod 0600 "$CA_FILE" +test -s "$CA_FILE" + +HTTP_CODE="$(curl --silent --show-error --output /dev/null --write-out '%{http_code}' \ + --connect-timeout 10 --max-time 30 \ + --cacert "$CA_FILE" --cert "$CERT" --key "$CERT" \ + "${API_SERVER%/}/api/v1/nodes/$NODE_NAME")" +if [ "$HTTP_CODE" != "200" ]; then + printf 'daemon certificate Node GET returned HTTP %s, expected 200\n' "$HTTP_CODE" >&2 + exit 1 +fi +cleanup +trap - EXIT + +sleep 30 +systemctl is-active --quiet "$SERVICE" +PID_AFTER="$(systemctl show --property MainPID --value "$SERVICE")" +test "$PID_AFTER" = "$PID_BEFORE" +test "$(readlink -f "/proc/$PID_AFTER/exe")" = "$LIVE_EXE" +printf 'verified stable certificate-backed access for Node %s\n' "$NODE_NAME" +EOF ``` +An HTTP `200` proves baseline certificate authentication and authorization to +read that daemon's own Node. It does not prove feature-specific authorization +for `MachineOperation` resources or an in-cluster machine client's Kubernetes +service-proxy endpoint. If those features are enabled, verify their +`aks-flex-node-daemons` group RBAC and exercise those paths separately before +migration. + Then explicitly remove the obsolete binding: ```bash @@ -69,26 +170,50 @@ Then explicitly remove the obsolete binding: --remove-legacy-node-role-binding ``` -This migration is idempotent. It automatically deletes only the canonical `aks-flex-node-role` object created by older helpers. If another binding grants the same unsafe edge, or that object has extra subjects, the helper refuses to guess and identifies the objects for manual review. Bootstrap-token config generation refuses to create a token while any such binding exists, rather than either issuing an over-privileged token or unexpectedly breaking an old daemon. +This migration is idempotent. It automatically deletes only the plain, +canonical `ClusterRoleBinding/aks-flex-node-role` created by older helpers. It +refuses automatic deletion if that object has extra subjects, ownership or +lifecycle metadata, or custom labels or annotations. Any other unsafe +`ClusterRoleBinding` is reported for manual review. A namespaced `RoleBinding` +can also reference the `system:node` ClusterRole; the helper reports these but +never deletes them automatically. Inspect its owners and other subjects, then +remove only the unsafe bootstrap-group edge through the owning deployment or a +careful manual edit. Bootstrap-token config generation refuses to create a +token while any direct unsafe binding remains. To verify no binding still grants the bootstrap group `system:node`, run: ```bash -kubectl get clusterrolebinding -o json | jq -r ' +kubectl get clusterrolebindings,rolebindings --all-namespaces -o json | jq -r ' .items[] - | select(.roleRef.kind == "ClusterRole" and .roleRef.name == "system:node") - | .metadata.name as $binding - | .subjects[]? - | select(.kind == "Group" and .name == "system:bootstrappers:aks-flex-node") - | $binding' + | select( + .roleRef.apiGroup == "rbac.authorization.k8s.io" + and .roleRef.kind == "ClusterRole" + and .roleRef.name == "system:node" + ) + | select([ + .subjects[]? + | select( + .apiGroup == "rbac.authorization.k8s.io" + and .kind == "Group" + and .name == "system:bootstrappers:aks-flex-node" + ) + ] | length > 0) + | if .kind == "RoleBinding" then + "RoleBinding/\(.metadata.namespace)/\(.metadata.name)" + else + "ClusterRoleBinding/\(.metadata.name)" + end' ``` -The expected result is no output. The canonical `aks-flex-node-role` object is -deleted; a safe, repurposed object with that name is preserved. Once certificate -issuance has been verified, both the kubelet and long-running Flex daemon use -issued client certificates, so removing the unsafe binding does not interrupt -joined nodes. New and in-progress joins retain the CSR permissions installed -above. +The expected result is no output. This is intentionally a narrow audit for a +direct bootstrap-group-to-`system:node` binding. It is not a full effective +authorization review and does not analyze aggregated or custom ClusterRoles or +other indirect authorization paths. The canonical `aks-flex-node-role` object +is deleted; a safe, repurposed object with that name is preserved. Once the +checks above pass, both the kubelet and long-running Flex daemon use issued +client certificates, so removing the unsafe binding does not interrupt joined +nodes. New and in-progress joins retain the CSR permissions installed above. Do not roll back a migrated host to an older or development-mode agent that still uses the bootstrap token for ordinary Kubernetes API requests. After this binding is removed, those requests correctly receive `403 Forbidden`. Restore a supported certificate-using agent instead of restoring the broad binding. diff --git a/scripts/aks-flex-config b/scripts/aks-flex-config index 85dbdb4f..5cfcaf0f 100755 --- a/scripts/aks-flex-config +++ b/scripts/aks-flex-config @@ -28,6 +28,7 @@ RBAC_API_GROUP = "rbac.authorization.k8s.io" LEGACY_NODE_ROLE_BINDING = "aks-flex-node-role" FLEX_NODE_BOOTSTRAP_GROUP = "system:bootstrappers:aks-flex-node" RBAC_AUTOUPDATE_ANNOTATION = "rbac.authorization.kubernetes.io/autoupdate" +KUBECTL_LAST_APPLIED_ANNOTATION = "kubectl.kubernetes.io/last-applied-configuration" MANAGED_BOOTSTRAP_BINDINGS = ( ("aks-flex-node-bootstrapper", "system:node-bootstrapper"), ( @@ -91,20 +92,26 @@ def setup_node_rbac(args: argparse.Namespace) -> None: require_legacy_node_role_binding_absent(bindings) -def cluster_role_bindings() -> list[dict[str, object]]: - raw = run(["kubectl", "get", "clusterrolebindings", "-o", "json"], capture=True) +def rbac_bindings() -> list[dict[str, object]]: + # A RoleBinding may reference the cluster-scoped system:node role and grant + # its namespaced permissions. Inventory both scopes so the migration cannot + # report success while that equivalent unsafe edge remains in a namespace. + raw = run( + ["kubectl", "get", "clusterrolebindings,rolebindings", "--all-namespaces", "-o", "json"], + capture=True, + ) try: payload = json.loads(raw) except json.JSONDecodeError as err: - raise SystemExit(f"ERROR: could not parse ClusterRoleBinding inventory: {err}") from err + raise SystemExit(f"ERROR: could not parse RBAC binding inventory: {err}") from err if not isinstance(payload, dict): - raise SystemExit("ERROR: ClusterRoleBinding inventory is not a JSON object") + raise SystemExit("ERROR: RBAC binding inventory is not a JSON object") items = payload.get("items") if not isinstance(items, list): - raise SystemExit("ERROR: ClusterRoleBinding inventory does not contain an items list") + raise SystemExit("ERROR: RBAC binding inventory does not contain an items list") if not all(isinstance(item, dict) for item in items): - raise SystemExit("ERROR: ClusterRoleBinding inventory contains a malformed item") + raise SystemExit("ERROR: RBAC binding inventory contains a malformed item") return items @@ -142,6 +149,8 @@ def managed_binding_inventory(bindings: list[dict[str, object]]) -> dict[str, di expected_names = {name for name, _ in MANAGED_BOOTSTRAP_BINDINGS} managed: dict[str, dict[str, object]] = {} for binding in bindings: + if binding.get("kind") != "ClusterRoleBinding": + continue metadata = binding.get("metadata") name = metadata.get("name") if isinstance(metadata, dict) else None if name not in expected_names: @@ -188,7 +197,7 @@ def desired_managed_binding(name: str, role_name: str) -> dict[str, object]: def reconcile_bootstrap_rbac_bindings() -> list[dict[str, object]]: - before = cluster_role_bindings() + before = rbac_bindings() managed = managed_binding_inventory(before) actions: list[tuple[str, str, dict[str, object]]] = [] @@ -235,7 +244,7 @@ def reconcile_bootstrap_rbac_bindings() -> list[dict[str, object]]: log_info(f"{operation} managed bootstrap RBAC binding {name}") run(["kubectl", operation, "-f", "-"], input_text=json.dumps(binding), capture=True) - after = cluster_role_bindings() + after = rbac_bindings() reconciled = managed_binding_inventory(after) for name, role_name in MANAGED_BOOTSTRAP_BINDINGS: binding = reconciled.get(name) @@ -247,7 +256,7 @@ def reconcile_bootstrap_rbac_bindings() -> list[dict[str, object]]: def unsafe_node_role_bindings(bindings: list[dict[str, object]] | None = None) -> list[dict[str, object]]: if bindings is None: - bindings = cluster_role_bindings() + bindings = rbac_bindings() unsafe = [] for item in bindings: @@ -258,14 +267,14 @@ def unsafe_node_role_bindings(bindings: list[dict[str, object]] | None = None) - if not isinstance(role_ref, dict) or not isinstance(subjects, list): continue if ( - role_ref.get("apiGroup") != "rbac.authorization.k8s.io" + role_ref.get("apiGroup") != RBAC_API_GROUP or role_ref.get("kind") != "ClusterRole" or role_ref.get("name") != "system:node" ): continue if any( isinstance(subject, dict) - and subject.get("apiGroup") == "rbac.authorization.k8s.io" + and subject.get("apiGroup") == RBAC_API_GROUP and subject.get("kind") == "Group" and subject.get("name") == FLEX_NODE_BOOTSTRAP_GROUP for subject in subjects @@ -277,20 +286,47 @@ def unsafe_node_role_bindings(bindings: list[dict[str, object]] | None = None) - def unsafe_binding_names(bindings: list[dict[str, object]]) -> str: names = [] for binding in bindings: + kind = binding.get("kind") metadata = binding.get("metadata") name = metadata.get("name") if isinstance(metadata, dict) else None - names.append(name if isinstance(name, str) and name else "") + name = name if isinstance(name, str) and name else "" + if kind == "RoleBinding": + namespace = metadata.get("namespace") if isinstance(metadata, dict) else None + namespace = namespace if isinstance(namespace, str) and namespace else "" + names.append(f"RoleBinding/{namespace}/{name}") + elif kind == "ClusterRoleBinding": + names.append(f"ClusterRoleBinding/{name}") + else: + names.append(f"/{name}") return ", ".join(sorted(names)) def is_canonical_legacy_node_role_binding(binding: dict[str, object]) -> bool: metadata = binding.get("metadata") subjects = binding.get("subjects") - if not isinstance(metadata, dict) or metadata.get("name") != LEGACY_NODE_ROLE_BINDING: + if ( + binding.get("apiVersion") != f"{RBAC_API_GROUP}/v1" + or binding.get("kind") != "ClusterRoleBinding" + or binding.get("roleRef") != expected_role_ref("system:node") + or not isinstance(metadata, dict) + or metadata.get("name") != LEGACY_NODE_ROLE_BINDING + or metadata.get("namespace") + ): + return False + if any(metadata.get(field) for field in ("ownerReferences", "finalizers", "deletionTimestamp")): + return False + labels = metadata.get("labels") + if labels not in (None, {}): + return False + annotations = metadata.get("annotations") + if annotations is not None and ( + not isinstance(annotations, dict) + or any(key != KUBECTL_LAST_APPLIED_ANNOTATION for key in annotations) + ): return False return subjects == [ { - "apiGroup": "rbac.authorization.k8s.io", + "apiGroup": RBAC_API_GROUP, "kind": "Group", "name": FLEX_NODE_BOOTSTRAP_GROUP, } @@ -302,12 +338,12 @@ def require_legacy_node_role_binding_absent(bindings: list[dict[str, object]] | if not bindings: return raise SystemExit( - "ERROR: ClusterRoleBinding(s) " + "ERROR: RBAC binding(s) " f"{unsafe_binding_names(bindings)} still grant {FLEX_NODE_BOOTSTRAP_GROUP!r} the " "system:node role. Some older or development agents may still depend on that access. " "First upgrade them to a release with daemon client certificates (v0.1.1 or later), " - "verify /etc/aks-flex-node/daemon-credentials/daemon-controller-current.pem exists " - "and the agent remains healthy after restart, then rerun setup-node-rbac with " + "verify the running agent is stable and uses its issued daemon certificate for API access, " + "then rerun setup-node-rbac with " "--remove-legacy-node-role-binding." ) @@ -469,8 +505,8 @@ def remove_legacy_node_role_binding(bindings: list[dict[str, object]] | None = N return if len(bindings) != 1 or not is_canonical_legacy_node_role_binding(bindings[0]): raise SystemExit( - "ERROR: refusing automatic removal because the unsafe ClusterRoleBinding set is " - f"not the canonical {LEGACY_NODE_ROLE_BINDING!r} object: " + "ERROR: refusing automatic removal because the unsafe RBAC binding set is not the " + f"canonical ClusterRoleBinding {LEGACY_NODE_ROLE_BINDING!r}: " f"{unsafe_binding_names(bindings)}. Review and remove only the bootstrap-group " "subjects manually." ) @@ -698,7 +734,7 @@ def build_parser() -> argparse.ArgumentParser: action="store_true", help=( "Remove the canonical obsolete aks-flex-node-role binding after existing agents " - "have issued daemon certificates and remain healthy after restart." + "have issued daemon certificates and verified stable certificate-backed API access." ), ) rbac.set_defaults(func=setup_node_rbac) diff --git a/scripts/aks_flex_config_test.go b/scripts/aks_flex_config_test.go index c0226ee2..9fe3714e 100644 --- a/scripts/aks_flex_config_test.go +++ b/scripts/aks_flex_config_test.go @@ -531,7 +531,7 @@ func TestSetupNodeRBACPreservesLegacyBindingWithoutExplicitMigration(t *testing. } if !strings.Contains(output, "--remove-legacy-node-role-binding") || !strings.Contains(output, "v0.1.1") || - !strings.Contains(output, "/etc/aks-flex-node/daemon-credentials/daemon-controller-current.pem") { + !strings.Contains(output, "issued daemon certificate for API access") { t.Fatalf("setup-node-rbac did not explain the compatible migration path:\n%s", output) } @@ -738,6 +738,12 @@ func TestSetupNodeRBACRefusesAmbiguousUnsafeBindings(t *testing.T) { }{ {name: "canonical name with additional subject", state: "customized"}, {name: "unexpected binding name", state: "renamed"}, + {name: "namespaced role binding", state: "namespaced"}, + {name: "owner-managed canonical binding", state: "owned"}, + {name: "finalized canonical binding", state: "finalized"}, + {name: "terminating canonical binding", state: "terminating"}, + {name: "labeled canonical binding", state: "labeled"}, + {name: "annotated canonical binding", state: "annotated"}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -764,6 +770,27 @@ func TestSetupNodeRBACRefusesAmbiguousUnsafeBindings(t *testing.T) { } } +func TestSetupNodeRBACIgnoresSameNamedNamespacedBindingDuringReconciliation(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + if err := os.WriteFile(harness.legacyState, []byte("safe-namespaced\n"), 0o600); err != nil { + t.Fatalf("write fake legacy state: %v", err) + } + output, err := harness.runSetupNodeRBAC(false) + if err != nil { + t.Fatalf("setup-node-rbac confused a namespaced RoleBinding with its managed ClusterRoleBinding: %v\n%s", err, output) + } + + bindings := readManagedState(t, harness.managedState) + if len(bindings) != 2 { + t.Fatalf("managed ClusterRoleBinding count = %d, want 2", len(bindings)) + } + if !hasSubject(findManagedBinding(t, bindings, bootstrapBindingName).Subjects, bootstrapGroup) { + t.Fatalf("managed ClusterRoleBinding %q is missing bootstrap group", bootstrapBindingName) + } +} + func TestSetupNodeRBACPreservesSameNamedNonLegacyBinding(t *testing.T) { t.Parallel() @@ -853,6 +880,27 @@ func TestGenerateBootstrapTokenRequiresCompletedLegacyMigration(t *testing.T) { } } +func TestGenerateBootstrapTokenRejectsUnsafeNamespacedRoleBinding(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + if err := os.WriteFile(harness.legacyState, []byte("namespaced\n"), 0o600); err != nil { + t.Fatalf("write fake legacy state: %v", err) + } + output, err := harness.runGenerateNodeConfig() + if err == nil { + t.Fatalf("generate-node-config minted a token while an unsafe RoleBinding remained\n%s", output) + } + if !strings.Contains(output, "RoleBinding/kube-system/legacy-node-access") || + !strings.Contains(output, "--remove-legacy-node-role-binding") { + t.Fatalf("failure did not identify the unsafe namespaced binding:\n%s", output) + } + applyIndexes, deleteIndexes := kubectlOperationIndexes(readCommandCalls(t, harness.commandLogPath)) + if len(applyIndexes) != 0 || len(deleteIndexes) != 0 { + t.Fatal("unsafe namespaced binding check mutated cluster state") + } +} + func TestKubeadmRBACReconciliation(t *testing.T) { t.Parallel() @@ -906,7 +954,7 @@ func TestRepositoryDoesNotBindBootstrapGroupsToLegacyNodeRole(t *testing.T) { } repositoryRoot := filepath.Dir(workingDir) sourceRoots := []string{"cmd", "hack", "pkg", "scripts"} - bindingPattern := regexp.MustCompile(`(?m)^[ \t]*kind:[ \t]*ClusterRoleBinding[ \t]*$`) + bindingPattern := regexp.MustCompile(`(?m)^[ \t]*kind:[ \t]*(?:ClusterRoleBinding|RoleBinding)[ \t]*$`) legacyRolePattern := regexp.MustCompile(`(?m)^[ \t]*name:[ \t]*system:node[ \t]*$`) bootstrapGroupPattern := regexp.MustCompile(`(?m)^[ \t]*name:[ \t]*system:bootstrappers:[^ \t\n]+[ \t]*$`) documentSeparator := regexp.MustCompile(`(?m)^[ \t]*---[ \t]*$`) @@ -1636,10 +1684,12 @@ get) exit "$get_exit" fi - case " $* " in - *" clusterrolebindings "*) ;; - *) exit 46 ;; - esac + if [ "${2:-}" != "clusterrolebindings,rolebindings" ] || + [ "${3:-}" != "--all-namespaces" ] || + [ "${4:-}" != "-o" ] || + [ "${5:-}" != "json" ]; then + exit 46 + fi python3 - "${AKS_FLEX_CONFIG_TEST_MANAGED_STATE:?}" "${AKS_FLEX_CONFIG_TEST_LEGACY_STATE:?}" <<'PY' import json @@ -1655,11 +1705,51 @@ subject = { "kind": "Group", "name": "system:bootstrappers:aks-flex-node", } -if state == "present": +if state in {"present", "owned", "finalized", "terminating", "labeled", "annotated"}: + metadata = { + "name": "aks-flex-node-role", + "uid": "legacy-uid", + "resourceVersion": "7", + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "{}", + }, + } + if state == "owned": + metadata["ownerReferences"] = [{ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRole", + "name": "external-owner", + "uid": "owner-uid", + }] + elif state == "finalized": + metadata["finalizers"] = ["example.test/protect"] + elif state == "terminating": + metadata["deletionTimestamp"] = "2026-08-26T00:00:00Z" + elif state == "labeled": + metadata["labels"] = {"app.kubernetes.io/managed-by": "external-controller"} + elif state == "annotated": + metadata["annotations"]["meta.helm.sh/release-name"] = "external-release" items.append({ "apiVersion": "rbac.authorization.k8s.io/v1", "kind": "ClusterRoleBinding", - "metadata": {"name": "aks-flex-node-role", "uid": "legacy-uid", "resourceVersion": "7"}, + "metadata": metadata, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "system:node", + }, + "subjects": [subject], + }) +elif state == "namespaced": + items.append({ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "RoleBinding", + "metadata": { + "name": "legacy-node-access", + "namespace": "kube-system", + "uid": "namespaced-uid", + "resourceVersion": "11", + }, "roleRef": { "apiGroup": "rbac.authorization.k8s.io", "kind": "ClusterRole", @@ -1667,6 +1757,27 @@ if state == "present": }, "subjects": [subject], }) +elif state == "safe-namespaced": + items.append({ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "RoleBinding", + "metadata": { + "name": "aks-flex-node-bootstrapper", + "namespace": "default", + "uid": "safe-namespaced-uid", + "resourceVersion": "12", + }, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "view", + }, + "subjects": [{ + "apiGroup": "rbac.authorization.k8s.io", + "kind": "Group", + "name": "readers", + }], + }) elif state == "customized": items.append({ "apiVersion": "rbac.authorization.k8s.io/v1", From e7d517d4177e22ff95814b34f90da55ca62ced50 Mon Sep 17 00:00:00 2001 From: Wen Huang <50309350+wenhug@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:40:22 +0000 Subject: [PATCH 3/3] Harden bootstrap token default-group guard --- docs/usages/aks-flex-config.md | 31 +++++++++---- scripts/aks-flex-config | 9 ++-- scripts/aks_flex_config_test.go | 81 +++++++++++++++++++++++++-------- 3 files changed, 88 insertions(+), 33 deletions(-) diff --git a/docs/usages/aks-flex-config.md b/docs/usages/aks-flex-config.md index d1f406c3..8615216c 100644 --- a/docs/usages/aks-flex-config.md +++ b/docs/usages/aks-flex-config.md @@ -46,7 +46,14 @@ Run this once per cluster for bootstrap-token joins: --subscription "$SUBSCRIPTION_ID" ``` -This applies only the CSR creation and approval `ClusterRoleBinding` objects for the `system:bootstrappers:aks-flex-node` group. If any binding still grants that group the obsolete `system:node` role, the command stops after applying the safe bindings and explains how to migrate. It does not silently remove the binding because older and development-mode agents may still use their bootstrap token after joining. +This applies only the CSR creation and approval `ClusterRoleBinding` objects for +the `system:bootstrappers:aks-flex-node` group. Kubernetes automatically places +every bootstrap token in `system:bootstrappers`; the token's +`auth-extra-groups` value adds the Flex-specific group. If any binding still +grants either group the obsolete `system:node` role, the command stops after +applying the safe bindings and explains how to migrate. It does not silently +remove the binding because older and development-mode agents may still use +their bootstrap token after joining. `v0.1.1` introduced a separate daemon client certificate. Before removing the legacy binding, upgrade every bootstrap-token agent to `v0.1.1` or later @@ -177,11 +184,11 @@ lifecycle metadata, or custom labels or annotations. Any other unsafe `ClusterRoleBinding` is reported for manual review. A namespaced `RoleBinding` can also reference the `system:node` ClusterRole; the helper reports these but never deletes them automatically. Inspect its owners and other subjects, then -remove only the unsafe bootstrap-group edge through the owning deployment or a -careful manual edit. Bootstrap-token config generation refuses to create a +remove only the unsafe bootstrap-token-group edge through the owning deployment +or a careful manual edit. Bootstrap-token config generation refuses to create a token while any direct unsafe binding remains. -To verify no binding still grants the bootstrap group `system:node`, run: +To verify neither bootstrap-token group is still bound to `system:node`, run: ```bash kubectl get clusterrolebindings,rolebindings --all-namespaces -o json | jq -r ' @@ -196,7 +203,10 @@ kubectl get clusterrolebindings,rolebindings --all-namespaces -o json | jq -r ' | select( .apiGroup == "rbac.authorization.k8s.io" and .kind == "Group" - and .name == "system:bootstrappers:aks-flex-node" + and ( + .name == "system:bootstrappers" + or .name == "system:bootstrappers:aks-flex-node" + ) ) ] | length > 0) | if .kind == "RoleBinding" then @@ -207,11 +217,12 @@ kubectl get clusterrolebindings,rolebindings --all-namespaces -o json | jq -r ' ``` The expected result is no output. This is intentionally a narrow audit for a -direct bootstrap-group-to-`system:node` binding. It is not a full effective -authorization review and does not analyze aggregated or custom ClusterRoles or -other indirect authorization paths. The canonical `aks-flex-node-role` object -is deleted; a safe, repurposed object with that name is preserved. Once the -checks above pass, both the kubelet and long-running Flex daemon use issued +direct bootstrap-token-group-to-`system:node` binding. It is not a full +effective authorization review and does not analyze aggregated or custom +ClusterRoles or other indirect authorization paths. The canonical +`aks-flex-node-role` object is deleted; a safe, repurposed object with that name +is preserved. Once the checks above pass, both the kubelet and long-running Flex +daemon use issued client certificates, so removing the unsafe binding does not interrupt joined nodes. New and in-progress joins retain the CSR permissions installed above. diff --git a/scripts/aks-flex-config b/scripts/aks-flex-config index 5cfcaf0f..9669531a 100755 --- a/scripts/aks-flex-config +++ b/scripts/aks-flex-config @@ -26,6 +26,7 @@ RESOURCE_MANAGER_ENDPOINT = "https://management.azure.com" DEFAULT_AGENT_POOL_NAME = "aksflexnodes" RBAC_API_GROUP = "rbac.authorization.k8s.io" LEGACY_NODE_ROLE_BINDING = "aks-flex-node-role" +BOOTSTRAP_DEFAULT_GROUP = "system:bootstrappers" FLEX_NODE_BOOTSTRAP_GROUP = "system:bootstrappers:aks-flex-node" RBAC_AUTOUPDATE_ANNOTATION = "rbac.authorization.kubernetes.io/autoupdate" KUBECTL_LAST_APPLIED_ANNOTATION = "kubectl.kubernetes.io/last-applied-configuration" @@ -276,7 +277,7 @@ def unsafe_node_role_bindings(bindings: list[dict[str, object]] | None = None) - isinstance(subject, dict) and subject.get("apiGroup") == RBAC_API_GROUP and subject.get("kind") == "Group" - and subject.get("name") == FLEX_NODE_BOOTSTRAP_GROUP + and subject.get("name") in (BOOTSTRAP_DEFAULT_GROUP, FLEX_NODE_BOOTSTRAP_GROUP) for subject in subjects ): unsafe.append(item) @@ -339,8 +340,8 @@ def require_legacy_node_role_binding_absent(bindings: list[dict[str, object]] | return raise SystemExit( "ERROR: RBAC binding(s) " - f"{unsafe_binding_names(bindings)} still grant {FLEX_NODE_BOOTSTRAP_GROUP!r} the " - "system:node role. Some older or development agents may still depend on that access. " + f"{unsafe_binding_names(bindings)} still grant a bootstrap-token group the system:node " + "role. Some older or development agents may still depend on that access. " "First upgrade them to a release with daemon client certificates (v0.1.1 or later), " "verify the running agent is stable and uses its issued daemon certificate for API access, " "then rerun setup-node-rbac with " @@ -507,7 +508,7 @@ def remove_legacy_node_role_binding(bindings: list[dict[str, object]] | None = N raise SystemExit( "ERROR: refusing automatic removal because the unsafe RBAC binding set is not the " f"canonical ClusterRoleBinding {LEGACY_NODE_ROLE_BINDING!r}: " - f"{unsafe_binding_names(bindings)}. Review and remove only the bootstrap-group " + f"{unsafe_binding_names(bindings)}. Review and remove only the bootstrap-token-group " "subjects manually." ) diff --git a/scripts/aks_flex_config_test.go b/scripts/aks_flex_config_test.go index 9fe3714e..2a4d5df2 100644 --- a/scripts/aks_flex_config_test.go +++ b/scripts/aks_flex_config_test.go @@ -739,6 +739,7 @@ func TestSetupNodeRBACRefusesAmbiguousUnsafeBindings(t *testing.T) { {name: "canonical name with additional subject", state: "customized"}, {name: "unexpected binding name", state: "renamed"}, {name: "namespaced role binding", state: "namespaced"}, + {name: "default bootstrap token group", state: "default-bootstrap-group"}, {name: "owner-managed canonical binding", state: "owned"}, {name: "finalized canonical binding", state: "finalized"}, {name: "terminating canonical binding", state: "terminating"}, @@ -815,6 +816,22 @@ func TestSetupNodeRBACPreservesSameNamedNonLegacyBinding(t *testing.T) { } } +func TestSetupNodeRBACIgnoresUnrelatedBootstrapSubgroup(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + if err := os.WriteFile(harness.legacyState, []byte("unrelated-bootstrap-subgroup\n"), 0o600); err != nil { + t.Fatalf("write fake legacy state: %v", err) + } + output, err := harness.runSetupNodeRBAC(false) + if err != nil { + t.Fatalf("setup-node-rbac rejected an unrelated bootstrap subgroup: %v\n%s", err, output) + } + if _, deleteIndexes := kubectlOperationIndexes(readCommandCalls(t, harness.commandLogPath)); len(deleteIndexes) != 0 { + t.Fatal("unrelated bootstrap subgroup binding was deleted") + } +} + func TestGenerateBootstrapTokenRequiresCompletedLegacyMigration(t *testing.T) { t.Parallel() @@ -880,24 +897,38 @@ func TestGenerateBootstrapTokenRequiresCompletedLegacyMigration(t *testing.T) { } } -func TestGenerateBootstrapTokenRejectsUnsafeNamespacedRoleBinding(t *testing.T) { +func TestGenerateBootstrapTokenRejectsUnsafeBindings(t *testing.T) { t.Parallel() - harness := newConfigScriptHarness(t, false, 0) - if err := os.WriteFile(harness.legacyState, []byte("namespaced\n"), 0o600); err != nil { - t.Fatalf("write fake legacy state: %v", err) - } - output, err := harness.runGenerateNodeConfig() - if err == nil { - t.Fatalf("generate-node-config minted a token while an unsafe RoleBinding remained\n%s", output) - } - if !strings.Contains(output, "RoleBinding/kube-system/legacy-node-access") || - !strings.Contains(output, "--remove-legacy-node-role-binding") { - t.Fatalf("failure did not identify the unsafe namespaced binding:\n%s", output) + tests := []struct { + name string + state string + wantBinding string + }{ + {name: "namespaced role binding", state: "namespaced", wantBinding: "RoleBinding/kube-system/legacy-node-access"}, + {name: "default bootstrap token group", state: "default-bootstrap-group", wantBinding: "ClusterRoleBinding/all-bootstrap-node-role"}, } - applyIndexes, deleteIndexes := kubectlOperationIndexes(readCommandCalls(t, harness.commandLogPath)) - if len(applyIndexes) != 0 || len(deleteIndexes) != 0 { - t.Fatal("unsafe namespaced binding check mutated cluster state") + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + harness := newConfigScriptHarness(t, false, 0) + if err := os.WriteFile(harness.legacyState, []byte(test.state+"\n"), 0o600); err != nil { + t.Fatalf("write fake legacy state: %v", err) + } + output, err := harness.runGenerateNodeConfig() + if err == nil { + t.Fatalf("generate-node-config minted a token while an unsafe binding remained\n%s", output) + } + if !strings.Contains(output, test.wantBinding) || + !strings.Contains(output, "--remove-legacy-node-role-binding") { + t.Fatalf("failure did not identify the unsafe binding:\n%s", output) + } + applyIndexes, deleteIndexes := kubectlOperationIndexes(readCommandCalls(t, harness.commandLogPath)) + if len(applyIndexes) != 0 || len(deleteIndexes) != 0 { + t.Fatal("unsafe binding check mutated cluster state") + } + }) } } @@ -956,7 +987,7 @@ func TestRepositoryDoesNotBindBootstrapGroupsToLegacyNodeRole(t *testing.T) { sourceRoots := []string{"cmd", "hack", "pkg", "scripts"} bindingPattern := regexp.MustCompile(`(?m)^[ \t]*kind:[ \t]*(?:ClusterRoleBinding|RoleBinding)[ \t]*$`) legacyRolePattern := regexp.MustCompile(`(?m)^[ \t]*name:[ \t]*system:node[ \t]*$`) - bootstrapGroupPattern := regexp.MustCompile(`(?m)^[ \t]*name:[ \t]*system:bootstrappers:[^ \t\n]+[ \t]*$`) + bootstrapGroupPattern := regexp.MustCompile(`(?m)^[ \t]*name:[ \t]*system:bootstrappers(?::[^ \t\n]+)?[ \t]*$`) documentSeparator := regexp.MustCompile(`(?m)^[ \t]*---[ \t]*$`) for _, sourceRoot := range sourceRoots { @@ -1794,17 +1825,29 @@ elif state == "customized": "name": "another-group", }], }) -elif state == "renamed": +elif state in {"renamed", "default-bootstrap-group", "unrelated-bootstrap-subgroup"}: + binding_name = "custom-bootstrap-node-role" + group_name = "system:bootstrappers:aks-flex-node" + if state == "default-bootstrap-group": + binding_name = "all-bootstrap-node-role" + group_name = "system:bootstrappers" + elif state == "unrelated-bootstrap-subgroup": + binding_name = "unrelated-bootstrap-node-role" + group_name = "system:bootstrappers:unrelated" items.append({ "apiVersion": "rbac.authorization.k8s.io/v1", "kind": "ClusterRoleBinding", - "metadata": {"name": "custom-bootstrap-node-role", "uid": "renamed-uid", "resourceVersion": "9"}, + "metadata": {"name": binding_name, "uid": "renamed-uid", "resourceVersion": "9"}, "roleRef": { "apiGroup": "rbac.authorization.k8s.io", "kind": "ClusterRole", "name": "system:node", }, - "subjects": [subject], + "subjects": [{ + "apiGroup": "rbac.authorization.k8s.io", + "kind": "Group", + "name": group_name, + }], }) elif state == "safe-customized": items.append({