Skip to content

Commit

Permalink
chore: Set up CDK Nag (#25)
Browse files Browse the repository at this point in the history
* add CDK nag for fmops

* Reduce the scope of CDK nag suppressions for mlflow modules

* Reduce the scope of CDK nag suppressions for endpoint module

* fix formatting

* add CDK nag to sagemaker studio module

* fix static checkers

* fix mypy error

* add cdk nag to the templates module

* Add CDK nag to sagemaker-notebook

* add CDK nag unit test checks

* fix formatting for fmops module

* fix sagemaker templates tests

* add CDK nag unit test for mlflow-farhate

* add CDK nag unit test for mlflow-image

* add CDK nag unit test for sagemaker-endpoint

* update tests for sagemaker-studio

* fix Lambda version suppression for sagemaker-studio
  • Loading branch information
LeonLuttenberger authored Mar 11, 2024
1 parent 6ce6fff commit ac5b229
Show file tree
Hide file tree
Showing 25 changed files with 321 additions and 93 deletions.
3 changes: 3 additions & 0 deletions modules/fmops/sagemaker-jumpstart-fm-endpoint/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import "source-map-support/register";
import * as cdk from "aws-cdk-lib";
import * as cdk_nag from "cdk-nag";
import { SagemakerJumpStartFmEndpointStack } from "./lib/sagemaker-jumpstart-fm-endpoint-stack";

const account = process.env.CDK_DEFAULT_ACCOUNT;
Expand Down Expand Up @@ -35,4 +36,6 @@ new cdk.CfnOutput(stack, "metadata", {
}),
});

cdk.Aspects.of(app).add(new cdk_nag.AwsSolutionsChecks({ logIgnores: true }));

app.synth();
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
SageMakerInstanceType,
JumpStartSageMakerEndpoint,
} from "@cdklabs/generative-ai-cdk-constructs";
import * as cdk_nag from "cdk-nag";

interface SagemakerJumpStartFmEndpointStackProps extends cdk.StackProps {
projectName?: string;
Expand Down Expand Up @@ -91,5 +92,12 @@ export class SagemakerJumpStartFmEndpointStack extends cdk.Stack {
role: this.role,
vpcConfig: vpcConfig,
});

cdk_nag.NagSuppressions.addResourceSuppressions(this.role, [
{
id: "AwsSolutions-IAM5",
reason: "Resource access restriced to S3 buckets (with a prefix) and ECR images",
},
]);
}
}
13 changes: 7 additions & 6 deletions modules/fmops/sagemaker-jumpstart-fm-endpoint/package-lock.json

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

1 change: 1 addition & 0 deletions modules/fmops/sagemaker-jumpstart-fm-endpoint/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"@types/jest": "^29.5.5",
"@types/node": "20.7.1",
"aws-cdk": "2.130.0",
"cdk-nag": "^2.28.55",
"cypress": "^13.6.1",
"jest": "^29.7.0",
"prettier": "^3.1.1",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import * as cdk from "aws-cdk-lib";
import { Template } from "aws-cdk-lib/assertions";
import { Annotations, Match, Template } from "aws-cdk-lib/assertions";
import { SagemakerJumpStartFmEndpointStack } from "../lib/sagemaker-jumpstart-fm-endpoint-stack";

test("Synth stack", () => {
describe("Sagemaker JumpStart Fm Endpoint Stack", () => {
const app = new cdk.App();

const projectName = "mlops";
Expand All @@ -26,9 +26,16 @@ test("Synth stack", () => {
env: { account, region },
});

const template = Template.fromStack(stack);
test("Synth stack", () => {
const template = Template.fromStack(stack);

template.hasResource("AWS::SageMaker::Endpoint", {});
template.hasResource("AWS::IAM::Role", {});
template.hasResource("AWS::EC2::SecurityGroup", {});
template.hasResource("AWS::SageMaker::Endpoint", {});
template.hasResource("AWS::IAM::Role", {});
template.hasResource("AWS::EC2::SecurityGroup", {});
});

test("No CDK Nag Errors", () => {
const errors = Annotations.fromStack(stack).findError("*", Match.stringLikeRegexp("AwsSolutions-.*"));
expect(errors).toHaveLength(0);
});
});
3 changes: 3 additions & 0 deletions modules/mlflow/mlflow-fargate/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from typing import Optional, cast

import aws_cdk
import cdk_nag

from stack import MlflowFargateStack, RDSSettings

Expand Down Expand Up @@ -116,4 +117,6 @@ def _param(name: str) -> str:
),
)

aws_cdk.Aspects.of(app).add(cdk_nag.AwsSolutionsChecks(log_ignores=True))

