Use `configure-aws-credentials` workflow instead of passing `secret_access_key` #1363

vudiep411 · 2024-11-26T23:48:52Z

Summary

This PR fixes #1346 where we can get rid of the long term credentials by using OpenID Connect. OpenID Connect (OIDC) allows your GitHub Actions workflows to access resources in Amazon Web Services (AWS), without needing to store the AWS credentials as long-lived GitHub secrets.

Changes

We can remove these secrets that were passed in previously:

token: ${{ secrets.GITHUB_TOKEN }}
      bucket: ${{ secrets.AWS_S3_BUCKET }}
      access_key_id: ${{ secrets.AWS_S3_ACCESS_KEY_ID }}
      secret_access_key: ${{ secrets.AWS_S3_ACCESS_KEY }}

Instead we only need the role-to-assume arn. For more information OIDC.

Prerequisites

Before merging this PR, we need to make sure to set up the proper Identity providers on the production AWS account. Follow this guides.
Quick guide:

Create an Identity provider with OpenID Connect.
Provider url: https://token.actions.githubusercontent.com

Audience: sts.amazonaws.com

Assign the role into this new provider with this trust policy like below

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Principal": {
                "Federated": "arn:aws:iam::<aws_account_id>:oidc-provider/token.actions.githubusercontent.com"
            },
            "Action": "sts:AssumeRoleWithWebIdentity",
            "Condition": {
                "StringLike": {
                    "token.actions.githubusercontent.com:sub": [
                        "repo:valkey-io/valkey:ref:refs/heads/unstable",
                        "repo:valkey-io/valkey:ref:refs/tags/*"
                    ],
                    "token.actions.githubusercontent.com:aud": "sts.amazonaws.com"
                }
            }
        }
    ]
}

In Github secrets, Add these secrets:

Results

Github action run:

Files in S3 Dev env:

Note: the failed workflow can be found in this issue #1343 which will be in a separate PR.

Signed-off-by: vudiep411 <[email protected]>

codecov · 2024-11-27T10:22:27Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.56%. Comparing base (9305b49) to head (cd1fe2d).
Report is 12 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #1363      +/-   ##
============================================
- Coverage     70.62%   70.56%   -0.06%     
============================================
  Files           117      117              
  Lines         63315    63315              
============================================
- Hits          44714    44679      -35     
- Misses        18601    18636      +35

see 11 files with indirect coverage changes

enjoy-binbin

seems ok, @roshkhatri please take a look

roshkhatri

LGTM, just had a few questions.

roshkhatri · 2024-11-27T18:14:34Z

.github/workflows/build-release-packages.yml

@@ -11,6 +11,7 @@ on:
        required: true

 permissions:
+  id-token: write


Why do we need the write permissions here?

This is to ensure that your workflow has the necessary permission to perform OIDC authentication with AWS.

roshkhatri · 2024-11-27T18:15:01Z

.github/workflows/call-build-linux-arm-packages.yml


 permissions:
+  id-token: write


why do we need write permissions here?

Same as the comment above. Each job spawned in the matrix runs in a separate environment and each of this env needs permission to perform OIDC authentication with AWS.

roshkhatri · 2024-11-27T18:18:45Z

.github/workflows/call-build-linux-x86-packages.yml

-      - name: Install AWS cli.
-        run: |
-          sudo apt-get install -y awscli
-


Is the aws cli installed by aws-actions/configure-aws-credentials@v4 ??

Yes, aws-actions/configure-aws-credentials@v4 will configured the runner environment with the proper AWS credentials and CLI

roshkhatri · 2024-11-27T18:21:23Z

We will also need to do the prerequisites steps for the main repo before we merge this.

roshkhatri · 2024-11-27T18:31:23Z

@vudiep411, will it be possible to add a test, where we uploads a test build binary to a test s3 bucket when ever changes are made to these workflows.

With this we can be sure that when we release stuff, it doesn't break on the main valkey repository

vudiep411 · 2024-11-27T18:40:07Z

