Skip to content
This repository has been archived by the owner on Jul 4, 2024. It is now read-only.

Commit

Permalink
[hotfix]Fix account creation (#2020)
Browse files Browse the repository at this point in the history 
* Fix account creation

(cherry picked from commit 646ac59)

* Bump versions

* Increase director TestDefinition memory

Co-authored-by: Desislava Asenova <[email protected]>
  • Loading branch information
@nyordanoff @desislavaa
nyordanoff and desislavaa authored Sep 15, 2021
1 parent 0308732 commit c70dede
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 47 deletions.
3 changes: 3 additions & 0 deletions chart/compass/templates/tests/director/director-test.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ spec:
imagePullPolicy: IfNotPresent
command: ["/bin/sh"]
args: ["-c", "/director.test -test.v; exit_code=$?; echo code is $exit_code; echo 'killing pilot-agent...'; curl -XPOST http://127.0.0.1:15020/quitquitquit; sleep 4; exit $exit_code;"]
resources:
limits:
memory: "256Mi"
env:
- name: DIRECTOR_URL
value: "https://{{ .Values.global.gateway.tls.host }}.{{ .Values.global.ingress.domainName }}{{ .Values.global.director.prefix }}"
Expand Down
4 changes: 2 additions & 2 deletions chart/compass/values.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ global:
version: "PR-2003"
director:
dir:
version: "PR-1998"
version: "PR-2020"
gateway:
dir:
version: "PR-2003"
Expand Down Expand Up @@ -104,7 +104,7 @@ global:
version: "PR-42"
e2e_tests:
dir:
version: "PR-1998"
version: "PR-2020"
isLocalEnv: false
oauth2:
host: oauth2
Expand Down
10 changes: 8 additions & 2 deletions components/director/internal/tenantfetchersvc/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,19 @@ func (h *handler) getProvisioningRequest(body []byte, region string) (*TenantPro
return nil, err
}

return &TenantProvisioningRequest{
req := &TenantProvisioningRequest{
AccountTenantID: properties[h.config.TenantIDProperty],
SubaccountTenantID: properties[h.config.SubaccountTenantIDProperty],
CustomerTenantID: properties[h.config.CustomerIDProperty],
Subdomain: properties[h.config.SubdomainProperty],
Region: region,
}, nil
}

if req.AccountTenantID == req.SubaccountTenantID {
req.SubaccountTenantID = ""
}

return req, nil
}

