Fix CA ordering so fresh installs boot with etcd mTLS

cli/commands/server.go

@@ -364,6 +364,17 @@ func Server(ctx *Context, opts serverconfig.CLIFlags) error {
 		if labs.DistributedRunners() {
 			ctx.Log.Info("setting up etcd mTLS for distributed runners")
 
+			// SetupEtcdTLS reads the CA from disk and requires it to already
+			// exist. On a fresh install the coordinator's own create-if-missing
+			// LoadCA doesn't run until much later (Coordinator.Start), so ensure
+			// the CA is materialized here first. Otherwise the first boot fails
+			// "CA certificate not found" and crash-loops until an out-of-band
+			// auth generate creates it.
+			if _, err := coordinate.EnsureCA(ctx.Log, cfg.Server.GetDataPath()); err != nil {
+				ctx.Log.Error("failed to ensure CA for etcd TLS", "error", err)
+				return err
+			}
+
 			var err error
 			etcdTLSSetup, err = coordinate.SetupEtcdTLS(ctx.Log, cfg.Server.GetDataPath(), cfg.TLS.AdditionalNames, ipSet.RawIPs())
 			if err != nil {

components/coordinate/coordinate.go

@@ -455,59 +455,81 @@ func validateAPICertificate(cert *x509.Certificate, expectedNames []string, expe
 	return nil
 }
 
-func (c *Coordinator) LoadCA(ctx context.Context) error {
-	cert := filepath.Join(c.DataPath, "server", "ca.crt")
-	keyPath := filepath.Join(c.DataPath, "server", "ca.key")
+// EnsureCA makes sure the server CA certificate and key exist on disk under
+// <dataPath>/server, creating them if they're missing, and returns the loaded
+// authority. It's safe to call before the coordinator starts.
+//
+// This is what lets the server bootstrap its own etcd mTLS: SetupEtcdTLS reads
+// the CA from disk and requires it to already exist, so the CA must be
+// materialized before that runs. Historically the only create-if-missing path
+// was Coordinator.LoadCA, which doesn't run until Coordinator.Start, well after
+// the etcd TLS block, so a fresh install with distributed runners enabled had
+// no CA yet. Calling EnsureCA early closes that gap.
+func EnsureCA(log *slog.Logger, dataPath string) (*caauth.Authority, error) {
+	cert := filepath.Join(dataPath, "server", "ca.crt")
+	keyPath := filepath.Join(dataPath, "server", "ca.key")
 
-	if data, err := os.ReadFile(cert); err == nil {
-		c.Log.Info("loading existing CA", "path", cert)
+	data, err := os.ReadFile(cert)
+	switch {
+	case err == nil:
+		log.Info("loading existing CA", "path", cert)
 
 		key, err := os.ReadFile(keyPath)
 		if err != nil {
-			return fmt.Errorf("missing key for CA: %w", err)
+			return nil, fmt.Errorf("missing key for CA: %w", err)
 		}
 
 		ca, err := caauth.LoadFromPEM(data, key)
 		if err != nil {
-			return fmt.Errorf("failed to load CA: %w", err)
+			return nil, fmt.Errorf("failed to load CA: %w", err)
 		}
 
-		c.authority = ca
-	} else {
-		c.Log.Info("generating new CA", "path", cert)
+		return ca, nil
+	case !errors.Is(err, os.ErrNotExist):
+		// The CA cert exists but couldn't be read (permissions, transient IO,
+		// etc). Bail rather than fall through and regenerate, which would
+		// overwrite a valid CA and invalidate every cert it ever issued.
+		return nil, fmt.Errorf("failed to read CA cert at %s: %w", cert, err)
+	}
 
-		ca, err := caauth.New(caauth.Options{
-			CommonName:   "miren-server",
-			Organization: "miren",
-			ValidFor:     10 * year,
-		})
-		if err != nil {
-			return fmt.Errorf("failed to generate CA: %w", err)
-		}
+	log.Info("generating new CA", "path", cert)
 
-		err = os.MkdirAll(filepath.Dir(cert), 0755)
-		if err != nil {
-			return fmt.Errorf("failed to create CA directory: %w", err)
-		}
+	ca, err := caauth.New(caauth.Options{
+		CommonName:   "miren-server",
+		Organization: "miren",
+		ValidFor:     10 * year,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate CA: %w", err)
+	}
 
-		cd, kd, err := ca.ExportPEM()
-		if err != nil {
-			return fmt.Errorf("failed to export CA: %w", err)
-		}
+	if err := os.MkdirAll(filepath.Dir(cert), 0755); err != nil {
+		return nil, fmt.Errorf("failed to create CA directory: %w", err)
+	}
 
-		err = os.WriteFile(cert, cd, 0644)
-		if err != nil {
-			return fmt.Errorf("failed to write CA cert: %w", err)
-		}
+	cd, kd, err := ca.ExportPEM()
+	if err != nil {
+		return nil, fmt.Errorf("failed to export CA: %w", err)
+	}
 
-		err = os.WriteFile(keyPath, kd, 0600)
-		if err != nil {
-			return fmt.Errorf("failed to write CA key: %w", err)
-		}
+	if err := os.WriteFile(cert, cd, 0644); err != nil {
+		return nil, fmt.Errorf("failed to write CA cert: %w", err)
+	}
 
-		c.authority = ca
+	if err := os.WriteFile(keyPath, kd, 0600); err != nil {
+		return nil, fmt.Errorf("failed to write CA key: %w", err)
+	}
+
+	return ca, nil
+}
+
+func (c *Coordinator) LoadCA(ctx context.Context) error {
+	ca, err := EnsureCA(c.Log, c.DataPath)
+	if err != nil {
+		return err
 	}
 
+	c.authority = ca
 	return nil
 }
 

components/coordinate/etcd_tls_test.go

@@ -0,0 +1,125 @@
+package coordinate_test
+
+import (
+	"crypto/x509"
+	"encoding/pem"
+	"log/slog"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"miren.dev/runtime/components/coordinate"
+)
+
+// TestEtcdTLSColdBoot pins the ordering invariant behind MIR-1464: a fresh data
+// dir has no CA, so SetupEtcdTLS must fail until the CA is materialized, and
+// EnsureCA (which the server now calls before the etcd TLS block) is what makes
+// it succeed. This reproduces a fresh `server install` with distributed runners
+// enabled, where nothing has bootstrapped a CA yet.
+func TestEtcdTLSColdBoot(t *testing.T) {
+	r := require.New(t)
+	log := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelDebug}))
+
+	dataPath := t.TempDir()
+
+	// Cold boot: no CA on disk yet, so SetupEtcdTLS can't proceed.
+	_, err := coordinate.SetupEtcdTLS(log, dataPath, nil, nil)
+	r.Error(err, "SetupEtcdTLS should fail before a CA exists")
+
+	// EnsureCA is the early bootstrap step. It should create the CA on disk.
+	ca, err := coordinate.EnsureCA(log, dataPath)
+	r.NoError(err)
+	r.NotNil(ca)
+
+	caCert := filepath.Join(dataPath, "server", "ca.crt")
+	caKey := filepath.Join(dataPath, "server", "ca.key")
+	r.FileExists(caCert)
+	r.FileExists(caKey)
+
+	// With the CA in place, etcd TLS setup now succeeds.
+	result, err := coordinate.SetupEtcdTLS(log, dataPath, nil, nil)
+	r.NoError(err, "SetupEtcdTLS should succeed once EnsureCA has run")
+	r.NotNil(result)
+
+	// The issued etcd server cert must chain to the CA we created.
+	roots := x509.NewCertPool()
+	caPEM, err := os.ReadFile(result.CAFile)
+	r.NoError(err)
+	r.True(roots.AppendCertsFromPEM(caPEM), "CA cert should be parseable")
+
+	serverCert := parseCert(t, filepath.Join(result.CertsDir, "server.crt"))
+	_, err = serverCert.Verify(x509.VerifyOptions{
+		Roots:     roots,
+		DNSName:   "localhost",
+		KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+	})
+	r.NoError(err, "etcd server cert should chain to the CA and cover localhost")
+
+	// The coordinator client cert should chain to the same CA.
+	clientCert := parseCert(t, result.ClientCertFile)
+	_, err = clientCert.Verify(x509.VerifyOptions{
+		Roots:     roots,
+		KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
+	})
+	r.NoError(err, "etcd client cert should chain to the CA")
+}
+
+// TestEnsureCAIdempotent confirms EnsureCA reuses an existing CA rather than
+// regenerating it, so calling it early (server boot) and again later
+// (auth generate, Coordinator.Start) all see the same authority.
+func TestEnsureCAIdempotent(t *testing.T) {
+	r := require.New(t)
+	log := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelDebug}))
+
+	dataPath := t.TempDir()
+
+	_, err := coordinate.EnsureCA(log, dataPath)
+	r.NoError(err)
+
+	caCert := filepath.Join(dataPath, "server", "ca.crt")
+	first, err := os.ReadFile(caCert)
+	r.NoError(err)
+
+	_, err = coordinate.EnsureCA(log, dataPath)
+	r.NoError(err)
+
+	second, err := os.ReadFile(caCert)
+	r.NoError(err)
+	r.Equal(first, second, "EnsureCA should reuse the existing CA on subsequent calls")
+}
+
+// TestEnsureCADoesNotOverwriteUnreadableCA guards against silently regenerating
+// a CA when its cert exists but can't be read. A transient read error must not
+// be mistaken for absence, since regenerating would clobber a valid CA and
+// invalidate every cert it issued. We simulate an unreadable cert by making
+// ca.crt a directory, which yields a non-ErrNotExist read error on every OS and
+// even as root (unlike chmod 0000).
+func TestEnsureCADoesNotOverwriteUnreadableCA(t *testing.T) {
+	r := require.New(t)
+	log := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelDebug}))
+
+	dataPath := t.TempDir()
+	caCert := filepath.Join(dataPath, "server", "ca.crt")
+	r.NoError(os.MkdirAll(caCert, 0755))
+
+	_, err := coordinate.EnsureCA(log, dataPath)
+	r.Error(err, "EnsureCA should refuse to proceed when the CA cert is unreadable")
+
+	// The path must be left untouched: still a directory, no key written next to it.
+	info, statErr := os.Stat(caCert)
+	r.NoError(statErr)
+	r.True(info.IsDir(), "EnsureCA must not overwrite the existing path")
+	r.NoFileExists(filepath.Join(dataPath, "server", "ca.key"))
+}
+
+func parseCert(t *testing.T, path string) *x509.Certificate {
+	t.Helper()
+	pemBytes, err := os.ReadFile(path)
+	require.NoError(t, err)
+	block, _ := pem.Decode(pemBytes)
+	require.NotNil(t, block, "expected a PEM block in %s", path)
+	cert, err := x509.ParseCertificate(block.Bytes)
+	require.NoError(t, err)
+	return cert
+}