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

chore: Set up CDK Nag #25

Merged
merged 17 commits into from
Mar 11, 2024
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: 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:
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, this is great. CDK nag runs at synth, but we didn't have a way to capture those errors when PRs are raised unless we deploy the modules manually.

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
Loading