Skip to content

Refactor templates to have fewer imported values #37

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

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft

Refactor templates to have fewer imported values #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
60d1885
refactor branch validation and variable defaults
cat5inthecradle Jan 12, 2024
db4d85d
create reviewable change set instead of blind deploy
cat5inthecradle Jan 12, 2024
8dcffc0
create reviewable change set instead of blind deploy
cat5inthecradle Jan 12, 2024
99f251f
remove permissions boundary
cat5inthecradle Jan 12, 2024
ac1ffa1
tweak comments
cat5inthecradle Jan 12, 2024
22f56be
eliminate ImportValue from app template
cat5inthecradle Jan 12, 2024
2bd3f13
rearrange and add comments
cat5inthecradle Jan 12, 2024
4b513a6
remove migrated role
cat5inthecradle Jan 12, 2024
477bdc3
keep old role to avoid breaking active stacks
cat5inthecradle Jan 12, 2024
8fa1504
support stack creation
cat5inthecradle Jan 12, 2024
3ae221f
fix !Sub and !ImportValue syntax
cat5inthecradle Jan 12, 2024
edbdbc7
revert switch to create-change-set
cat5inthecradle Jan 12, 2024
9f3b3b5
fix template ref typo
cat5inthecradle Jan 12, 2024
9f40ff9
ensure logs aren't destroyed
cat5inthecradle Jan 12, 2024
3e123f9
silence useless output
cat5inthecradle Jan 12, 2024
9b7d879
remove trailing commas
cat5inthecradle Jan 12, 2024
69aa260
tweak array syntax
cat5inthecradle Jan 12, 2024
4853973
import ecs task execution role
cat5inthecradle Jan 12, 2024
1c50a8f
comment unused parameter
cat5inthecradle Jan 12, 2024
e9fdbd3
comment out unused parameter
cat5inthecradle Jan 12, 2024
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
8 changes: 3 additions & 5 deletions cicd/1-setup/cicd-dependencies.template.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ Resources:
- codepipeline.amazonaws.com
Version: '2012-10-17'
Path: /service-role/
PermissionsBoundary: !ImportValue IAM-DevPermissions
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a tentative removal. I don't think we need it here. This stack is being deployed by an Admin anyway. I might be overlooking something though.

