diff --git a/cmd/spinifex/cmd/service.go b/cmd/spinifex/cmd/service.go index 18879d2ee..a053cf9c9 100644 --- a/cmd/spinifex/cmd/service.go +++ b/cmd/spinifex/cmd/service.go @@ -1301,9 +1301,12 @@ func init() { serviceCmd.AddCommand(viperblockCmd) + // These override spinifex.toml only when set, and the start command tests them + // for emptiness to decide that. A non-empty default would make the override + // unconditional, discarding the configured value on every start. viperblockCmd.PersistentFlags().String("s3-host", "", "Predastore (S3) host URI") - viperblockCmd.PersistentFlags().String("s3-bucket", "predastore", "Predastore (S3) bucket") - viperblockCmd.PersistentFlags().String("s3-region", "ap-southeast-2", "Predastore (S3) region") + viperblockCmd.PersistentFlags().String("s3-bucket", "", "Predastore (S3) bucket") + viperblockCmd.PersistentFlags().String("s3-region", "", "Predastore (S3) region") viperblockCmd.PersistentFlags().String("plugin-path", "/opt/spinifex/lib/nbdkit-viperblock-plugin.so", "Pathname to the nbdkit viperblockplugin") bindViperblockEnv() diff --git a/cmd/spinifex/cmd/service_bind_collision_test.go b/cmd/spinifex/cmd/service_bind_collision_test.go index ffb1eae74..890790b05 100644 --- a/cmd/spinifex/cmd/service_bind_collision_test.go +++ b/cmd/spinifex/cmd/service_bind_collision_test.go @@ -129,16 +129,19 @@ func TestViperblockFlagsLookUpTheirOwnCommand(t *testing.T) { } // TestViperblockFlagDefaultsReachViper drives the real bindViperblockEnv, so -// a lookup pointed at the wrong command fails here: an unbound flag leaves -// its default invisible and s3-bucket resolves "" rather than "predastore". +// a lookup pointed at the wrong command fails here: an unbound flag leaves its +// default invisible and plugin-path resolves "" rather than the install path. +// +// The s3-* flags override spinifex.toml and so must default empty, which leaves +// plugin-path as the only default that can prove the binding. func TestViperblockFlagDefaultsReachViper(t *testing.T) { t.Cleanup(func() { viper.Reset() }) viper.Reset() bindViperblockEnv() - assert.Equal(t, "predastore", viper.GetString("s3-bucket")) - assert.Equal(t, "ap-southeast-2", viper.GetString("s3-region")) assert.Equal(t, "/opt/spinifex/lib/nbdkit-viperblock-plugin.so", viper.GetString("plugin-path")) + assert.Empty(t, viper.GetString("s3-bucket")) + assert.Empty(t, viper.GetString("s3-region")) } // TestViperblockEnvBeatsUnchangedFlagDefault pins viper's precedence: wiring diff --git a/cmd/spinifex/cmd/templates/predastore.toml b/cmd/spinifex/cmd/templates/predastore.toml index dec28e5ac..3adee8288 100644 --- a/cmd/spinifex/cmd/templates/predastore.toml +++ b/cmd/spinifex/cmd/templates/predastore.toml @@ -66,7 +66,7 @@ role = "state-replica" [[buckets]] name = "predastore" -region = "ap-southeast-2" +region = "{{.Region}}" type = "distributed" public = false encryption = "" diff --git a/cmd/spinifex/cmd/viperblock_region_test.go b/cmd/spinifex/cmd/viperblock_region_test.go new file mode 100644 index 000000000..d9648424f --- /dev/null +++ b/cmd/spinifex/cmd/viperblock_region_test.go @@ -0,0 +1,48 @@ +package cmd + +import ( + "testing" + + toml "github.com/pelletier/go-toml/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The viperblock start command applies each of these over spinifex.toml whenever +// viper reports a non-empty value, so a non-empty flag default makes the override +// unconditional and discards the configured value on every start. +func TestViperblockConfigOverrideFlagsDefaultEmpty(t *testing.T) { + t.Parallel() + + for _, name := range []string{"s3-host", "s3-bucket", "s3-region"} { + flag := viperblockCmd.PersistentFlags().Lookup(name) + require.NotNil(t, flag, "flag %s must exist", name) + assert.Empty(t, flag.DefValue, "flag %s must default empty so an unset flag cannot clobber spinifex.toml", name) + } +} + +// predastore verifies each request's SigV4 credential scope against its own +// configured region, so a bucket pinned to a literal region breaks every client +// on a cluster deployed anywhere else. +func TestPredastoreTemplateBucketsUseConfiguredRegion(t *testing.T) { + t.Parallel() + + settings := northstarSettings() + settings.Region = "bare-metal" + content := renderTemplate(t, predastoreTomlTemplate, settings) + + var parsed struct { + Region string `toml:"region"` + Buckets []struct { + Name string `toml:"name"` + Region string `toml:"region"` + } `toml:"buckets"` + } + require.NoError(t, toml.Unmarshal([]byte(content), &parsed)) + + assert.Equal(t, "bare-metal", parsed.Region) + require.NotEmpty(t, parsed.Buckets) + for _, bucket := range parsed.Buckets { + assert.Equal(t, "bare-metal", bucket.Region, "bucket %s", bucket.Name) + } +}