Skip to content

Commit

Permalink
Revert "ROX-12344: Add organisation name to central request" (#736)
Browse files Browse the repository at this point in the history
Revert "ROX-12344: Add organisation name to central request (#683)"

This reverts commit ab36d4f.
  • Loading branch information
dhaus67 authored Jan 17, 2023
1 parent e31c30d commit 5feb0f9
Show file tree
Hide file tree
Showing 17 changed files with 139 additions and 442 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ db/start:
.PHONY: db/start

db/migrate:
$(GO) run ./cmd/fleet-manager migrate
OCM_ENV=integration $(GO) run ./cmd/fleet-manager migrate
.PHONY: db/migrate

db/teardown:
Expand Down
7 changes: 3 additions & 4 deletions internal/dinosaur/pkg/api/dbapi/central_request_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,9 @@ type CentralRequest struct {
OwnerUserID string `json:"owner_user_id"`
// Instance-independent part of the Central's hostname. For example, this
// can be `rhacs-dev.com`, `acs-stage.rhcloud.com`, etc.
Host string `json:"host"`
OrganisationID string `json:"organisation_id" gorm:"index"`
OrganisationName string `json:"organisation_name"`
FailedReason string `json:"failed_reason"`
Host string `json:"host"`
OrganisationID string `json:"organisation_id" gorm:"index"`
FailedReason string `json:"failed_reason"`
// PlacementID field should be updated every time when a CentralRequest is assigned to an OSD cluster (even if it's the same one again)
PlacementID string `json:"placement_id"`
Central api.JSON `json:"central"` // Schema is defined by dbapi.CentralSpec
Expand Down
2 changes: 0 additions & 2 deletions internal/dinosaur/pkg/api/private/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -437,8 +437,6 @@ components:
type: string
ownerOrgId:
type: string
ownerOrgName:
type: string
issuer:
type: string
ManagedCentral_allOf_spec_uiEndpoint_tls:
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions internal/dinosaur/pkg/handlers/cloud_accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ func (h *cloudAccountsHandler) actionFunc(r *http.Request) func() (i interface{}
if err != nil {
return nil, errors.NewWithCause(errors.ErrorForbidden, err, "cannot make request without orgID claim")
}
organization, err := h.client.GetOrganisationFromExternalID(orgID)
organizationID, err := h.client.GetOrganisationIDFromExternalID(orgID)
if err != nil {
return nil, errors.OrganisationNotFound(orgID, err)
}

cloudAccounts, err := h.client.GetCustomerCloudAccounts(organization.ID(), []string{quota.RHACSMarketplaceQuotaID})
cloudAccounts, err := h.client.GetCustomerCloudAccounts(organizationID, []string{quota.RHACSMarketplaceQuotaID})
if err != nil {
return nil, errors.NewWithCause(errors.ErrorGeneral, err, "failed to fetch cloud accounts from AMS")
}
Expand Down
14 changes: 6 additions & 8 deletions internal/dinosaur/pkg/handlers/cloud_accounts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,8 @@ func TestGetSuccess(t *testing.T) {
Build()
assert.NoError(t, err)
c := ocm.ClientMock{
GetOrganisationFromExternalIDFunc: func(externalId string) (*v1.Organization, error) {
org, _ := v1.NewOrganization().ID("external-id").Build()
return org, nil
GetOrganisationIDFromExternalIDFunc: func(externalID string) (string, error) {
return "external-id", nil
},
GetCustomerCloudAccountsFunc: func(externalID string, quotaID []string) ([]*v1.CloudAccount, error) {
return []*v1.CloudAccount{
Expand Down Expand Up @@ -66,10 +65,9 @@ func TestGetSuccess(t *testing.T) {
func TestGetNoOrgId(t *testing.T) {
timesClientCalled := 0
c := ocm.ClientMock{
GetOrganisationFromExternalIDFunc: func(externalId string) (*v1.Organization, error) {
GetOrganisationIDFromExternalIDFunc: func(externalID string) (string, error) {
timesClientCalled++
org, _ := v1.NewOrganization().ID("external-id").Build()
return org, nil
return "external-id", nil
},
GetCustomerCloudAccountsFunc: func(externalID string, quotaID []string) ([]*v1.CloudAccount, error) {
timesClientCalled++
Expand Down Expand Up @@ -102,8 +100,8 @@ func TestGetNoOrgId(t *testing.T) {
func TestGetCannotGetOrganizationID(t *testing.T) {
timesClientCalled := 0
c := ocm.ClientMock{
GetOrganisationFromExternalIDFunc: func(externalId string) (*v1.Organization, error) {
return nil, errors.New("test failure")
GetOrganisationIDFromExternalIDFunc: func(externalID string) (string, error) {
return "", errors.New("test failure")
},
GetCustomerCloudAccountsFunc: func(externalID string, quotaID []string) ([]*v1.CloudAccount, error) {
timesClientCalled++
Expand Down
128 changes: 0 additions & 128 deletions internal/dinosaur/pkg/migrations/202212150000_add_organisation_name.go

This file was deleted.

34 changes: 15 additions & 19 deletions internal/dinosaur/pkg/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"

"github.com/go-gormigrate/gormigrate/v2"
"github.com/stackrox/acs-fleet-manager/pkg/client/ocm"
"github.com/stackrox/acs-fleet-manager/pkg/db"
)

Expand All @@ -25,33 +24,30 @@ import (
//
// 4. Create one function in a separate file that returns your Migration. Add that single function call
// to the end of this list.
func getMigrations(ocmConfig *ocm.OCMConfig) []*gormigrate.Migration {
return []*gormigrate.Migration{
addCentralRequest(),
addClusters(),
addLeaderLease(),
sampleMigration(),
addOwnerUserIDToCentralRequest(),
addResourcesToCentralRequest(),
addAuthConfigToCentralRequest(),
addCentralAuthLease(),
addSkipSchedulingToClusters(),
addClientOriginToCentralRequest(),
changeCentralClientOrigin(),
addCloudAccountIDToCentralRequest(),
addOrganisationNameToCentralRequest(ocmConfig),
}
var migrations = []*gormigrate.Migration{
addCentralRequest(),
addClusters(),
addLeaderLease(),
sampleMigration(),
addOwnerUserIDToCentralRequest(),
addResourcesToCentralRequest(),
addAuthConfigToCentralRequest(),
addCentralAuthLease(),
addSkipSchedulingToClusters(),
addClientOriginToCentralRequest(),
changeCentralClientOrigin(),
addCloudAccountIDToCentralRequest(),
}

// New ...
func New(dbConfig *db.DatabaseConfig, ocmConfig *ocm.OCMConfig) (*db.Migration, func(), error) {
migrations := getMigrations(ocmConfig)
func New(dbConfig *db.DatabaseConfig) (*db.Migration, func(), error) {
m, f, err := db.NewMigration(dbConfig, &gormigrate.Options{
TableName: "migrations",
IDColumnName: "id",
IDColumnSize: 255,
UseTransaction: false,
}, migrations)

if err != nil {
return m, f, fmt.Errorf("assembling database migration: %w", err)
}
Expand Down
1 change: 0 additions & 1 deletion internal/dinosaur/pkg/presenters/managedcentral.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ func (c *ManagedCentralPresenter) PresentManagedCentral(from *dbapi.CentralReque
ClientSecret: from.AuthConfig.ClientSecret, // pragma: allowlist secret
ClientOrigin: from.AuthConfig.ClientOrigin,
OwnerOrgId: from.OrganisationID,
OwnerOrgName: from.OrganisationName,
OwnerUserId: from.OwnerUserID,
Issuer: from.AuthConfig.Issuer,
},
Expand Down
18 changes: 2 additions & 16 deletions internal/dinosaur/pkg/services/dinosaur.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/stackrox/acs-fleet-manager/pkg/api"
"github.com/stackrox/acs-fleet-manager/pkg/auth"
"github.com/stackrox/acs-fleet-manager/pkg/client/aws"
"github.com/stackrox/acs-fleet-manager/pkg/client/ocm"
"github.com/stackrox/acs-fleet-manager/pkg/db"
"github.com/stackrox/acs-fleet-manager/pkg/errors"
"github.com/stackrox/acs-fleet-manager/pkg/logger"
Expand Down Expand Up @@ -122,11 +121,10 @@ type dinosaurService struct {
authService authorization.Authorization
dataplaneClusterConfig *config.DataplaneClusterConfig
clusterPlacementStrategy ClusterPlacementStrategy
amsClient ocm.AMSClient
}

// NewDinosaurService ...
func NewDinosaurService(connectionFactory *db.ConnectionFactory, clusterService ClusterService, iamService sso.IAMService, dinosaurConfig *config.CentralConfig, dataplaneClusterConfig *config.DataplaneClusterConfig, awsConfig *config.AWSConfig, quotaServiceFactory QuotaServiceFactory, awsClientFactory aws.ClientFactory, authorizationService authorization.Authorization, clusterPlacementStrategy ClusterPlacementStrategy, amsClient ocm.AMSClient) *dinosaurService {
func NewDinosaurService(connectionFactory *db.ConnectionFactory, clusterService ClusterService, iamService sso.IAMService, dinosaurConfig *config.CentralConfig, dataplaneClusterConfig *config.DataplaneClusterConfig, awsConfig *config.AWSConfig, quotaServiceFactory QuotaServiceFactory, awsClientFactory aws.ClientFactory, authorizationService authorization.Authorization, clusterPlacementStrategy ClusterPlacementStrategy) *dinosaurService {
return &dinosaurService{
connectionFactory: connectionFactory,
clusterService: clusterService,
Expand All @@ -138,7 +136,6 @@ func NewDinosaurService(connectionFactory *db.ConnectionFactory, clusterService
authService: authorizationService,
dataplaneClusterConfig: dataplaneClusterConfig,
clusterPlacementStrategy: clusterPlacementStrategy,
amsClient: amsClient,
}
}

Expand Down Expand Up @@ -331,23 +328,12 @@ func (k *dinosaurService) PrepareDinosaurRequest(dinosaurRequest *dbapi.CentralR
return nil
}

// Obtain organisation name from AMS to store in central request.
org, err := k.amsClient.GetOrganisationFromExternalID(dinosaurRequest.OrganisationID)
if err != nil {
return errors.OrganisationNotFound(dinosaurRequest.OrganisationID, err)
}
orgName := org.Name()
if orgName == "" {
return errors.OrganisationNameInvalid(dinosaurRequest.OrganisationID, orgName)
}

// Update the fields of the CentralRequest record in the database.
updatedCentralRequest := &dbapi.CentralRequest{
Meta: api.Meta{
ID: dinosaurRequest.ID,
},
OrganisationName: orgName,
Status: dinosaurConstants.CentralRequestStatusProvisioning.String(),
Status: dinosaurConstants.CentralRequestStatusProvisioning.String(),
}
if err := k.Update(updatedCentralRequest); err != nil {
return errors.NewWithCause(errors.ErrorGeneral, err, "failed to update central request")
Expand Down
12 changes: 6 additions & 6 deletions internal/dinosaur/pkg/services/quota/ams_quota_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ var supportedAMSBillingModels = map[string]struct{}{

// CheckIfQuotaIsDefinedForInstanceType ...
func (q amsQuotaService) CheckIfQuotaIsDefinedForInstanceType(dinosaur *dbapi.CentralRequest, instanceType types.DinosaurInstanceType) (bool, *errors.ServiceError) {
org, err := q.amsClient.GetOrganisationFromExternalID(dinosaur.OrganisationID)
orgID, err := q.amsClient.GetOrganisationIDFromExternalID(dinosaur.OrganisationID)
if err != nil {
return false, errors.OrganisationNotFound(dinosaur.OrganisationID, err)
}

hasQuota, err := q.hasConfiguredQuotaCost(org.ID(), instanceType.GetQuotaType())
hasQuota, err := q.hasConfiguredQuotaCost(orgID, instanceType.GetQuotaType())
if err != nil {
return false, errors.NewWithCause(errors.ErrorGeneral, err, fmt.Sprintf("failed to get assigned quota of type %v for organization with external id %v and id %v", instanceType.GetQuotaType(), dinosaur.OrganisationID, org.ID()))
return false, errors.NewWithCause(errors.ErrorGeneral, err, fmt.Sprintf("failed to get assigned quota of type %v for organization with id %v", instanceType.GetQuotaType(), orgID))
}

return hasQuota, nil
Expand Down Expand Up @@ -132,11 +132,11 @@ func (q amsQuotaService) ReserveQuota(dinosaur *dbapi.CentralRequest, instanceTy

rr := newBaseQuotaReservedResourceResourceBuilder()

org, err := q.amsClient.GetOrganisationFromExternalID(dinosaur.OrganisationID)
orgID, err := q.amsClient.GetOrganisationIDFromExternalID(dinosaur.OrganisationID)
if err != nil {
return "", errors.OrganisationNotFound(dinosaur.OrganisationID, err)
}
bm, err := q.selectBillingModelFromDinosaurInstanceType(org.ID(), dinosaur.CloudProvider, dinosaur.CloudAccountID, instanceType)
bm, err := q.selectBillingModelFromDinosaurInstanceType(orgID, dinosaur.CloudProvider, dinosaur.CloudAccountID, instanceType)
if err != nil {
svcErr := errors.ToServiceError(err)
return "", errors.NewWithCause(svcErr.Code, svcErr, "Error getting billing model")
Expand All @@ -145,7 +145,7 @@ func (q amsQuotaService) ReserveQuota(dinosaur *dbapi.CentralRequest, instanceTy
glog.Infof("Billing model of Central request %s with quota type %s has been set to %s.", dinosaur.ID, instanceType.GetQuotaType(), bm)

if bm != string(amsv1.BillingModelStandard) {
if err := q.verifyCloudAccountInAMS(dinosaur, org.ID()); err != nil {
if err := q.verifyCloudAccountInAMS(dinosaur, orgID); err != nil {
return "", err
}
rr.BillingMarketplaceAccount(dinosaur.CloudAccountID)
Expand Down
Loading

0 comments on commit 5feb0f9

Please sign in to comment.