Implement SetConfig, but in a stupid way

go/daemon/client.go

@@ -124,3 +124,13 @@ func (c *rpcClient) RemoveHost(ctx context.Context, hostName nebula.HostName) (e
 	)
 	return
 }
+
+func (c *rpcClient) SetConfig(ctx context.Context, n1 daecommon.NetworkConfig) (err error) {
+	err = c.client.Call(
+		ctx,
+		nil,
+		"SetConfig",
+		n1,
+	)
+	return
+}

go/daemon/daecommon/config.go

@@ -239,10 +239,8 @@ func LoadConfig(userConfigPath string) (Config, error) {
 // corresponds with the given alloc from the daemon config. This will panic if
 // no associated instance can be found.
 func BootstrapGarageHostForAlloc(
-	host bootstrap.Host,
+	host bootstrap.Host, alloc ConfigStorageAllocation,
-	alloc ConfigStorageAllocation,
 ) bootstrap.GarageHostInstance {
-
 	for _, inst := range host.Garage.Instances {
 		if inst.RPCPort == alloc.RPCPort {
 			return inst

go/daemon/daemon.go

@@ -457,6 +457,21 @@ func (d *Daemon) GetConfig(
 	)
 }
 
+func (d *Daemon) SetConfig(
+	ctx context.Context, config daecommon.NetworkConfig,
+) error {
+	_, err := withNetwork(
+		ctx,
+		d,
+		func(ctx context.Context, n network.Network) (struct{}, error) {
+			// TODO needs to check that public addresses aren't being shared
+			// across networks, and whatever else happens in Config.Validate.
+			return struct{}{}, n.SetConfig(ctx, config)
+		},
+	)
+	return err
+}
+
 // Shutdown blocks until all resources held or created by the daemon,
 // including child processes it has started, have been cleaned up.
 //

go/daemon/network/network.go

@@ -113,6 +113,10 @@ type RPC interface {
 
 	// GetConfig returns the configuration currently in use.
 	GetConfig(context.Context) (daecommon.NetworkConfig, error)
+
+	// SetConfig overrides the current config with the given one, adjusting any
+	// running child processes as needed.
+	SetConfig(context.Context, daecommon.NetworkConfig) error
 }
 
 // Network manages membership in a single micropelago network. Each Network
@@ -145,12 +149,19 @@ type Network interface {
 // Network instance. A nil Opts is equivalent to a zero value.
 type Opts struct {
 	ChildrenOpts *children.Opts
+
+	GarageAdminToken string // Will be randomly generated if left unset.
 }
 
 func (o *Opts) withDefaults() *Opts {
 	if o == nil {
 		o = new(Opts)
 	}
+
+	if o.GarageAdminToken == "" {
+		o.GarageAdminToken = toolkit.RandStr(32)
+	}
+
 	return o
 }
 
@@ -165,7 +176,6 @@ type network struct {
 	opts *Opts
 
 	secretsStore secrets.Store
-	garageAdminToken string
 
 	l             sync.RWMutex
 	children      *children.Children
@@ -192,7 +202,6 @@ func instatiateNetwork(
 		stateDir:      stateDir,
 		runtimeDir:    runtimeDir,
 		opts:          opts.withDefaults(),
-		garageAdminToken: toolkit.RandStr(32),
 		shutdownCh:    make(chan struct{}),
 	}
 }
@@ -427,8 +436,8 @@ func (n *network) initialize(
 		n.secretsStore,
 		n.networkConfig,
 		n.runtimeDir,
-		n.garageAdminToken,
+		n.opts.GarageAdminToken,
-		currBootstrap,
+		n.currBootstrap,
 		n.opts.ChildrenOpts,
 	)
 	if err != nil {
@@ -443,6 +452,14 @@ func (n *network) initialize(
 		return fmt.Errorf("performing post-initialization: %w", err)
 	}
 
+	// Do this now so that everything is stable before returning. This also
+	// serves a dual-purpose, as it makes sure that the PUT from the postInit
+	// above has propagated from the local garage instance, if there is one.
+	n.logger.Info(ctx, "Reloading bootstrap from network")
+	if err = n.reload(ctx); err != nil {
+		return fmt.Errorf("Reloading network bootstrap: %w", err)
+	}
+
 	// TODO annotate this context with creation params
 	ctx, cancel := context.WithCancel(context.Background())
 	n.wg.Add(1)
@@ -466,7 +483,11 @@ func (n *network) postInit(ctx context.Context) error {
 	if len(n.networkConfig.Storage.Allocations) > 0 {
 		n.logger.Info(ctx, "Applying garage layout")
 		if err := garageApplyLayout(
-			ctx, n.logger, n.networkConfig, n.garageAdminToken, n.currBootstrap,
+			ctx,
+			n.logger,
+			n.networkConfig,
+			n.opts.GarageAdminToken,
+			n.currBootstrap,
 		); err != nil {
 			return fmt.Errorf("applying garage layout: %w", err)
 		}
@@ -486,7 +507,7 @@ func (n *network) postInit(ctx context.Context) error {
 			ctx,
 			n.logger,
 			n.networkConfig,
-			n.garageAdminToken,
+			n.opts.GarageAdminToken,
 			n.currBootstrap,
 		)
 		if err != nil {
@@ -506,14 +527,6 @@ func (n *network) postInit(ctx context.Context) error {
 		return fmt.Errorf("updating host info in garage: %w", err)
 	}
 
-	// Do this now so that everything is stable before returning. This also
-	// serves a dual-purpose, as it makes sure that the PUT above has propagated
-	// from the local garage instance, if there is one.
-	n.logger.Info(ctx, "Reloading bootstrap from network")
-	if err = n.reload(ctx); err != nil {
-		return fmt.Errorf("Reloading network bootstrap: %w", err)
-	}
-
 	return nil
 }
 
@@ -622,6 +635,7 @@ func (n *network) getBootstrap() (
 }
 
 func (n *network) GetHosts(ctx context.Context) ([]bootstrap.Host, error) {
+	// TODO use withCurrBootstrap in here
 	b, err := n.getBootstrap()
 	if err != nil {
 		return nil, fmt.Errorf("retrieving bootstrap: %w", err)
@@ -887,6 +901,55 @@ func (n *network) GetConfig(context.Context) (daecommon.NetworkConfig, error) {
 	return n.networkConfig, nil
 }
 
+func (n *network) SetConfig(
+	ctx context.Context, config daecommon.NetworkConfig,
+) error {
+	newBootstrap, err := coalesceNetworkConfigAndBootstrap(
+		config, n.currBootstrap,
+	)
+	if err != nil {
+		return fmt.Errorf("combining configuration into bootstrap: %w", err)
+	}
+
+	n.l.Lock()
+	defer n.l.Unlock()
+
+	n.logger.Info(ctx, "Shutting down children")
+	n.children.Shutdown()
+
+	err = writeBootstrapToStateDir(n.stateDir.Path, newBootstrap)
+	if err != nil {
+		return fmt.Errorf("writing bootstrap to state dir: %w", err)
+	}
+
+	n.networkConfig = config
+	n.currBootstrap = newBootstrap
+
+	n.logger.Info(ctx, "Creating child processes")
+	n.children, err = children.New(
+		ctx,
+		n.logger.WithNamespace("children"),
+		n.envBinDirPath,
+		n.secretsStore,
+		n.networkConfig,
+		n.runtimeDir,
+		n.opts.GarageAdminToken,
+		n.currBootstrap,
+		n.opts.ChildrenOpts,
+	)
+	if err != nil {
+		return fmt.Errorf("creating child processes: %w", err)
+	}
+
+	n.logger.Info(ctx, "Child processes re-created")
+
+	if err := n.postInit(ctx); err != nil {
+		return fmt.Errorf("performing post-initialization: %w", err)
+	}
+
+	return nil
+}
+
 func (n *network) GetNetworkCreationParams(
 	ctx context.Context,
 ) (

go/daemon/network/network_it_test.go

@@ -1,8 +1,14 @@
 package network
 
 import (
-	"reflect"
+	"isle/bootstrap"
+	"isle/daemon/daecommon"
+	"isle/garage"
+	"isle/jsonutil"
+	"isle/nebula"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestCreate(t *testing.T) {
@@ -12,15 +18,8 @@ func TestCreate(t *testing.T) {
 	)
 
 	gotCreationParams, err := LoadCreationParams(network.stateDir)
-	if err != nil {
+	assert.NoError(t, err)
-		t.Fatalf("calling LoadCreationParams: %v", err)
+	assert.Equal(t, gotCreationParams, network.creationParams)
-	} else if network.creationParams != gotCreationParams {
-		t.Fatalf(
-			"expected CreationParams %+v, got %+v",
-			network.creationParams,
-			gotCreationParams,
-		)
-	}
 }
 
 func TestLoad(t *testing.T) {
@@ -32,9 +31,7 @@ func TestLoad(t *testing.T) {
 	)
 
 	t.Log("Shutting down network")
-	if err := network.Shutdown(); err != nil {
+	assert.NoError(t, network.Shutdown())
-		t.Fatal(err)
-	}
 
 	t.Log("Calling Load")
 	loadedNetwork, err := Load(
@@ -46,14 +43,11 @@ func TestLoad(t *testing.T) {
 		h.mkDir(t, "runtime"),
 		network.opts,
 	)
-	if err != nil {
+	assert.NoError(t, err)
-		t.Fatalf("Load failed: %v", err)
+
-	}
 	t.Cleanup(func() {
 		t.Log("Shutting down loadedNetwork")
-		if err := loadedNetwork.Shutdown(); err != nil {
+		assert.NoError(t, loadedNetwork.Shutdown())
-			t.Logf("Shutting down loadedNetwork failed: %v", err)
-		}
 	})
 }
 
@@ -65,22 +59,12 @@ func TestJoin(t *testing.T) {
 	)
 
 	primusHosts, err := primus.GetHosts(h.ctx)
-	if err != nil {
+	assert.NoError(t, err)
-		t.Fatalf("getting hosts from primus: %v", err)
-	}
 
 	secondusHosts, err := secondus.GetHosts(h.ctx)
-	if err != nil {
+	assert.NoError(t, err)
-		t.Fatalf("getting hosts from secondus: %v", err)
-	}
 
-	if !reflect.DeepEqual(primusHosts, secondusHosts) {
+	assert.Equal(t, primusHosts, secondusHosts)
-		t.Fatalf(
-			"expected primusHosts %+v to equal secondusHosts %+v",
-			primusHosts,
-			secondusHosts,
-		)
-	}
 }
 
 func TestNetwork_GetConfig(t *testing.T) {
@@ -90,17 +74,83 @@ func TestNetwork_GetConfig(t *testing.T) {
 	)
 
 	config, err := network.GetConfig(h.ctx)
-	if err != nil {
+	assert.NoError(t, err)
-		t.Fatal(err)
+
+	assert.Equal(t, config, network.networkConfig)
+}
+
+func TestNetwork_SetConfig(t *testing.T) {
+	t.Run("adding storage alloc", func(t *testing.T) {
+		var (
+			h       = newIntegrationHarness(t)
+			network = h.createNetwork(t, "primus", nil)
+		)
+
+		network.networkConfig.Storage.Allocations = append(
+			network.networkConfig.Storage.Allocations,
+			daecommon.ConfigStorageAllocation{
+				DataPath:  h.mkDir(t, "data").Path,
+				MetaPath:  h.mkDir(t, "meta").Path,
+				Capacity:  1,
+				S3APIPort: 4900,
+				RPCPort:   4901,
+				AdminPort: 4902,
+			},
+		)
+
+		assert.NoError(t, network.SetConfig(h.ctx, network.networkConfig))
+
+		// Check that the Host information was updated
+		newHosts, err := network.GetHosts(h.ctx)
+		assert.NoError(t, err)
+
+		newHostsByName := map[nebula.HostName]bootstrap.Host{}
+		for _, h := range newHosts {
+			newHostsByName[h.Name] = h
 		}
 
-	if !reflect.DeepEqual(config, network.networkConfig) {
+		newHost, ok := newHostsByName[network.hostName]
-		t.Fatalf(
+		assert.True(t, ok)
-			"Config doesn't match the one used to create the Network\n"+
+
-				"exp: %+v\n"+
+		allocs := newHost.HostConfigured.Garage.Instances
-				"got: %+v",
+		assert.Len(t, allocs, 4)
-			config,
+
+		newAlloc := allocs[3]
+		assert.NotEmpty(t, newAlloc.ID)
+		newAlloc.ID = ""
+		assert.Equal(t, bootstrap.GarageHostInstance{
+			S3APIPort: 4900,
+			RPCPort:   4901,
+		}, newAlloc)
+
+		// Check that the bootstrap file was written with the new host config
+		var storedBootstrap bootstrap.Bootstrap
+		assert.NoError(t, jsonutil.LoadFile(
+			&storedBootstrap, bootstrap.StateDirPath(network.stateDir.Path),
+		))
+		assert.Equal(t, newHostsByName, storedBootstrap.Hosts)
+
+		// Check that garage layout contains the new allocation
+		garageAdminClient := newGarageAdminClient(
+			h.logger,
 			network.networkConfig,
+			network.opts.GarageAdminToken,
+			storedBootstrap,
 		)
+
+		layout, err := garageAdminClient.GetLayout(h.ctx)
+		assert.NoError(t, err)
+
+		expPeers := make([]garage.PeerLayout, len(allocs))
+		for i := range allocs {
+			expPeers[i] = garage.PeerLayout{
+				ID:       allocs[i].ID,
+				Capacity: 1_000_000_000,
+				Zone:     string(network.hostName),
+				Tags:     []string{},
 			}
+		}
+
+		assert.ElementsMatch(t, expPeers, layout.Peers)
+	})
 }

go/daemon/network/network_it_util_test.go

@@ -17,6 +17,8 @@ import (
 	"testing"
 
 	"dev.mediocregopher.com/mediocre-go-lib.git/mlog"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"gopkg.in/yaml.v3"
 )
 
@@ -83,9 +85,8 @@ func newIntegrationHarness(t *testing.T) *integrationHarness {
 	toolkit.MarkIntegrationTest(t)
 
 	rootDir, err := os.MkdirTemp("", "isle-network-it-test.*")
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatalf("creating root temp dir: %v", err)
+
-	}
 	t.Logf("Temporary test directory: %q", rootDir)
 
 	t.Cleanup(func() {
@@ -95,14 +96,14 @@ func newIntegrationHarness(t *testing.T) *integrationHarness {
 		}
 
 		t.Logf("Deleting temp directory %q", rootDir)
-		if err := os.RemoveAll(rootDir); err != nil {
+		assert.NoError(t, os.RemoveAll(rootDir))
-			t.Errorf("failed to remove %q: %v", rootDir, err)
-		}
 	})
 
 	return &integrationHarness{
 		ctx: context.Background(),
-		logger:  mlog.NewTestLogger(t),
+		logger: mlog.NewLogger(&mlog.LoggerOpts{
+			MessageHandler: mlog.NewTestMessageHandler(t),
+		}),
 		rootDir: toolkit.Dir{Path: rootDir},
 	}
 }
@@ -112,9 +113,7 @@ func (h *integrationHarness) mkDir(t *testing.T, name string) toolkit.Dir {
 
 	t.Logf("Creating directory %q", fullName)
 	d, err := h.rootDir.MkChildDir(fullName, false)
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatalf("creating %q: %v", fullName, err)
-	}
 
 	return d
 }
@@ -147,9 +146,7 @@ func (h *integrationHarness) mkNetworkConfig(
 	}
 
 	allocsJSON, err := json.Marshal(allocs)
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatalf("marshaling allocs: %v", err)
-	}
 
 	return mustParseNetworkConfigf(`
         vpn:
@@ -174,13 +171,10 @@ func (h *integrationHarness) mkChildrenOpts(
 	)
 
 	childrenLogFile, err := os.Create(childrenLogFilePath)
-	if err != nil {
+	require.NoError(t, err)
-		t.Fatalf("creating %q: %v", childrenLogFilePath, err)
+
-	}
 	t.Cleanup(func() {
-		if err := childrenLogFile.Close(); err != nil {
+		assert.NoError(t, err)
-			t.Errorf("closing %q: %v", childrenLogFilePath, err)
-		}
 	})
 
 	if os.Getenv("ISLE_INTEGRATION_TEST_CHILDREN_LOG_STDOUT") == "" {
@@ -218,6 +212,7 @@ func (o *createNetworkOpts) withDefaults() *createNetworkOpts {
 type integrationHarnessNetwork struct {
 	Network
 
+	hostName             nebula.HostName
 	creationParams       bootstrap.CreationParams
 	networkConfig        daecommon.NetworkConfig
 	stateDir, runtimeDir toolkit.Dir
@@ -246,6 +241,7 @@ func (h *integrationHarness) createNetwork(
 
 		networkOpts = &Opts{
 			ChildrenOpts:     h.mkChildrenOpts(t, runtimeDir),
+			GarageAdminToken: "admin_token",
 		}
 	)
 
@@ -276,6 +272,7 @@ func (h *integrationHarness) createNetwork(
 
 	return integrationHarnessNetwork{
 		network,
+		hostName,
 		opts.creationParams,
 		networkConfig,
 		stateDir,
@@ -306,6 +303,7 @@ func (h *integrationHarness) joinNetwork(
 		hostName      = nebula.HostName(hostNameStr)
 		networkOpts   = &Opts{
 			ChildrenOpts:     h.mkChildrenOpts(t, runtimeDir),
+			GarageAdminToken: "admin_token",
 		}
 	)
 
@@ -341,6 +339,7 @@ func (h *integrationHarness) joinNetwork(
 
 	return integrationHarnessNetwork{
 		joinedNetwork,
+		hostName,
 		network.creationParams,
 		networkConfig,
 		stateDir,

go/garage/admin_client.go

@@ -273,10 +273,23 @@ func (c *AdminClient) GrantBucketPermissions(
 // PeerLayout describes the properties of a garage peer in the context of the
 // layout of the cluster.
 type PeerLayout struct {
-	ID       string
+	ID       string   `json:"id"`
-	Capacity int // Gb (SI units)
+	Capacity int      `json:"capacity"` // Gb (SI units)
-	Zone     string
+	Zone     string   `json:"zone"`
-	Tags     []string
+	Tags     []string `json:"tags"`
+}
+
+// ClusterLayout describes the layout of the cluster as a whole.
+type ClusterLayout struct {
+	Peers []PeerLayout `json:"roles"`
+}
+
+// GetLayout returns the currently applied ClusterLayout.
+func (c *AdminClient) GetLayout(ctx context.Context) (ClusterLayout, error) {
+	// https://garagehq.deuxfleurs.fr/api/garage-admin-v1.html#tag/Layout/operation/GetLayout
+	var res ClusterLayout
+	err := c.do(ctx, &res, "GET", "/v1/layout", nil)
+	return res, err
 }
 
 // ApplyLayout modifies the layout of the garage cluster. Only layout of the
@@ -284,21 +297,9 @@ type PeerLayout struct {
 func (c *AdminClient) ApplyLayout(
 	ctx context.Context, peers []PeerLayout,
 ) error {
-	type peerLayout struct {
-		ID       string   `json:"id"`
-		Capacity int      `json:"capacity"`
-		Zone     string   `json:"zone"`
-		Tags     []string `json:"tags"`
-	}
-
 	{
 		// https://garagehq.deuxfleurs.fr/api/garage-admin-v1.html#tag/Layout/operation/ApplyLayout
-		clusterLayout := make([]peerLayout, len(peers))
+		err := c.do(ctx, nil, "POST", "/v1/layout", peers)
-		for i := range peers {
-			clusterLayout[i] = peerLayout(peers[i])
-		}
-
-		err := c.do(ctx, nil, "POST", "/v1/layout", clusterLayout)
 		if err != nil {
 			return fmt.Errorf("staging layout changes: %w", err)
 		}
@@ -307,7 +308,7 @@ func (c *AdminClient) ApplyLayout(
 	// https://garagehq.deuxfleurs.fr/api/garage-admin-v1.html#tag/Layout/operation/GetLayout
 	var clusterLayout struct {
 		Version           int          `json:"version"`
-		StagedRoleChanges []peerLayout `json:"stagedRoleChanges"`
+		StagedRoleChanges []PeerLayout `json:"stagedRoleChanges"`
 	}
 
 	if err := c.do(ctx, &clusterLayout, "GET", "/v1/layout", nil); err != nil {

go/go.mod

@@ -10,12 +10,14 @@ require (
 	github.com/minio/minio-go/v7 v7.0.28
 	github.com/slackhq/nebula v1.6.1
 	github.com/spf13/pflag v1.0.5
+	github.com/stretchr/testify v1.9.0
 	github.com/tv42/httpunix v0.0.0-20191220191345-2ba4b9c3382c
 	golang.org/x/exp v0.0.0-20240613232115-7f521ea00fb8
 	gopkg.in/yaml.v3 v3.0.1
 )
 
 require (
+	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/dustin/go-humanize v1.0.0 // indirect
 	github.com/google/uuid v1.1.1 // indirect
 	github.com/gopherjs/gopherjs v1.17.2 // indirect
@@ -28,6 +30,7 @@ require (
 	github.com/mitchellh/go-homedir v1.1.0 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/rs/xid v1.2.1 // indirect
 	github.com/sirupsen/logrus v1.8.1 // indirect
 	github.com/smartystreets/assertions v1.13.0 // indirect

go/go.sum

@@ -1,9 +1,5 @@
 code.betamike.com/micropelago/pmux v0.0.0-20240719134913-f5fce902e8c4 h1:n4pGP12kgdH5kCTF4TZYpOjWuAR/zZLpM9j3neeVMEk=
 code.betamike.com/micropelago/pmux v0.0.0-20240719134913-f5fce902e8c4/go.mod h1:WlEWacLREVfIQl1IlBjKzuDgL56DFRvyl7YiL5gGP4w=
-dev.mediocregopher.com/mediocre-go-lib.git v0.0.0-20240511135822-4ab1176672d7 h1:wKQ3bXzG+KQDtRAN/xaRZ4aQtJe1pccleG6V43MvFxw=
-dev.mediocregopher.com/mediocre-go-lib.git v0.0.0-20240511135822-4ab1176672d7/go.mod h1:nP+AtQWrc3k5qq5y3ABiBLkOfUPlk/FO9fpTFpF+jgs=
-dev.mediocregopher.com/mediocre-go-lib.git v0.0.0-20241023133804-cedd67cf2652 h1:onyZbHYR1GJJOVd8iRtwVpgrowukWNl3EA0gWYRxixs=
-dev.mediocregopher.com/mediocre-go-lib.git v0.0.0-20241023133804-cedd67cf2652/go.mod h1:nP+AtQWrc3k5qq5y3ABiBLkOfUPlk/FO9fpTFpF+jgs=
 dev.mediocregopher.com/mediocre-go-lib.git v0.0.0-20241023182613-55984cdf5233 h1:Ea4HixNfDNDPh7zMngPpEeDf8gpociSPEROBFBedqIY=
 dev.mediocregopher.com/mediocre-go-lib.git v0.0.0-20241023182613-55984cdf5233/go.mod h1:nP+AtQWrc3k5qq5y3ABiBLkOfUPlk/FO9fpTFpF+jgs=
 github.com/adrg/xdg v0.4.0 h1:RzRqFcjH4nE5C6oTAxhBtoE2IRyjBSa62SCbyPidvls=
@@ -69,8 +65,8 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
-github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY=
+github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
-github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 github.com/tv42/httpunix v0.0.0-20191220191345-2ba4b9c3382c h1:u6SKchux2yDvFQnDHS3lPnIRmfVJ5Sxy3ao2SIdysLQ=
 github.com/tv42/httpunix v0.0.0-20191220191345-2ba4b9c3382c/go.mod h1:hzIxponao9Kjc7aWznkXaL4U4TWaDSs8zcsY4Ka08nM=
 golang.org/x/crypto v0.0.0-20220331220935-ae2d96664a29 h1:tkVvjkPTB7pnW3jnid7kNyAMPVWllTNOf/qKDze4p9o=

go/nebula/certificate.go

@@ -1,13 +1,35 @@
 package nebula
 
-import "github.com/slackhq/nebula/cert"
+import (
+	"fmt"
+
+	"github.com/slackhq/nebula/cert"
+)
 
 // Certificate wraps a NebulaCertificate to provide convenient (and consistent)
-// text (un)marshaling methods.
+// text (un)marshaling methods as well as normalization for equality checking.
 type Certificate struct {
 	inner cert.NebulaCertificate
 }
 
+// NewCertificate returns a Certificate wrapping the given one.
+func NewCertificate(inner cert.NebulaCertificate) (Certificate, error) {
+	// normalize the inner cert by marshaling to and unmarshaling from the PEM.
+	// This allows equality checking in tests to work between certs which have
+	// never been written to disk and those which have.
+	b, err := inner.MarshalToPEM()
+	if err != nil {
+		return Certificate{}, fmt.Errorf("marshaling to PEM: %w", err)
+	}
+
+	normInner, _, err := cert.UnmarshalNebulaCertificateFromPEM(b)
+	if err != nil {
+		return Certificate{}, fmt.Errorf("unmarshaling from PEM: %w", err)
+	}
+
+	return Certificate{inner: *normInner}, nil
+}
+
 // Unwrap returns the wrapped NebulaCertificate type.
 func (c Certificate) Unwrap() *cert.NebulaCertificate {
 	return &c.inner

go/nebula/nebula.go

@@ -90,7 +90,12 @@ func NewHostCert(
 		return Certificate{}, fmt.Errorf("signing host cert with ca.key: %w", err)
 	}
 
-	return Certificate{hostCert}, nil
+	c, err := NewCertificate(hostCert)
+	if err != nil {
+		return Certificate{}, fmt.Errorf("wrapping cert: %w", err)
+	}
+
+	return c, nil
 }
 
 // NewHostCredentials generates a new key/cert for a nebula host using the CA
@@ -136,7 +141,7 @@ func NewCACredentials(domain string, subnet IPNet) (CACredentials, error) {
 		expireAt                      = now.Add(2 * 365 * 24 * time.Hour)
 	)
 
-	caCert := cert.NebulaCertificate{
+	caCertInner := cert.NebulaCertificate{
 		Details: cert.NebulaCertificateDetails{
 			Name:      fmt.Sprintf("%s isle root cert", domain),
 			Subnets:   []*net.IPNet{(*net.IPNet)(&subnet)},
@@ -147,13 +152,18 @@ func NewCACredentials(domain string, subnet IPNet) (CACredentials, error) {
 		},
 	}
 
-	if err := signCert(&caCert, signingPrivKey); err != nil {
+	if err := signCert(&caCertInner, signingPrivKey); err != nil {
 		return CACredentials{}, fmt.Errorf("signing caCert: %w", err)
 	}
 
+	caCert, err := NewCertificate(caCertInner)
+	if err != nil {
+		return CACredentials{}, fmt.Errorf("wrapping caCert: %w", err)
+	}
+
 	return CACredentials{
 		Public: CAPublicCredentials{
-			Cert:       Certificate{caCert},
+			Cert:       caCert,
 			SigningKey: signingPubKey,
 		},
 		SigningPrivateKey: signingPrivKey,