fix(oci): both credential routes in the guide, and a nightly topology that proves the principal one (#1173)

* docs(oci): offer both credential routes, and default the guide to the release

Step 2 was one route only, the scoped API key, with the instance principal
mentioned in a TIP at the end of it. It is now "2. OCI Credentials for
Spinifex" with the two routes as 2.1 and 2.2, a table to choose between them,
and the instance principal first because it leaves no key on any node.

The guide also assumed the dev branch throughout. `git switch dev` and
`--channel dev` are now an optional note for installing a pre-release, and the
documented path is the published release.

* test(oci): add a vm-single-principal topology, and cover the credential branch

Instance-principal authentication was broken for a release while the OCI
nightly stayed green, because the run's two axes never crossed: every topology
that built the address allocator authenticated with an API key, and every run
that authenticated as the instance was dispatched with no external pool, so the
allocator was never constructed and the credential never used.

vm-single-principal closes that. It is vm-single with the credential fixed by
the topology rather than by a separate switch, so the name alone decides it, and
pairing it with --no-external-pool is refused rather than run: without the
allocator there is nothing for it to exercise. It joins the nightly's default
list, and naming it alone is the way to test this one thing in a quarter of the
full run.

A unit test covers the same branch without a tenancy. newClient now reaches its
two constructors through package variables a test can swap, which is the only
way to observe the choice off an OCI instance - both real constructors need
either an instance identity or a key file on disk.

* fix(oci): let the tfvars choose the credential, and stop the guide hardcoding paths

The deploy driver passed instance_principal as a -var, which outranks a
tfvars file, so a deployment that set it there had it silently replaced with
off and the nodes went looking for a key. Same defect class as the shape and
node count, and the same fix: the flag is a choice about one run and still
outranks everything, and without it the deployment's own tfvars decides.

The driver now reads instance_principal from terraform.tfvars and
*.auto.tfvars for its pre-apply gates, taking the last assignment as Terraform
does, and checks that guess against a new Terraform output once the apply has
run. They have to agree, because the credential handoff is skipped on the
strength of it and a wrong answer leaves a node with no credential at all.

The guide now carries one export rather than two more steps. SPX_PRINCIPAL is
set in step 2.1, expanded into the tfvars step 4 already writes, and read back
from there in step 5, so an instance-principal deployment runs the same
command as a key-based one.

Getting into the node needed an address copied out of the log. Every topology
now writes its hosts file, not only the multi-node one that hands it to
install-node.sh, and the run prints the ssh and destroy commands when --keep
leaves the infrastructure up. The guide exports TOPOLOGY once, so the paths
read the same for one VM, three, or bare metal.

* fix(oci): print the ssh key path as ~ rather than expanded

The --keep line read ssh -i /home/you/.ssh/oci-spx, which is the invoking
user's home and differs on every machine. Collapsed to ~ when the key is under
$HOME, left alone when it is not.

scripts/terraform/oci-spx/e2e-oci.sh

@@ -7,7 +7,9 @@
 # Configured entirely by environment variables so a dispatch needs no arguments:
 #
 #   SPX_REF           Ref to prove. Default: the spinifex checkout's current branch.
-#   OCI_TOPOLOGIES    Space-separated. Default: "vm-single vm-multi bm".
+#   OCI_TOPOLOGIES    Space-separated. Default: "vm-single vm-multi bm
+#                     vm-single-principal". Set one name to test one thing: a
+#                     single VM topology is roughly a quarter of the full run.
 #   OCI_SOURCE        tree (build SPX_REF) or release (the published installer).
 #   OCI_VERSION       With OCI_SOURCE=release, install this exact release tag and
 #                     ignore OCI_CHANNEL. Preferred for anything repeatable: a tag
@@ -22,11 +24,14 @@
 #                     list. Set to "" to skip the workbook phase entirely.
 #   OCI_INSTANCE_PRINCIPAL  Authenticate the node's allocator as the instance
 #                     principal. Default 1, and it needs the tenancy's dynamic
-#                     group to exist.
+#                     group to exist. The vm-single-principal topology ignores it
+#                     and always authenticates that way, which is its purpose.
 #   OCI_NO_EXTERNAL_POOL    1 to form with no allocator at all, for a tenancy whose
 #                     dynamic group does not exist yet. The allocator and every
 #                     workbook are then recorded SKIPPED, never PASS, so a green
 #                     run with this set is not a claim about guest networking.
+#                     Not applied to vm-single-principal, which the allocator is
+#                     the entire test of; that pairing is refused rather than run.
 #   OCI_SSH_PUBLIC_KEY / OCI_SSH_PRIVATE_KEY   Paths. Default ~/.ssh/oci-spx[.pub].
 #   OCI_CREDENTIAL_HOOK     Executable run on each topology after formation and
 #                     before the pool, as "hook <ssh-key> <host>...". An API-key
@@ -47,7 +52,7 @@ SPINIFEX_ROOT="$(cd "$HERE/../../.." && pwd)"
 # under the workspace. Deriving one from the other is only right in the first case.
 MULGA_ROOT="${MULGA_ROOT:-$SPINIFEX_ROOT/..}"
 
-TOPOLOGIES="${OCI_TOPOLOGIES:-vm-single vm-multi bm}"
+TOPOLOGIES="${OCI_TOPOLOGIES:-vm-single vm-multi bm vm-single-principal}"
 SOURCE="${OCI_SOURCE:-tree}"
 CHANNEL="${OCI_CHANNEL:-latest}"
 INSTALL_VERSION="${OCI_VERSION:-}"
@@ -141,12 +146,14 @@ summary() {
     # In the header, not left to a SKIPPED row further down. A run with no allocator
     # is not evidence about public addressing, and that is precisely the claim a
     # reader of a green table would otherwise take from it.
+    # Said per topology rather than once for the run: vm-single-principal pins its
+    # own credential, so a single claim over a mixed run is wrong for one of them.
     if [ "${OCI_NO_EXTERNAL_POOL:-0}" = 1 ]; then
-        printf '> **No external address pool.** Public-address allocation and every workbook needing a public address were skipped, so nothing here speaks to guest ingress.\n\n'
+        printf '> **No external address pool.** Public-address allocation and every workbook needing a public address were skipped, so nothing here speaks to guest ingress. `vm-single-principal`, if it ran, is the exception and did build one.\n\n'
     elif [ "${OCI_INSTANCE_PRINCIPAL:-1}" = 1 ]; then
         printf 'Allocator authenticated as the instance principal.\n\n'
     else
-        printf 'Allocator authenticated with an API key.\n\n'
+        printf 'Allocator authenticated with an API key, except `vm-single-principal` which authenticates as the instance.\n\n'
     fi
 
     local topology results workbooks gate status detail name secs
@@ -275,7 +282,13 @@ for topology in $TOPOLOGIES; do
     # Two separate choices. --no-external-pool is the one that forms without any
     # allocator, for a tenancy whose dynamic group does not exist yet; it records
     # the allocator and the workbooks as SKIPPED rather than passing them.
-    if [ "${OCI_NO_EXTERNAL_POOL:-0}" = 1 ]; then
+    #
+    # vm-single-principal opts out of both: it sets its own credential, and a run
+    # of it without an allocator would exercise nothing. validate-topology.sh
+    # refuses that pairing, so skipping it here keeps a mixed run from aborting.
+    if [ "$topology" = vm-single-principal ]; then
+        :
+    elif [ "${OCI_NO_EXTERNAL_POOL:-0}" = 1 ]; then
         args+=(--no-external-pool)
     elif [ "${OCI_INSTANCE_PRINCIPAL:-1}" = 1 ]; then
         args+=(--instance-principal)

scripts/terraform/oci-spx/outputs.tf

@@ -8,9 +8,12 @@ output "security_list_ocids" { value = { public = oci_core_security_list.public.
 output "compute_ocids" { value = [for instance in oci_core_instance.mulgadc : instance.id] }
 output "data_volume_ocids" { value = [for volume in oci_core_volume.mulgadc_data : volume.id] }
 
-# Read back rather than assumed: the shape can come from a tfvars file, so this is
-# the only place that knows what was actually built.
+# Read back rather than assumed: both can come from a tfvars file, so this is the
+# only place that knows what was actually built. The deploy driver skips the
+# credential handoff on instance_principal, so a wrong guess leaves a node with no
+# credential at all.
 output "compute_shape" { value = var.compute_shape }
+output "instance_principal" { value = var.instance_principal }
 
 # What the Spinifex deploy path consumes. The primary VNIC is the host plane, so
 # these are the addresses update-nodes.sh and install-node.sh connect to.

scripts/terraform/oci-spx/validate-topology.sh

@@ -33,21 +33,31 @@ WORKBOOKS_SET=0
 CHANNEL=latest
 INSTALL_VERSION=""
 WORKBOOKS=""
+# Declared here, not at the apply that fills it: the EXIT trap reads it, and under
+# set -u an unset array is an error rather than an empty one.
+HOSTS=()
 
-TOPOLOGIES="bm vm-single vm-multi"
+TOPOLOGIES="bm vm-single vm-multi vm-single-principal"
 
 # What a topology is worth when nothing says otherwise, so a CI run of a named
 # topology is the same every time. Supplied as TF_VAR_, which terraform.auto.tfvars
-# outranks, so a deployment sets its shape and node count in that file and no
-# flag has to carry them.
+# outranks, so a deployment sizes itself in that file and no flag has to carry it.
+#
+# TOPO_FORCE_PRINCIPAL is the exception: vm-single-principal exists to exercise
+# instance-principal authentication, so the credential is the topology rather than
+# a separate switch that could be left at the other value.
 #
 # A case rather than an associative array: macOS ships bash 3.2, where declare -A
 # is an indexed assignment and the key is read as an arithmetic variable.
 topo_defaults() {
+    TOPO_FORCE_PRINCIPAL=0
     case "$1" in
         bm)        TOPO_DEFAULT_SHAPE="BM.Standard.E2.64";  TOPO_DEFAULT_NODES=1 ;;
         vm-single) TOPO_DEFAULT_SHAPE="VM.Standard.E6.Flex"; TOPO_DEFAULT_NODES=1 ;;
         vm-multi)  TOPO_DEFAULT_SHAPE="VM.Standard.E6.Flex"; TOPO_DEFAULT_NODES=3 ;;
+        vm-single-principal)
+            TOPO_DEFAULT_SHAPE="VM.Standard.E6.Flex"; TOPO_DEFAULT_NODES=1
+            TOPO_FORCE_PRINCIPAL=1 ;;
         *) return 1 ;;
     esac
 }
