Skip to content
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### **Added**

- added permission boundary as optional input to all modules
- added permission boundary as optional input to all modules
- added optional `s3_access_logs_bucket_arn` parameter to `sagemaker-templates` and `sagemaker-model-cicd` modules for S3 access logging (CT.S3.PR.2 Control Tower compliance)

### **Changed**

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ export function getModuleParameters() {
const deploymentGroups = getSeedFarmerParamter('DEPLOYMENT_GROUPS');
const permissionsBoundaryName =
process.env.SEEDFARMER_PARAMETER_PERMISSIONS_BOUNDARY_NAME;
const s3AccessLogsBucketArn =
process.env.SEEDFARMER_PARAMETER_S3_ACCESS_LOGS_BUCKET_ARN;

// validate parameters
try {
Expand All @@ -67,6 +69,7 @@ export function getModuleParameters() {
modelBuildRepo,
deploymentGroups,
permissionsBoundaryName,
s3AccessLogsBucketArn,
});
} catch (err) {
const validationError = fromError(err, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ export const MLOpsCodePipelinePropsSchema = z.object({
* IAM Policy Name to attach to all roles as permissions boundary.
*/
permissionsBoundaryName: z.string().optional(),
/**
* S3 bucket ARN for server access logging. When provided, all S3 buckets
* will be configured to send access logs to this bucket.
*/
s3AccessLogsBucketArn: z.string().optional(),
});

export type MLOpsCodePipelineStackProps = z.infer<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class MLOpsCodePipelineStack extends cdk.Stack {
modelBuildRepo,
deploymentGroups,
tags,
s3AccessLogsBucketArn,
} = props;

// get the infra codecommit repo that will be used as source for model deploy codepipeline
Expand All @@ -78,7 +79,11 @@ export class MLOpsCodePipelineStack extends cdk.Stack {
// this pipeline will already be updated during `seedfarmer apply`
selfMutation: false,
crossAccountKeys: true,
artifactBucket: utils.createPipelineArtifactsBucket(this),
artifactBucket: utils.createPipelineArtifactsBucket(
this,
s3AccessLogsBucketArn,
`${projectName}-infra-pipeline-artifacts/`,
),
synth: new cdk.pipelines.CodeBuildStep('Synth', {
input: cdk.pipelines.CodePipelineSource.codeCommit(
this.infraRepo,
Expand Down Expand Up @@ -127,6 +132,7 @@ export class MLOpsCodePipelineStack extends cdk.Stack {
modelPackageGroupName,
modelApprovalTopicName: modelApprovalNotificationsTopicName,
deployEnvironments,
s3AccessLogsBucketArn,
// Model build support resources stack should be deployed to model build account
env: {
account: buildEnvironment.account,
Expand All @@ -153,6 +159,7 @@ export class MLOpsCodePipelineStack extends cdk.Stack {
sagemakerExecutionRoleName,
codeBuildAssumeRoleName,
modelPackageGroupName,
s3AccessLogsBucketArn,
// this pipeline should be created in tooling account, which will trigger the model build in target account
env: toolingEnvironment,
description: `Model build pipeline for ${projectName} ${deploymentGroup.name}`,
Expand All @@ -175,6 +182,7 @@ export class MLOpsCodePipelineStack extends cdk.Stack {
deploymentGroup,
modelPackageGroupName,
sagemakerArtifactsBucketName,
s3AccessLogsBucketArn,
// this pipeline should be created in tooling account, which will trigger the model deploy in target accounts
env: toolingEnvironment,
description: `Model deploy pipeline for ${projectName} ${deploymentGroup.name}`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as kms from 'aws-cdk-lib/aws-kms';
import * as s3 from 'aws-cdk-lib/aws-s3';
import * as sagemaker from 'aws-cdk-lib/aws-sagemaker';
import * as sns from 'aws-cdk-lib/aws-sns';
import { NagSuppressions } from 'cdk-nag';
import { Construct } from 'constructs';
import { DeployEnvironment } from '../mlops-code-pipeline-stack-props';

Expand All @@ -18,6 +19,7 @@ export interface ModelBuildSupportStackProps extends cdk.StackProps {
readonly modelPackageGroupName: string;
readonly modelApprovalTopicName: string;
readonly deployEnvironments: DeployEnvironment[];
readonly s3AccessLogsBucketArn?: string;
}

export class ModelBuildSupportStack extends cdk.Stack {
Expand All @@ -39,6 +41,7 @@ export class ModelBuildSupportStack extends cdk.Stack {
modelApprovalTopicName,
deployEnvironments,
toolingEnvironment,
s3AccessLogsBucketArn,
} = props;

const deploymentAccountPrincipals: iam.AccountPrincipal[] =
Expand Down Expand Up @@ -87,14 +90,32 @@ export class ModelBuildSupportStack extends cdk.Stack {
targetKey: kmsKey,
});

const accessLogsBucket = s3AccessLogsBucketArn
? s3.Bucket.fromBucketArn(this, 'AccessLogsBucket', s3AccessLogsBucketArn)
: undefined;

const logsBucket = new s3.Bucket(this, 'LogsBucket', {
encryption: s3.BucketEncryption.KMS,
encryptionKey: kmsKey,
removalPolicy: cdk.RemovalPolicy.DESTROY,
autoDeleteObjects: true,
enforceSSL: true,
serverAccessLogsBucket: accessLogsBucket,
serverAccessLogsPrefix: accessLogsBucket
? `${sagemakerArtifactsBucketName}-logs/`
: undefined,
});

if (!accessLogsBucket) {
NagSuppressions.addResourceSuppressions(logsBucket, [
{
id: 'AwsSolutions-S1',
reason:
'This is itself a logging bucket; enabling access logs would create a circular dependency.',
},
]);
}

const sagemakerArtifactsBucket = new s3.Bucket(this, 'ArtifactsBucket', {
bucketName: sagemakerArtifactsBucketName,
encryption: s3.BucketEncryption.KMS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface ModelBuildCodePipelineProps extends cdk.StackProps {
readonly sagemakerExecutionRoleName: string;
readonly codeBuildAssumeRoleName: string;
readonly modelPackageGroupName: string;
readonly s3AccessLogsBucketArn?: string;
}

export class ModelBuildCodePipelineStack extends cdk.Stack {
Expand Down Expand Up @@ -118,7 +119,11 @@ export class ModelBuildCodePipelineStack extends cdk.Stack {

const buildPipeline = new codepipeline.Pipeline(this, 'BuildPipeline', {
pipelineName,
artifactBucket: utils.createPipelineArtifactsBucket(this),
artifactBucket: utils.createPipelineArtifactsBucket(
this,
props.s3AccessLogsBucketArn,
`${pipelineName}-artifacts/`,
),
});
this.pipeline = buildPipeline;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export interface ModelDeployCodePipelineStackProps extends cdk.StackProps {
readonly ssmParamName?: string;
readonly modelPackageGroupName: string;
readonly sagemakerArtifactsBucketName: string;
readonly s3AccessLogsBucketArn?: string;
}

export class ModelDeployCodePipelineStack extends cdk.Stack {
Expand Down Expand Up @@ -89,7 +90,11 @@ export class ModelDeployCodePipelineStack extends cdk.Stack {
// this pipeline will be updated by project-infra pipeline
selfMutation: false,
crossAccountKeys: true,
artifactBucket: utils.createPipelineArtifactsBucket(this),
artifactBucket: utils.createPipelineArtifactsBucket(
this,
props.s3AccessLogsBucketArn,
`${pipelineName}-artifacts/`,
),
synth: new cdk.pipelines.CodeBuildStep('Synth', {
input: cdk.pipelines.CodePipelineSource.codeCommit(
infraRepo,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import * as s3 from 'aws-cdk-lib/aws-s3';
import { NagSuppressions } from 'cdk-nag';
import { Construct } from 'constructs';

export function createPipelineArtifactsBucket(scope: Construct): s3.Bucket {
export function createPipelineArtifactsBucket(
scope: Construct,
s3AccessLogsBucketArn?: string,
logsPrefix?: string,
): s3.Bucket {
const removalPolicy = cdk.RemovalPolicy.DESTROY;
const autoDeleteObjects = removalPolicy === cdk.RemovalPolicy.DESTROY;

Expand All @@ -17,6 +21,14 @@ export function createPipelineArtifactsBucket(scope: Construct): s3.Bucket {
},
);

const accessLogsBucket = s3AccessLogsBucketArn
? s3.Bucket.fromBucketArn(
scope,
'PipelineAccessLogsBucket',
s3AccessLogsBucketArn,
)
: undefined;

const pipelineArtifactsBucket = new s3.Bucket(
scope,
'PipelineArtifactBucket',
Expand All @@ -26,13 +38,20 @@ export function createPipelineArtifactsBucket(scope: Construct): s3.Bucket {
autoDeleteObjects,
bucketKeyEnabled: true,
enforceSSL: true,
serverAccessLogsBucket: accessLogsBucket,
serverAccessLogsPrefix: accessLogsBucket
? logsPrefix || 'pipeline-artifacts/'
: undefined,
},
);
NagSuppressions.addResourceSuppressions(pipelineArtifactsBucket, [
{
id: 'AwsSolutions-S1',
reason: 'The bucket stores pipeline artifacts, no logging required.',
},
]);
if (!accessLogsBucket) {
NagSuppressions.addResourceSuppressions(pipelineArtifactsBucket, [

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is the nag suppression in the "real code" ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If a user does not provide access logging, then they do not have a strict CT.S3.PR ruleset regime. In this scenario we revert to the existing behavior of suppressing. Why it's here, I'm not totally sure. Not ideal, but also do not see a priority to organize today.

{
id: 'AwsSolutions-S1',
reason:
'S3 access logging is optional and was not configured for this deployment.',
},
]);
}
return pipelineArtifactsBucket;
}
Original file line number Diff line number Diff line change
Expand Up @@ -1661,7 +1661,7 @@ exports[`MLOpsCodePipelineStack to match snapshot 1`] = `
"rules_to_suppress": [
{
"id": "AwsSolutions-S1",
"reason": "The bucket stores pipeline artifacts, no logging required.",
"reason": "S3 access logging is optional and was not configured for this deployment.",
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,7 @@ exports[`ModelBuildCodePipelineStack to match snapshot 1`] = `
"rules_to_suppress": [
{
"id": "AwsSolutions-S1",
"reason": "The bucket stores pipeline artifacts, no logging required.",
"reason": "S3 access logging is optional and was not configured for this deployment.",
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,16 @@ exports[`ModelBuildSupportStack to match snapshot 1`] = `
},
"LogsBucket9C4D8843": {
"DeletionPolicy": "Delete",
"Metadata": {
"cdk_nag": {
"rules_to_suppress": [
{
"id": "AwsSolutions-S1",
"reason": "This is itself a logging bucket; enabling access logs would create a circular dependency.",
},
],
},
},
"Properties": {
"AccessControl": "LogDeliveryWrite",
"BucketEncryption": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ exports[`ModelDeployCodePipelineStack to match snapshot 1`] = `
"rules_to_suppress": [
{
"id": "AwsSolutions-S1",
"reason": "The bucket stores pipeline artifacts, no logging required.",
"reason": "S3 access logging is optional and was not configured for this deployment.",
},
],
},
Expand Down
Loading