Yes it is possible, we can use github environment for that. So we can write something like this:

- name: Configure AWS Credentials
        uses: aws-actions/configure-aws-credentials@v2
        with:
          role-to-assume: ${{ env.ROLE_TO_ASSUME }}
          role-session-name: ${{ env.ROLE_SESSION_NAME }}
          aws-region: ${{ env.AWS_REGION }}

We use env to dynamically assigned the role to assume and env. But this would required us to set up multiple OCID on different accounts not just in the workflow itself so it should be in a separate issue. For now we can just set this one up and after this looks good, I will open another issue for it if that's ok.
@roshkhatri

roshkhatri · 2024-11-28T00:08:41Z

I think we can also do it based on the github event trigger maybe?

We set two OCID for two buckets, releases and test.
When the changes are made to release workflow files, for a pull_request event we run the same workflow with secrets.AWS_TEST_ROLE_TO_ASSUME and secrets.AWS_TEST_S3_BUCKET and for a workflow_dispatch and release
event trigger we run with the current setup.
We would do it at the head of unstable.
Once we upload the test binaries, we can confirm the upload, delete the delete the file and pass the test.

Also, I think we should add the test on the same PR so we can run the test on this PR and know that the tests also work. We would also have to do the pre-requisites only once for both the scenarios.

vudiep411 · 2024-11-28T00:19:18Z

That's a really good suggestion. We can look into that. That way it would automated the testing of a PR with a test bucket. Definitely doable

Signed-off-by: vudiep411 <[email protected]>

vudiep411 · 2024-12-02T22:27:24Z

.github/workflows/build-release-packages.yml

@@ -56,7 +77,7 @@ jobs:
      - uses: ./.github/actions/generate-package-build-matrix
        id: set-matrix
        with:
-          ref: ${{ inputs.version || github.ref_name }}
+          ref: ${{ needs.release-build-get-meta.outputs.version }}


We can use version output from release-build-get-meta

vudiep411 · 2024-12-02T22:53:03Z

.github/workflows/build-release-packages.yml

+      - name: Set bucket name
+        id: check-env
+        run: |
+          if [[ "${{ github.event_name }}" == "pull_request" ]]; then
+            echo "IS_TEST=true" >> $GITHUB_OUTPUT
+          else
+            echo "IS_TEST=false" >> $GITHUB_OUTPUT
+          fi
+        shell: bash
+


Check if whether it is a pull request or it's an actual release (manual or automated) and output true or false. This step purpose is to know if we should use a test bucket or a prod bucket

vudiep411 · 2024-12-02T22:54:34Z

.github/workflows/build-release-packages.yml

-      bucket: ${{ secrets.AWS_S3_BUCKET }}
-      access_key_id: ${{ secrets.AWS_S3_ACCESS_KEY_ID }}
-      secret_access_key: ${{ secrets.AWS_S3_ACCESS_KEY }}
+      bucket_name: ${{ needs.release-build-get-meta.outputs.is_test == 'true' && secrets.AWS_TEST_BUCKET || secrets.AWS_S3_BUCKET }}


Passing in the either the test_bucket or prod_bucket to run the reusable workflow.

vudiep411 · 2024-12-02T22:56:55Z

New Changes

When a PR is raised, the build-release will run if there are changes to the release workflow or utils/releasetools
Get the version step will output the version that is getting built. eg. 8.0.0, unstable,...
Set bucket name test will output true if the event is a PR else it will output false. This step is to determine if we want to use the test bucket or prod bucket to store the binary files.

Set up

Trust policy for github-action-role

{
	"Version": "2012-10-17",
	"Statement": [
		{
			"Effect": "Allow",
			"Principal": {
				"Federated": "arn:aws:iam::<aws_account_number>:oidc-provider/token.actions.githubusercontent.com"
			},
			"Action": "sts:AssumeRoleWithWebIdentity",
			"Condition": {
				"StringLike": {
					"token.actions.githubusercontent.com:sub": [
						"repo:valkey/valkey:ref:refs/heads/*",
						"repo:vudiep411/valkey:ref:refs/tags/*",
						"repo:vudiep411/valkey:pull_request"
					],
					"token.actions.githubusercontent.com:aud": "sts.amazonaws.com"
				}
			}
		}
	]
}

