enforce site-scoped effective TargetedInstanceCreation on instance, machine, and vpc actions

Author: hwadekar-nv

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

@@ -861,6 +861,18 @@ func (cih CreateInstanceHandler) Handle(c echo.Context) error {
 				return cutil.NewAPIError(http.StatusBadRequest, fmt.Sprintf("Machine specified in request does not belong to Site: %s", site.Name), nil)
 			}
 
+			// Site-scoped gate: the targetedInstanceCreation capability must be
+			// effective for this Machine's Site (ceiling + per-site opt-in).
+			enabledForSite, derr := common.EffectiveTargetedInstanceCreation(ctx, tx, cih.dbSession, tenant, machine.SiteID)
+			if derr != nil {
+				logger.Error().Err(derr).Msg("error checking effective targeted instance creation for Machine's Site")
+				return cutil.NewAPIError(http.StatusInternalServerError, "Failed to verify capability for Machine's Site", nil)
+			}
+			if !enabledForSite {
+				logger.Warn().Msg("tenant does not have targeted instance creation enabled for the Machine's Site")
+				return cutil.NewAPIError(http.StatusForbidden, "Tenant does not have capability to create Instances using specific Machine ID for this Site", nil)
+			}
+
 			// Validate Machine availability. Note: allowUnhealthyMachine also bypasses
 			// the Ready status check, not just health - consider renaming the parameter later.
 			if apiRequest.AllowUnhealthyMachine != nil {
@@ -4741,16 +4753,37 @@ func (dih DeleteInstanceHandler) Handle(c echo.Context) error {
 		// requires the tenant to carry the TargetedInstanceCreation
 		// capability. By the time `ToProto` runs the request is safe
 		// to trust.
+		// Prepare the delete/release request workflow object
+		releaseInstanceRequest := apiRequest.ToProto(instance)
+
+		// This is for enhanced break-fix flow:
+		if apiRequest.MachineHealthIssue != nil {
+			releaseInstanceRequest.Issue = &cwssaws.Issue{
+				Category: cwssaws.IssueCategory(model.MachineIssueCategoriesFromAPIToProtobuf[apiRequest.MachineHealthIssue.Category]),
+			}
+			if apiRequest.MachineHealthIssue.Summary != nil {
+				releaseInstanceRequest.Issue.Summary = *apiRequest.MachineHealthIssue.Summary
+			}
+			if apiRequest.MachineHealthIssue.Details != nil {
+				releaseInstanceRequest.Issue.Details = *apiRequest.MachineHealthIssue.Details
+			}
+		}
+
+		// if caller attempt to set IsRepairTenant then the tenant must have the
+		// targetedInstanceCreation capability effective for the Instance's Site.
 		if apiRequest.IsRepairTenant != nil && *apiRequest.IsRepairTenant {
-			if !common.TenantHasTargetedInstanceCreation(instance.Tenant) {
-				logger.Warn().Msg("tenant does not have capability to set IsRepairTenant")
+			enabledForSite, derr := common.EffectiveTargetedInstanceCreation(ctx, tx, dih.dbSession, instance.Tenant, instance.SiteID)
+			if derr != nil {
+				logger.Error().Err(derr).Msg("error checking effective targeted instance creation for Instance's Site")
+				return cutil.NewAPIError(http.StatusInternalServerError, "Failed to verify capability for Instance's Site", nil)
+			}
+			if !enabledForSite {
+				logger.Warn().Msg("tenant does not have capability to set IsRepairTenant for the Instance's Site")
 				return cutil.NewAPIError(http.StatusForbidden, "Tenant does not have capability to set IsRepairTenant", nil)
 			}
 		}
 
-		// Prepare the delete/release request workflow object
-		releaseInstanceRequest := apiRequest.ToProto(instance)
-
+		// Trigger Site workflow to delete instance
 		workflowOptions := temporalClient.StartWorkflowOptions{
 			ID:                       "instance-delete-" + instance.ID.String(),
 			TaskQueue:                queue.SiteTaskQueue,

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

@@ -132,6 +132,9 @@ func testInstanceSetupSchema(t *testing.T, dbSession *cdb.Session) {
 	// create NetworkSecurityGroup table
 	err = dbSession.DB.ResetModel(context.Background(), (*cdbm.NetworkSecurityGroup)(nil))
 	assert.Nil(t, err)
+	// create TenantSiteCapabilityAssociation table
+	err = dbSession.DB.ResetModel(context.Background(), (*cdbm.TenantSiteCapabilityAssociation)(nil))
+	assert.Nil(t, err)
 }
 
 func testInstanceSiteBuildInfrastructureProvider(t *testing.T, dbSession *cdb.Session, name string, org string, user *cdbm.User) *cdbm.InfrastructureProvider {
@@ -692,6 +695,11 @@ func TestCreateInstanceHandler_Handle(t *testing.T) {
 	ts1 := testBuildTenantSiteAssociation(t, dbSession, tnOrg, tn1.ID, st1.ID, tnu1.ID)
 	assert.NotNil(t, ts1)
 
+	// Enable the TargetedInstanceCreation capability for the privileged tenant on its sites
+	// so the site-scoped effective check passes for targeted (MachineID) instance creation.
+	cdbm.TestBuildTenantSiteCapabilityAssociation(t, dbSession, tn1, st1, true, tnu1)
+	cdbm.TestBuildTenantSiteCapabilityAssociation(t, dbSession, tn1, st2, true, tnu1)
+
 	al1 := testInstanceSiteBuildAllocation(t, dbSession, st1, tn1, "test-allocation-1", ipu)
 	assert.NotNil(t, al1)
 

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

@@ -757,8 +757,13 @@ func (umh UpdateMachineHandler) Handle(c echo.Context) error {
 
 	// Validate if Tenant is allowed to update Machine
 	if tenant != nil {
-		// Check if Tenant is privileged
-		if common.TenantHasTargetedInstanceCreation(tenant) {
+		// Check if the capability is effective for the Machine's Site (ceiling + per-site opt-in)
+		enabledForSite, serr := common.EffectiveTargetedInstanceCreation(ctx, nil, umh.dbSession, tenant, machine.SiteID)
+		if serr != nil {
+			logger.Error().Err(serr).Msg("error checking effective targeted instance creation for Machine's Site")
+			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Error verifying capability for Machine's Site, DB error", nil)
+		}
+		if enabledForSite {
 			// Check if privileged Tenant has an account with Infrastructure Provider
 			taDAO := cdbm.NewTenantAccountDAO(umh.dbSession)
 			_, taCount, serr := taDAO.GetAll(ctx, nil, cdbm.TenantAccountFilterInput{

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

@@ -1632,6 +1632,9 @@ func TestMachineHandler_Update(t *testing.T) {
 	tenant2 := testMachineBuildTenant(t, dbSession, tnOrg2, "testTenant2")
 	_ = testMachineUpdateTenantCapability(t, dbSession, tenant2)
 	_ = common.TestBuildTenantAccount(t, dbSession, ip, &tenant2.ID, tnOrg2, cdbm.TenantAccountStatusReady, tnu2)
+	// Enable the TargetedInstanceCreation capability for the privileged tenant on the Machine's Site
+	// so the site-scoped effective check passes during Machine update.
+	cdbm.TestBuildTenantSiteCapabilityAssociation(t, dbSession, tenant2, site, true, tnu2)
 
 	instanceType1 := testMachineBuildInstanceType(t, dbSession, ip, site, "testInstanceType1")
 	instanceType2 := testMachineBuildInstanceType(t, dbSession, ip, site, "testInstanceType2")

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

@@ -1283,14 +1283,47 @@ func IsTenant(ctx context.Context, logger zerolog.Logger, dbSession *cdb.Session
 // "ceiling"). It is nil-safe so callers don't have to repeat the
 // tenant/Config nil checks.
 //
-// This is the single chokepoint for the tenant-level capability read. A
-// later phase will introduce a site-scoped EffectiveTargetedInstanceCreation
-// that composes this ceiling with per-site association rows and the provider
-// gate; until then this preserves the existing tenant-global behavior.
+// This is the single chokepoint for the tenant-level (ceiling) capability read.
+// It still governs broad discovery/listing paths (e.g. expanding a privileged
+// tenant's visible Sites). Site-scoped capability *actions* should instead use
+// EffectiveTargetedInstanceCreation, which layers the per-site opt-in on top of
+// this ceiling.
 func TenantHasTargetedInstanceCreation(tenant *cdbm.Tenant) bool {
 	return tenant != nil && tenant.Config != nil && tenant.Config.TargetedInstanceCreation
 }
 
+// EffectiveTargetedInstanceCreation reports whether the TargetedInstanceCreation
+// capability is effective for a Tenant on a specific Site. It composes the
+// tenant-level ceiling (TenantHasTargetedInstanceCreation) with the per-site
+// opt-in stored in TenantSiteCapabilityAssociation:
+//
+//	effective = ceiling AND association_enabled(tenant, site)
+//
+// The provider gate (a Ready TenantAccount with the Site's Infrastructure
+// Provider) is intentionally NOT enforced here. Call sites that require it
+// already perform that check where the API mandates it, so keeping it out of
+// this helper avoids double-enforcement and keeps the helper composable.
+//
+// The default when no association row exists is strict (false). The Phase-1
+// conservative backfill materialized enabled rows for previously-implicit
+// Sites, so existing privileged tenants are unaffected.
+func EffectiveTargetedInstanceCreation(ctx context.Context, tx *cdb.Tx, dbSession *cdb.Session, tenant *cdbm.Tenant, siteID uuid.UUID) (bool, error) {
+	if !TenantHasTargetedInstanceCreation(tenant) {
+		return false, nil
+	}
+
+	tscaDAO := cdbm.NewTenantSiteCapabilityAssociationDAO(dbSession)
+	tsca, err := tscaDAO.GetByTenantIDAndSiteID(ctx, tx, tenant.ID, siteID, nil)
+	if err != nil {
+		if errors.Is(err, cdb.ErrDoesNotExist) {
+			return false, nil
+		}
+		return false, err
+	}
+
+	return tsca.TargetedInstanceCreation, nil
+}
+
 // IsProviderOrTenant ensures that user is authorized to act as a Provider Admin or/and Tenant Admin for the org.
 // if authorized it returns the tenant otherwise a relevant error.
 func IsProviderOrTenant(ctx context.Context, logger zerolog.Logger, dbSession *cdb.Session, org string, user *cdbm.User, allowViewerRole bool, requirePrivilegedTenant bool) (infrastructureProvider *cdbm.InfrastructureProvider, tenant *cdbm.Tenant, apiError *cutil.APIError) {

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

@@ -2630,3 +2630,80 @@ func TestTenantHasTargetedInstanceCreation(t *testing.T) {
 		})
 	}
 }
+
+func TestEffectiveTargetedInstanceCreation(t *testing.T) {
+	ctx := context.Background()
+	dbSession := testCommonInitDB(t)
+	defer dbSession.Close()
+
+	testCommonSetupSchema(t, dbSession)
+	err := dbSession.DB.ResetModel(ctx, (*cdbm.TenantSiteCapabilityAssociation)(nil))
+	assert.Nil(t, err)
+
+	org := "test-eff-org"
+	user := testCommonBuildUser(t, dbSession, uuid.NewString(), []string{org}, []string{authz.TenantAdminRole})
+	ip := testCommonBuildInfrastructureProvider(t, dbSession, "Test Provider", org, user)
+	site := testCommonBuildSite(t, dbSession, ip, "Test Site", user)
+
+	// Tenant with the ceiling enabled.
+	tnDAO := cdbm.NewTenantDAO(dbSession)
+	tenantWithCeiling, err := tnDAO.CreateFromParams(ctx, nil, "ceiling-tenant", cdb.GetStrPtr("Ceiling Tenant"), org, nil,
+		&cdbm.TenantConfig{TargetedInstanceCreation: true}, user)
+	assert.Nil(t, err)
+	// Tenant without the ceiling.
+	tenantNoCeiling := testCommonBuildTenant(t, dbSession, "no-ceiling-tenant", org+"-2", user)
+
+	// Association row for the ceiling tenant on the site, enabled.
+	cdbm.TestBuildTenantSiteCapabilityAssociation(t, dbSession, tenantWithCeiling, site, true, user)
+	// A disabled association on a second site.
+	site2 := testCommonBuildSite(t, dbSession, ip, "Test Site 2", user)
+	cdbm.TestBuildTenantSiteCapabilityAssociation(t, dbSession, tenantWithCeiling, site2, false, user)
+	// A site with no association row at all.
+	site3 := testCommonBuildSite(t, dbSession, ip, "Test Site 3", user)
+
+	tests := []struct {
+		name     string
+		tenant   *cdbm.Tenant
+		siteID   uuid.UUID
+		expected bool
+	}{
+		{
+			name:     "nil tenant",
+			tenant:   nil,
+			siteID:   site.ID,
+			expected: false,
+		},
+		{
+			name:     "ceiling disabled",
+			tenant:   tenantNoCeiling,
+			siteID:   site.ID,
+			expected: false,
+		},
+		{
+			name:     "ceiling enabled and association enabled",
+			tenant:   tenantWithCeiling,
+			siteID:   site.ID,
+			expected: true,
+		},
+		{
+			name:     "ceiling enabled but association disabled",
+			tenant:   tenantWithCeiling,
+			siteID:   site2.ID,
+			expected: false,
+		},
+		{
+			name:     "ceiling enabled but no association row",
+			tenant:   tenantWithCeiling,
+			siteID:   site3.ID,
+			expected: false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got, gerr := EffectiveTargetedInstanceCreation(ctx, nil, dbSession, tc.tenant, tc.siteID)
+			assert.Nil(t, gerr)
+			assert.Equal(t, tc.expected, got)
+		})
+	}
+}

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

@@ -238,8 +238,14 @@ func (cvh CreateVPCHandler) Handle(c echo.Context) error {
 	var routingProfile *string
 	if apiRequest.RoutingProfile != nil {
 		// For now, we gate on TargetedInstanceCreation permission,
-		// Which implies a "privileged tenant"
-		if !common.TenantHasTargetedInstanceCreation(tenant) {
+		// Which implies a "privileged tenant". The capability must be effective
+		// for the VPC's Site (ceiling + per-site opt-in).
+		enabledForSite, derr := common.EffectiveTargetedInstanceCreation(ctx, nil, cvh.dbSession, tenant, site.ID)
+		if derr != nil {
+			logger.Error().Err(derr).Msg("error checking effective targeted instance creation for Site")
+			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to verify privileges for Site", nil)
+		}
+		if !enabledForSite {
 			logger.Warn().Msg("tenant does not have sufficient privileges to set `routingProfile`")
 			return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Tenant does not have sufficient privileges to set `routingProfile`", nil)
 		}

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

@@ -91,6 +91,9 @@ func testVPCSetupSchema(t *testing.T, dbSession *cdb.Session) {
 	// create VPC table
 	err = dbSession.DB.ResetModel(context.Background(), (*cdbm.Vpc)(nil))
 	assert.Nil(t, err)
+	// create TenantSiteCapabilityAssociation table
+	err = dbSession.DB.ResetModel(context.Background(), (*cdbm.TenantSiteCapabilityAssociation)(nil))
+	assert.Nil(t, err)
 }
 
 func testVPCSiteBuildInfrastructureProvider(t *testing.T, dbSession *cdb.Session, name string, org string, user *cdbm.User) *cdbm.InfrastructureProvider {
@@ -359,6 +362,12 @@ func TestCreateVPCHandler_Handle(t *testing.T) {
 	ts1t3 := testBuildTenantSiteAssociation(t, dbSession, tnOrg3, tn3.ID, st1.ID, tnu3.ID)
 	assert.NotNil(t, ts1t3)
 
+	// Enable the TargetedInstanceCreation capability for the privileged tenant on its sites
+	// so the site-scoped effective check passes for routingProfile.
+	cdbm.TestBuildTenantSiteCapabilityAssociation(t, dbSession, tn, st1, true, tnu)
+	cdbm.TestBuildTenantSiteCapabilityAssociation(t, dbSession, tn, st2, true, tnu)
+	cdbm.TestBuildTenantSiteCapabilityAssociation(t, dbSession, tn, st3, true, tnu)
+
 	// NSG for tenant 1 on site 1
 	nsgTenant1Site1 := testBuildNetworkSecurityGroup(t, dbSession, "test-nsg-1", tn, st1, cdbm.NetworkSecurityGroupStatusReady)
 	assert.NotNil(t, nsgTenant1Site1)