fix(advisor): own the web-targets allowlist, and carry enrolment across an upgrade

[traviswu-bigstack]

What type of PR is this?

/kind bug

What this PR does / why we need it

Two gaps in the Advisor agent's node-side support, folded together because they share every file.

1. Nothing owned the web-targets allowlist. cube-advisor-agent only dials endpoints listed in /etc/cube-advisor-agent/web-targets.json, and no part of cubecos created or managed it — so an enrolled node could serve no web target until someone hand-wrote JSON on it, and there was no CLI to inspect or change it.

• config_advisor's Commit seeds it on every node, so each node has its own: cube-cos → this cluster's own dashboard, plus cube-cmp and app-fw-idp → <ingress>:443 when the app framework's ingress address is known. Both CMP entries share one address on purpose — the portal and its identity provider must share an origin, or the OIDC state cookie is set on one and the callback arrives at the other.
• The ingress address is discovered once and distributed. advisor_targets_discover fetches a framework kubeconfig from rancher, resolves the address, and writes it to every node as a short string; each node's Commit reads it locally. No node needs a kubeconfig at commit time.
• advisor_targets_list/_set/_unset manage it through jq, validating and writing atomically.
• advisor targets, advisor target_set, advisor target_unset expose it in hex_cli.

2. An upgraded node silently dropped off the fleet. CubeCOS upgrades by booting the other partition, carrying across only what a module registers — and config_advisor.cpp registered nothing, so an enrolled node came up with no agent and no identity, looking exactly like one that was never enrolled.

• Migrate what an enrolled node needs: the agent binary, its identity, the allowlist, the console CA and its sshd drop-in.
• Put the service under hex_config, where every other cubecos service lives — a Commit hook with the standard IsUndef/CommitCheck guard calling SystemdCommitService(IsEnrolled(), …), ordered after networking via CONFIG_REQUIRES(advisor, net_static). advisor_agent_service_enable became advisor_agent_service_start: it starts the unit and never enables it, so hex_config stays the only owner.
• Teach cluster check about the advisor — a cluster-wide check naming any enrolled node whose agent is not running, repairable manually and automatically.

Which issue(s) this PR fixes

Fixes #1481
Fixes #1483

The API and UI need the component declared to display it: bigstack-oss/cube-cos-api#659, bigstack-oss/cube-cos-ui#873.

Special notes for your reviewer

Verified on hardware (1cc r630, cube-combined). A node was enrolled on the previous image, then upgraded into this one. Without a second reboot:

Check	Result
Identity, binary migrated	agent.crt/agent.key/enrollment-ca.crt/server intact, binary sha unchanged
is-enabled	disabled — enabled is the failure signal
is-active	active
Who started it	hex_config's commit pass, not multi-user.target
systemd-hexctl-sys.service	Result=success, no boot stall
Allowlist seeded by Commit	created on first boot of the new slot
cluster check	ClusterSys … advisor(v)
Agent rejoined unattended	agent: connected to <advisor>:8443 after the framework came back

Trusting the Advisor's CA at enrolment. An Advisor serving its own certificate — the normal case offline, where there is no public CA to lean on — could not be enrolled against at all: the release fetch died at curl: (60) SSL certificate problem: self-signed certificate before any signature was checked, and the agent that follows speaks to the same endpoint through Go's TLS. Both read the system trust store and neither takes a CA path. advisor_trust_ca installs a CA the operator supplies out of band, as an optional third argument to advisor enroll.

-k was never the alternative — the pairing token is a bearer credential on that request. A file that is not a certificate is refused at the door rather than installed to fail every later handshake, and an anchor whose trust store will not rebuild is removed rather than left claiming a trust that is not in effect.

It is deliberately not migrated across an upgrade: this is enrolment-time trust, while the tunnel's trust is the enrollment CA the agent pins from its identity directory, which is migrated. Confirmed on the r630 — after the upgrade that dropped this anchor, the agent had reconnected on its own while curl to the same address still refused. Verified live: REFUSED (self-signed) → VERIFIES, with the releases endpoint then answering 401 instead of failing at TLS. This is the node-side half of bigstack-oss/cube-ai-advisor#215.

Why discovery never ran, and what fixed it. That same hardware run exposed a defect this PR introduced: app_ingress_address and app_helm_release_deployed both refused unless /opt/appfw/kubeconfig was readable, and nothing creates that file — app_framework_deploy fetches its own from rancher into a temp file and discards it. Both helpers therefore answered "no framework" on every cluster, so advisor_targets_discover returned at its first line and no install or enrolment ever declared cube-cmp or app-fw-idp. The node under test came up with a cube-cos-only allowlist.

Their unit tests passed throughout, because those tests point APPFW_KUBECONFIG at a fixture — they stubbed the exact thing that was broken. app_kubeconfig now fetches the kubeconfig the way the product does, reading the framework's name back from rancher rather than assuming it (a driver-installed framework is appfw; app_framework_deploy's own is app-framework). APPFW_RANCHER joins APPFW_KUBECONFIG as an injectable path, because a hardcoded absolute path is one no test can reach. Confirmed on the same node: discovery now writes cube-cmp and app-fw-idp → 10.32.1.110:443, and advisor status reports 3 targets.

Why there is no systemctl call in the migrate path. Earlier revisions had one, wrong three times: enable --now from a post-migrate hook deadlocks the boot (it runs inside rc.sysinit, which is Before=sysinit.target with TimeoutSec=infinity, waiting on a job that cannot run until it returns); plain enable never starts anything (the default.target transaction is computed at manager startup, so a later .wants symlink adds no job); enable + start --no-block works and is still not how this codebase does it. hex_config owns service lifecycle, so the hook was deleted entirely.

