potiuk commented on code in PR #66: URL: https://github.com/apache/airflow-ci-infra/pull/66#discussion_r4360741555
########## terraform/runs-on-access/main.tf: ########## @@ -0,0 +1,217 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +locals { + account_id = data.aws_caller_identity.current.account_id + stack_arn = "arn:aws:cloudformation:us-east-2:${local.account_id}:stack/runs-on/*" + state = "arn:aws:s3:::airflow-ci-tfstate/tf-state/runs-on.tfstate" + + # Immutable OIDC subject (owner and repository IDs next to their names), so a renamed or + # re-created repository cannot match. Fork pull requests get no OIDC token at all, and the + # runs-on-prod environment deploys only from main after a required reviewer approves (.asf.yaml). + github_subject = "repo:apache@47359/airflow-ci-infra@321691041" + github_subjects = { + plan = ["${local.github_subject}:pull_request", "${local.github_subject}:ref:refs/heads/main"] + apply = ["${local.github_subject}:environment:runs-on-prod"] + } + + # IAM names the RunsOn template generates for the runs-on stack: <stack>-<logical id>-<suffix>. + # Matching the logical-ID prefixes rather than "runs-on-*" keeps every other role in the account, + # including the ones defined here, out of reach of the CloudFormation service role. + stack_role_prefixes = [ + "RunsOn", "EC2Instance", "ManagedLaunchTemplate", "SchedulerInvoke", "StackConfigMaterializer", + "NetworkFirewallEndpointResolver", "AlertTopicSlackWebhook", + ] + stack_iam_resources = concat( + [for prefix in local.stack_role_prefixes : "arn:aws:iam::${local.account_id}:role/runs-on-${prefix}*"], + [ + "arn:aws:iam::${local.account_id}:instance-profile/runs-on-EC2InstanceProfile*", + "arn:aws:iam::${local.account_id}:role/aws-service-role/*", + ], + ) +} + +data "aws_caller_identity" "current" {} + +# Shared with apache/airflow's GithubActionApacheAirflow role, so it is read here, never managed. +data "aws_iam_openid_connect_provider" "github" { + url = "https://token.actions.githubusercontent.com"; +} + +data "aws_iam_policy_document" "github_trust" { + for_each = local.github_subjects + + statement { + actions = ["sts:AssumeRoleWithWebIdentity"] + principals { + type = "Federated" + identifiers = [data.aws_iam_openid_connect_provider.github.arn] + } + condition { + test = "StringEquals" + variable = "token.actions.githubusercontent.com:aud" + values = ["sts.amazonaws.com"] + } + condition { + test = "StringEquals" + variable = "token.actions.githubusercontent.com:sub" + values = each.value + } + } +} + +resource "aws_iam_role" "terraform" { + for_each = local.github_subjects + + name = "runs-on-terraform-${each.key}" + description = "terraform ${each.key} of terraform/runs-on from apache/airflow-ci-infra GitHub Actions" + assume_role_policy = data.aws_iam_policy_document.github_trust[each.key].json +} + +# What terraform plan needs: refresh the stack, prove the license-key parameter exists and read +# the state. Plans run with -lock=false, so this policy grants no writes. +data "aws_iam_policy_document" "read" { + statement { + actions = ["cloudformation:DescribeStacks", "cloudformation:DescribeStackEvents", "cloudformation:GetTemplate"] + resources = [local.stack_arn] + } + statement { + actions = ["ssm:GetParameter"] + resources = ["arn:aws:ssm:us-east-2:${local.account_id}:parameter/runs-on/license-key"] + } + # The aws/ssm key policy lets any principal in the account decrypt through SSM, so + # GetParameter alone would also return the plain license key. Terraform never asks for it. + statement { + effect = "Deny" + actions = ["kms:Decrypt"] + resources = ["*"] + } + statement { + actions = ["s3:ListBucket"] + resources = ["arn:aws:s3:::airflow-ci-tfstate"] + condition { + test = "StringEquals" + variable = "s3:prefix" + values = ["tf-state/runs-on.tfstate"] + } + } + statement { + actions = ["s3:GetObject"] + resources = [local.state] + } +} + +resource "aws_iam_role_policy" "read" { + for_each = aws_iam_role.terraform + + name = "terraform-read" + role = each.value.name + policy = data.aws_iam_policy_document.read.json +} + +# The apply role can only resubmit the stack from RunsOn's published templates. CloudFormation +# then acts as the stack's service role below, so this role holds no infrastructure permissions. +data "aws_iam_policy_document" "apply" { + statement { + actions = ["cloudformation:UpdateStack"] + resources = [local.stack_arn] + condition { + test = "StringLike" + variable = "cloudformation:TemplateUrl" + values = ["https://runs-on.s3.eu-west-1.amazonaws.com/cloudformation/template-v*.yaml";] + } + } + statement { + actions = ["iam:PassRole"] + resources = [aws_iam_role.cloudformation.arn] + condition { + test = "StringEquals" + variable = "iam:PassedToService" + values = ["cloudformation.amazonaws.com"] + } + } + statement { + actions = ["s3:PutObject"] + resources = [local.state] + } + statement { + actions = ["s3:GetObject", "s3:PutObject", "s3:DeleteObject"] + resources = ["${local.state}.tflock"] + } +} + +resource "aws_iam_role_policy" "apply" { + name = "terraform-apply" + role = aws_iam_role.terraform["apply"].name + policy = data.aws_iam_policy_document.apply.json +} + +data "aws_iam_policy_document" "cloudformation_trust" { + statement { + actions = ["sts:AssumeRole"] + principals { + type = "Service" + identifiers = ["cloudformation.amazonaws.com"] + } + } +} + +resource "aws_iam_role" "cloudformation" { + name = "runs-on-cloudformation" + description = "Service role CloudFormation uses to update the runs-on stack" + assume_role_policy = data.aws_iam_policy_document.cloudformation_trust.json +} + +# RunsOn documents only "admin rights" for deploying its stack: the template creates IAM, VPC, +# ECS, Lambda, API Gateway, SQS, DynamoDB, S3, SNS and Budgets resources, and the set changes +# between releases. AdministratorAccess follows that; the denies below keep the role away from +# everything in the account that is not the stack's own. +resource "aws_iam_role_policy_attachment" "cloudformation" { + role = aws_iam_role.cloudformation.name + policy_arn = "arn:aws:iam::aws:policy/AdministratorAccess" +} + +data "aws_iam_policy_document" "cloudformation_guardrails" { + statement { + sid = "OnlyTheStackIam" + effect = "Deny" + actions = [ + "iam:Add*", "iam:Attach*", "iam:Change*", "iam:Create*", "iam:Deactivate*", "iam:Delete*", + "iam:Detach*", "iam:Enable*", "iam:PassRole", "iam:Put*", "iam:Remove*", "iam:Reset*", + "iam:Set*", "iam:Tag*", "iam:Untag*", "iam:Update*", "iam:Upload*", + ] + not_resources = local.stack_iam_resources Review Comment: The apply role can resubmit the allowed template with arbitrary parameters, and the v3.3.2 template attaches `AppCustomPolicy` to the ECS task role and pulls the control-plane image from `AppRegistry` ([template L7062](https://github.com/runs-on/runs-on/blob/v3.3.2/cloudformation/template.yaml#L7062)). Since this deny exempts `runs-on-RunsOn*` roles, `AppCustomPolicy=arn:aws:iam::aws:policy/AdministratorAccess` plus an attacker-controlled image yields a container with admin credentials — and none of these guardrails apply to it, so it could rewrite the plan/apply roles and the state. Suggest denying `iam:AttachRolePolicy` / `iam:PutRolePolicy` unless `iam:PolicyARN` is in an approved list (covering `AppCustomPolicy` and `RunnerCustomPolicy`), or requiring a permissions boundary on stack-created roles. Relatedly, `template-v*.yaml` accepts any published version, including older ones — worth pinning tighter if possible. ########## terraform/runs-on-access/main.tf: ########## @@ -0,0 +1,217 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +locals { + account_id = data.aws_caller_identity.current.account_id + stack_arn = "arn:aws:cloudformation:us-east-2:${local.account_id}:stack/runs-on/*" + state = "arn:aws:s3:::airflow-ci-tfstate/tf-state/runs-on.tfstate" + + # Immutable OIDC subject (owner and repository IDs next to their names), so a renamed or + # re-created repository cannot match. Fork pull requests get no OIDC token at all, and the + # runs-on-prod environment deploys only from main after a required reviewer approves (.asf.yaml). + github_subject = "repo:apache@47359/airflow-ci-infra@321691041" + github_subjects = { + plan = ["${local.github_subject}:pull_request", "${local.github_subject}:ref:refs/heads/main"] + apply = ["${local.github_subject}:environment:runs-on-prod"] + } + + # IAM names the RunsOn template generates for the runs-on stack: <stack>-<logical id>-<suffix>. + # Matching the logical-ID prefixes rather than "runs-on-*" keeps every other role in the account, + # including the ones defined here, out of reach of the CloudFormation service role. + stack_role_prefixes = [ + "RunsOn", "EC2Instance", "ManagedLaunchTemplate", "SchedulerInvoke", "StackConfigMaterializer", + "NetworkFirewallEndpointResolver", "AlertTopicSlackWebhook", + ] + stack_iam_resources = concat( + [for prefix in local.stack_role_prefixes : "arn:aws:iam::${local.account_id}:role/runs-on-${prefix}*"], + [ + "arn:aws:iam::${local.account_id}:instance-profile/runs-on-EC2InstanceProfile*", + "arn:aws:iam::${local.account_id}:role/aws-service-role/*", + ], + ) +} + +data "aws_caller_identity" "current" {} + +# Shared with apache/airflow's GithubActionApacheAirflow role, so it is read here, never managed. +data "aws_iam_openid_connect_provider" "github" { + url = "https://token.actions.githubusercontent.com"; +} + +data "aws_iam_policy_document" "github_trust" { + for_each = local.github_subjects + + statement { + actions = ["sts:AssumeRoleWithWebIdentity"] + principals { + type = "Federated" + identifiers = [data.aws_iam_openid_connect_provider.github.arn] + } + condition { + test = "StringEquals" + variable = "token.actions.githubusercontent.com:aud" + values = ["sts.amazonaws.com"] + } + condition { + test = "StringEquals" + variable = "token.actions.githubusercontent.com:sub" + values = each.value + } + } +} + +resource "aws_iam_role" "terraform" { + for_each = local.github_subjects + + name = "runs-on-terraform-${each.key}" + description = "terraform ${each.key} of terraform/runs-on from apache/airflow-ci-infra GitHub Actions" + assume_role_policy = data.aws_iam_policy_document.github_trust[each.key].json +} + +# What terraform plan needs: refresh the stack, prove the license-key parameter exists and read +# the state. Plans run with -lock=false, so this policy grants no writes. +data "aws_iam_policy_document" "read" { + statement { + actions = ["cloudformation:DescribeStacks", "cloudformation:DescribeStackEvents", "cloudformation:GetTemplate"] + resources = [local.stack_arn] + } + statement { + actions = ["ssm:GetParameter"] + resources = ["arn:aws:ssm:us-east-2:${local.account_id}:parameter/runs-on/license-key"] + } + # The aws/ssm key policy lets any principal in the account decrypt through SSM, so + # GetParameter alone would also return the plain license key. Terraform never asks for it. + statement { + effect = "Deny" + actions = ["kms:Decrypt"] Review Comment: The deny on `*` also covers S3, so if the state bucket uses (or later switches to) SSE-KMS, both roles lose read access to the state. Scoping it with `kms:ViaService = ssm.us-east-2.amazonaws.com` would keep the license-key protection without that side effect. ########## .github/workflows/terraform-runs-on.yml: ########## @@ -0,0 +1,109 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +--- +# Plans terraform/runs-on on pull requests and on main; applies the plan made on main once a +# required reviewer approves the runs-on-prod environment. The AWS roles live in +# terraform/runs-on-access and are applied by hand. +name: Terraform (RunsOn) + +on: # yamllint disable-line rule:truthy + pull_request: + paths: + - "terraform/runs-on/**" + - ".github/workflows/terraform-runs-on.yml" + push: + branches: [main] + paths: + - "terraform/runs-on/**" + - ".github/workflows/terraform-runs-on.yml" + +permissions: {} + +concurrency: + group: terraform-runs-on-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +env: + AWS_REGION: us-east-2 + TF_IN_AUTOMATION: "true" + TF_INPUT: "false" + +jobs: + plan: + # Fork pull requests get no OIDC token, so they cannot plan. + if: >- + github.event_name == 'push' || + github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-24.04 + permissions: + contents: read + id-token: write + defaults: + run: + working-directory: terraform/runs-on + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: aws-actions/configure-aws-credentials@e6de054238d6b7531b4efff3b6587d9aade6a06c # v6.2.3 + with: + role-to-assume: arn:aws:iam::827901512104:role/runs-on-terraform-plan + aws-region: ${{ env.AWS_REGION }} + - uses: hashicorp/setup-terraform@dfe3c3f87815947d99a8997f908cb6525fc44e9e # v4.0.1 + with: + terraform_version: 1.16.2 + terraform_wrapper: false + - run: terraform init -lockfile=readonly + # The plan role cannot write the lock file; apply takes the lock and refuses a stale plan. + - run: terraform plan -lock=false -out=runs-on.tfplan + - if: github.event_name == 'push' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: runs-on-tfplan + path: terraform/runs-on/runs-on.tfplan + retention-days: 1 Review Comment: The apply job waits for a reviewer, so a merge on Friday approved on Monday finds the artifact expired and the apply fails — with no `workflow_dispatch` to re-trigger. Maybe a longer retention plus a `workflow_dispatch` trigger? ########## terraform/runs-on-access/main.tf: ########## @@ -0,0 +1,217 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +locals { + account_id = data.aws_caller_identity.current.account_id + stack_arn = "arn:aws:cloudformation:us-east-2:${local.account_id}:stack/runs-on/*" + state = "arn:aws:s3:::airflow-ci-tfstate/tf-state/runs-on.tfstate" + + # Immutable OIDC subject (owner and repository IDs next to their names), so a renamed or + # re-created repository cannot match. Fork pull requests get no OIDC token at all, and the + # runs-on-prod environment deploys only from main after a required reviewer approves (.asf.yaml). + github_subject = "repo:apache@47359/airflow-ci-infra@321691041" + github_subjects = { + plan = ["${local.github_subject}:pull_request", "${local.github_subject}:ref:refs/heads/main"] + apply = ["${local.github_subject}:environment:runs-on-prod"] + } + + # IAM names the RunsOn template generates for the runs-on stack: <stack>-<logical id>-<suffix>. + # Matching the logical-ID prefixes rather than "runs-on-*" keeps every other role in the account, + # including the ones defined here, out of reach of the CloudFormation service role. + stack_role_prefixes = [ + "RunsOn", "EC2Instance", "ManagedLaunchTemplate", "SchedulerInvoke", "StackConfigMaterializer", + "NetworkFirewallEndpointResolver", "AlertTopicSlackWebhook", + ] + stack_iam_resources = concat( + [for prefix in local.stack_role_prefixes : "arn:aws:iam::${local.account_id}:role/runs-on-${prefix}*"], + [ + "arn:aws:iam::${local.account_id}:instance-profile/runs-on-EC2InstanceProfile*", + "arn:aws:iam::${local.account_id}:role/aws-service-role/*", + ], + ) +} + +data "aws_caller_identity" "current" {} + +# Shared with apache/airflow's GithubActionApacheAirflow role, so it is read here, never managed. +data "aws_iam_openid_connect_provider" "github" { + url = "https://token.actions.githubusercontent.com"; +} + +data "aws_iam_policy_document" "github_trust" { + for_each = local.github_subjects + + statement { + actions = ["sts:AssumeRoleWithWebIdentity"] + principals { + type = "Federated" + identifiers = [data.aws_iam_openid_connect_provider.github.arn] + } + condition { + test = "StringEquals" + variable = "token.actions.githubusercontent.com:aud" + values = ["sts.amazonaws.com"] + } + condition { + test = "StringEquals" + variable = "token.actions.githubusercontent.com:sub" + values = each.value + } + } +} + +resource "aws_iam_role" "terraform" { + for_each = local.github_subjects + + name = "runs-on-terraform-${each.key}" + description = "terraform ${each.key} of terraform/runs-on from apache/airflow-ci-infra GitHub Actions" + assume_role_policy = data.aws_iam_policy_document.github_trust[each.key].json +} + +# What terraform plan needs: refresh the stack, prove the license-key parameter exists and read +# the state. Plans run with -lock=false, so this policy grants no writes. +data "aws_iam_policy_document" "read" { + statement { + actions = ["cloudformation:DescribeStacks", "cloudformation:DescribeStackEvents", "cloudformation:GetTemplate"] + resources = [local.stack_arn] + } + statement { + actions = ["ssm:GetParameter"] + resources = ["arn:aws:ssm:us-east-2:${local.account_id}:parameter/runs-on/license-key"] + } + # The aws/ssm key policy lets any principal in the account decrypt through SSM, so + # GetParameter alone would also return the plain license key. Terraform never asks for it. + statement { + effect = "Deny" + actions = ["kms:Decrypt"] + resources = ["*"] + } + statement { + actions = ["s3:ListBucket"] + resources = ["arn:aws:s3:::airflow-ci-tfstate"] + condition { + test = "StringEquals" + variable = "s3:prefix" + values = ["tf-state/runs-on.tfstate"] + } + } + statement { + actions = ["s3:GetObject"] + resources = [local.state] + } +} + +resource "aws_iam_role_policy" "read" { + for_each = aws_iam_role.terraform + + name = "terraform-read" + role = each.value.name + policy = data.aws_iam_policy_document.read.json +} + +# The apply role can only resubmit the stack from RunsOn's published templates. CloudFormation +# then acts as the stack's service role below, so this role holds no infrastructure permissions. +data "aws_iam_policy_document" "apply" { + statement { + actions = ["cloudformation:UpdateStack"] + resources = [local.stack_arn] + condition { + test = "StringLike" + variable = "cloudformation:TemplateUrl" + values = ["https://runs-on.s3.eu-west-1.amazonaws.com/cloudformation/template-v*.yaml";] + } + } + statement { + actions = ["iam:PassRole"] + resources = [aws_iam_role.cloudformation.arn] + condition { + test = "StringEquals" + variable = "iam:PassedToService" + values = ["cloudformation.amazonaws.com"] + } + } + statement { + actions = ["s3:PutObject"] + resources = [local.state] + } + statement { + actions = ["s3:GetObject", "s3:PutObject", "s3:DeleteObject"] + resources = ["${local.state}.tflock"] + } +} + +resource "aws_iam_role_policy" "apply" { + name = "terraform-apply" + role = aws_iam_role.terraform["apply"].name + policy = data.aws_iam_policy_document.apply.json +} + +data "aws_iam_policy_document" "cloudformation_trust" { + statement { + actions = ["sts:AssumeRole"] + principals { + type = "Service" + identifiers = ["cloudformation.amazonaws.com"] Review Comment: This role carries `AdministratorAccess` (minus guardrails) but any principal with a broad `iam:PassRole` can hand it to CloudFormation for an unrelated stack. Consider adding `aws:SourceAccount` and `aws:SourceArn` (`arn:aws:cloudformation:us-east-2:<acct>:stack/runs-on/*`) conditions to the trust policy. ########## .github/workflows/terraform-runs-on.yml: ########## @@ -0,0 +1,109 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +--- +# Plans terraform/runs-on on pull requests and on main; applies the plan made on main once a +# required reviewer approves the runs-on-prod environment. The AWS roles live in +# terraform/runs-on-access and are applied by hand. +name: Terraform (RunsOn) + +on: # yamllint disable-line rule:truthy + pull_request: + paths: + - "terraform/runs-on/**" + - ".github/workflows/terraform-runs-on.yml" + push: + branches: [main] + paths: + - "terraform/runs-on/**" + - ".github/workflows/terraform-runs-on.yml" + +permissions: {} + +concurrency: + group: terraform-runs-on-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +env: + AWS_REGION: us-east-2 + TF_IN_AUTOMATION: "true" + TF_INPUT: "false" + +jobs: + plan: + # Fork pull requests get no OIDC token, so they cannot plan. + if: >- + github.event_name == 'push' || + github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-24.04 + permissions: + contents: read + id-token: write + defaults: + run: + working-directory: terraform/runs-on + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: aws-actions/configure-aws-credentials@e6de054238d6b7531b4efff3b6587d9aade6a06c # v6.2.3 + with: + role-to-assume: arn:aws:iam::827901512104:role/runs-on-terraform-plan + aws-region: ${{ env.AWS_REGION }} + - uses: hashicorp/setup-terraform@dfe3c3f87815947d99a8997f908cb6525fc44e9e # v4.0.1 + with: + terraform_version: 1.16.2 + terraform_wrapper: false + - run: terraform init -lockfile=readonly + # The plan role cannot write the lock file; apply takes the lock and refuses a stale plan. + - run: terraform plan -lock=false -out=runs-on.tfplan + - if: github.event_name == 'push' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: runs-on-tfplan + path: terraform/runs-on/runs-on.tfplan + retention-days: 1 + + apply: Review Comment: Every matching push to main creates a pending `runs-on-prod` deployment, even when the plan is empty (e.g. README-only edits). `terraform plan -detailed-exitcode` exposed as a `has_changes` job output, and `if: needs.plan.outputs.has_changes == 'true'` here, would avoid approving no-op applies. ########## terraform/runs-on-access/main.tf: ########## @@ -0,0 +1,217 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +locals { + account_id = data.aws_caller_identity.current.account_id + stack_arn = "arn:aws:cloudformation:us-east-2:${local.account_id}:stack/runs-on/*" + state = "arn:aws:s3:::airflow-ci-tfstate/tf-state/runs-on.tfstate" + + # Immutable OIDC subject (owner and repository IDs next to their names), so a renamed or + # re-created repository cannot match. Fork pull requests get no OIDC token at all, and the + # runs-on-prod environment deploys only from main after a required reviewer approves (.asf.yaml). + github_subject = "repo:apache@47359/airflow-ci-infra@321691041" + github_subjects = { + plan = ["${local.github_subject}:pull_request", "${local.github_subject}:ref:refs/heads/main"] + apply = ["${local.github_subject}:environment:runs-on-prod"] + } + + # IAM names the RunsOn template generates for the runs-on stack: <stack>-<logical id>-<suffix>. + # Matching the logical-ID prefixes rather than "runs-on-*" keeps every other role in the account, + # including the ones defined here, out of reach of the CloudFormation service role. + stack_role_prefixes = [ + "RunsOn", "EC2Instance", "ManagedLaunchTemplate", "SchedulerInvoke", "StackConfigMaterializer", + "NetworkFirewallEndpointResolver", "AlertTopicSlackWebhook", + ] + stack_iam_resources = concat( + [for prefix in local.stack_role_prefixes : "arn:aws:iam::${local.account_id}:role/runs-on-${prefix}*"], + [ + "arn:aws:iam::${local.account_id}:instance-profile/runs-on-EC2InstanceProfile*", + "arn:aws:iam::${local.account_id}:role/aws-service-role/*", + ], + ) +} + +data "aws_caller_identity" "current" {} + +# Shared with apache/airflow's GithubActionApacheAirflow role, so it is read here, never managed. +data "aws_iam_openid_connect_provider" "github" { + url = "https://token.actions.githubusercontent.com"; +} + +data "aws_iam_policy_document" "github_trust" { + for_each = local.github_subjects + + statement { + actions = ["sts:AssumeRoleWithWebIdentity"] + principals { + type = "Federated" + identifiers = [data.aws_iam_openid_connect_provider.github.arn] + } + condition { + test = "StringEquals" + variable = "token.actions.githubusercontent.com:aud" + values = ["sts.amazonaws.com"] + } + condition { + test = "StringEquals" + variable = "token.actions.githubusercontent.com:sub" + values = each.value + } + } +} + +resource "aws_iam_role" "terraform" { + for_each = local.github_subjects + + name = "runs-on-terraform-${each.key}" + description = "terraform ${each.key} of terraform/runs-on from apache/airflow-ci-infra GitHub Actions" + assume_role_policy = data.aws_iam_policy_document.github_trust[each.key].json +} + +# What terraform plan needs: refresh the stack, prove the license-key parameter exists and read +# the state. Plans run with -lock=false, so this policy grants no writes. +data "aws_iam_policy_document" "read" { + statement { + actions = ["cloudformation:DescribeStacks", "cloudformation:DescribeStackEvents", "cloudformation:GetTemplate"] + resources = [local.stack_arn] + } + statement { + actions = ["ssm:GetParameter"] + resources = ["arn:aws:ssm:us-east-2:${local.account_id}:parameter/runs-on/license-key"] + } + # The aws/ssm key policy lets any principal in the account decrypt through SSM, so + # GetParameter alone would also return the plain license key. Terraform never asks for it. + statement { + effect = "Deny" + actions = ["kms:Decrypt"] + resources = ["*"] + } + statement { + actions = ["s3:ListBucket"] + resources = ["arn:aws:s3:::airflow-ci-tfstate"] + condition { + test = "StringEquals" + variable = "s3:prefix" Review Comment: The S3 backend also lists the bucket with the workspace prefix (`env:/`) during `terraform init`, so with only `tf-state/runs-on.tfstate` allowed, init is likely to fail with "Failed to get existing workspaces" for both roles. The PR's plan run used admin credentials, so this path hasn't been exercised. Adding `env:/*` (or `StringLike` on both) should fix it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
