gsadmin/spinifex
1
0
Fork 0
mirror of https://github.com/mulgadc/spinifex.git synced 2026-10-19 19:01:42 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: remove pre-Phase4 volume tenant fallback

Browse Source 
Simplify all 6 tenant-check sites in volume handlers from
`if accountID != "" && tenantID != "" && tenantID != accountID`
to `if tenantID != accountID`. No pre-Phase4 volumes with empty
TenantID exist in any environment — this code was unreachable.
Remove vol-legacy test fixture and empty-accountID-sees-all assertion.
This commit is contained in:
Julian Sommer
2026-03-25 09:59:42 +10:00
parent bdf0522895
commit ae316dba47
2 changed files with 9 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spinifex/handlers/ec2/volume/service_impl.go
+6 -9
View File
@@ -266,8 +266,7 @@ func (s *VolumeServiceImpl) DescribeVolumes(input *ec2.DescribeVolumesInput, acc
} }
// Skip volumes not owned by the caller's account. // Skip volumes not owned by the caller's account.
// Pre-Phase4 volumes (empty TenantID) are visible to all accounts. if result.tenantID != accountID {
if accountID != "" && result.tenantID != "" && result.tenantID != accountID {
continue continue
} }
@@ -303,7 +302,7 @@ func (s *VolumeServiceImpl) DescribeVolumeStatus(input *ec2.DescribeVolumeStatus
return nil, errors.New(awserrors.ErrorInvalidVolumeNotFound) return nil, errors.New(awserrors.ErrorInvalidVolumeNotFound)
} }
// Skip volumes not owned by the caller's account // Skip volumes not owned by the caller's account
if accountID != "" && tenantID != "" && tenantID != accountID { if tenantID != accountID {
return nil, errors.New(awserrors.ErrorInvalidVolumeNotFound) return nil, errors.New(awserrors.ErrorInvalidVolumeNotFound)
} }
statusItems = append(statusItems, item) statusItems = append(statusItems, item)
@@ -326,8 +325,7 @@ func (s *VolumeServiceImpl) DescribeVolumeStatus(input *ec2.DescribeVolumeStatus
} }
// Skip volumes not owned by the caller's account. // Skip volumes not owned by the caller's account.
// Pre-Phase4 volumes (empty TenantID) are visible to all accounts. if tenantID != accountID {
if accountID != "" && tenantID != "" && tenantID != accountID {
continue continue
} }
@@ -433,8 +431,7 @@ func (s *VolumeServiceImpl) fetchVolumesByIDs(volumeIDs []*string, accountID str
} }
// Skip volumes not owned by the caller's account. // Skip volumes not owned by the caller's account.
// Pre-Phase4 volumes (empty TenantID) are visible to all accounts. if result.tenantID != accountID {
if accountID != "" && result.tenantID != "" && result.tenantID != accountID {
return return
} }
@@ -669,7 +666,7 @@ func (s *VolumeServiceImpl) ModifyVolume(input *ec2.ModifyVolumeInput, accountID
} }
// Verify caller owns this volume // Verify caller owns this volume
if accountID != "" && cfg.VolumeMetadata.TenantID != "" && cfg.VolumeMetadata.TenantID != accountID { if cfg.VolumeMetadata.TenantID != accountID {
return nil, errors.New(awserrors.ErrorInvalidVolumeNotFound) return nil, errors.New(awserrors.ErrorInvalidVolumeNotFound)
} }
@@ -758,7 +755,7 @@ func (s *VolumeServiceImpl) DeleteVolume(input *ec2.DeleteVolumeInput, accountID
} }
// Verify caller owns this volume // Verify caller owns this volume
if accountID != "" && cfg.VolumeMetadata.TenantID != "" && cfg.VolumeMetadata.TenantID != accountID { if cfg.VolumeMetadata.TenantID != accountID {
return nil, errors.New(awserrors.ErrorInvalidVolumeNotFound) return nil, errors.New(awserrors.ErrorInvalidVolumeNotFound)
} }
spinifex/handlers/ec2/volume/service_impl_test.go
+3 -13
View File
@@ -815,18 +815,15 @@ func TestDescribeVolumes_AccountScoping_SlowPath(t *testing.T) {
store := objectstore.NewMemoryObjectStore() store := objectstore.NewMemoryObjectStore()
svc := newTestVolumeServiceWithStore("ap-southeast-2a", store) svc := newTestVolumeServiceWithStore("ap-southeast-2a", store)
// Create volumes for two different accounts and one pre-Phase4 (empty TenantID) // Create volumes for two different accounts
createVolumeInStoreWithMeta(t, store, "vol-acctA", viperblock.VolumeMetadata{ createVolumeInStoreWithMeta(t, store, "vol-acctA", viperblock.VolumeMetadata{
VolumeID: "vol-acctA", SizeGiB: 10, State: "available", TenantID: "111111111111", VolumeID: "vol-acctA", SizeGiB: 10, State: "available", TenantID: "111111111111",
}) })
createVolumeInStoreWithMeta(t, store, "vol-acctB", viperblock.VolumeMetadata{ createVolumeInStoreWithMeta(t, store, "vol-acctB", viperblock.VolumeMetadata{
VolumeID: "vol-acctB", SizeGiB: 10, State: "available", TenantID: "222222222222", VolumeID: "vol-acctB", SizeGiB: 10, State: "available", TenantID: "222222222222",
}) })
createVolumeInStoreWithMeta(t, store, "vol-legacy", viperblock.VolumeMetadata{
VolumeID: "vol-legacy", SizeGiB: 10, State: "available", TenantID: "",
})
// Account A sees its own volume + legacy (empty TenantID) // Account A sees only its own volume
output, err := svc.DescribeVolumes(&ec2.DescribeVolumesInput{}, "111111111111") output, err := svc.DescribeVolumes(&ec2.DescribeVolumesInput{}, "111111111111")
require.NoError(t, err) require.NoError(t, err)
ids := map[string]bool{} ids := map[string]bool{}
@@ -835,9 +832,8 @@ func TestDescribeVolumes_AccountScoping_SlowPath(t *testing.T) {
} }
assert.True(t, ids["vol-acctA"], "Account A should see its own volume") assert.True(t, ids["vol-acctA"], "Account A should see its own volume")
assert.False(t, ids["vol-acctB"], "Account A should NOT see Account B's volume") assert.False(t, ids["vol-acctB"], "Account A should NOT see Account B's volume")
assert.True(t, ids["vol-legacy"], "Account A should see pre-Phase4 volumes")
// Account B sees its own volume + legacy // Account B sees only its own volume
output, err = svc.DescribeVolumes(&ec2.DescribeVolumesInput{}, "222222222222") output, err = svc.DescribeVolumes(&ec2.DescribeVolumesInput{}, "222222222222")
require.NoError(t, err) require.NoError(t, err)
ids = map[string]bool{} ids = map[string]bool{}
@@ -846,12 +842,6 @@ func TestDescribeVolumes_AccountScoping_SlowPath(t *testing.T) {
} }
assert.True(t, ids["vol-acctB"], "Account B should see its own volume") assert.True(t, ids["vol-acctB"], "Account B should see its own volume")
assert.False(t, ids["vol-acctA"], "Account B should NOT see Account A's volume") assert.False(t, ids["vol-acctA"], "Account B should NOT see Account A's volume")
assert.True(t, ids["vol-legacy"], "Account B should see pre-Phase4 volumes")
// Empty accountID sees all volumes (backward compat)
output, err = svc.DescribeVolumes(&ec2.DescribeVolumesInput{}, "")
require.NoError(t, err)
assert.Len(t, output.Volumes, 3)
} }
func TestDescribeVolumes_AccountScoping_FastPath(t *testing.T) { func TestDescribeVolumes_AccountScoping_FastPath(t *testing.T) {