From 0a8e6bca15115575b83da9ff2f20cb62ecabff15 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Wed, 15 Jul 2026 09:00:35 +1000 Subject: [PATCH] fix(admin): always pin mgmt-bridge IP in server cert SAN (#540) Server-cert IP SANs were built solely from live interface enumeration (DiscoverLocalIPs) plus bindIP, so the canonical mgmt-bridge IP entered the SAN set only when br-mgmt happened to be up at cert-generation time. On a host where br-mgmt is down when the cert is minted, 10.15.8.1 was silently dropped and the EKS control-plane publish to https://10.15.8.1 failed cert verify, leaving the cluster stuck CREATING. Pin config.DefaultMgmtBridgeIP into the extra SANs for both the init and join cert paths so publish succeeds regardless of interface state at generation time. Add a regression test asserting the SAN is present when br-mgmt is not a live interface. --- spinifex/admin/admin.go | 10 ++++++++-- spinifex/admin/admin_test.go | 33 +++++++++++++++++++++++++++++++++ spinifex/config/config.go | 7 +++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/spinifex/admin/admin.go b/spinifex/admin/admin.go index 2915ce4d2..d190649d6 100644 --- a/spinifex/admin/admin.go +++ b/spinifex/admin/admin.go @@ -21,6 +21,7 @@ import ( "text/template" "time" + "github.com/mulgadc/spinifex/spinifex/config" toml "github.com/pelletier/go-toml/v2" "gopkg.in/ini.v1" ) @@ -219,7 +220,10 @@ func GenerateCertificatesIfNeeded(configDir string, force bool, bindIP string, a if force || !FileExists(serverCertPath) || !FileExists(serverKeyPath) { extraDNS := AWSGWServiceDNSNames(awsRegion, internalSuffix) - if err := GenerateSignedCert(serverCertPath, serverKeyPath, caCertPath, caKeyPath, []string{bindIP}, extraDNS); err != nil { + // Always pin the canonical mgmt-bridge IP; the control plane publishes to + // it regardless of whether br-mgmt is up when this cert is minted. + extraIPs := []string{bindIP, config.DefaultMgmtBridgeIP} + if err := GenerateSignedCert(serverCertPath, serverKeyPath, caCertPath, caKeyPath, extraIPs, extraDNS); err != nil { fmt.Fprintf(os.Stderr, "Error generating server certificate: %v\n", err) os.Exit(1) } @@ -247,7 +251,9 @@ func GenerateServerCertOnly(configDir string, bindIP, awsRegion, internalSuffix } extraDNS := AWSGWServiceDNSNames(awsRegion, internalSuffix) - return GenerateSignedCert(serverCertPath, serverKeyPath, caCertPath, caKeyPath, []string{bindIP}, extraDNS) + // Always pin the canonical mgmt-bridge IP (see GenerateCertificatesIfNeeded). + extraIPs := []string{bindIP, config.DefaultMgmtBridgeIP} + return GenerateSignedCert(serverCertPath, serverKeyPath, caCertPath, caKeyPath, extraIPs, extraDNS) } func CreateServiceDirectories(spxRoot string) { diff --git a/spinifex/admin/admin_test.go b/spinifex/admin/admin_test.go index 91084c5ad..6fc86daab 100644 --- a/spinifex/admin/admin_test.go +++ b/spinifex/admin/admin_test.go @@ -11,10 +11,28 @@ import ( "strings" "testing" + "github.com/mulgadc/spinifex/spinifex/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// certHasIP reports whether a PEM-encoded cert carries ip as an IP SAN. +func certHasIP(t *testing.T, certPath, ip string) bool { + t.Helper() + certPEM, err := os.ReadFile(certPath) + require.NoError(t, err) + block, _ := pem.Decode(certPEM) + require.NotNil(t, block) + cert, err := x509.ParseCertificate(block.Bytes) + require.NoError(t, err) + for _, got := range cert.IPAddresses { + if got.Equal(net.ParseIP(ip)) { + return true + } + } + return false +} + // --- Key / Token generation --- func TestGenerateAWSAccessKey_Format(t *testing.T) { @@ -672,6 +690,17 @@ func TestGenerateCertificatesIfNeeded(t *testing.T) { _, err = srvCert.Verify(x509.VerifyOptions{Roots: pool, KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}}) assert.NoError(t, err, "re-signed server cert must verify against preserved CA") }) + + // The mgmt-bridge IP must be a SAN even though br-mgmt is not a live + // interface here (interface enumeration cannot discover it), mirroring a + // host where br-mgmt is down when the cert is minted. Without the explicit + // pin the control-plane publish to https:// would fail cert verify. + t.Run("MgmtBridgeIPAlwaysInSAN", func(t *testing.T) { + certDir := t.TempDir() + GenerateCertificatesIfNeeded(certDir, false, "10.0.0.5", "us-east-1", "spinifex.internal") + assert.True(t, certHasIP(t, filepath.Join(certDir, "server.pem"), config.DefaultMgmtBridgeIP), + "server cert must carry the canonical mgmt-bridge IP SAN regardless of br-mgmt state") + }) } func TestGenerateServerCertOnly(t *testing.T) { @@ -708,6 +737,10 @@ func TestGenerateServerCertOnly(t *testing.T) { // AWS-parity ECR SANs must be present. assert.Contains(t, cert.DNSNames, "ecr.us-east-1.spinifex.internal") assert.Contains(t, cert.DNSNames, "*.dkr.ecr.us-east-1.spinifex.internal") + + // Joining nodes must also pin the mgmt-bridge IP regardless of br-mgmt state. + assert.True(t, certHasIP(t, filepath.Join(dir, "server.pem"), config.DefaultMgmtBridgeIP), + "server cert must carry the canonical mgmt-bridge IP SAN") }) t.Run("MissingCA", func(t *testing.T) { diff --git a/spinifex/config/config.go b/spinifex/config/config.go index 4fc9796c4..8f8a1c9c9 100644 --- a/spinifex/config/config.go +++ b/spinifex/config/config.go @@ -27,6 +27,13 @@ const ( DefaultAWSInternalSuffix = "spinifex.internal" ) +// DefaultMgmtBridgeIP is the canonical br-mgmt host address the control plane +// advertises: the EKS gateway URL, predastore endpoint, and node-group userdata +// all target it. Kept in sync with setup-ovn.sh MGMT_CIDR. Server certs must +// always carry this as a SAN so publish succeeds even when br-mgmt happens to be +// down at cert-generation time (interface enumeration would otherwise miss it). +const DefaultMgmtBridgeIP = "10.15.8.1" + // AWSConfig holds cluster-wide AWS-parity settings shared across services. // Region scopes the default AWS region; InternalSuffix is the internal DNS // suffix used to build service endpoints (e.g. ecr.{region}.{suffix}).