Address review comments

hwadekar-nv · hwadekar-nv · commit f1269e5afa21 · 2026-06-26T08:51:07.000-07:00
diff --git a/rest-api/api/pkg/api/handler/infinibandinterface.go b/rest-api/api/pkg/api/handler/infinibandinterface.go
@@ -175,7 +175,7 @@ func (gaibih GetAllInfiniBandInterfaceHandler) Handle(c echo.Context) error {
 	// Get Tenant for this org
 	tnDAO := cdbm.NewTenantDAO(gaibih.dbSession)
 
-	tenants, err := tnDAO.GetAllByOrg(ctx, nil, org, nil)
+	tenants, _, err := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error retrieving Tenant for this org")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant", nil)
diff --git a/rest-api/api/pkg/api/handler/instance.go b/rest-api/api/pkg/api/handler/instance.go
@@ -3964,7 +3964,7 @@ func (gih GetInstanceHandler) Handle(c echo.Context) error {
 	// Get Tenant for this org
 	tnDAO := cdbm.NewTenantDAO(gih.dbSession)
 
-	tenants, err := tnDAO.GetAllByOrg(ctx, nil, org, nil)
+	tenants, _, err := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error retrieving Tenant for this org")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant", nil)
@@ -4209,7 +4209,7 @@ func (gaih GetAllInstanceHandler) Handle(c echo.Context) error {
 	// Get Tenant for this org
 	tnDAO := cdbm.NewTenantDAO(gaih.dbSession)
 
-	tenants, err := tnDAO.GetAllByOrg(ctx, nil, org, nil)
+	tenants, _, err := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error retrieving Tenant for this org")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant for org", nil)
@@ -5070,7 +5070,7 @@ func (gisdh GetInstanceStatusDetailsHandler) Handle(c echo.Context) error {
 
 	// Get Tenant for this org
 	tnDAO := cdbm.NewTenantDAO(gisdh.dbSession)
-	tenants, err := tnDAO.GetAllByOrg(ctx, nil, org, nil)
+	tenants, _, err := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error retrieving Tenant for this org")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant", nil)
diff --git a/rest-api/api/pkg/api/handler/nvlinkinterface.go b/rest-api/api/pkg/api/handler/nvlinkinterface.go
@@ -176,7 +176,7 @@ func (gaish GetAllNVLinkInterfaceHandler) Handle(c echo.Context) error {
 	// Get Tenant for this org
 	tnDAO := cdbm.NewTenantDAO(gaish.dbSession)
 
-	tenants, err := tnDAO.GetAllByOrg(ctx, nil, org, nil)
+	tenants, _, err := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error retrieving Tenant for this org")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant", nil)
diff --git a/rest-api/api/pkg/api/handler/serviceaccount.go b/rest-api/api/pkg/api/handler/serviceaccount.go
@@ -85,7 +85,7 @@ func (gcsah GetCurrentServiceAccountHandler) Handle(c echo.Context) error {
 
 	var tn *cdbm.Tenant
 
-	tns, err := tnDAO.GetAllByOrg(ctx, nil, org, nil)
+	tns, _, err := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error retrieving Tenant for this org")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant for org, DB error", nil)
diff --git a/rest-api/api/pkg/api/handler/serviceaccount_test.go b/rest-api/api/pkg/api/handler/serviceaccount_test.go
@@ -20,6 +20,8 @@ import (
 	"github.com/labstack/echo/v4"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	cutil "github.com/NVIDIA/infra-controller/rest-api/common/pkg/util"
 )
 
 func TestServiceAccountHandler_GetCurrent(t *testing.T) {
@@ -124,7 +126,7 @@ func TestServiceAccountHandler_GetCurrent(t *testing.T) {
 				ips, ipErr := ipDAO.GetAllByOrg(ctx, nil, org1, nil)
 				require.NoError(t, ipErr)
 				require.Len(t, ips, 1)
-				tns, tnErr := tnDAO.GetAllByOrg(ctx, nil, org1, nil)
+				tns, _, tnErr := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org1}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 				require.NoError(t, tnErr)
 				require.Len(t, tns, 1)
 
diff --git a/rest-api/api/pkg/api/handler/sshkey.go b/rest-api/api/pkg/api/handler/sshkey.go
@@ -418,7 +418,7 @@ func (uskh UpdateSSHKeyHandler) Handle(c echo.Context) error {
 	// Get Tenant for this org
 	tnDAO := cdbm.NewTenantDAO(uskh.dbSession)
 
-	tenants, err := tnDAO.GetAllByOrg(ctx, nil, org, nil)
+	tenants, _, err := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error retrieving Tenant for this org")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant for org", nil)
@@ -586,7 +586,7 @@ func (gskh GetSSHKeyHandler) Handle(c echo.Context) error {
 	// Get Tenant for this org
 	tnDAO := cdbm.NewTenantDAO(gskh.dbSession)
 
-	tenants, err := tnDAO.GetAllByOrg(ctx, nil, org, nil)
+	tenants, _, err := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error retrieving Tenant for this org")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant for org", nil)
@@ -706,7 +706,7 @@ func (gaskh GetAllSSHKeyHandler) Handle(c echo.Context) error {
 	// Get Tenant for this org
 	tnDAO := cdbm.NewTenantDAO(gaskh.dbSession)
 
-	tenants, err := tnDAO.GetAllByOrg(ctx, nil, org, nil)
+	tenants, _, err := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error retrieving Tenant for this org")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant for org", nil)
@@ -852,7 +852,7 @@ func (dskh DeleteSSHKeyHandler) Handle(c echo.Context) error {
 	// Get Tenant for this org
 	tnDAO := cdbm.NewTenantDAO(dskh.dbSession)
 
-	tenants, err := tnDAO.GetAllByOrg(ctx, nil, org, nil)
+	tenants, _, err := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error retrieving Tenant for this org")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant for org", nil)
diff --git a/rest-api/api/pkg/api/handler/tenant.go b/rest-api/api/pkg/api/handler/tenant.go
@@ -158,7 +158,7 @@ func (gcth GetCurrentTenantHandler) Handle(c echo.Context) error {
 
 		// Re-read inside the tx so the existence check and any create/update
 		// happen against the same locked snapshot.
-		tns, derr := tnDAO.GetAllByOrg(ctx, tx, org, nil)
+		tns, _, derr := tnDAO.GetAll(ctx, tx, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 		if derr != nil {
 			logger.Error().Err(derr).Msg("error retrieving Tenant for this org")
 			return nil, cutil.NewAPIError(http.StatusInternalServerError, "Failed to retrieve current Tenant", nil)
@@ -274,7 +274,7 @@ func (gcth GetCurrentTenantStatsHandler) Handle(c echo.Context) error {
 	// Get Tenant for this org
 	tnDAO := cdbm.NewTenantDAO(gcth.dbSession)
 
-	tns, err := tnDAO.GetAllByOrg(ctx, nil, org, nil)
+	tns, _, err := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error retrieving Tenant for this org")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant", nil)
diff --git a/rest-api/api/pkg/api/handler/tenantaccount.go b/rest-api/api/pkg/api/handler/tenantaccount.go
@@ -141,7 +141,7 @@ func (ctah CreateTenantAccountHandler) Handle(c echo.Context) error {
 		tenantOrg = &tenant.Org
 	} else {
 		tenantOrg = apiRequest.TenantOrg
-		tenants, serr := tnDAO.GetAllByOrg(ctx, nil, *tenantOrg, nil)
+		tenants, _, serr := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{*tenantOrg}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 		if serr != nil {
 			logger.Warn().Err(err).Msg("error retrieving tenant")
 			return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Failed to retrieve Tenant specified in request", nil)
diff --git a/rest-api/api/pkg/api/handler/util/common/common.go b/rest-api/api/pkg/api/handler/util/common/common.go
@@ -100,7 +100,7 @@ func GetInfrastructureProviderForOrg(ctx context.Context, tx *cdb.Tx, dbSession
 func GetTenantForOrg(ctx context.Context, tx *cdb.Tx, dbSession *cdb.Session, org string) (*cdbm.Tenant, error) {
 	tnDAO := cdbm.NewTenantDAO(dbSession)
 
-	ts, err := tnDAO.GetAllByOrg(ctx, tx, org, nil)
+	ts, _, err := tnDAO.GetAll(ctx, tx, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		return nil, err
 	}
diff --git a/rest-api/api/pkg/api/handler/vpc.go b/rest-api/api/pkg/api/handler/vpc.go
@@ -1424,7 +1424,7 @@ func (gavh GetAllVPCHandler) Handle(c echo.Context) error {
 	// Get Tenant for this org
 	tnDAO := cdbm.NewTenantDAO(gavh.dbSession)
 
-	tenants, err := tnDAO.GetAllByOrg(ctx, nil, org, nil)
+	tenants, _, err := tnDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error retrieving Tenant for this org")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant for org", nil)
diff --git a/rest-api/api/pkg/api/handler/vpcpeering.go b/rest-api/api/pkg/api/handler/vpcpeering.go
@@ -530,19 +530,21 @@ func (gavph GetAllVpcPeeringHandler) Handle(c echo.Context) error {
 	}
 
 	// Get status from query param
-	if statusStrings := qParams["status"]; len(statusStrings) != 0 {
-		gavph.tracerSpan.SetAttribute(handlerSpan, attribute.StringSlice("status", statusStrings), logger)
-		for _, status := range statusStrings {
+	qStatuses := qParams["status"]
+	if len(qStatuses) > 0 {
+		gavph.tracerSpan.SetAttribute(handlerSpan, attribute.StringSlice("status", qStatuses), logger)
+		for _, status := range qStatuses {
 			if !cdbm.VpcPeeringStatusMap[status] {
 				logger.Warn().Msg(fmt.Sprintf("invalid value in status query: %v", status))
-				return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Invalid Status value in query", nil)
+				return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Invalid Status value %v in query", status), nil)
 			}
 			filterInput.Statuses = append(filterInput.Statuses, status)
 		}
 	}
 
 	// Get vpcId from query param
-	if vpcIDStrs := qParams["vpcId"]; len(vpcIDStrs) != 0 {
+	vpcIDStrs := qParams["vpcId"]
+	if len(vpcIDStrs) > 0 {
 		gavph.tracerSpan.SetAttribute(handlerSpan, attribute.StringSlice("vpcId", vpcIDStrs), logger)
 		vpcIDs := make([]uuid.UUID, 0, len(vpcIDStrs))
 		for _, vpcIDStr := range vpcIDStrs {
@@ -579,7 +581,8 @@ func (gavph GetAllVpcPeeringHandler) Handle(c echo.Context) error {
 	}
 
 	// Get peerTenantId from query param
-	if peerTenantIDStrs := qParams["peerTenantId"]; len(peerTenantIDStrs) != 0 {
+	peerTenantIDStrs := qParams["peerTenantId"]
+	if len(peerTenantIDStrs) > 0 {
 		gavph.tracerSpan.SetAttribute(handlerSpan, attribute.StringSlice("peerTenantId", peerTenantIDStrs), logger)
 		peerTenantIDs := make([]uuid.UUID, 0, len(peerTenantIDStrs))
 		for _, peerTenantIDStr := range peerTenantIDStrs {
diff --git a/rest-api/db/pkg/db/model/tenant.go b/rest-api/db/pkg/db/model/tenant.go
@@ -129,8 +129,6 @@ type TenantDAO interface {
 	//
 	GetAll(ctx context.Context, tx *db.Tx, filter TenantFilterInput, page paginator.PageInput, includeRelations []string) ([]Tenant, int, error)
 	//
-	GetAllByOrg(ctx context.Context, tx *db.Tx, org string, includeRelations []string) ([]Tenant, error)
-	//
 	Create(ctx context.Context, tx *db.Tx, input TenantCreateInput) (*Tenant, error)
 	//
 	Update(ctx context.Context, tx *db.Tx, input TenantUpdateInput) (*Tenant, error)
@@ -228,32 +226,6 @@ func (tsd TenantSQLDAO) GetAll(ctx context.Context, tx *db.Tx, filter TenantFilt
 	return tns, pag.Total, nil
 }
 
-// GetAllByOrg returns all Tenants for an Org
-func (tsd TenantSQLDAO) GetAllByOrg(ctx context.Context, tx *db.Tx, org string, includeRelations []string) ([]Tenant, error) {
-	// Create a child span and set the attributes for current request
-	ctx, tnDAOSpan := tsd.tracerSpan.CreateChildInCurrentContext(ctx, "TenantDAO.GetAllByOrg")
-	if tnDAOSpan != nil {
-		defer tnDAOSpan.End()
-
-		tsd.tracerSpan.SetAttribute(tnDAOSpan, "org", org)
-	}
-
-	tns := []Tenant{}
-
-	query := db.GetIDB(tx, tsd.dbSession).NewSelect().Model(&tns).Where("tn.org = ?", org)
-
-	for _, relation := range includeRelations {
-		query = query.Relation(relation)
-	}
-
-	err := query.Scan(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	return tns, nil
-}
-
 // Create creates a new Tenant from the given input
 func (tsd TenantSQLDAO) Create(ctx context.Context, tx *db.Tx, input TenantCreateInput) (*Tenant, error) {
 	// Create a child span and set the attributes for current request
diff --git a/rest-api/db/pkg/db/model/tenant_test.go b/rest-api/db/pkg/db/model/tenant_test.go
@@ -207,7 +207,8 @@ func TestTenantSQLDAO_GetAllByOrg(t *testing.T) {
 			ipsd := TenantSQLDAO{
 				dbSession: tt.fields.dbSession,
 			}
-			got, err := ipsd.GetAllByOrg(tt.args.ctx, nil, tt.args.org, nil)
+
+			got, _, err := ipsd.GetAll(ctx, nil, TenantFilterInput{Orgs: []string{org}}, paginator.PageInput{Limit: cutil.GetPtr(paginator.TotalLimit)}, nil)
 			assert.NoError(t, err)
 
 			for i, tn := range tt.want {
diff --git a/rest-api/db/pkg/migrations/migrations_test.go b/rest-api/db/pkg/migrations/migrations_test.go
@@ -904,7 +904,7 @@ func Test_tenantConfigUpMigration(t *testing.T) {
 
 	// GetAll operating systems and verify
 	tnDAO := model.NewTenantDAO(dbSession)
-	tns, err := tnDAO.GetAllByOrg(context.Background(), nil, tenant1.Org, nil)
+	tns, _, err := tnDAO.GetAll(ctx, nil, model.TenantFilterInput{Orgs: []string{tenant1.Org}}, paginator.PageInput{Limit: cutil.GetPtr(paginator.TotalLimit)}, nil)
 	assert.Nil(t, err)
 	assert.Equal(t, 1, len(tns))
 	assert.Equal(t, tenant1.ID, tns[0].ID)
diff --git a/rest-api/workflow/pkg/activity/networksecuritygroup/networksecuritygroup.go b/rest-api/workflow/pkg/activity/networksecuritygroup/networksecuritygroup.go
@@ -106,7 +106,7 @@ func (mv ManageNetworkSecurityGroup) UpdateNetworkSecurityGroupsInDB(ctx context
 			// tenants, so if we see a tenant we don't know about, we'll
 			// query and cache it.
 			if !foundTenant {
-				tenants, err := tenantDAO.GetAllByOrg(ctx, nil, controllerNetworkSecurityGroup.TenantOrganizationId, nil)
+				tenants, _, err := tenantDAO.GetAll(ctx, nil, cdbm.TenantFilterInput{Orgs: []string{controllerNetworkSecurityGroup.TenantOrganizationId}}, cdbp.PageInput{Limit: cwutil.GetPtr(cdbp.TotalLimit)}, nil)
 				if err != nil {
 					slogger.Error().Err(err).Msg("failed to query for tenant ID for " + controllerNetworkSecurityGroup.TenantOrganizationId)
 					return err

Original file line number	Diff line number	Diff line change
`@@ -100,7 +100,7 @@ func GetInfrastructureProviderForOrg(ctx context.Context, tx *cdb.Tx, dbSession`
`100`	`100`	`func GetTenantForOrg(ctx context.Context, tx cdb.Tx, dbSession cdb.Session, org string) (*cdbm.Tenant, error) {`
`101`	`101`	`tnDAO := cdbm.NewTenantDAO(dbSession)`
`102`	`102`
`103`		`- ts, err := tnDAO.GetAllByOrg(ctx, tx, org, nil)`
	`103`	`+ ts, _, err := tnDAO.GetAll(ctx, tx, cdbm.TenantFilterInput{Orgs: []string{org}}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)`
`104`	`104`	`if err != nil {`
`105`	`105`	`return nil, err`
`106`	`106`	`}`