Policies:
- PolicyName: AiProxyPassRole
PolicyDocument:
Expand All @@ -41,9 +40,10 @@ Resources:
- Effect: Allow
Action:
- "cloudformation:DescribeStacks"
- "cloudformation:DescribeStackEvents"
- "cloudformation:CreateStack"
- "cloudformation:UpdateStack"
Resource: "*"
Resource: "*" # TODO scope to specific stacks
- PolicyName: CodeBuildResourcesAccess
PolicyDocument:
Statement:
Expand All @@ -68,11 +68,10 @@ Resources:
- s3:GetObjectVersion
Resource:
- !Sub ${ArtifactStore.Arn}/*
# TODO: Scope to specific ECR Repos?
- Effect: Allow
Action:
- ecr:GetAuthorizationToken
Resource: '*'
Resource: '*' # TODO scope to specific ECR repos
- Effect: Allow
Action: codestar-connections:UseConnection
Resource:
Expand Down Expand Up @@ -103,7 +102,6 @@ Resources:
- codebuild.amazonaws.com
Version: '2012-10-17'
Path: /service-role/
PermissionsBoundary: !ImportValue IAM-DevPermissions
Policies:
- PolicyName: PublicCodeBuildSecretsAccess
PolicyDocument:
Expand Down
2 changes: 1 addition & 1 deletion cicd/1-setup/deploy-cicd-dependencies.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ TEMPLATE_FILE=cicd/1-setup/cicd-dependencies.template.yml
echo Validating cloudformation template...
aws cloudformation validate-template \
--template-body file://${TEMPLATE_FILE} \
| cat
> /dev/null

ACCOUNT=$(aws sts get-caller-identity --query "Account" --output text)

Expand Down
142 changes: 104 additions & 38 deletions cicd/2-cicd/cicd.template.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,11 @@ Conditions:

Resources:

# The Elastic Container Registry Repository will store our built docker
# images.
#-------------------------------------
# Elastic Container Registry (ECR)
# - Store built docker images
#-------------------------------------

EcrRepository:
Type: AWS::ECR::Repository
Properties:
Expand Down Expand Up @@ -113,6 +116,10 @@ Resources:
- 'kms:GenerateDataKeyWithoutPlaintext'
Resource: '*'

#-------------------------------------
# CodeBuild Projects
#-------------------------------------

# The CodeBuild Project is triggered by pull requests targeting $GitHubBranch
# It will perform any steps defined in the pr-buildspec.yml file.
PullRequestBuildProject:
Expand Down Expand Up @@ -191,32 +198,9 @@ Resources:
Artifacts:
Type: CODEPIPELINE

# Grant the AiProxy CodeBuild Role additional permissions for resources in
# this template. This allows us to avoid granting permission to * resources.
AiProxyRolePolicy:
Type: 'AWS::IAM::Policy'
Properties:
PolicyName: !Sub "${AWS::StackName}-codebuild-policy"
PolicyDocument:
Version: "2012-10-17"
Statement:
- Effect: Allow
Action:
- codebuild:*
Resource:
- !GetAtt AppBuildProject.Arn
- !GetAtt IntegrationTestBuildProject.Arn
- Effect: Allow
Action:
- codebuild:CreateReportGroup
- codebuild:CreateReport
- codebuild:UpdateReport
- codebuild:BatchPutTestCases
- codebuild:BatchPutCodeCoverage
Resource:
- !Sub arn:aws:codebuild:us-east-1:165336972514:report-group/${AWS::StackName}-${GitHubBranch}-pr-build
Roles:
- !ImportValue AiProxyCodeBuildRoleName
#-------------------------------------
# Pipeline
#-------------------------------------

Pipeline:
Type: AWS::CodePipeline::Pipeline
Expand Down Expand Up @@ -278,11 +262,25 @@ Resources:
ActionMode: CREATE_UPDATE
TemplatePath: appBuildResults::packaged-app-template.yml
TemplateConfiguration: appBuildResults::cicd/3-app/aiproxy/dev.config.json
# ParameterOverrides must be a JSON string, not an object
ParameterOverrides: !Join
- ''
- - '{ "SubdomainName": "'
- !Sub "aiproxy-dev-${GitHubBranch}"
- '" }'
- ''
- - '{'
- !Sub '"SubdomainName": "aiproxy-dev-${GitHubBranch}",'
# - !Sub '"ECRRepositoryArn": "${EcrRepository.Arn}",'
- !Sub
- '"VPC": "${VPCValue}",'
- { VPCValue: !ImportValue VPC }
- !Sub
- '"SecurityGroup": "${SecurityGroupValue}",'
- { SecurityGroupValue: !ImportValue VPC-ELBSecurityGroup }
- !Sub
- '"PublicSubnets": "${PublicSubnetsValue}",'
- { PublicSubnetsValue: !Join [",", [!ImportValue VPC-PublicSubnetB, !ImportValue VPC-PublicSubnetC, !ImportValue VPC-PublicSubnetD, !ImportValue VPC-PublicSubnetE]] }
- !Sub
- '"PrivateSubnets": "${PrivateSubnetsValue}"'
- { PrivateSubnetsValue: !Join [",", [!ImportValue VPC-SubnetB, !ImportValue VPC-SubnetC, !ImportValue VPC-SubnetD, !ImportValue VPC-SubnetE]] }
- '}'
Capabilities: CAPABILITY_AUTO_EXPAND,CAPABILITY_IAM
RoleArn: !Sub arn:aws:iam::${AWS::AccountId}:role/admin/CloudFormationService
- !Ref AWS::NoValue
Expand All @@ -304,11 +302,27 @@ Resources:
ActionMode: CREATE_UPDATE
TemplatePath: appBuildResults::packaged-app-template.yml
TemplateConfiguration: appBuildResults::cicd/3-app/aiproxy/test.config.json
# ParameterOverrides must be a JSON string, not an object
ParameterOverrides: !Join
- ''
- - '{ "SubdomainName": "'
- !If [ TargetsMainBranch, 'aiproxy-test', !Sub 'aiproxy-test-${GitHubBranch}' ]
- '" }'
- - '{'
- !Sub
- '"SubdomainName": "${SubdomainName}",'
- { SubdomainName: !If [ TargetsMainBranch, 'aiproxy-test', !Sub 'aiproxy-test-${GitHubBranch}' ] }
- !Sub '"ECRRepositoryArn": "${EcrRepository.Arn}",'
- !Sub
- '"VPC": "${VPCValue}",'
- { VPCValue: !ImportValue VPC }
- !Sub
- '"SecurityGroup": "${SecurityGroupValue}",'
- { SecurityGroupValue: !ImportValue VPC-ELBSecurityGroup }
- !Sub
- '"PublicSubnets": "[${PublicSubnetsValue}]",'
- { PublicSubnetsValue: !Join [",", [!ImportValue VPC-PublicSubnetB, !ImportValue VPC-PublicSubnetC, !ImportValue VPC-PublicSubnetD, !ImportValue VPC-PublicSubnetE]] }
- !Sub
- '"PrivateSubnets": "[${PrivateSubnetsValue}]"'
- { PrivateSubnetsValue: !Join [",", [!ImportValue VPC-SubnetB, !ImportValue VPC-SubnetC, !ImportValue VPC-SubnetD, !ImportValue VPC-SubnetE]] }
- '}'
Capabilities: CAPABILITY_AUTO_EXPAND,CAPABILITY_IAM
RoleArn: !Sub arn:aws:iam::${AWS::AccountId}:role/admin/CloudFormationService
- !Ref AWS::NoValue
Expand Down Expand Up @@ -353,11 +367,27 @@ Resources:
ActionMode: CREATE_UPDATE
TemplatePath: appBuildResults::packaged-app-template.yml
TemplateConfiguration: appBuildResults::cicd/3-app/aiproxy/production.config.json
# ParameterOverrides must be a JSON string, not an object
ParameterOverrides: !Join
- ''
- - '{ "SubdomainName": "'
- !If [ TargetsMainBranch, 'aiproxy', !Sub 'aiproxy-${GitHubBranch}' ]
- '" }'
- - '{'
- !Sub
- '"SubdomainName": "${SubdomainName}",'
- { SubdomainName: !If [ TargetsMainBranch, 'aiproxy', !Sub 'aiproxy-${GitHubBranch}' ] }
- !Sub '"ECRRepositoryArn": "${EcrRepository.Arn}",'
- !Sub
- '"VPC": "${VPCValue}",'
- { VPCValue: !ImportValue VPC }
- !Sub
- '"SecurityGroup": "${SecurityGroupValue}",'
- { SecurityGroupValue: !ImportValue VPC-ELBSecurityGroup }
- !Sub
- '"PublicSubnets": "[${PublicSubnetsValue}]",'
- { PublicSubnetsValue: !Join [",", [!ImportValue VPC-PublicSubnetB, !ImportValue VPC-PublicSubnetC, !ImportValue VPC-PublicSubnetD, !ImportValue VPC-PublicSubnetE]] }
- !Sub
- '"PrivateSubnets": "[${PrivateSubnetsValue}]"'
- { PrivateSubnetsValue: !Join [",", [!ImportValue VPC-SubnetB, !ImportValue VPC-SubnetC, !ImportValue VPC-SubnetD, !ImportValue VPC-SubnetE]] }
- '}'
Capabilities: CAPABILITY_AUTO_EXPAND,CAPABILITY_IAM
RoleArn: !Sub arn:aws:iam::${AWS::AccountId}:role/admin/CloudFormationService
- !Ref AWS::NoValue
Expand All @@ -384,6 +414,42 @@ Resources:
- Name: smokeTestResults
- !Ref AWS::NoValue


#-------------------------------------
# IAM Roles & Policies
#-------------------------------------

# Grant the AiProxy CodeBuild Role additional permissions for resources in
# this template. This allows us to avoid granting permission to * resources.
AiProxyRolePolicy:
Type: 'AWS::IAM::Policy'
Properties:
PolicyName: !Sub "${AWS::StackName}-codebuild-policy"
PolicyDocument:
Version: "2012-10-17"
Statement:
- Effect: Allow
Action:
- codebuild:*
Resource:
- !GetAtt AppBuildProject.Arn
- !GetAtt IntegrationTestBuildProject.Arn
- Effect: Allow
Action:
- codebuild:CreateReportGroup
- codebuild:CreateReport
- codebuild:UpdateReport
- codebuild:BatchPutTestCases
- codebuild:BatchPutCodeCoverage
Resource:
- !Sub arn:aws:codebuild:us-east-1:165336972514:report-group/${AWS::StackName}-${GitHubBranch}-pr-build
Roles:
- !ImportValue AiProxyCodeBuildRoleName

#-------------------------------------
# Metrics & Notifications
#-------------------------------------

# Send pipeline events to an SNS topic.
# Note:
# Integration with Slack via AWS ChatBot is configured manually via AWS
Expand Down
2 changes: 1 addition & 1 deletion cicd/2-cicd/deploy-cicd.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ TEMPLATE_FILE=cicd/2-cicd/cicd.template.yml
echo Validating cloudformation template...
aws cloudformation validate-template \
--template-body file://${TEMPLATE_FILE} \
| cat
> /dev/null

ACCOUNT=$(aws sts get-caller-identity --query "Account" --output text)

Expand Down
77 changes: 58 additions & 19 deletions cicd/3-app/aiproxy/template.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ AWSTemplateFormatVersion: 2010-09-09
Transform: AWS::Serverless-2016-10-31
Description: Provision an instance of the AI Proxy service.

# Dependencies: This template has dependencies, look for !ImportValue in the Resources section.

Parameters:
BaseDomainName:
Type: String
Expand All @@ -14,9 +12,20 @@ Parameters:
SubdomainName:
Type: String
Description: Subdomain name for aiproxy service (e.g. 'aiproxy' in 'aiproxy.code.org').
# ECRRepositoryArn:
# Type: String
# Description: ARN of the ECR repository for this service.
AppImageUri:
Type: String
Description: URI of the Docker image in ECR.
VPC:
Type: AWS::EC2::VPC::Id
SecurityGroup:
Type: AWS::EC2::SecurityGroup::Id
PublicSubnets:
Type: List<AWS::EC2::Subnet::Id>
PrivateSubnets:
Type: List<AWS::EC2::Subnet::Id>

# Conditions:
# IsDevCondition: !Equals [!Ref BaseDomainName, "dev-code.org"]
Expand Down Expand Up @@ -63,16 +72,11 @@ Resources:
LoadBalancerAttributes:
- Key: idle_timeout.timeout_seconds
Value: 180
SecurityGroups:
- !ImportValue VPC-ELBSecurityGroup
Subnets:
# Place load balancer in public subnets, so it's accessible from the internet.
# We may want to move this to the private subnets, so only internal resources
# can access it, but this is very convenient for local development.
- !ImportValue VPC-PublicSubnetB
- !ImportValue VPC-PublicSubnetC
- !ImportValue VPC-PublicSubnetD
- !ImportValue VPC-PublicSubnetE
SecurityGroups: [!Ref SecurityGroup]
# Place load balancer in public subnets, so it's accessible from the internet.
# We may want to move this to the private subnets, so only internal resources
# can access it, but this is very convenient for local development.
Subnets: !Ref PublicSubnets

HttpListener:
Type: AWS::ElasticLoadBalancingV2::Listener
Expand Down Expand Up @@ -114,7 +118,7 @@ Resources:
TargetGroup:
Type: AWS::ElasticLoadBalancingV2::TargetGroup
Properties:
VpcId: !ImportValue VPC
VpcId: !Ref VPC
Port: 80
TargetType: ip
Protocol: HTTP
Expand Down Expand Up @@ -147,12 +151,8 @@ Resources:
AssignPublicIp: DISABLED
SecurityGroups:
- !Ref ECSSecurityGroup
Subnets:
# Place ECS Service in private subnets, but traffic should use the LoadBalancer.
- !ImportValue VPC-SubnetB
- !ImportValue VPC-SubnetC
- !ImportValue VPC-SubnetD
- !ImportValue VPC-SubnetE
Subnets: !Ref PrivateSubnets
LoadBalancers:
- ContainerName: aiproxy
ContainerPort: 80
Expand All @@ -163,7 +163,7 @@ Resources:
Properties:
GroupDescription: Security Group for ECS Service
# TODO: This copies geocoder, but we should probably have a separate VPC for this service.
VpcId: !ImportValue VPC
VpcId: !Ref VPC
SecurityGroupIngress:
- IpProtocol: tcp
FromPort: 80
Expand Down Expand Up @@ -195,12 +195,51 @@ Resources:
awslogs-region: !Ref AWS::Region
awslogs-stream-prefix: ecs

# ECSTaskExecutionRole:
# Type: AWS::IAM::Role
# Properties:
# AssumeRolePolicyDocument:
# Version: "2012-10-17"
# Statement:
# - Effect: Allow
# Principal:
# Service:
# - ecs-tasks.amazonaws.com
# Action:
# - sts:AssumeRole
# Policies:
# - PolicyName: ECRPolicy
# PolicyDocument:
# Version: "2012-10-17"
# Statement:
# - Effect: Allow
# Action:
# - ecr:GetAuthorizationToken
# - ecr:BatchCheckLayerAvailability
# - ecr:GetDownloadUrlForLayer
# - ecr:BatchGetImage
# Resource: !Ref ECRRepositoryArn
# - PolicyName: LogsPolicy
# PolicyDocument:
# Version: "2012-10-17"
# Statement:
# - Effect: Allow
# Action:
# - logs:CreateLogGroup
# - logs:CreateLogStream
# - logs:PutLogEvents
# Resource:
# - !GetAtt LogGroup.Arn
# - !Sub "${LogGroup.Arn}:*"

# ------------------
# Logging & Alerts
# ------------------

LogGroup:
Type: AWS::Logs::LogGroup
DeletionPolicy: Retain
UpdateReplacePolicy: Retain
Properties:
LogGroupName: !Sub "${AWS::StackName}"

Expand Down