Skip to content

BREAKING CHANGE - Align content bucket name with url #394

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

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion beta-template.yml.erb
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ Resources:
ContentBucket:
Type: AWS::S3::Bucket
Properties:
BucketName: !If [IsDevCondition, !Sub "cdo-dev-${SubdomainName}-content", !Sub "cdo-${SubdomainName}-content"]
BucketName: !Sub "${SubdomainName}-content.${BaseDomainName}"
CorsConfiguration:
CorsRules:
- AllowedMethods: [GET, PUT]
Expand Down
8 changes: 4 additions & 4 deletions cicd/2-cicd/cicd.template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ Resources:
- - '{ "SubdomainName": "'
- !Sub "javabuilder-dev-${GitHubBranch}"
- '" }'
Capabilities: CAPABILITY_AUTO_EXPAND
Capabilities: CAPABILITY_AUTO_EXPAND,CAPABILITY_IAM
RoleArn: !Sub arn:aws:iam::${AWS::AccountId}:role/admin/CloudFormationService
- !Ref AWS::NoValue

Expand All @@ -343,7 +343,7 @@ Resources:
- - '{ "SubdomainName": "'
- !If [ TargetsMainBranch, 'javabuilder-test', !Sub 'javabuilder-${GitHubBranch}-test' ]
- '" }'
Capabilities: CAPABILITY_AUTO_EXPAND
Capabilities: CAPABILITY_AUTO_EXPAND,CAPABILITY_IAM
RoleArn: !Sub arn:aws:iam::${AWS::AccountId}:role/admin/CloudFormationService
- !Ref AWS::NoValue

Expand Down Expand Up @@ -392,7 +392,7 @@ Resources:
- - '{ "SubdomainName": "'
- !If [ TargetsMainBranch, 'javabuilder', !Sub 'javabuilder-${GitHubBranch}' ]
- '" }'
Capabilities: CAPABILITY_AUTO_EXPAND
Capabilities: CAPABILITY_AUTO_EXPAND,CAPABILITY_IAM
RoleArn: !Sub arn:aws:iam::${AWS::AccountId}:role/admin/CloudFormationService
- Name: app-demo-deploy
ActionTypeId:
Expand Down Expand Up @@ -422,7 +422,7 @@ Resources:
!Sub "javabuilder-demo-${GitHubBranch}",
]
- '" }'
Capabilities: CAPABILITY_AUTO_EXPAND
Capabilities: CAPABILITY_AUTO_EXPAND,CAPABILITY_IAM
RoleArn: !Sub arn:aws:iam::${AWS::AccountId}:role/admin/CloudFormationService
- !Ref AWS::NoValue

Expand Down
33 changes: 32 additions & 1 deletion cicd/3-app/javabuilder/template.yml.erb
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ Resources:
ContentBucket:
Type: AWS::S3::Bucket
Properties:
BucketName: !If [IsDevCondition, !Sub "cdo-dev-${SubdomainName}-content", !Sub "cdo-${SubdomainName}-content"]
BucketName: !Sub "${SubdomainName}-content.${BaseDomainName}"
CorsConfiguration:
CorsRules:
- AllowedMethods: [GET, PUT]
Expand All @@ -500,6 +500,37 @@ Resources:
Resource: !Sub "arn:aws:s3:::${ContentBucket}/*"
Principal: '*'

# TODO: the cloudformation role used to deploy this does not have permission to putRolePolicy. I'd like to find a way to allow creation of roles within this template.
ContentBucketWritePolicy:
Type: AWS::IAM::Policy
Properties:
PolicyName: !Sub "${AWS::StackName}-content-bucket-write-policy"
PolicyDocument:
Statement:
- Action:
- 's3:PutObject'
Effect: Allow
Resource: !Sub "arn:aws:s3:::${ContentBucket}/*"
Roles:
# TODO: do not hard code these!
- javabuilder-iam-PutSourcesLambdaRole-1R0GH6YNAXIC3
- javabuilder-iam-BuildAndRunLambdaRole-ZR48U5GF0610