Why there is no release stash. An earlier revision stashed the signed release so the new partition could reinstall and re-verify offline. It sat on the same root-writable filesystem as the binary, so re-verification defended against nothing an attacker could not also reach; its only real effect — a rotated key disabling old agents at upgrade — is revocation, which belongs at the SaaS. Migrating the binary is simpler and removes a failure mode.

The allowlist read path uses jq, not a regex. The first implementation hand-parsed it, and a file with "key": "value" — a space after the colon, what an editor or jq . produces — made list return nothing, after which set rebuilt the file from that empty list and silently deleted every entry. Silent data loss on a security allowlist. jq already ships in the image and 26 other sdk_sh modules use it; a file that does not parse is now refused rather than clobbered.

Seeding is not reconciling. Commit writes the allowlist only when the file is absent — it never adds, removes or reconciles entries in one that exists, because advisor target_unset cube-cmp is a deliberate act and a commit that quietly put it back would make the command meaningless. "Self-healing" means a node with no allowlist gets one, not every node reconciled to a canonical set. A test pins it: making Commit reconcile shows the operator's removed entry reappearing.

Where cube-cos points. The seed used to be 127.0.0.1:8080. That port is httpd, which answers 403 to everything -- nginx serves the dashboard on the management address. A target pointed at loopback looks configured and refuses every request, with nothing in any log to say why; in the browser it presented as the proxied tab showing "403 Forbidden". advisor_dashboard_address resolves where the UI actually answers -- the control VIP on an HA cluster, this node's management address otherwise -- and when neither is known the seed omits cube-cos entirely rather than writing an address that cannot work; advisor target_set adds it once the address exists. The installer half of the same bug is bigstack-oss/cube-cos-driver#116.

The endpoints the dashboard links out to are targets too. CubeCOS serves Keycloak on :10443, Skyline on :9999 and the Ceph dashboard on :7443, and the dashboard links the browser at all three -- so a node that allows cube-cos and not those allows links that cannot be followed, and a login that cannot complete. advisor_own_targets returns the whole set the node can name for itself, one name and address per line, which the seed renders the way it already renders the discovered ones: one function to stub in the tests rather than one per port, and one place to add the next endpoint. None of them can be discovered by reading what the cluster serves -- the address arrives as a bare string in /api/v1/datacenters and script appends the port in the browser. The proxy-side support is bigstack-oss/cube-ai-advisor#227.

Why per-node seeding exists. The agent runs on every node and each reads its own local allowlist, but seeding used to happen only inside advisor_enroll, on one control node. On the lab cluster the allowlist existed on one node and was absent on the other two, so a session routed to either refused every target. Commit runs during each node's own hex_config commit, so a node that was down or added later gets one without anyone intervening.

Who sets the targets, and what is reachable. Seeding covers cube-cos and, where the app framework is present, the two CMP targets. Anything else is operator-declared; nothing else auto-discovers, deliberately. On a cluster with both an appfw-hosted portal and a separately-installed CMP, a guess would point cube-cmp at the wrong host, surfacing as a proxied page whose OIDC redirect_uri does not match — a broken login that reads as a proxy bug. An absent target is a better failure than a wrong one.

Reachability is whatever the node can route to. The agent dials with no interface binding, so it inherits the node's full routing table, and CubeCOS nodes are multi-homed — a CMP on a provider network is reachable if the node has an interface on it. The SaaS never learns the address; it only ever sends the symbolic name. The check before filing a ticket: if you can curl it from the node, the web console can reach it. A CMP the node has no route to cannot be reached at all — the agent is the only path in, by design.

Health check semantics. cluster check runs only on control nodes but the agent runs on every node, so the check fans out across CUBE_NODE_LIST_HOSTNAMES. Codes: 0 no node enrolled — healthy, since most clusters never roll into the Advisor; 1 an enrolled node whose agent is down, named explicitly, auto-repaired by starting it; 2 enrolled with the binary missing, needing re-enrolment. Auto-repair deliberately skips code 2: the failure count only resets on a passing check, so retrying an unfixable state would burn the maxerr budget and gate auto-repair off permanently.

enabled is identity-only. Every module this idiom came from also filters on IsControl; this one does not, and a test fails if that filter is reintroduced.

Coverage gaps, stated rather than hidden. The stub cannot reach CommitCheck's non-bootstrap branch, so only the IsBootstrap() path is pinned. CONFIG_REQUIRES(advisor, net_static) is argued, not observed — resolution happens at static init of a fully linked binary, and an unknown name is HexLogFatal, so a jail build fails loudly. Several behaviours are pinned only by core/sdk_sh/tests/*.sh, which CI never runs (#1484). Building these files needs gcc-toolset-11.

Additional documentation

advisor enroll [<url> [<version> [<ca-file>]]]
                                         — ca-file is optional; needed when the
                                           Advisor serves its own certificate
advisor targets                          — list what this node allows
advisor target_set <name> <host:port>    — allow an endpoint; address is whatever the node can route to
advisor target_unset <name>              — stop allowing it; nothing puts it back
cluster check                            — reports the advisor module under Cluster System
cluster check_repair                     — starts a stopped agent on any enrolled node

🤖 Generated with Claude Code

https://claude.ai/code/session_01PZ5umjjCedZwWtbAbiMjfj