From 284a861af6a050022e954b198658fc1eea1ab50e Mon Sep 17 00:00:00 2001 From: Darin Webb Date: Wed, 24 May 2023 12:23:11 -0500 Subject: [PATCH 01/11] align content bucket name with url BREAKING CHANGE: requires bucket replacement, must be deployed carefully and with downtime. --- cicd/3-app/javabuilder/template.yml.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cicd/3-app/javabuilder/template.yml.erb b/cicd/3-app/javabuilder/template.yml.erb index 3a54d82e..8f907de9 100644 --- a/cicd/3-app/javabuilder/template.yml.erb +++ b/cicd/3-app/javabuilder/template.yml.erb @@ -465,7 +465,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] From 01097001dd9b313bfd577ee0b38f79920683cf6c Mon Sep 17 00:00:00 2001 From: Darin Webb Date: Wed, 21 Jun 2023 11:52:39 -0500 Subject: [PATCH 02/11] rename bucket in old beta template --- beta-template.yml.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beta-template.yml.erb b/beta-template.yml.erb index d519f46f..5036eea4 100644 --- a/beta-template.yml.erb +++ b/beta-template.yml.erb @@ -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] From e96e35d6a503660834abc6fbf9b73fc670ea32f5 Mon Sep 17 00:00:00 2001 From: Darin Webb Date: Wed, 21 Jun 2023 11:53:28 -0500 Subject: [PATCH 03/11] migrate content bucket permissions moved from global iam stack to per-environment app stack --- cicd/3-app/javabuilder/template.yml.erb | 25 +++++++++++++++++++++++ iam.yml | 27 ++++++------------------- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/cicd/3-app/javabuilder/template.yml.erb b/cicd/3-app/javabuilder/template.yml.erb index 8f907de9..c417baa4 100644 --- a/cicd/3-app/javabuilder/template.yml.erb +++ b/cicd/3-app/javabuilder/template.yml.erb @@ -492,6 +492,31 @@ Resources: Resource: !Sub "arn:aws:s3:::${ContentBucket}/*" Principal: '*' + ContentBucketWritePolicy: + Type: AWS::IAM::Policy + Properties: + PolicyDocument: + Statement: + - Action: + - 's3:PutObject' + Effect: Allow + Resource: !Sub "arn:aws:s3:::${ContentBucket}/*" + Roles: + - !ImportValue JavabuilderPutSourcesLambdaRole + - !ImportValue JavabuilderBuildAndRunLambdaRole + + ContentBucketReadPolicy: + Type: AWS::IAM::Policy + Properties: + PolicyDocument: + Statement: + - Action: + - 's3:GetObject' + Effect: Allow + Resource: !Sub "arn:aws:s3:::${ContentBucket}/*" + Roles: + - !ImportValue JavabuilderBuildAndRunLambdaRole + ContentApiCertificate: Type: AWS::CertificateManager::Certificate Properties: diff --git a/iam.yml b/iam.yml index 04c1f679..7e00f87a 100644 --- a/iam.yml +++ b/iam.yml @@ -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: @@ -32,12 +34,6 @@ 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: @@ -149,6 +145,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: @@ -160,15 +158,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: @@ -221,12 +210,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: @@ -333,8 +316,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: From 1a34e992f6e0bdf3033fd3a1e5c8ff5cc40000b8 Mon Sep 17 00:00:00 2001 From: Darin Webb Date: Wed, 21 Jun 2023 11:53:40 -0500 Subject: [PATCH 04/11] update test to use more realistic url --- .../test/java/org/code/javabuilder/AWSContentManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/AWSContentManagerTest.java b/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/AWSContentManagerTest.java index cf77efe3..4aa4c18e 100644 --- a/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/AWSContentManagerTest.java +++ b/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/AWSContentManagerTest.java @@ -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)); From 0c1bb2e12efb2891a1b1ac24f7ee78504d56da7b Mon Sep 17 00:00:00 2001 From: Darin Webb Date: Wed, 21 Jun 2023 13:54:25 -0500 Subject: [PATCH 05/11] Fix linting error in app stack --- cicd/3-app/javabuilder/template.yml.erb | 2 ++ cicd/README.md | 1 + 2 files changed, 3 insertions(+) diff --git a/cicd/3-app/javabuilder/template.yml.erb b/cicd/3-app/javabuilder/template.yml.erb index 6e2da7c7..1d59cfab 100644 --- a/cicd/3-app/javabuilder/template.yml.erb +++ b/cicd/3-app/javabuilder/template.yml.erb @@ -503,6 +503,7 @@ Resources: ContentBucketWritePolicy: Type: AWS::IAM::Policy Properties: + PolicyName: !Sub "${AWS::StackName}-content-bucket-write-policy" PolicyDocument: Statement: - Action: @@ -516,6 +517,7 @@ Resources: ContentBucketReadPolicy: Type: AWS::IAM::Policy Properties: + PolicyName: !Sub "${AWS::StackName}-content-bucket-read-policy" PolicyDocument: Statement: - Action: diff --git a/cicd/README.md b/cicd/README.md index 1c259fa6..f64596fa 100644 --- a/cicd/README.md +++ b/cicd/README.md @@ -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. From 305801c13bd98e69215bf2463a4f7bb385ed4a16 Mon Sep 17 00:00:00 2001 From: Darin Webb Date: Wed, 21 Jun 2023 17:03:38 -0500 Subject: [PATCH 06/11] add capabilities to stack create --- cicd/2-cicd/cicd.template.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cicd/2-cicd/cicd.template.yml b/cicd/2-cicd/cicd.template.yml index c0a8ac66..b2ab6405 100644 --- a/cicd/2-cicd/cicd.template.yml +++ b/cicd/2-cicd/cicd.template.yml @@ -392,7 +392,9 @@ 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: @@ -422,7 +424,9 @@ 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 From a15d8dc256d2f4e7a606560b383c092d830fea3d Mon Sep 17 00:00:00 2001 From: Darin Webb Date: Wed, 21 Jun 2023 17:03:46 -0500 Subject: [PATCH 07/11] add iam capability --- cicd/2-cicd/cicd.template.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cicd/2-cicd/cicd.template.yml b/cicd/2-cicd/cicd.template.yml index b2ab6405..d8e7384c 100644 --- a/cicd/2-cicd/cicd.template.yml +++ b/cicd/2-cicd/cicd.template.yml @@ -317,7 +317,9 @@ 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 @@ -343,7 +345,9 @@ 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 From 3762862bee15f63dd68b06f603dd5c07b1cd529a Mon Sep 17 00:00:00 2001 From: Darin Webb Date: Wed, 21 Jun 2023 17:29:28 -0500 Subject: [PATCH 08/11] fix json syntax --- cicd/2-cicd/cicd.template.yml | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/cicd/2-cicd/cicd.template.yml b/cicd/2-cicd/cicd.template.yml index d8e7384c..9c9e1448 100644 --- a/cicd/2-cicd/cicd.template.yml +++ b/cicd/2-cicd/cicd.template.yml @@ -317,9 +317,7 @@ Resources: - - '{ "SubdomainName": "' - !Sub "javabuilder-dev-${GitHubBranch}" - '" }' - Capabilities: - - CAPABILITY_AUTO_EXPAND - - CAPABILITY_IAM + Capabilities: CAPABILITY_AUTO_EXPAND,CAPABILITY_IAM RoleArn: !Sub arn:aws:iam::${AWS::AccountId}:role/admin/CloudFormationService - !Ref AWS::NoValue @@ -345,9 +343,7 @@ Resources: - - '{ "SubdomainName": "' - !If [ TargetsMainBranch, 'javabuilder-test', !Sub 'javabuilder-${GitHubBranch}-test' ] - '" }' - Capabilities: - - CAPABILITY_AUTO_EXPAND - - CAPABILITY_IAM + Capabilities: CAPABILITY_AUTO_EXPAND,CAPABILITY_IAM RoleArn: !Sub arn:aws:iam::${AWS::AccountId}:role/admin/CloudFormationService - !Ref AWS::NoValue @@ -396,9 +392,7 @@ Resources: - - '{ "SubdomainName": "' - !If [ TargetsMainBranch, 'javabuilder', !Sub 'javabuilder-${GitHubBranch}' ] - '" }' - Capabilities: - - CAPABILITY_AUTO_EXPAND - - CAPABILITY_IAM + Capabilities: CAPABILITY_AUTO_EXPAND,CAPABILITY_IAM RoleArn: !Sub arn:aws:iam::${AWS::AccountId}:role/admin/CloudFormationService - Name: app-demo-deploy ActionTypeId: @@ -428,9 +422,7 @@ Resources: !Sub "javabuilder-demo-${GitHubBranch}", ] - '" }' - Capabilities: - - CAPABILITY_AUTO_EXPAND - - CAPABILITY_IAM + Capabilities: CAPABILITY_AUTO_EXPAND,CAPABILITY_IAM RoleArn: !Sub arn:aws:iam::${AWS::AccountId}:role/admin/CloudFormationService - !Ref AWS::NoValue From 963f67d66d29759fbc68cafdf18cf434c94aba7f Mon Sep 17 00:00:00 2001 From: Darin Webb Date: Wed, 21 Jun 2023 18:07:11 -0500 Subject: [PATCH 09/11] try hardcoding role names --- cicd/3-app/javabuilder/template.yml.erb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cicd/3-app/javabuilder/template.yml.erb b/cicd/3-app/javabuilder/template.yml.erb index 1d59cfab..4fb19767 100644 --- a/cicd/3-app/javabuilder/template.yml.erb +++ b/cicd/3-app/javabuilder/template.yml.erb @@ -511,8 +511,9 @@ Resources: Effect: Allow Resource: !Sub "arn:aws:s3:::${ContentBucket}/*" Roles: - - !ImportValue JavabuilderPutSourcesLambdaRole - - !ImportValue JavabuilderBuildAndRunLambdaRole + # TODO: do not hard code these! + - javabuilder-iam-PutSourcesLambdaRole-1R0GH6YNAXIC3 + - javabuilder-iam-BuildAndRunLambdaRole-ZR48U5GF0610 ContentBucketReadPolicy: Type: AWS::IAM::Policy @@ -525,7 +526,8 @@ Resources: Effect: Allow Resource: !Sub "arn:aws:s3:::${ContentBucket}/*" Roles: - - !ImportValue JavabuilderBuildAndRunLambdaRole + # TODO: do not hard code this! + - javabuilder-iam-BuildAndRunLambdaRole-ZR48U5GF0610 ContentApiCertificate: Type: AWS::CertificateManager::Certificate From 89b8d5041d82b56f2d6b7f1b73320c7d13a387a8 Mon Sep 17 00:00:00 2001 From: Darin Webb Date: Wed, 21 Jun 2023 18:17:13 -0500 Subject: [PATCH 10/11] remove commented code --- iam.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/iam.yml b/iam.yml index 7e00f87a..4c6eac55 100644 --- a/iam.yml +++ b/iam.yml @@ -39,11 +39,6 @@ Resources: Action: - "sqs:ReceiveMessage" - "sqs:DeleteMessage" - # May need the following - # - "sqs:GetQueueAttributes" - # - "sqs:GetQueueUrl" - # - "sqs:ListDeadLetterSourceQueues" - # - "sqs:ListQueues" Resource: # TODO: limit this - '*' From 4fd6a561088266915d9502aa2d7b575c229c8302 Mon Sep 17 00:00:00 2001 From: Darin Webb Date: Wed, 21 Jun 2023 18:20:32 -0500 Subject: [PATCH 11/11] add comment to failing template --- cicd/3-app/javabuilder/template.yml.erb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cicd/3-app/javabuilder/template.yml.erb b/cicd/3-app/javabuilder/template.yml.erb index 4fb19767..9c60bc84 100644 --- a/cicd/3-app/javabuilder/template.yml.erb +++ b/cicd/3-app/javabuilder/template.yml.erb @@ -500,6 +500,7 @@ 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: @@ -515,6 +516,7 @@ Resources: - 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: