Fix suggested by cr

Author: hwadekar-nv

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

@@ -347,24 +347,14 @@ func (gaemh GetAllExpectedMachineHandler) Handle(c echo.Context) error {
 	}
 
 	if tenant != nil {
-		privileged, err := common.TenantHasTargetedInstanceCreation(ctx, nil, gaemh.dbSession, tenant, nil)
+		// Get the Sites where the Tenant has effective TargetedInstanceCreation,
+		// honoring per-site TenantSite.config overrides.
+		privilegedSiteIDs, err := common.GetPrivilegedAccessSiteIDsForTenant(ctx, nil, gaemh.dbSession, tenant)
 		if err != nil {
-			logger.Error().Err(err).Msg("error resolving TargetedInstanceCreation for Tenant")
+			logger.Error().Err(err).Msg("error resolving privileged Site access for Tenant")
 			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to resolve Tenant capability due to DB error", nil)
 		}
-		if privileged {
-			// Get IDs for all Sites the privileged Tenant has an access with
-			tenantSiteDAO := cdbm.NewTenantSiteDAO(gaemh.dbSession)
-			tenantSites, _, err := tenantSiteDAO.GetAll(ctx, nil, cdbm.TenantSiteFilterInput{TenantIDs: []uuid.UUID{tenant.ID}}, paginator.PageInput{Limit: cutil.GetPtr(math.MaxInt)}, nil)
-			if err != nil {
-				logger.Error().Err(err).Msg("error retrieving Tenant Sites from DB")
-				return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant Sites due to DB error", nil)
-			}
-
-			for _, tenantSite := range tenantSites {
-				filterInput.SiteIDs = append(filterInput.SiteIDs, tenantSite.SiteID)
-			}
-		}
+		filterInput.SiteIDs = append(filterInput.SiteIDs, privilegedSiteIDs...)
 	}
 
 	siteIDStr := c.QueryParam("siteId")
@@ -388,7 +378,7 @@ func (gaemh GetAllExpectedMachineHandler) Handle(c echo.Context) error {
 		}
 
 		if !isAssociated && tenant != nil {
-			// We've already populated the filter with Providers the Tenant has an account with
+			// filterInput.SiteIDs already holds the Tenant's effective privileged Sites.
 			isAssociated = slices.Contains(filterInput.SiteIDs, site.ID)
 		}
 

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

@@ -886,43 +886,15 @@ func (gash GetAllSiteHandler) Handle(c echo.Context) error {
 			tsMap[ts.SiteID] = &ts
 		}
 
-		// If Tenant has TargetedInstanceCreation on any Ready TenantAccount,
-		// also retrieve all Sites from Providers where that capability is enabled.
-		privileged, serr := common.TenantHasTargetedInstanceCreation(ctx, nil, gash.dbSession, tenant, nil)
+		// Add the Sites where the Tenant has effective TargetedInstanceCreation,
+		// honoring per-site TenantSite.config overrides.
+		privilegedSiteIDs, serr := common.GetPrivilegedAccessSiteIDsForTenant(ctx, nil, gash.dbSession, tenant)
 		if serr != nil {
-			logger.Error().Err(serr).Msg("error resolving TargetedInstanceCreation for Tenant")
+			logger.Error().Err(serr).Msg("error resolving privileged Site access for Tenant")
 			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to resolve Tenant capability, DB error", nil)
 		}
-		if privileged {
-			taDAO := cdbm.NewTenantAccountDAO(gash.dbSession)
-			tas, _, serr := taDAO.GetAll(ctx, nil, cdbm.TenantAccountFilterInput{
-				TenantIDs: []uuid.UUID{tenant.ID},
-				Statuses:  []string{cdbm.TenantAccountStatusReady},
-			}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
-			if serr != nil {
-				logger.Error().Err(serr).Msg("error retrieving Tenant Accounts for privileged Tenant")
-				return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant Accounts", nil)
-			}
-
-			if len(tas) > 0 {
-				providerIDs := make([]uuid.UUID, 0, len(tas))
-				for _, ta := range tas {
-					if ta.Config.TargetedInstanceCreation {
-						providerIDs = append(providerIDs, ta.InfrastructureProviderID)
-					}
-				}
-
-				if len(providerIDs) > 0 {
-					providerSites, _, serr := stDAO.GetAll(ctx, nil, cdbm.SiteFilterInput{InfrastructureProviderIDs: providerIDs}, paginator.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
-					if serr != nil {
-						logger.Error().Err(serr).Msg("error retrieving Sites for Providers from Tenant Accounts")
-						return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Sites for one or more Providers", nil)
-					}
-					for _, site := range providerSites {
-						siteIDs.Add(site.ID)
-					}
-				}
-			}
+		for _, siteID := range privilegedSiteIDs {
+			siteIDs.Add(siteID)
 		}
 	}
 

rest-api/db/pkg/db/model/tenantsite.go

@@ -295,6 +295,12 @@ func (tssd TenantSiteSQLDAO) Update(ctx context.Context, tx *db.Tx, input Tenant
 		ID: input.TenantSiteID,
 	}
 
+	// Validate mutually exclusive inputs before applying any write so a
+	// rejected request cannot leave the row partially updated.
+	if input.Config != nil && input.RemoveTargetedInstanceCreation {
+		return nil, errors.Wrap(db.ErrInvalidParams, "Config and RemoveTargetedInstanceCreation are mutually exclusive")
+	}
+
 	updatedFields := []string{}
 
 	if input.EnableSerialConsole != nil {
@@ -312,10 +318,6 @@ func (tssd TenantSiteSQLDAO) Update(ctx context.Context, tx *db.Tx, input Tenant
 		}
 	}
 
-	if input.Config != nil && input.RemoveTargetedInstanceCreation {
-		return nil, errors.Wrap(db.ErrInvalidParams, "Config and RemoveTargetedInstanceCreation are mutually exclusive")
-	}
-
 	switch {
 	case input.Config != nil:
 		_, err := db.GetIDB(tx, tssd.dbSession).NewUpdate().

rest-api/db/pkg/db/model/tenantsite_test.go

@@ -286,7 +286,7 @@ func TestTenantSiteSQLDAO_GetAll(t *testing.T) {
 
 	sites := []*Site{}
 	siteCount := 30
-	for i := 0; i < siteCount; i++ {
+	for i := range siteCount {
 		site := TestBuildSite(t, dbSession, ip, fmt.Sprintf("test-site-%d", i), ipu)
 		sites = append(sites, site)
 		if i%2 == 0 {

rest-api/workflow/pkg/activity/dpuextensionservice/dpuextensionservice_test.go

@@ -97,7 +97,7 @@ func TestManageDpuExtensionService_UpdateDpuExtensionServicesInDB(t *testing.T)
 
 	// Build test user and tenant
 	user := util.TestBuildUser(t, dbSession, uuid.NewString(), []string{"test-org"}, []string{"ADMIN"})
-	tenant := util.TestBuildTenant(t, dbSession, "test-tenant", "test-org", user)
+	tenant := util.TestBuildTenant(t, dbSession, "test-org", "test-tenant", user)
 
 	st := util.TestBuildSite(t, dbSession, ip, "test-site", cdbm.SiteStatusRegistered, nil, user)
 	st2 := util.TestBuildSite(t, dbSession, ip, "test-site-2", cdbm.SiteStatusRegistered, nil, user)