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.

Sign up for GitHub

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

Jump to bottom

feat: swb main study role #710

Merged
merged 12 commits into from
Sep 21, 2021
Merged

feat: swb main study role #710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5b29186
Working create iam role for egress
Sep 14, 2021
592df15
Update 2
Sep 15, 2021
599e1a3
Only enableEgressStore when a file was actually added
Sep 15, 2021
94ed178
Remove unnecessary comments
Sep 15, 2021
ab4f4a2
Update failing tests
Sep 15, 2021
d611b3e
New role name
Sep 15, 2021
c24b3df
PR comment
Sep 16, 2021
dbd42ae
Remove unused lodash function
Sep 16, 2021
fa5012f
Update tests
Sep 16, 2021
9448361
Create role only for EnableEgressStore
Sep 17, 2021
bb485a9
Allow terminating Egress Store even when store is in Created state
Sep 20, 2021
8b43698
Add unit tests
Sep 20, 2021
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
80 changes: 73 additions & 7 deletions ...ase-raas/packages/base-raas-services/lib/data-egress/__test__/data-egress-service.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,11 @@ describe('DataEgressService', () => {
return undefined;
},
};
const roleArn = 'test-RoleArn';
const environmentId = 'test-id';
const requestContext = {};
const rawEnvironment = {
id: 'test-id',
id: environmentId,
name: 'test-raw-environment-name',
createdBy: 'test-raw-environment-createdby',
updatedBy: 'test-updatedBy',
Expand Down Expand Up @@ -291,6 +293,7 @@ describe('DataEgressService', () => {
arn: `arn:aws:s3:::test-egressStoreBucketName/${rawEnvironment.id}/`,
},
],
roleArn,
};
AWSMock.mock('KMS', 'describeKey', (params, callback) => {
expect(params).toMatchObject({
Expand All @@ -312,6 +315,30 @@ describe('DataEgressService', () => {
});
});

const createPolicyArn = 'test-policy-arn';
AWSMock.mock('IAM', 'createPolicy', (params, callback) => {
expect(params.PolicyName).toEqual(`study-${environmentId}`);
callback(null, {
Policy: {
Arn: createPolicyArn,
},
});
});
AWSMock.mock('IAM', 'createRole', (params, callback) => {
expect(params.RoleName).toEqual(`study-${environmentId}`);
callback(null, {
Role: {
Arn: roleArn,
},
});
});
AWSMock.mock('IAM', 'attachRolePolicy', (params, callback) => {
expect(params).toMatchObject({
RoleName: `study-${environmentId}`,
PolicyArn: createPolicyArn,
});
callback();
});
const putBucketPolicyMock = jest.fn((params, callback) => {
expect(params).toMatchObject({
Bucket: 'test-egressStoreBucketName',
Expand Down Expand Up @@ -460,13 +487,14 @@ describe('DataEgressService', () => {
},
};

const egressStoreId = 'test-egress-store-id';
dbService.table.scan.mockResolvedValueOnce([
{
status: 'PROCESSED',
workspaceId: 'test-workspace-id',
s3BucketName: 'test-s3BucketName',
s3BucketPath: 'test-s3BucketPath',
id: 'test-egress-store-id',
id: egressStoreId,
},
]);
const requestContext = {};
Expand Down Expand Up @@ -495,7 +523,6 @@ describe('DataEgressService', () => {
Policy: JSON.stringify(s3Policy),
});
});

const putBucketPolicyMock = jest.fn((params, callback) => {
expect(params).toMatchObject({
Bucket: 'test-egressStoreBucketName',
Expand All @@ -506,6 +533,8 @@ describe('DataEgressService', () => {
lockService.tryWriteLockAndRun = jest.fn((_params, callback) => callback());
AWSMock.mock('S3', 'putBucketPolicy', putBucketPolicyMock);

mockDeleteEgressStoreRole(egressStoreId);

await dataEgressService.terminateEgressStore(requestContext, envId);
expect(dataEgressService.removeEgressStoreBucketPolicy).toHaveBeenCalledTimes(1);
expect(dataEgressService.removeEgressStoreBucketPolicy).toHaveBeenCalledWith(
Expand All @@ -527,7 +556,42 @@ describe('DataEgressService', () => {
);
});

it('should successfully delete egress store while egress store is not touched since created', async () => {
function mockDeleteEgressStoreRole(egressStoreId) {
const policyArn = 'test-PermissionBoundaryArn';
AWSMock.mock('IAM', 'listAttachedRolePolicies', (params, callback) => {
expect(params.RoleName).toEqual(`study-${egressStoreId}`);
callback(null, {
AttachedPolicies: [
{
PolicyName: 'test-PermissionBoundaryName',
PolicyArn: policyArn,
},
],
});
});
AWSMock.mock('IAM', 'detachRolePolicy', (params, callback) => {
expect(params).toMatchObject({ RoleName: `study-${egressStoreId}`, PolicyArn: policyArn });
callback();
});
AWSMock.mock('IAM', 'deleteRole', (params, callback) => {
expect(params).toMatchObject({ RoleName: `study-${egressStoreId}` });
callback();
});
AWSMock.mock('IAM', 'deletePolicy', (params, callback) => {
expect(params).toMatchObject({ PolicyArn: policyArn });
callback();
});
}

it('should successfully delete egress store that is in CREATED state: deleteEgressStoreInCreatedStateTest = true', async () => {
await deleteEgressStoreInCreatedStateTest(true);
});

it('should successfully delete egress store that is in CREATED state: deleteEgressStoreInCreatedStateTest = false', async () => {
await deleteEgressStoreInCreatedStateTest(false);
});

async function deleteEgressStoreInCreatedStateTest(isAbleToSubmitEgressRequest) {
dataEgressService.removeEgressStoreBucketPolicy = jest.fn();
const s3Policy = testS3PolicyFn();
dataEgressService._settings = {
Expand All @@ -545,14 +609,15 @@ describe('DataEgressService', () => {
},
};

const egressStoreId = 'test-egress-store-id';
dbService.table.scan.mockResolvedValueOnce([
{
status: 'CREATED',
workspaceId: 'test-workspace-id',
s3BucketName: 'test-s3BucketName',
s3BucketPath: 'test-s3BucketPath',
id: 'test-egress-store-id',
isAbleToSubmitEgressRequest: false,
id: egressStoreId,
isAbleToSubmitEgressRequest,
},
]);
const requestContext = {};
Expand Down Expand Up @@ -591,6 +656,7 @@ describe('DataEgressService', () => {
// Mock locking so that the putBucketPolicy actually gets called
lockService.tryWriteLockAndRun = jest.fn((_params, callback) => callback());
AWSMock.mock('S3', 'putBucketPolicy', putBucketPolicyMock);
mockDeleteEgressStoreRole(egressStoreId);

await dataEgressService.terminateEgressStore(requestContext, envId);
expect(dataEgressService.removeEgressStoreBucketPolicy).toHaveBeenCalledTimes(1);
Expand All @@ -611,7 +677,7 @@ describe('DataEgressService', () => {
},
'test-accountId',
);
});
}

it('should remove bucket policy', async () => {
dataEgressService.getS3BucketAndPolicy = jest.fn().mockResolvedValueOnce({
Expand Down
124 changes: 116 additions & 8 deletions addons/addon-base-raas/packages/base-raas-services/lib/data-egress/data-egress-service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const {
getRevisedS3Statements,
removeAccountFromStatement,
} = require('../helpers/utils');
const { StudyPolicy } = require('../helpers/iam/study-policy');

const settingKeys = {
tableName: 'dbEgressStore',
Expand Down Expand Up @@ -116,8 +117,10 @@ class DataEgressService extends Service {
throw this.boom.badRequest(`Error in creating egress store:${folderName} in bucket: ${bucketName}`, true);
}

// prepare info for ddb and update egress store info
const egressStoreId = environment.id;
const roleArn = await this.createMainAccountEgressStoreRole(requestContext, egressStoreId);

// prepare info for ddb and update egress store info
const creationTime = new Date().toISOString;
const dbObject = {
id: egressStoreId,
Expand All @@ -134,6 +137,7 @@ class DataEgressService extends Service {
ver: 0,
isAbleToSubmitEgressRequest: false,
egressStoreObjectListLocation: null,
roleArn,
};

const lockService = await this.service('lockService');
Expand Down Expand Up @@ -175,6 +179,7 @@ class DataEgressService extends Service {
arn: `arn:aws:s3:::${bucketName}/${environment.id}/`,
},
],
roleArn,
};
const memberAccountId = await this.getMemberAccountId(requestContext, environment.id);

Expand Down Expand Up @@ -208,20 +213,15 @@ class DataEgressService extends Service {
true,
);
}

const s3Service = await this.service('s3Service');
const egressStoreStatus = egressStoreInfo.status;
const isEgressStoreNotTouched =
egressStoreStatus.toUpperCase() === CREATED_STATUS_CODE && egressStoreInfo.isAbleToSubmitEgressRequest === false;

if (egressStoreStatus.toUpperCase() === PROCESSING_STATUS_CODE) {
throw this.boom.forbidden(
`Egress store: ${egressStoreInfo.id} is still in processing. The egress store is not terminated and the workspce can not be terminated before egress request is processed.`,
`Egress store: ${egressStoreInfo.id} is still in processing. The egress store is not terminated and the workspace can not be terminated before egress request is processed.`,
true,
);
} else if (egressStoreStatus.toUpperCase() === PROCESSED_STATUS_CODE || isEgressStoreNotTouched) {
// ONLY terminate the egress store if it has been processed or the egress store is empty

} else if ([PROCESSED_STATUS_CODE, CREATED_STATUS_CODE].includes(egressStoreStatus.toUpperCase())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this branch not being unit tested before? Can we add a test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

try {
await s3Service.clearPath(egressStoreInfo.s3BucketName, egressStoreInfo.s3BucketPath);
} catch (error) {
Expand All @@ -230,6 +230,7 @@ class DataEgressService extends Service {
true,
);
}
await this.deleteMainAccountEgressStoreRole(egressStoreInfo.id);

const lockService = await this.service('lockService');
const egressStoreDdbLockId = `egress-store-ddb-access-${egressStoreInfo.id}`;
Expand Down Expand Up @@ -455,6 +456,11 @@ class DataEgressService extends Service {
return new aws.sdk.KMS();
}

async getIAM() {
const aws = await this.getAWS();
return new aws.sdk.IAM();
}

async getAWS() {
const aws = await this.service('aws');
return aws;
Expand Down Expand Up @@ -537,6 +543,108 @@ class DataEgressService extends Service {
return memberAccount.accountId;
}

getMainAccountEgressStoreRole(egressStoreId) {
return `study-${egressStoreId}`;
}

getMainAccountEgressStoreRolePolicyName(egressStoreId) {
return `study-${egressStoreId}`;
}

/**
* Create the main account IAM Role and Policy to allow a workspace on the member account to access
* the egress store S3 bucket
* @param requestContext
* @param egressStoreId
* @returns role Arn
*/
async createMainAccountEgressStoreRole(requestContext, egressStoreId) {
const egressStoreBucketName = this.settings.get('egressStoreBucketName');
const kmsArn = await this.getKmsKeyIdArn();
const memberAccountId = await this.getMemberAccountId(requestContext, egressStoreId);
const mainAccountRoleName = this.getMainAccountEgressStoreRole(egressStoreId);
const permissionBoundaryArn = this.settings.get('permissionBoundaryPolicyStudyBucketArn');

const egressStudyPolicy = new StudyPolicy();

const study = {
bucket: egressStoreBucketName,
folder: [egressStoreId],
permission: {
read: true,
write: true,
},
kmsArn,
};
egressStudyPolicy.addStudy(study);
const permissionPolicy = egressStudyPolicy.toPolicyDoc();

const iam = await this.getIAM();
const createPolicyResponse = await iam
.createPolicy({
PolicyName: this.getMainAccountEgressStoreRolePolicyName(egressStoreId),
PolicyDocument: JSON.stringify(permissionPolicy),
})
.promise();

const createRoleResponse = await iam
.createRole({
AssumeRolePolicyDocument: JSON.stringify({
Version: '2012-10-17',
Statement: [
{
Effect: 'Allow',
Principal: {
AWS: `arn:aws:iam::${memberAccountId}:root`,
},
Action: 'sts:AssumeRole',
},
],
}),
Path: '/',
RoleName: mainAccountRoleName,
PermissionsBoundary: permissionBoundaryArn,
})
.promise();

await iam
.attachRolePolicy({
RoleName: mainAccountRoleName,
PolicyArn: createPolicyResponse.Policy.Arn,
})
.promise();

return createRoleResponse.Role.Arn;
}

async deleteMainAccountEgressStoreRole(egressStoreId) {
const iam = await this.getIAM();
const listRolePoliciesResponse = await iam
.listAttachedRolePolicies({
RoleName: this.getMainAccountEgressStoreRole(egressStoreId),
})
.promise();
const policyArns = listRolePoliciesResponse.AttachedPolicies.map(policy => {
return policy.PolicyArn;
});
await Promise.all(
policyArns.map(arn => {
return iam
.detachRolePolicy({
RoleName: this.getMainAccountEgressStoreRole(egressStoreId),
PolicyArn: arn,
})
.promise();
}),
);
await iam.deleteRole({ RoleName: this.getMainAccountEgressStoreRole(egressStoreId) }).promise();
await Promise.all(
policyArns.map(arn => {
return iam.deletePolicy({ PolicyArn: arn }).promise();
}),
);
}

async lockAndUpdate(lockId, dbKey, dbObject) {
const lockService = await this.service('lockService');
await lockService.tryWriteLockAndRun({ id: lockId }, async () => {
Expand Down
7 changes: 4 additions & 3 deletions ...ges/base-raas-services/lib/environment/service-catalog/environment-config-vars-service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class EnvironmentConfigVarsService extends Service {
constructor() {
super();
this.dependency([
'aws',
'userService',
'environmentScService',
'environmentScKeypairService',
Expand Down Expand Up @@ -227,13 +228,13 @@ class EnvironmentConfigVarsService extends Service {
const iamPolicyDocument = await this.getEnvRolePolicy(requestContext, {
environment,
studies,
memberAccountId: accountId,
accountId,
});

const s3Mounts = await this.getS3Mounts(requestContext, {
environment,
studies,
memberAccountId: accountId,
accountId,
});

let egressStoreIamPolicyDocument = {};
Expand All @@ -244,7 +245,7 @@ class EnvironmentConfigVarsService extends Service {
egressStoreIamPolicyDocument = await this.getEnvEgressStorePolicy(requestContext, {
environment,
egressStore: egressStoreMount,
memberAccountId: accountId,
accountId,
});
}

Expand Down
Loading