Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Provision environment pulls namespace value from stack id #575

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Parameters:
Description: An environment name that will be prefixed to resource names
SolutionNamespace:
Type: String
Description: Environment name of the solution. It should be the same value as provided in onboard-account.cfn.yml for "Namespace"
Description: The namespace value provided when onboarding the Member account
IsAppStreamEnabled:
Type: String
AllowedValues: [true, false]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Parameters:
Description: An environment name that will be prefixed to resource names
SolutionNamespace:
Type: String
Description: Environment name of the solution. It should be the same value as provided in onboard-account.cfn.yml for "Namespace"
Description: The namespace value provided when onboarding the Member account
IsAppStreamEnabled:
Type: String
AllowedValues: [true, false]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Parameters:
Description: An environment name that will be prefixed to resource names
SolutionNamespace:
Type: String
Description: Environment name of the solution. It should be the same value as provided in onboard-account.cfn.yml for "Namespace"
Description: The namespace value provided when onboarding the Member account
IsAppStreamEnabled:
Type: String
AllowedValues: [true, false]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ const EnvironmentSCKeyPairServiceMock = require('../environment-sc-keypair-servi
jest.mock('../../../data-egress/data-egress-service.js');
const DataEgressService = require('../../../data-egress/data-egress-service.js');

jest.mock('../../../account/account-service.js');
const AccountService = require('../../../account/account-service.js');

const EnvironmentConfigVarsService = require('../environment-config-vars-service');

describe('EnvironmentSCService', () => {
Expand All @@ -73,6 +76,7 @@ describe('EnvironmentSCService', () => {
let environmentAmiService = null;
let userService = null;
let settingsService = null;
let accountService = null;

beforeEach(async () => {
const container = new ServicesContainer();
Expand All @@ -92,6 +96,7 @@ describe('EnvironmentSCService', () => {
container.register('environmentScKeypairService', new EnvironmentSCKeyPairServiceMock());
container.register('studyService', new StudyServiceMock());
container.register('dataEgressService', new DataEgressService());
container.register('accountService', new AccountService());
await container.initServices();

// suppress expected console errors
Expand All @@ -106,7 +111,14 @@ describe('EnvironmentSCService', () => {
environmentAmiService = await container.find('environmentAmiService');
userService = await container.find('userService');
settingsService = await container.find('settings');
accountService = await container.find('accountService');

accountService.mustFind = jest.fn(() => {
return Promise.resolve({
stackId:
'arn:aws:cloudformation:eu-west-1:123456789012:stack/initial-stack-1625689755737/ff9a0dc0-df61-11eb-8b32-024312ba26d9',
});
});
// Skip authorization by default
service.assertAuthorized = jest.fn();
});
Expand Down Expand Up @@ -289,7 +301,7 @@ describe('EnvironmentSCService', () => {
mockSettingsService({
environmentInstanceFiles: '{}',
isAppStreamEnabled: 'true',
solutionNamespace: 'gamma',
solutionNamespace: 'initial-stack-1625689755737',
});
const expectedResponse = {
accountId: '123456789012',
Expand All @@ -315,7 +327,7 @@ describe('EnvironmentSCService', () => {
vpcId: 'VpcId-Test',
xAccEnvMgmtRoleArn: 'xAccEnvMgmtRole-Test',
isAppStreamEnabled: 'true',
solutionNamespace: 'gamma',
solutionNamespace: 'initial-stack-1625689755737',
};

// EXECUTE & CHECK
Expand All @@ -329,7 +341,7 @@ describe('EnvironmentSCService', () => {
mockSettingsService({
environmentInstanceFiles: '{}',
isAppStreamEnabled: 'true',
solutionNamespace: 'gamma',
solutionNamespace: 'initial-stack-1625689755737',
enableEgressStore: 'true',
});
const requestContext = 'sampleRequestContext';
Expand Down Expand Up @@ -402,7 +414,7 @@ describe('EnvironmentSCService', () => {
vpcId: 'VpcId-Test',
xAccEnvMgmtRoleArn: 'xAccEnvMgmtRole-Test',
isAppStreamEnabled: 'true',
solutionNamespace: 'gamma',
solutionNamespace: 'initial-stack-1625689755737',
};

// EXECUTE & CHECK
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const settingKeys = {
enableEgressStore: 'enableEgressStore',
environmentInstanceFiles: 'environmentInstanceFiles',
isAppStreamEnabled: 'isAppStreamEnabled',
solutionNamespace: 'solutionNamespace',
};

/**
Expand All @@ -50,6 +49,7 @@ class EnvironmentConfigVarsService extends Service {
'pluginRegistryService',
'auditWriterService',
'dataEgressService',
'accountService',
]);
}

Expand Down Expand Up @@ -157,8 +157,7 @@ class EnvironmentConfigVarsService extends Service {
},
{
name: 'solutionNamespace',
desc:
'Environment name of the solution. It should be the same value as provided in onboard-account.cfn.yml for "Namespace"',
desc: 'The namespace value provided when onboarding the Member account',
},
];
}
Expand Down Expand Up @@ -250,21 +249,6 @@ class EnvironmentConfigVarsService extends Service {
});
}

// TODO: If the ami sharing gets moved (because it doesn't contribute to an env var)
// then move the update local resource policies too.
// Using the account root provides basically the same level of security because in either
// case we have to trust that the member account hasn't altered the role's assume role policy to allow other
// principals assume it
// if (s3Prefixes.length > 0) {
// await environmentMountService.addRoleArnToLocalResourcePolicies(`arn:aws:iam::${accountId}:root`, s3Prefixes);
// }

// Check if the environment being launched needs an admin key-pair to be created in the target account
// If the configuration being used has any parameter that uses the "adminKeyPairName" variable then it means
// we need to provision that key in the target account and provide the name of the generated key as the
// "adminKeyPairName" variable
// Disabling "no-template-curly-in-string" lint rule because we need to compare with the string literal "${adminKeyPairName}"
// i.e., without any string interpolation
// eslint-disable-next-line no-template-curly-in-string
const isAdminKeyPairRequired = !!_.find(envTypeConfig.params, p => p.value === '${adminKeyPairName}');
let adminKeyPairName = '';
Expand Down Expand Up @@ -295,7 +279,7 @@ class EnvironmentConfigVarsService extends Service {
iamPolicyDocument: JSON.stringify(iamPolicyDocument),
environmentInstanceFiles: this.settings.get(settingKeys.environmentInstanceFiles),
isAppStreamEnabled: this.settings.get(settingKeys.isAppStreamEnabled),
solutionNamespace: this.settings.get(settingKeys.solutionNamespace),
solutionNamespace: await this.getSolutionNamespace(requestContext, externalId, accountId),
// s3Prefixes // This variable is no longer relevant it is being removed, the assumption is that
// this variable has not been used in any of the product templates.
uid: user.uid,
Expand All @@ -312,6 +296,12 @@ class EnvironmentConfigVarsService extends Service {
return result;
}

async getSolutionNamespace(requestContext, externalId, accountId) {
const [accountService] = await this.service(['accountService']);
const { stackId } = await accountService.mustFind(requestContext, { id: accountId });
return stackId.match(/\d{12}:stack\/(.+)\//)[1];
}

async getEnvRolePolicy(requestContext, { environment, studies, memberAccountId }) {
const policyDoc = new StudyPolicy();
const pluginRegistryService = await this.service('pluginRegistryService');
Expand Down
1 change: 0 additions & 1 deletion main/solution/backend/config/infra/functions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -193,5 +193,4 @@ workflowLoopRunner:
APP_EGRESS_STORE_BUCKET_NAME: ${self:custom.settings.egressStoreBucketName}
APP_EGRESS_STORE_KMS_KEY_ALIAS_ARN: ${self:custom.settings.egressStoreKmsKeyAliasArn}
APP_EGRESS_STORE_KMS_POLICY_WORKSPACE_SID: ${self:custom.settings.egressStoreKmsPolicyWorkspaceSid}
APP_SOLUTION_NAMESPACE: ${self:custom.settings.envName}
APP_IS_APP_STREAM_ENABLED: ${self:custom.settings.isAppStreamEnabled}