app.synth()
32 changes: 20 additions & 12 deletions modules/mlflow/mlflow-fargate/stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def __init__(

cdk.Tags.of(scope=cast(IConstruct, self)).add(key="Deployment", value=app_prefix[:64])

role = iam.Role(
task_role = iam.Role(
self,
"TaskRole",
assumed_by=iam.ServicePrincipal(service="ecs-tasks.amazonaws.com"),
Expand All @@ -58,7 +58,7 @@ def __init__(

# Grant artifacts bucket read-write permissions
model_bucket = s3.Bucket.from_bucket_name(self, "ArtifactsBucket", bucket_name=artifacts_bucket_name)
model_bucket.grant_read_write(role)
model_bucket.grant_read_write(task_role)

vpc = ec2.Vpc.from_lookup(self, "Vpc", vpc_id=vpc_id)
subnets = [ec2.Subnet.from_subnet_id(self, f"Subnet {subnet_id}", subnet_id) for subnet_id in subnet_ids]
Expand All @@ -76,7 +76,7 @@ def __init__(
task_definition = ecs.FargateTaskDefinition(
self,
"MlflowTask",
task_role=role,
task_role=task_role,
cpu=task_cpu_units,
memory_limit_mib=task_memory_limit_mb,
)
Expand Down Expand Up @@ -172,8 +172,8 @@ def __init__(
fs.connections.allow_default_port_from(fargate_service.service.connections)

# Setup security group
fargate_securigy_group = fargate_service.service.connections.security_groups[0]
fargate_securigy_group.add_ingress_rule(
fargate_security_group = fargate_service.service.connections.security_groups[0]
fargate_security_group.add_ingress_rule(
peer=ec2.Peer.ipv4(vpc.vpc_cidr_block),
connection=ec2.Port.tcp(5000),
description="Allow inbound from VPC for mlflow",
Expand All @@ -185,7 +185,7 @@ def __init__(
self, "RDS Security Group", rds_settings["security_group_id"]
)
rds_security_group.add_ingress_rule(
peer=ec2.Peer.security_group_id(fargate_securigy_group.security_group_id),
peer=ec2.Peer.security_group_id(fargate_security_group.security_group_id),
connection=ec2.Port.tcp(rds_settings["port"]),
description="Allow inbound access to RDS for mlflow",
)
Expand All @@ -199,12 +199,10 @@ def __init__(
scale_out_cooldown=cdk.Duration.seconds(60),
)

# Add CDK nag solutions checks
cdk.Aspects.of(self).add(cdk_nag.AwsSolutionsChecks(log_ignores=True))

cdk_nag.NagSuppressions.add_stack_suppressions(
self,
apply_to_nested_stacks=True,
# Add CDK nag suppressions
cdk_nag.NagSuppressions.add_resource_suppressions(
[task_role, task_definition.execution_role], # type: ignore[list-item]
apply_to_children=True,
suppressions=[
cdk_nag.NagPackSuppression(
id="AwsSolutions-IAM4",
Expand All @@ -214,10 +212,20 @@ def __init__(
id="AwsSolutions-IAM5",
reason="Resource access restricted to resources",
),
],
)
cdk_nag.NagSuppressions.add_resource_suppressions(
task_definition,
suppressions=[
cdk_nag.NagPackSuppression(
id="AwsSolutions-ECS2",
reason="Not passing secrets via env variables",
),
],
)
cdk_nag.NagSuppressions.add_resource_suppressions(
lb_access_logs_bucket,
suppressions=[
cdk_nag.NagPackSuppression(
id="AwsSolutions-S1",
reason="Access logs not required for access logs bucket",
Expand Down
23 changes: 19 additions & 4 deletions modules/mlflow/mlflow-fargate/tests/test_stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
from unittest import mock

import aws_cdk as cdk
import cdk_nag
import pytest
from aws_cdk.assertions import Template
from aws_cdk.assertions import Annotations, Match, Template


@pytest.fixture(scope="function")
Expand All @@ -20,8 +21,8 @@ def stack_defaults():
yield


@pytest.mark.parametrize("use_rds", [False, True])
def test_synthesize_stack(stack_defaults, use_rds) -> None:
@pytest.fixture(scope="function")
def stack(stack_defaults, use_rds: bool) -> cdk.Stack:
import stack

app = cdk.App()
Expand Down Expand Up @@ -49,7 +50,7 @@ def test_synthesize_stack(stack_defaults, use_rds) -> None:
else:
rds_settings = None

stack = stack.MlflowFargateStack(
return stack.MlflowFargateStack(
scope=app,
id=app_prefix,
app_prefix=app_prefix,
Expand All @@ -71,6 +72,20 @@ def test_synthesize_stack(stack_defaults, use_rds) -> None:
),
)


@pytest.mark.parametrize("use_rds", [False, True])
def test_synthesize_stack(stack: cdk.Stack, use_rds: bool) -> None:
template = Template.from_stack(stack)
template.resource_count_is("AWS::ECS::Cluster", 1)
template.resource_count_is("AWS::ECS::TaskDefinition", 1)


@pytest.mark.parametrize("use_rds", [False, True])
def test_no_cdk_nag_errors(stack: cdk.Stack, use_rds: bool) -> None:
cdk.Aspects.of(stack).add(cdk_nag.AwsSolutionsChecks())

nag_errors = Annotations.from_stack(stack).find_error(
"*",
Match.string_like_regexp(r"AwsSolutions-.*"),
)
assert not nag_errors, f"Found {len(nag_errors)} CDK nag errors"
3 changes: 3 additions & 0 deletions modules/mlflow/mlflow-image/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os

import aws_cdk
import cdk_nag

from stack import MlflowImagePublishingStack

Expand Down Expand Up @@ -51,4 +52,6 @@ def _param(name: str) -> str:
),
)

aws_cdk.Aspects.of(app).add(cdk_nag.AwsSolutionsChecks(log_ignores=True))

app.synth()
8 changes: 4 additions & 4 deletions modules/mlflow/mlflow-image/stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,16 @@ def __init__(
dest=ecr_deployment.DockerImageName(self.image_uri),
)

# Add CDK nag solutions checks
cdk.Aspects.of(self).add(cdk_nag.AwsSolutionsChecks(log_ignores=True))

# Add CDK nag suppressions
cdk_nag.NagSuppressions.add_stack_suppressions(
self,
apply_to_nested_stacks=True,
suppressions=[
cdk_nag.NagPackSuppression(
id="AwsSolutions-IAM4",
reason="Managed Policies are for src account roles only",
applies_to=[
"Policy::arn:<AWS::Partition>:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole",
],
),
cdk_nag.NagPackSuppression(
id="AwsSolutions-IAM5",
Expand Down
20 changes: 17 additions & 3 deletions modules/mlflow/mlflow-image/tests/test_stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
import sys

import aws_cdk as cdk
import cdk_nag
import pytest
from aws_cdk.assertions import Template
from aws_cdk.assertions import Annotations, Match, Template


@pytest.fixture(scope="function")
Expand All @@ -16,7 +17,8 @@ def stack_defaults() -> None:
del sys.modules["stack"]


def test_synthesize_stack() -> None:
@pytest.fixture(scope="function")
def stack(stack_defaults) -> cdk.Stack:
import stack

app = cdk.App()
Expand All @@ -28,7 +30,7 @@ def test_synthesize_stack() -> None:

ecr_repo_name = "repo"

stack = stack.MlflowImagePublishingStack(
return stack.MlflowImagePublishingStack(
scope=app,
id=app_prefix,
app_prefix=app_prefix,
Expand All @@ -39,5 +41,17 @@ def test_synthesize_stack() -> None:
),
)


def test_synthesize_stack(stack: cdk.Stack) -> None:
template = Template.from_stack(stack)
template.resource_count_is("Custom::CDKBucketDeployment", 1)


def test_no_cdk_nag_errors(stack: cdk.Stack) -> None:
cdk.Aspects.of(stack).add(cdk_nag.AwsSolutionsChecks())

nag_errors = Annotations.from_stack(stack).find_error(
"*",
Match.string_like_regexp(r"AwsSolutions-.*"),
)
assert not nag_errors, f"Found {len(nag_errors)} CDK nag errors"
4 changes: 3 additions & 1 deletion modules/sagemaker/sagemaker-endpoint/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import os

import aws_cdk
import cdk_nag

from stack import DeployEndpointStack

Expand Down Expand Up @@ -84,7 +85,7 @@ def _param(name: str) -> str:
id="metadata",
value=stack.to_json_string(
{
"ModelExecutionRoleArn": stack.model_execution_role_arn,
"ModelExecutionRoleArn": stack.model_execution_role.role_arn,
"ModelName": stack.model.model_name,
"ModelPackageArn": stack.model_package_arn,
"EndpointName": stack.endpoint.attr_endpoint_name,
Expand All @@ -93,5 +94,6 @@ def _param(name: str) -> str:
),
)

aws_cdk.Aspects.of(app).add(cdk_nag.AwsSolutionsChecks(log_ignores=True))

app.synth()
Loading

0 comments on commit ac5b229

Please sign in to comment.