# TODO: the cloudformation role used to deploy this does not have permission to putRolePolicy. I'd like to find a way to allow creation of roles within this template.
ContentBucketReadPolicy:
Type: AWS::IAM::Policy
Properties:
PolicyName: !Sub "${AWS::StackName}-content-bucket-read-policy"
PolicyDocument:
Statement:
- Action:
- 's3:GetObject'
Effect: Allow
Resource: !Sub "arn:aws:s3:::${ContentBucket}/*"
Roles:
# TODO: do not hard code this!
- javabuilder-iam-BuildAndRunLambdaRole-ZR48U5GF0610

ContentApiCertificate:
Type: AWS::CertificateManager::Certificate
Properties:
Expand Down
1 change: 1 addition & 0 deletions cicd/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ You can create a Development (aka 'adhoc') environment by setting the `ENVIRONME

Notes:

* branch names are used as a unique identifier (stack name, among others). Therefore we can only deploy one CI/CD pipeline per branch per AWS Account.
* your branch name cannot contain the character `/`, as this causes issues in AWS. Note that resources will be deployed with the tags `{EnvType = development}`.
* for now, these must deployed to the production AWS account. There is planned work to enable these to be deployed to the Dev AWS account.

Expand Down
32 changes: 6 additions & 26 deletions iam.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ Parameters:

Resources:
# Permissions for the long-running lambda that compiles/runs student projects
# S3 Read/Write permissions are granted within the app stack
# TODO: create per-environment in app stack
BuildAndRunLambdaRole:
Type: AWS::IAM::Role
Properties:
Expand All @@ -32,22 +34,11 @@ Resources:
- PolicyName: BuildAndRunLambdaExecutionPolicy
PolicyDocument:
Statement:
# BuildAndRunJavaProject Lambda needs to put objects to the content bucket.
- Effect: Allow
Action:
- 's3:PutObject'
- 's3:GetObject'
Resource: 'arn:aws:s3:::cdo-*javabuilder*-content/*'
# Read from Javabuilder SQS Queue
- Effect: Allow
Action:
- "sqs:ReceiveMessage"
- "sqs:DeleteMessage"
# May need the following
# - "sqs:GetQueueAttributes"
# - "sqs:GetQueueUrl"
# - "sqs:ListDeadLetterSourceQueues"
# - "sqs:ListQueues"
Resource:
# TODO: limit this
- '*'
Expand Down Expand Up @@ -149,6 +140,8 @@ Resources:
- '*'

# Permissions for the lambda that uploads student code to S3
# Per-bucket PutObject permissions are granted in the app template
# TODO move this role to the app stack
PutSourcesLambdaRole:
Type: AWS::IAM::Role
Properties:
Expand All @@ -160,15 +153,6 @@ Resources:
Principal: {Service: [lambda.amazonaws.com]}
ManagedPolicyArns:
- !Ref JavabuilderLoggingPolicy
Policies:
- PolicyName: named
PolicyDocument:
Statement:
# Put objects to the content bucket.
- Effect: Allow
Action:
- 's3:putObject'
Resource: 'arn:aws:s3:::cdo-*javabuilder*-content/*'

# Shared permissions that several lambdas need
JavabuilderLoggingPolicy:
Expand Down Expand Up @@ -221,12 +205,6 @@ Resources:
- "cloudformation:DescribeStacks"
Resource: '*'

# BuildAndRunJavaProject Lambda needs to put objects to the content bucket.
- Effect: Allow
Action:
- 's3:PutObject'
Resource: 'arn:aws:s3:::cdo-*javabuilder*-content/*'

# All Lambdas need logging permissions.
- Effect: Allow
Action:
Expand Down Expand Up @@ -333,8 +311,10 @@ Resources:
- !Sub "arn:aws:s3:::${TemplateBucket}/*"
- Effect: Allow
Action:
# TODO: limit this permission to only actions required by cloudformation (low priority)
- "s3:*"
Resource:
# TODO: use new bucket name
- "arn:aws:s3:::cdo-*javabuilder*-content"
- Effect: Allow
Action:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public void testGenerateAssetUrlDoesNotReturnStubUrlIfNotDashboard() {
Properties.setCanAccessDashboardAssets(false);

final String filename = "file";
final String actualUrl = "cdo-javabuilderbeta-content/file.wav";
final String actualUrl = "javabuilder-test.code.org-content/file.wav";
when(projectData.getAssetUrl(filename)).thenReturn(actualUrl);

assertEquals(actualUrl, contentManager.getAssetUrl(filename));
Expand Down