Skip to content

Commit ffd8c7b

Browse files
committed
Fixed failing test cases
1 parent 48b362d commit ffd8c7b

9 files changed

Lines changed: 55 additions & 10 deletions

File tree

rest-api/api/pkg/api/handler/dpureprovision_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func TestReprovisionMachineDpuHandlerAllowsPrivilegedTenant(t *testing.T) {
6060
require.NoError(t, err)
6161
provider, err := cdbm.NewInfrastructureProviderDAO(fixture.DBSession).GetByID(ctx, nil, machine.InfrastructureProviderID, []string{})
6262
require.NoError(t, err)
63-
common.TestBuildTenantAccount(t, fixture.DBSession, provider, &tenant.ID, tenant.Org, cdbm.TenantAccountStatusReady, tenantUser)
63+
common.TestBuildTenantAccountWithTargetedInstanceCreation(t, fixture.DBSession, provider, &tenant.ID, tenant.Org, cdbm.TenantAccountStatusReady, tenantUser)
6464

6565
fixture.Org = tenantOrg
6666
fixture.User = tenantUser

rest-api/api/pkg/api/handler/expectedmachine.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -314,13 +314,19 @@ func (gaemh GetAllExpectedMachineHandler) Handle(c echo.Context) error {
314314
return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve current user", nil)
315315
}
316316

317-
// ensure our user is a provider for the org
318-
infrastructureProvider, tenant, apiError := common.IsProviderOrTenant(ctx, logger, gaemh.dbSession, org, dbUser, true, true)
317+
// ensure our user is a provider or tenant for the org. We do not request the
318+
// privileged-tenant pre-gate (requirePrivilegedTenant=false): a Tenant's
319+
// visibility is scoped below to the Sites it has TenantSite access to (and
320+
// only when TargetedInstanceCreation is enabled), so a non-privileged Tenant
321+
// receives an empty list rather than a spurious 403.
322+
infrastructureProvider, tenant, apiError := common.IsProviderOrTenant(ctx, logger, gaemh.dbSession, org, dbUser, true, false)
319323
if apiError != nil {
320324
return cutil.NewAPIErrorResponse(c, apiError.Code, apiError.Message, apiError.Data)
321325
}
322326

323-
filterInput := cdbm.ExpectedMachineFilterInput{}
327+
// Initialize SiteIDs to a non-nil empty slice so an unscoped caller (e.g. a
328+
// non-privileged Tenant) matches no Sites instead of every Site.
329+
filterInput := cdbm.ExpectedMachineFilterInput{SiteIDs: []uuid.UUID{}}
324330

325331
if infrastructureProvider != nil {
326332
// Get all Sites for the org's Infrastructure Provider

rest-api/api/pkg/api/handler/expectedmachine_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1547,14 +1547,16 @@ func TestTenantWithTargetedInstanceCreationCapability(t *testing.T) {
15471547
_, err = dbSession.DB.NewInsert().Model(tenantUser).Exec(ctx)
15481548
assert.Nil(t, err)
15491549

1550-
// Create TenantAccount linking tenant to infrastructure provider
1550+
// Create TenantAccount linking tenant to infrastructure provider. Privilege
1551+
// (TargetedInstanceCreation) is resolved from the Ready TenantAccount config.
15511552
tenantAccount := &cdbm.TenantAccount{
15521553
ID: uuid.New(),
15531554
AccountNumber: "TA-12345",
15541555
TenantID: &tenant.ID,
15551556
TenantOrg: tenantOrg,
15561557
InfrastructureProviderID: ip.ID,
15571558
Status: cdbm.TenantAccountStatusReady,
1559+
Config: cdbm.TenantAccountConfig{TargetedInstanceCreation: true},
15581560
CreatedBy: tenantUser.ID,
15591561
}
15601562
_, err = dbSession.DB.NewInsert().Model(tenantAccount).Exec(ctx)

rest-api/api/pkg/api/handler/instance_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,9 @@ func TestCreateInstanceHandler_Handle(t *testing.T) {
739739
tnu1 := testInstanceBuildUser(t, dbSession, "test-starfleet-id-2", tnOrg, tnOrgRoles)
740740
tn1 := testInstanceBuildTenant(t, dbSession, "test-tenant", tnOrg, tnu1)
741741
tn1 = testInstanceUpdateTenantCapability(t, dbSession, tn1)
742+
// Privilege is resolved from a Ready TenantAccount config, so enable
743+
// TargetedInstanceCreation on tn1's account with the site's provider.
744+
_ = common.TestBuildTenantAccountWithTargetedInstanceCreation(t, dbSession, ip, &tn1.ID, tnOrg, cdbm.TenantAccountStatusReady, tnu1)
742745

743746
ts1 := testBuildTenantSiteAssociation(t, dbSession, tnOrg, tn1.ID, st1.ID, tnu1.ID)
744747
assert.NotNil(t, ts1)

rest-api/api/pkg/api/handler/machine.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,15 @@ func (gamh GetAllMachineHandler) Handle(c echo.Context) error {
202202
return cutil.NewAPIErrorResponse(c, http.StatusForbidden, fmt.Sprintf("Failed to validate membership for org: %s", org), nil)
203203
}
204204

205-
// Validate role: Provider Admins or Viewers, or privileged Tenant Admins (site-effective TargetedInstanceCreation; see filters below).
206-
infrastructureProvider, tenant, apiError := common.IsProviderOrTenant(ctx, logger, gamh.dbSession, org, dbUser, true, true)
205+
// Validate role: Provider Admins or Viewers, or Tenant Admins. We do not
206+
// request the privileged-tenant pre-gate here (requirePrivilegedTenant=false):
207+
// that gate keys off a Ready TenantAccount without site context and would
208+
// reject site-privileged tenants (or those whose privilege is per-site)
209+
// before machine.SiteID is known. The site-scoped
210+
// EffectiveTargetedInstanceCreation checks below are authoritative and
211+
// filter results to the providers/sites where the capability is effective,
212+
// yielding an empty list rather than a spurious 403.
213+
infrastructureProvider, tenant, apiError := common.IsProviderOrTenant(ctx, logger, gamh.dbSession, org, dbUser, true, false)
207214
if apiError != nil {
208215
return cutil.NewAPIErrorResponse(c, apiError.Code, apiError.Message, apiError.Data)
209216
}

rest-api/api/pkg/api/handler/machinehealth_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ func configureMachineHealthFixtureForPrivilegedTenant(t *testing.T, fixture *com
169169
require.NoError(t, err)
170170
provider, err := cdbm.NewInfrastructureProviderDAO(fixture.DBSession).GetByID(ctx, nil, machine.InfrastructureProviderID, []string{})
171171
require.NoError(t, err)
172-
common.TestBuildTenantAccount(t, fixture.DBSession, provider, &tenant.ID, tenant.Org, cdbm.TenantAccountStatusReady, tenantUser)
172+
common.TestBuildTenantAccountWithTargetedInstanceCreation(t, fixture.DBSession, provider, &tenant.ID, tenant.Org, cdbm.TenantAccountStatusReady, tenantUser)
173173

174174
fixture.Org = tenantOrg
175175
fixture.User = tenantUser

rest-api/api/pkg/api/handler/sku_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,8 @@ func TestGetAllSkuHandler_Handle(t *testing.T) {
196196
TenantID: &tenantWithCapability.ID,
197197
TenantOrg: tenantOrg,
198198
InfrastructureProviderID: infraProv.ID,
199-
Status: "active",
199+
Status: cdbm.TenantAccountStatusReady,
200+
Config: cdbm.TenantAccountConfig{TargetedInstanceCreation: true},
200201
}
201202
_, err = dbSession.DB.NewInsert().Model(tenantAccount).Exec(ctx)
202203
assert.Nil(t, err)
@@ -479,7 +480,8 @@ func TestGetSkuHandler_Handle(t *testing.T) {
479480
TenantID: &tenantWithCapability.ID,
480481
TenantOrg: tenantOrg,
481482
InfrastructureProviderID: infraProv.ID,
482-
Status: "active",
483+
Status: cdbm.TenantAccountStatusReady,
484+
Config: cdbm.TenantAccountConfig{TargetedInstanceCreation: true},
483485
}
484486
_, err = dbSession.DB.NewInsert().Model(tenantAccount).Exec(ctx)
485487
assert.Nil(t, err)

rest-api/api/pkg/api/handler/util/common/testing.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,28 @@ func TestBuildTenantAccount(t *testing.T, dbSession *cdb.Session, ip *cdbm.Infra
340340
return ta
341341
}
342342

343+
// TestBuildTenantAccountWithTargetedInstanceCreation creates a test TenantAccount
344+
// with the TargetedInstanceCreation capability enabled in its config. Privilege is
345+
// resolved from a Ready TenantAccount config (see EffectiveTargetedInstanceCreation
346+
// and TenantHasTargetedInstanceCreation), so tests that exercise privileged Tenant
347+
// paths must enable it on the account rather than only on the legacy Tenant config.
348+
func TestBuildTenantAccountWithTargetedInstanceCreation(t *testing.T, dbSession *cdb.Session, ip *cdbm.InfrastructureProvider, tenantID *uuid.UUID, tenantOrg string, status string, user *cdbm.User) *cdbm.TenantAccount {
349+
taDAO := cdbm.NewTenantAccountDAO(dbSession)
350+
351+
ta, err := taDAO.Create(context.Background(), nil, cdbm.TenantAccountCreateInput{
352+
AccountNumber: GenerateAccountNumber(),
353+
TenantID: tenantID,
354+
TenantOrg: tenantOrg,
355+
InfrastructureProviderID: ip.ID,
356+
Status: status,
357+
Config: &cdbm.TenantAccountConfig{TargetedInstanceCreation: true},
358+
CreatedBy: user.ID,
359+
})
360+
assert.Nil(t, err)
361+
362+
return ta
363+
}
364+
343365
// TestBuildSite creates a test Site
344366
func TestBuildSite(t *testing.T, dbSession *cdb.Session, ip *cdbm.InfrastructureProvider, name string, user *cdbm.User) *cdbm.Site {
345367
stDAO := cdbm.NewSiteDAO(dbSession)

rest-api/api/pkg/api/handler/vpc_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,9 @@ func TestCreateVPCHandler_Handle(t *testing.T) {
326326
},
327327
})
328328
assert.NoError(t, err)
329+
// Privilege for `routingProfile` is resolved from a Ready TenantAccount
330+
// config, so enable TargetedInstanceCreation on tn's account with the provider.
331+
_ = common.TestBuildTenantAccountWithTargetedInstanceCreation(t, dbSession, ip, &tn.ID, tnOrg, cdbm.TenantAccountStatusReady, tnu)
329332

330333
tnu2 := testVPCBuildUser(t, dbSession, "test-starfleet-id-3", tnOrg, tnOrgRoles)
331334
tn2 := testVPCBuildTenant(t, dbSession, "test-tenant-2", tnOrg, tnu2)

0 commit comments

Comments
 (0)