@@ -60,18 +70,22 @@ die() {
 
 usage() {
     cat >&2 <<EOF
-usage: ${0##*/} --topology <bm|vm-single|vm-multi> [options]
+usage: ${0##*/} --topology <bm|vm-single|vm-multi|vm-single-principal> [options]
 
 Builds the topology, installs Spinifex, forms the cluster, runs a Terraform
 workbook against it, then destroys everything.
 
   --topology <name>       Required. No default: a command aimed at the wrong
                           topology is the easiest expensive mistake here.
+                          vm-single-principal is vm-single authenticating as the
+                          instance, and implies --instance-principal.
   --ssh-public-key PATH   Public half installed on each node. Default $SSH_PUBLIC_KEY
   --ssh-private-key PATH  Private half used to reach them. Default $SSH_PRIVATE_KEY
   --instance-principal    Configure the pool with oci_auth="instance_principal"
                           instead of a key file. Needs the dynamic group and
                           policy to exist already; see instance-principal.tf.
+                          Not needed when instance_principal is set in your
+                          tfvars, which this reads; the flag outranks it.
   --credential-hook PATH  Executable run after formation, before the pool, as
                           "hook <ssh-key> <host>...". Where an API-key deployment
                           installs its credential. Default \$OCI_CREDENTIAL_HOOK.
@@ -136,9 +150,44 @@ if [ -n "$DISTRO" ] || [ -n "$SETUP_SH" ]; then
     [ -r "$SETUP_SH" ] || die "--setup-sh is not readable: '$SETUP_SH'"
 fi
 
+if [ "$TOPO_FORCE_PRINCIPAL" = 1 ]; then
+    INSTANCE_PRINCIPAL=1
+    # The pool is the whole point of this topology: with no allocator built, the
+    # credential is never used and a green run says nothing about it. That is
+    # exactly how the auth branch stayed broken through passing nightlies.
+    [ "$SKIP_POOL" != 1 ] \
+        || die "--no-external-pool makes $TOPOLOGY meaningless: the allocator is what exercises the instance principal, so a run without it proves nothing"
+fi
+
+# Terraform's own order, as far as this script needs it: the last assignment across
+# terraform.tfvars then *.auto.tfvars alphabetically is the one that applies. Read
+# because the credential-hook gate has to run before the apply spends anything; the
+# apply's own output is checked against this afterwards.
+tfvars_instance_principal() {
+    local file mode="" found=""
+    for file in "$HERE/terraform.tfvars" "$HERE"/*.auto.tfvars; do
+        [ -r "$file" ] || continue
+        mode=$(sed -n 's/^[[:space:]]*instance_principal[[:space:]]*=[[:space:]]*"\([a-z]*\)".*/\1/p' \
+            "$file" | tail -1)
+        [ -n "$mode" ] && found="$mode"
+    done
+    [ -n "$found" ] || return 1
+    printf '%s\n' "$found"
+}
+
 # adopt, never create: this script's state is destroyed at the end of every run, and
 # the dynamic group and policy outlive every topology. setup-identity.sh owns them.
-PRINCIPAL_MODE=$([ "$INSTANCE_PRINCIPAL" = 1 ] && echo adopt || echo off)
+#
+# A flag goes in as -var and outranks the tfvars, because it is a choice about this
+# run. Without one the deployment's own tfvars decides, so a guide can set the
+# credential in the file it already writes and no flag has to carry it.
+tf_principal_var=()
+if [ "$INSTANCE_PRINCIPAL" = 1 ]; then
+    PRINCIPAL_MODE=adopt
+    tf_principal_var=(-var "instance_principal=$PRINCIPAL_MODE")
+else
+    PRINCIPAL_MODE="$(tfvars_instance_principal || echo off)"
+fi
 
 # What the apply actually built, filled in from the hosts_file output once it has.
 # Until then these are what the topology asked for, which is all there is to say.
@@ -174,10 +223,8 @@ tf() {
 # terraform output takes -state but not -var, so the two sets are kept apart. They
 # were one set once, and the -var made output fail into the default state file.
 tf_state=(-state "$STATE_DIR/terraform.tfstate")
-# instance_principal stays a -var, and outranks any tfvars, because --instance-principal
-# is a choice about this run rather than about the infrastructure's size.
 tf_vars=(
-    -var "instance_principal=$PRINCIPAL_MODE"
+    "${tf_principal_var[@]}"
     "${tf_state[@]}"
 )
 
@@ -243,6 +290,17 @@ cleanup() {
     local rc=$?
     if [ "$KEEP" = 1 ]; then
         log "--keep: leaving the infrastructure up, no verdict recorded"
+        # The command to get in, printed rather than left to be assembled from an
+        # address elsewhere in the log. HOSTS is empty if we never got as far as
+        # the apply, which is why this is guarded rather than assumed.
+        if [ "${#HOSTS[@]}" -gt 0 ]; then
+            # ~ rather than the expanded path, so the line is the same on every
+            # machine and reads as something a person would type.
+            local key="$SSH_PRIVATE_KEY"
+            case "$key" in "$HOME"/*) key="~${key#"$HOME"}" ;; esac
+            log "ssh in with: ssh -i $key ubuntu@${HOSTS[0]}"
+            log "destroy it with: $0 --topology $TOPOLOGY --destroy-only"
+        fi
         return
     fi
     # Only on a failure, and said out loud: the cost of forgetting is an OCI bare
@@ -361,6 +419,11 @@ for host in "${HOSTS[@]}"; do
         || die "the hosts_file output is not an address: '$host' -- something on the wrapper's stdout is in the capture"
 done
 
+# Written for every topology, not just the multi-node one that passes it to
+# install-node.sh: it is also how an operator gets the addresses afterwards
+# without a terraform invocation and the right -state path.
+printf '%s\n' "${HOSTS[@]}" > "$STATE_DIR/hosts"
+
 # node_count can come from a tfvars file, so the hosts the apply produced are the
 # authority on how many there are. Every later count check reads this.
 if [ "${#HOSTS[@]}" != "$NODES" ]; then
@@ -369,8 +432,18 @@ if [ "${#HOSTS[@]}" != "$NODES" ]; then
 fi
 SHAPE=$(tf output -raw "${tf_state[@]}" compute_shape) \
     || die "could not read the compute_shape output from $STATE_DIR/terraform.tfstate"
+
+# Terraform is the authority on which credential was built, and the reader above is
+# a guess made before the apply. They have to agree: the hook is skipped on the
+# strength of that guess, and a node left with no credential at all would otherwise
+# fail much later as an allocator that cannot resolve its VNIC.
+built_principal=$(tf output -raw "${tf_state[@]}" instance_principal) \
+    || die "could not read the instance_principal output from $STATE_DIR/terraform.tfstate"
+[ "$built_principal" = "$PRINCIPAL_MODE" ] \
+    || die "read instance_principal='$PRINCIPAL_MODE' from the tfvars and the apply built '$built_principal'; set it in one place"
+
 log "hosts: ${HOSTS[*]}"
-log "$SHAPE, $NODES node(s)"
+log "$SHAPE, $NODES node(s), oci_auth $([ "$PRINCIPAL_MODE" = off ] && echo "from a key file" || echo "as the instance principal")"
 
 # cloud-init owns the volume, the ports and br-wan, and all three are units now, so
 # the question is whether they converged rather than whether a runcmd happened to
@@ -462,7 +535,6 @@ if [ "$NODES" = 1 ]; then
     ' > "$STATE_DIR/form.log" 2>&1 \
         || die "single-node init failed; see $STATE_DIR/form.log"
 else
-    printf '%s\n' "${HOSTS[@]}" > "$STATE_DIR/hosts"
     log "forming the cluster"
     "$REPO_ROOT/scripts/install-node.sh" \
         --hosts-file "$STATE_DIR/hosts" \