func (h *handler) provisionTenants(ctx context.Context, request *TenantProvisioningRequest, region string) error {
Expand Down
118 changes: 75 additions & 43 deletions components/director/internal/tenantfetchersvc/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ const (
tenantSubdomain = "mytenant"
tenantRegion = "myregion"

subaccountTenantSubdomain = "myregionaltenant"
subaccountTenantExtID = "regional-tenant-external-id"
regionalTenantSubdomain = "myregionaltenant"
subaccountTenantExtID = "subaccount-tenant-external-id"

parentTenantExtID = "parent-tenant-external-id"

Expand All @@ -41,25 +41,22 @@ const (
var (
testError = errors.New("test error")
validHandlerConfig = tenantfetchersvc.HandlerConfig{
RegionPathParam: "region",
TenantProviderConfig: tenantfetchersvc.TenantProviderConfig{
TenantProvider: testProviderName,
TenantIDProperty: tenantProviderTenantIDProperty,
CustomerIDProperty: tenantProviderCustomerIDProperty,
SubdomainProperty: tenantProviderSubdomainProperty,
TenantProvider: testProviderName,
TenantIDProperty: tenantProviderTenantIDProperty,
SubaccountTenantIDProperty: tenantProviderSubaccountTenantIDProperty,
CustomerIDProperty: tenantProviderCustomerIDProperty,
SubdomainProperty: tenantProviderSubdomainProperty,
},
}
)

type tenantCreationRequest struct {
TenantID string `json:"tenantId"`
CustomerID string `json:"customerId"`
Subdomain string `json:"subdomain"`
}

type regionalTenantCreationRequest struct {
TenantID string `json:"subaccountTenantId"`
ParentID string `json:"tenantId"`
Subdomain string `json:"subdomain"`
TenantID string `json:"tenantId"`
SubaccountID string `json:"subaccountTenantId"`
CustomerID string `json:"customerId"`
Subdomain string `json:"subdomain"`
}

type errReader int
Expand Down Expand Up @@ -91,6 +88,13 @@ func TestService_Create(t *testing.T) {
Subdomain: tenantSubdomain,
})
assert.NoError(t, err)
bodyWithMathcingChild, err := json.Marshal(tenantCreationRequest{
TenantID: tenantExtID,
SubaccountID: tenantExtID,
CustomerID: parentTenantExtID,
Subdomain: tenantSubdomain,
})
assert.NoError(t, err)

bodyWithMissingTenantSubdomain, err := json.Marshal(tenantCreationRequest{
TenantID: tenantExtID,
Expand Down Expand Up @@ -141,6 +145,18 @@ func TestService_Create(t *testing.T) {
ExpectedSuccessOutput: compassURL,
ExpectedStatusCode: http.StatusOK,
},
{
Name: "Succeeds when matching child tenant ID is provided",
TxFn: txGen.ThatSucceeds,
TenantProvisionerFn: func() *automock.TenantProvisioner {
provisioner := &automock.TenantProvisioner{}
provisioner.On("ProvisionTenants", txtest.CtxWithDBMatcher(), accountProvisioningRequest).Return(nil).Once()
return provisioner
},
Request: httptest.NewRequest(http.MethodPut, target, bytes.NewBuffer(bodyWithMathcingChild)),
ExpectedSuccessOutput: compassURL,
ExpectedStatusCode: http.StatusOK,
},
{
Name: "Returns error when reading request body fails",
TxFn: txGen.ThatDoesntStartTransaction,
Expand Down Expand Up @@ -240,42 +256,45 @@ func TestService_CreateRegional(t *testing.T) {
target := "http://example.com/foo/:region"
txtest.CtxWithDBMatcher()

validRequestBody, err := json.Marshal(regionalTenantCreationRequest{
TenantID: subaccountTenantExtID,
ParentID: tenantExtID,
Subdomain: subaccountTenantSubdomain,
validRequestBody, err := json.Marshal(tenantCreationRequest{
SubaccountID: subaccountTenantExtID,
TenantID: tenantExtID,
Subdomain: regionalTenantSubdomain,
})
assert.NoError(t, err)

bodyWithMissingParent, err := json.Marshal(regionalTenantCreationRequest{
TenantID: subaccountTenantExtID,
Subdomain: tenantSubdomain,
bodyWithMissingAccountParent, err := json.Marshal(tenantCreationRequest{
SubaccountID: subaccountTenantExtID,
Subdomain: tenantSubdomain,
})
assert.NoError(t, err)

bodyWithMissingTenantSubdomain, err := json.Marshal(regionalTenantCreationRequest{
TenantID: subaccountTenantExtID,
ParentID: tenantExtID,
bodyWithMissingTenantSubdomain, err := json.Marshal(tenantCreationRequest{
SubaccountID: subaccountTenantExtID,
TenantID: tenantExtID,
})
assert.NoError(t, err)

validHandlerConfig := tenantfetchersvc.HandlerConfig{
RegionPathParam: "region",
TenantProviderConfig: tenantfetchersvc.TenantProviderConfig{
TenantProvider: testProviderName,
TenantIDProperty: tenantProviderTenantIDProperty,
SubaccountTenantIDProperty: tenantProviderSubaccountTenantIDProperty,
CustomerIDProperty: tenantProviderCustomerIDProperty,
SubdomainProperty: tenantProviderSubdomainProperty,
},
}
regionalTenant := tenantfetchersvc.TenantProvisioningRequest{
bodyWithMatchingAccountAndSubaccountIDs, err := json.Marshal(tenantCreationRequest{
TenantID: tenantExtID,
SubaccountID: tenantExtID,
Subdomain: regionalTenantSubdomain,
})
assert.NoError(t, err)

subaccountTenantReq := tenantfetchersvc.TenantProvisioningRequest{
SubaccountTenantID: subaccountTenantExtID,
AccountTenantID: tenantExtID,
Subdomain: subaccountTenantSubdomain,
Subdomain: regionalTenantSubdomain,
Region: region,
}

accountTenantReq := tenantfetchersvc.TenantProvisioningRequest{
AccountTenantID: tenantExtID,
Subdomain: regionalTenantSubdomain,
Region: region,
}

testCases := []struct {
Name string
provisionerFn func() *automock.TenantProvisioner
Expand All @@ -291,14 +310,27 @@ func TestService_CreateRegional(t *testing.T) {
TxFn: txGen.ThatSucceeds,
provisionerFn: func() *automock.TenantProvisioner {
provisioner := &automock.TenantProvisioner{}
provisioner.On("ProvisionRegionalTenants", txtest.CtxWithDBMatcher(), regionalTenant).Return(nil).Once()
provisioner.On("ProvisionRegionalTenants", txtest.CtxWithDBMatcher(), subaccountTenantReq).Return(nil).Once()
return provisioner
},
Request: httptest.NewRequest(http.MethodPut, target, bytes.NewBuffer(validRequestBody)),
Region: region,
ExpectedSuccessOutput: compassURL,
ExpectedStatusCode: http.StatusOK,
},
{
Name: "Succeeds to create account tenant when account ID and subaccount IDs are matching",
TxFn: txGen.ThatSucceeds,
provisionerFn: func() *automock.TenantProvisioner {
provisioner := &automock.TenantProvisioner{}
provisioner.On("ProvisionRegionalTenants", txtest.CtxWithDBMatcher(), accountTenantReq).Return(nil).Once()
return provisioner
},
Request: httptest.NewRequest(http.MethodPut, target, bytes.NewBuffer(bodyWithMatchingAccountAndSubaccountIDs)),
Region: region,
ExpectedSuccessOutput: compassURL,
ExpectedStatusCode: http.StatusOK,
},
{
Name: "Returns error when region path parameter is missing",
TxFn: txGen.ThatDoesntStartTransaction,
Expand All @@ -311,7 +343,7 @@ func TestService_CreateRegional(t *testing.T) {
Name: "Returns error when parent tenant is not found in body",
TxFn: txGen.ThatDoesntStartTransaction,
provisionerFn: func() *automock.TenantProvisioner { return &automock.TenantProvisioner{} },
Request: httptest.NewRequest(http.MethodPut, target, bytes.NewBuffer(bodyWithMissingParent)),
Request: httptest.NewRequest(http.MethodPut, target, bytes.NewBuffer(bodyWithMissingAccountParent)),
Region: region,
ExpectedStatusCode: http.StatusBadRequest,
ExpectedErrorOutput: fmt.Sprintf("mandatory property %q is missing from request body", tenantProviderTenantIDProperty),
Expand Down Expand Up @@ -340,15 +372,15 @@ func TestService_CreateRegional(t *testing.T) {
provisionerFn: func() *automock.TenantProvisioner { return &automock.TenantProvisioner{} },
Request: httptest.NewRequest(http.MethodPut, target, bytes.NewBuffer(validRequestBody)),
Region: region,
ExpectedErrorOutput: fmt.Sprintf(tenantCreationFailureMsgFmt, regionalTenant.SubaccountTenantID),
ExpectedErrorOutput: fmt.Sprintf(tenantCreationFailureMsgFmt, subaccountTenantReq.SubaccountTenantID),
ExpectedStatusCode: http.StatusInternalServerError,
},
{
Name: "Returns error when tenant creation fails",
TxFn: txGen.ThatDoesntExpectCommit,
provisionerFn: func() *automock.TenantProvisioner {
provisioner := &automock.TenantProvisioner{}
provisioner.On("ProvisionRegionalTenants", txtest.CtxWithDBMatcher(), regionalTenant).Return(testError).Once()
provisioner.On("ProvisionRegionalTenants", txtest.CtxWithDBMatcher(), subaccountTenantReq).Return(testError).Once()
return provisioner
},
Request: httptest.NewRequest(http.MethodPut, target, bytes.NewBuffer(validRequestBody)),
Expand All @@ -361,12 +393,12 @@ func TestService_CreateRegional(t *testing.T) {
TxFn: txGen.ThatFailsOnCommit,
provisionerFn: func() *automock.TenantProvisioner {
provisioner := &automock.TenantProvisioner{}
provisioner.On("ProvisionRegionalTenants", txtest.CtxWithDBMatcher(), regionalTenant).Return(nil).Once()
provisioner.On("ProvisionRegionalTenants", txtest.CtxWithDBMatcher(), subaccountTenantReq).Return(nil).Once()
return provisioner
},
Request: httptest.NewRequest(http.MethodPut, target, bytes.NewBuffer(validRequestBody)),
Region: region,
ExpectedErrorOutput: fmt.Sprintf(tenantCreationFailureMsgFmt, regionalTenant.SubaccountTenantID),
ExpectedErrorOutput: fmt.Sprintf(tenantCreationFailureMsgFmt, subaccountTenantReq.SubaccountTenantID),
ExpectedStatusCode: http.StatusInternalServerError,
},
}
Expand Down
17 changes: 17 additions & 0 deletions tests/tenant-fetcher/tests/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,23 @@ func TestOnboardingHandler(t *testing.T) {
assertTenant(t, tnt, tenant.TenantID, tenant.Subdomain)
})

t.Run("Successful account tenant creation with matching account and subaccount tenant IDs", func(t *testing.T) {
id := uuid.New().String()
tenant := Tenant{
TenantID: id,
SubaccountID: id,
Subdomain: defaultSubdomain,
}

addTenantExpectStatusCode(t, tenant, http.StatusOK)

tnt, err := fixtures.GetTenantByExternalID(dexGraphQLClient, tenant.TenantID)
require.NoError(t, err)

// THEN
assertTenant(t, tnt, tenant.TenantID, tenant.Subdomain)
})

t.Run("Should not add already existing tenants", func(t *testing.T) {
tenantWithCustomer := Tenant{
TenantID: uuid.New().String(),
Expand Down

0 comments on commit c70dede

Please sign in to comment.