Repo Secrets
AWS_ROLE_TO_ASSUME: ARN role created from the prerequisite
AWS_S3_BUCKET: Prod bucket name
AWS_TEST_BUCKET: Test bucket name

Results

This is the PR replicated in my env.
Check the CI build-release for this PR here

AWS Buckets
We will have 2 buckets. 1 is a test bucket and the other is a prod bucket

Summary

The PR CI build-release will build the binary files into the test bucket to check if these files are correctly build. After that we can merge the PR and whenever we release a new version, this workflow will run and build the binary files into the production bucket.

@roshkhatri

Improve aws creds workflow

cd1fe2d

Signed-off-by: vudiep411 <[email protected]>

enjoy-binbin requested a review from roshkhatri November 27, 2024 16:30

enjoy-binbin approved these changes Nov 27, 2024

View reviewed changes

roshkhatri reviewed Nov 27, 2024

View reviewed changes

vudiep411 force-pushed the aws-creds-workflow branch from 5c9474a to cd1fe2d Compare December 2, 2024 19:42

AWS Test env CI

6d4c457

Signed-off-by: vudiep411 <[email protected]>

vudiep411 commented Dec 2, 2024

View reviewed changes

vudiep411 mentioned this pull request Dec 2, 2024

Deprecate support for ubuntu 18 and Add support for Ubuntu 22 #1343

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `configure-aws-credentials` workflow instead of passing `secret_access_key` #1363

Use `configure-aws-credentials` workflow instead of passing `secret_access_key` #1363

vudiep411 commented Nov 26, 2024

codecov bot commented Nov 27, 2024 •

edited

Loading

enjoy-binbin left a comment

roshkhatri left a comment

roshkhatri Nov 27, 2024

vudiep411 Nov 27, 2024

roshkhatri Nov 27, 2024

vudiep411 Nov 27, 2024

roshkhatri Nov 27, 2024

vudiep411 Nov 27, 2024

roshkhatri commented Nov 27, 2024

roshkhatri commented Nov 27, 2024

vudiep411 commented Nov 27, 2024 •

edited

Loading

roshkhatri commented Nov 28, 2024 •

edited

Loading

vudiep411 commented Nov 28, 2024 •

edited

Loading

vudiep411 Dec 2, 2024

vudiep411 Dec 2, 2024

vudiep411 Dec 2, 2024 •

edited

Loading

vudiep411 commented Dec 2, 2024

Use configure-aws-credentials workflow instead of passing secret_access_key #1363

Are you sure you want to change the base?

Use configure-aws-credentials workflow instead of passing secret_access_key #1363

Conversation

vudiep411 commented Nov 26, 2024

Summary

Changes

Prerequisites

Results

codecov bot commented Nov 27, 2024 • edited Loading

Codecov Report

enjoy-binbin left a comment

Choose a reason for hiding this comment

roshkhatri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roshkhatri commented Nov 27, 2024

roshkhatri commented Nov 27, 2024

vudiep411 commented Nov 27, 2024 • edited Loading

roshkhatri commented Nov 28, 2024 • edited Loading

vudiep411 commented Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vudiep411 Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

vudiep411 commented Dec 2, 2024

New Changes

Set up

Results

Summary

Use `configure-aws-credentials` workflow instead of passing `secret_access_key` #1363

Use `configure-aws-credentials` workflow instead of passing `secret_access_key` #1363

codecov bot commented Nov 27, 2024 •

edited

Loading

vudiep411 commented Nov 27, 2024 •

edited

Loading

roshkhatri commented Nov 28, 2024 •

edited

Loading

vudiep411 commented Nov 28, 2024 •

edited

Loading

vudiep411 Dec 2, 2024 •

edited

Loading