-
Notifications
You must be signed in to change notification settings - Fork 677
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
base: unstable
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
contents: read | ||
|
||
jobs: | ||
|
@@ -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 | ||
|
@@ -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 | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
generate-build-matrix: | ||
name: Generating build matrix | ||
runs-on: ubuntu-latest | ||
|
@@ -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 }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can use version output from |
||
|
||
release-build-linux-x86-packages: | ||
needs: | ||
|
@@ -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 }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Passing in the either the |
||
role_to_assume: ${{ secrets.AWS_ROLE_TO_ASSUME }} | ||
|
||
release-build-linux-arm-packages: | ||
needs: | ||
|
@@ -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 }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need write permissions here? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: | ||
|
@@ -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: | ||
|
@@ -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/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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: | ||
|
@@ -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 | ||
|
||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, |
||
- 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/ |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.