fix: stop viperblockd discarding the configured S3 region (#677)

The viperblock start command applies --s3-host, --s3-bucket and --s3-region over
spinifex.toml whenever viper reports a non-empty value. s3-bucket and s3-region
carried non-empty defaults, so the override fired on every start even though the
service unit passes neither flag nor environment variable, and the region read
from the config was replaced with ap-southeast-2.

Any cluster deployed to another region could not launch an instance: viperblockd
signed its S3 requests with ap-southeast-2, predastore verified them against its
own region and rejected them as AuthorizationHeaderMalformed, the volume mount
failed and RunInstances ended in launch_failed. The bare-metal e2e cell, which
installs with --region bare-metal, failed every instance-backed test this way.

Default all three to empty, matching s3-host, so an unset flag leaves the
configured value alone while an explicit flag or SPINIFEX_VIPERBLOCK_* still
wins. The predastore.toml template also pinned the main bucket to a literal
ap-southeast-2 while the top-level key and the northstar bucket both rendered
the configured region; it now renders it too.

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()
 

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

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 = ""

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)
+	}
+}