diff --git a/storageprovider/dospaces/dospaces_coverage_test.go b/storageprovider/dospaces/dospaces_coverage_test.go new file mode 100644 index 0000000..ed5a31f --- /dev/null +++ b/storageprovider/dospaces/dospaces_coverage_test.go @@ -0,0 +1,90 @@ +package dospaces_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "instant.dev/common/storageprovider" + "instant.dev/common/storageprovider/dospaces" +) + +// TestGetters_AndCustomerEndpointFallbacks exercises every simple accessor on +// the Provider plus the fallback paths inside customerEndpointURL() that the +// happy-path tests don't reach. +// +// Why each branch matters: +// +// - MasterAccessKey / MasterSecretKey / Endpoint / Bucket / Region / UseTLS +// are read by the api at boot when building the presigner for broker mode; +// if any returns "" we ship an unsigned URL into a customer's logs. +// - PublicURL() with publicURL=="" must fall through customerEndpointURL() +// so we never return an empty string (broker-mode would silently 404). +// - customerEndpointURL() with UseTLS=false should prepend "http://", and +// when the endpoint already carries a scheme (some operators wire +// `https://...`) it should pass through unmodified. +func TestGetters_AndCustomerEndpointFallbacks(t *testing.T) { + t.Run("all_getters_return_configured_values", func(t *testing.T) { + p, err := dospaces.New(storageprovider.Config{ + Endpoint: "nyc3.digitaloceanspaces.com", + PublicURL: "https://s3.instanode.dev", + Region: "nyc3", + Bucket: "instant-shared", + MasterKey: "MK", + MasterSecret: "MS", + UseTLS: true, + }) + require.NoError(t, err) + prov := p.(*dospaces.Provider) + + assert.Equal(t, "MK", prov.MasterAccessKey()) + assert.Equal(t, "MS", prov.MasterSecretKey()) + assert.Equal(t, "nyc3.digitaloceanspaces.com", prov.Endpoint()) + assert.Equal(t, "https://s3.instanode.dev", prov.PublicURL()) + assert.Equal(t, "instant-shared", prov.Bucket()) + assert.Equal(t, "nyc3", prov.Region()) + assert.True(t, prov.UseTLS()) + }) + + t.Run("public_url_falls_back_to_scheme_prefixed_endpoint_no_tls", func(t *testing.T) { + p, err := dospaces.New(storageprovider.Config{ + Endpoint: "minio.local:9000", + MasterKey: "MK", + MasterSecret: "MS", + UseTLS: false, + }) + require.NoError(t, err) + prov := p.(*dospaces.Provider) + + // PublicURL must fall back to customerEndpointURL ("http://minio.local:9000"). + assert.Equal(t, "http://minio.local:9000", prov.PublicURL()) + assert.False(t, prov.UseTLS()) + }) + + t.Run("public_url_falls_back_to_scheme_prefixed_endpoint_tls", func(t *testing.T) { + p, err := dospaces.New(storageprovider.Config{ + Endpoint: "nyc3.digitaloceanspaces.com", + MasterKey: "MK", + MasterSecret: "MS", + UseTLS: true, + }) + require.NoError(t, err) + prov := p.(*dospaces.Provider) + assert.Equal(t, "https://nyc3.digitaloceanspaces.com", prov.PublicURL()) + }) + + t.Run("endpoint_with_existing_scheme_passes_through", func(t *testing.T) { + // Some operator wires OBJECT_STORE_ENDPOINT=https://foo.bar — the + // customerEndpointURL must NOT double-prefix. + p, err := dospaces.New(storageprovider.Config{ + Endpoint: "https://already-schemed.example", + MasterKey: "MK", + MasterSecret: "MS", + UseTLS: false, + }) + require.NoError(t, err) + prov := p.(*dospaces.Provider) + assert.Equal(t, "https://already-schemed.example", prov.PublicURL()) + }) +} diff --git a/storageprovider/s3/s3_coverage_test.go b/storageprovider/s3/s3_coverage_test.go new file mode 100644 index 0000000..09e5c55 --- /dev/null +++ b/storageprovider/s3/s3_coverage_test.go @@ -0,0 +1,216 @@ +package s3_test + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "instant.dev/common/storageprovider" + "instant.dev/common/storageprovider/s3" +) + +// TestS3_New_RequiresMasterCreds — sts:AssumeRole requires platform creds. +func TestS3_New_RequiresMasterCreds(t *testing.T) { + _, err := s3.New(storageprovider.Config{ + AWSRoleARN: "arn:aws:iam::123:role/x", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "OBJECT_STORE_ACCESS_KEY") +} + +// TestS3_Getters_AndCustomerEndpointFallbacks covers Master*/Endpoint/Bucket/ +// Region/PublicURL/Name + the two customerEndpointURL branches (already-schemed +// and bare-host). +func TestS3_Getters_AndCustomerEndpointFallbacks(t *testing.T) { + t.Run("bare_host_gets_https_prefix", func(t *testing.T) { + raw, err := s3.New(storageprovider.Config{ + AWSRoleARN: "arn:aws:iam::123:role/x", + MasterKey: "MK", + MasterSecret: "MS", + }) + require.NoError(t, err) + p := raw.(*s3.Provider) + + assert.Equal(t, "MK", p.MasterAccessKey()) + assert.Equal(t, "MS", p.MasterSecretKey()) + assert.Equal(t, "s3.us-east-1.amazonaws.com", p.Endpoint()) + assert.Equal(t, "instant-shared", p.Bucket()) + assert.Equal(t, "us-east-1", p.Region()) + assert.Equal(t, "s3", p.Name()) + assert.Equal(t, "https://s3.us-east-1.amazonaws.com", p.PublicURL()) + }) + + t.Run("endpoint_already_schemed_passes_through", func(t *testing.T) { + raw, err := s3.New(storageprovider.Config{ + Endpoint: "https://custom.example", + AWSRoleARN: "arn:aws:iam::123:role/x", + MasterKey: "MK", + MasterSecret: "MS", + }) + require.NoError(t, err) + p := raw.(*s3.Provider) + assert.Equal(t, "https://custom.example", p.PublicURL()) + }) + + t.Run("public_url_set_overrides_endpoint", func(t *testing.T) { + raw, err := s3.New(storageprovider.Config{ + Endpoint: "s3.amazonaws.com", + PublicURL: "https://cdn.example.dev", + AWSRoleARN: "arn:aws:iam::123:role/x", + MasterKey: "MK", + MasterSecret: "MS", + }) + require.NoError(t, err) + p := raw.(*s3.Provider) + assert.Equal(t, "https://cdn.example.dev", p.PublicURL()) + }) + + t.Run("region_defaults_and_bucket_defaults_apply", func(t *testing.T) { + raw, err := s3.New(storageprovider.Config{ + AWSRoleARN: "arn:aws:iam::123:role/x", + MasterKey: "MK", + MasterSecret: "MS", + }) + require.NoError(t, err) + p := raw.(*s3.Provider) + assert.Equal(t, "us-east-1", p.Region()) + assert.Equal(t, "instant-shared", p.Bucket()) + }) +} + +// TestS3_RevokeTenantCredentials_IsNoOp — STS sessions can't be revoked, the +// method must return nil regardless of input. +func TestS3_RevokeTenantCredentials_IsNoOp(t *testing.T) { + raw, err := s3.New(storageprovider.Config{ + AWSRoleARN: "arn:aws:iam::123:role/x", + MasterKey: "MK", + MasterSecret: "MS", + }) + require.NoError(t, err) + p := raw.(*s3.Provider) + + require.NoError(t, p.RevokeTenantCredentials(context.Background(), "")) + require.NoError(t, p.RevokeTenantCredentials(context.Background(), "any-key")) +} + +// TestS3_IssueTenantCredentials_DefaultsTTLAndPrefix covers: +// - TTL==0 falls through to the 1-hour default +// - Empty Prefix falls back to ResourceToken +// - Empty Bucket falls back to provider default +func TestS3_IssueTenantCredentials_DefaultsTTLAndPrefix(t *testing.T) { + raw, err := s3.New(storageprovider.Config{ + Region: "us-west-2", + Bucket: "my-default", + AWSRoleARN: "arn:aws:iam::123:role/x", + MasterKey: "MK", + MasterSecret: "MS", + }) + require.NoError(t, err) + p := raw.(*s3.Provider) + + var captured s3.AssumeRoleInput + p.SetAssumeRoleFunc(func(ctx context.Context, in s3.AssumeRoleInput) (*s3.AssumeRoleOutput, error) { + captured = in + return &s3.AssumeRoleOutput{ + AccessKeyID: "AK", + SecretAccessKey: "SK", + SessionToken: "TOK", + Expiration: time.Now().Add(time.Hour), + }, nil + }) + + creds, err := p.IssueTenantCredentials(context.Background(), storageprovider.IssueRequest{ + ResourceToken: "tok-ABC", + // No Prefix, no Bucket, no TTL — exercises every fallback line. + }) + require.NoError(t, err) + assert.Equal(t, "tok-ABC", creds.Prefix, "prefix must default to resource token") + assert.Equal(t, "my-default", creds.Bucket, "bucket must default to provider default") + assert.Equal(t, int32(3600), captured.DurationSeconds, "TTL==0 must default to 1h") +} + +// TestS3_SafeSessionName covers the trimming + min-length fallback. RoleSessionName +// is constrained by AWS to [\w+=,.@-] and length 2..64; safeSessionName must drop +// disallowed characters AND backfill "instanode-x" when the result is too short +// to be a valid session name. +func TestS3_SafeSessionName(t *testing.T) { + raw, err := s3.New(storageprovider.Config{ + AWSRoleARN: "arn:aws:iam::123:role/x", + MasterKey: "MK", + MasterSecret: "MS", + }) + require.NoError(t, err) + p := raw.(*s3.Provider) + + cases := []struct { + name string + token string + expected string + }{ + { + name: "all_disallowed_chars_falls_back_to_instanode_x", + token: "!!!@@@###", + expected: "instanode-instanode-x", + }, + { + name: "long_token_truncated_to_56", + token: strings.Repeat("a", 200), + expected: "instanode-" + strings.Repeat("a", 56), + }, + { + name: "digits_underscores_dashes_kept", + token: "tenant-01_abc", + expected: "instanode-tenant-01_abc", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + var captured s3.AssumeRoleInput + p.SetAssumeRoleFunc(func(ctx context.Context, in s3.AssumeRoleInput) (*s3.AssumeRoleOutput, error) { + captured = in + return &s3.AssumeRoleOutput{ + AccessKeyID: "AK", + SecretAccessKey: "SK", + SessionToken: "TOK", + Expiration: time.Now().Add(15 * time.Minute), + }, nil + }) + _, err := p.IssueTenantCredentials(context.Background(), storageprovider.IssueRequest{ + ResourceToken: c.token, + TTL: 15 * time.Minute, + }) + require.NoError(t, err) + assert.Equal(t, c.expected, captured.RoleSessionName) + }) + } +} + +// TestS3_AssumeRoleError_Propagates verifies the error from a failing +// AssumeRole call surfaces to the caller rather than being swallowed into +// a "best-effort" silent return. +func TestS3_AssumeRoleError_Propagates(t *testing.T) { + raw, err := s3.New(storageprovider.Config{ + AWSRoleARN: "arn:aws:iam::123:role/x", + MasterKey: "MK", + MasterSecret: "MS", + }) + require.NoError(t, err) + p := raw.(*s3.Provider) + p.SetAssumeRoleFunc(func(ctx context.Context, in s3.AssumeRoleInput) (*s3.AssumeRoleOutput, error) { + return nil, assertError("aws-sdk: throttled") + }) + _, err = p.IssueTenantCredentials(context.Background(), storageprovider.IssueRequest{ + ResourceToken: "x", + TTL: 15 * time.Minute, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "throttled") +} + +type assertError string + +func (a assertError) Error() string { return string(a) }