Skip to content
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

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

Open
wants to merge 2 commits into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
43 changes: 31 additions & 12 deletions .github/workflows/build-release-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,20 @@ name: Build Release Packages
on:
release:
types: [published]

pull_request:
paths:
- '.github/workflows/build-release-packages.yml'
- '.github/workflows/call-build-linux-arm-packages.yml'
- '.github/workflows/call-build-linux-x86_64-packages.yml'
- 'utils/releasetools/**'
workflow_dispatch:
inputs:
version:
description: Version of Valkey to build
required: true

permissions:
id-token: write
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need the write permissions here?

Copy link
Author

Choose a reason for hiding this comment

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

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

contents: read

jobs:
Expand All @@ -20,8 +26,8 @@ jobs:
runs-on: ubuntu-latest
outputs:
version: ${{ steps.get_version.outputs.VERSION }}
is_test: ${{ steps.check-env.outputs.IS_TEST }}
steps:

- run: |
echo "Version: ${{ inputs.version || github.ref_name }}"
shell: bash
Expand All @@ -32,8 +38,13 @@ jobs:
- name: Get the version
id: get_version
run: |
VERSION="${INPUT_VERSION}"
if [[ "${{ github.event_name }}" == "pull_request" ]]; then
VERSION="unstable"
else
VERSION="${INPUT_VERSION}"
fi
if [ -z "${VERSION}" ]; then
echo "Error: No version specified"
exit 1
fi
echo "VERSION=$VERSION" >> $GITHUB_OUTPUT
Expand All @@ -43,6 +54,16 @@ jobs:
# only ever be a tag
INPUT_VERSION: ${{ inputs.version || github.ref_name }}

- 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

Comment on lines +57 to +66
Copy link
Author

Choose a reason for hiding this comment

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

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

generate-build-matrix:
name: Generating build matrix
runs-on: ubuntu-latest
Expand All @@ -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 }}
Copy link
Author

Choose a reason for hiding this comment

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

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


release-build-linux-x86-packages:
needs:
Expand All @@ -67,11 +88,10 @@ jobs:
version: ${{ needs.release-build-get-meta.outputs.version }}
ref: ${{ inputs.version || github.ref_name }}
build_matrix: ${{ needs.generate-build-matrix.outputs.x86_64-build-matrix }}
region: us-west-2
secrets:
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 }}
bucket_name: ${{ needs.release-build-get-meta.outputs.is_test == 'true' && secrets.AWS_TEST_BUCKET || secrets.AWS_S3_BUCKET }}
Copy link
Author

@vudiep411 vudiep411 Dec 2, 2024

Choose a reason for hiding this comment

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

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

role_to_assume: ${{ secrets.AWS_ROLE_TO_ASSUME }}

release-build-linux-arm-packages:
needs:
Expand All @@ -82,8 +102,7 @@ jobs:
version: ${{ needs.release-build-get-meta.outputs.version }}
ref: ${{ inputs.version || github.ref_name }}
build_matrix: ${{ needs.generate-build-matrix.outputs.arm64-build-matrix }}
region: us-west-2
secrets:
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 }}
bucket_name: ${{ needs.release-build-get-meta.outputs.is_test == 'true' && secrets.AWS_TEST_BUCKET || secrets.AWS_S3_BUCKET }}
role_to_assume: ${{ secrets.AWS_ROLE_TO_ASSUME }}
39 changes: 17 additions & 22 deletions .github/workflows/call-build-linux-arm-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,20 @@ on:
description: The build targets to produce as a JSON matrix.
type: string
required: true
region:
description: The AWS region to push packages into.
type: string
required: true
secrets:
token:
description: The Github token or similar to authenticate with.
bucket_name:
description: The S3 bucket to push packages into.
required: true
role_to_assume:
description: The role to assume for the S3 bucket.
required: true
bucket:
description: The name of the S3 bucket to push packages into.
required: false
access_key_id:
description: The S3 access key id for the bucket.
required: false
secret_access_key:
description: The S3 secret access key for the bucket.
required: false

permissions:
id-token: write
Copy link
Member

Choose a reason for hiding this comment

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

why do we need write permissions here?

Copy link
Author

Choose a reason for hiding this comment

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

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.

contents: read

jobs:
Expand All @@ -46,6 +45,12 @@ jobs:
with:
ref: ${{ inputs.version }}

- name: Configure AWS credentials
uses: aws-actions/configure-aws-credentials@v4
with:
aws-region: ${{ inputs.region }}
role-to-assume: ${{ secrets.role_to_assume }}

- name: Make Valkey
uses: uraimo/run-on-arch-action@v2
with:
Expand All @@ -65,15 +70,5 @@ jobs:
mkdir -p packages-files
cp -rfv $TAR_FILE_NAME.tar* packages-files/

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

- name: Configure AWS credentials
run: |
aws configure set region us-west-2
aws configure set aws_access_key_id ${{ secrets.access_key_id }}
aws configure set aws_secret_access_key ${{ secrets.secret_access_key }}

- name: Sync to S3
run: aws s3 sync packages-files s3://${{secrets.bucket}}/releases/
run: aws s3 sync packages-files s3://${{ secrets.bucket_name }}/releases/
39 changes: 17 additions & 22 deletions .github/workflows/call-build-linux-x86-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,20 @@ on:
description: The build targets to produce as a JSON matrix.
type: string
required: true
region:
description: The AWS region to upload the packages to.
type: string
required: true
secrets:
token:
description: The Github token or similar to authenticate with.
bucket_name:
description: The name of the S3 bucket to upload the packages to.
required: true
role_to_assume:
description: The role to assume for the S3 bucket.
required: true
bucket:
description: The name of the S3 bucket to push packages into.
required: false
access_key_id:
description: The S3 access key id for the bucket.
required: false
secret_access_key:
description: The S3 secret access key for the bucket.
required: false

permissions:
id-token: write
contents: read

jobs:
Expand All @@ -46,6 +45,12 @@ jobs:
with:
ref: ${{ inputs.version }}

- name: Configure AWS credentials
uses: aws-actions/configure-aws-credentials@v4
with:
aws-region: ${{ inputs.region }}
role-to-assume: ${{ secrets.role_to_assume }}

- name: Install dependencies
run: sudo apt-get update && sudo apt-get install -y build-essential libssl-dev libsystemd-dev

Expand All @@ -63,15 +68,5 @@ jobs:
mkdir -p packages-files
cp -rfv $TAR_FILE_NAME.tar* packages-files/

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

Comment on lines -66 to -69
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

- name: Configure AWS credentials
run: |
aws configure set region us-west-2
aws configure set aws_access_key_id ${{ secrets.access_key_id }}
aws configure set aws_secret_access_key ${{ secrets.secret_access_key }}

- name: Sync to S3
run: aws s3 sync packages-files s3://${{secrets.bucket}}/releases/
run: aws s3 sync packages-files s3://${{ secrets.bucket_name }}/releases/
Loading