-
Notifications
You must be signed in to change notification settings - Fork 337
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
ci: add the github actions of pushing image to aws public ecr #4214
Conversation
WalkthroughThe recent changes across various GitHub Actions and workflows centralize around integrating AWS ECR for storing Docker images. New parameters and steps were added to authenticate with AWS, log in to Amazon ECR, and push the Docker images to both DockerHub and ECR. These modifications ensure that Docker images for different platforms are built and stored systematically in Amazon ECR and DockerHub repositories. Changes
Sequence Diagram(s)sequenceDiagram
actor Developer
participant GitHub Actions
participant AWS ECR
participant DockerHub
Developer->>GitHub Actions: Push code to repository
GitHub Actions->>GitHub Actions: Trigger workflow
GitHub Actions->>AWS ECR: Configure AWS credentials
GitHub Actions->>AWS ECR: Log in to Amazon ECR
GitHub Actions->>GitHub Actions: Build Docker images
GitHub Actions->>AWS ECR: Push images to Amazon ECR
GitHub Actions->>DockerHub: Push images to DockerHub
GitHub Actions->>GitHub Actions: Notify build results
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- .github/actions/build-dev-builder-images/action.yml (5 hunks)
- .github/actions/build-greptime-images/action.yml (3 hunks)
- .github/actions/build-images/action.yml (2 hunks)
- .github/actions/build-linux-artifacts/action.yml (5 hunks)
- .github/workflows/dev-build.yml (3 hunks)
- .github/workflows/nightly-build.yml (3 hunks)
- .github/workflows/release-dev-builder-images.yaml (1 hunks)
- .github/workflows/release.yml (5 hunks)
Additional comments not posted (18)
.github/actions/build-linux-artifacts/action.yml (2)
54-55
: Dynamic variable usage for image registry and namespaceUsing dynamic variables for
image-registry
andimage-namespace
enhances flexibility and allows easy updates to these configurations without code changes. This is in line with best practices for CI/CD pipelines.Also applies to: 67-68, 85-86, 97-98
34-35
: Ensure correct environment variable usage for IMAGE_REGISTRYThe usage of
${{ vars.aws-image-registry }}
forIMAGE_REGISTRY
is correct as it aligns with the PR's objective to integrate AWS ECR. However, ensure that thevars
context is correctly defined in the workflow or globally to avoid runtime errors.Verification successful
Environment variable usage for IMAGE_REGISTRY is correctly defined
The variable
vars.aws-image-registry
is correctly defined across multiple workflow files, ensuring consistent usage and avoiding runtime errors.
.github/workflows/release-dev-builder-images.yaml
.github/workflows/release.yml
.github/workflows/dev-build.yml
.github/workflows/nightly-build.yml
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `vars.aws-image-registry` is defined. # Test: Search for the definition of `vars.aws-image-registry`. Expect: Definition exists. rg --type yaml 'vars.aws-image-registry' .github/workflows/Length of output: 451
.github/workflows/release-dev-builder-images.yaml (1)
46-50
: Addition of AWS ECR configurationThe addition of AWS-specific parameters (
aws-image-registry
,aws-access-key-id
,aws-secret-access-key
,aws-ecr-region
) is essential for integrating AWS ECR. Ensure that these secrets and variables are correctly set in the GitHub repository to maintain security and functionality..github/actions/build-greptime-images/action.yml (2)
39-50
: Proper configuration for AWS credentials and ECR loginConfiguring AWS credentials and logging into Amazon ECR are crucial steps for pushing images to AWS. The added inputs and steps are aligned with the PR objectives. Ensure the inputs are used correctly in subsequent steps to avoid misconfigurations.
Also applies to: 61-72
129-130
: Ensure dynamic tagging of Docker imagesThe dynamic tagging of Docker images using both DockerHub and AWS ECR ensures redundancy and availability across different registries. This is a good practice for high availability and disaster recovery.
.github/actions/build-images/action.yml (2)
76-106
: Ensure correct configuration for pushing images to ECRThe steps to build and push standard and CentOS images to ECR are correctly configured with the necessary AWS credentials. This ensures that the images are securely pushed to the correct AWS region and registry.
31-42
: Addition of AWS ECR support in image buildingThe addition of AWS ECR-related inputs (
aws-access-key-id
,aws-secret-access-key
,aws-ecr-region
,aws-image-registry
) is crucial for enabling image pushes to AWS ECR. Ensure that these credentials are securely stored and that access is restricted to maintain security.Verification successful
Addition of AWS ECR support in image building
The addition of AWS ECR-related inputs (
aws-access-key-id
,aws-secret-access-key
,aws-ecr-region
,aws-image-registry
) is crucial for enabling image pushes to AWS ECR. Based on the verification:
- No hardcoded AWS credentials were found in the codebase.
- The credentials are likely securely stored elsewhere.
Please ensure that these credentials are securely stored and that access is restricted to maintain security.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that AWS credentials are securely stored. # Test: Check for the presence of AWS credentials in the repository settings. Expect: Credentials are securely stored. gh secret list --repo GreptimeTeam/greptimedbLength of output: 203
Script:
#!/bin/bash # Search for hardcoded AWS credentials in the codebase # Search for potential AWS access key IDs rg --pcre2 '(A3T[A-Z0-9]{16})' # Search for potential AWS secret access keys rg --pcre2 '([A-Za-z0-9/+=]{40})'Length of output: 165539
.github/actions/build-dev-builder-images/action.yml (3)
59-70
: Proper AWS configuration and ECR login stepsConfiguring AWS credentials and logging into Amazon ECR are correctly implemented. These steps are essential for pushing images to AWS ECR, aligning with the objectives of the PR.
82-96
: Ensure multi-platform builds and pushes are correctly configuredThe steps to build and push images for different platforms (Ubuntu, CentOS, Android) to both DockerHub and AWS ECR are correctly set up. This ensures that the images are available on both registries, increasing redundancy and availability.
Also applies to: 108-122, 134-145
34-45
: Secure handling of AWS credentialsThe inclusion of AWS credentials (
aws-access-key-id
,aws-secret-access-key
,aws-ecr-region
,aws-image-registry
) is critical for the functionality to push images to AWS ECR. It's vital to ensure these credentials are handled securely and are not exposed in logs or other outputs.Verification successful
Secure handling of AWS credentials
The AWS credentials (
aws-access-key-id
,aws-secret-access-key
) are securely referenced using GitHub Secrets in the identified workflow files. This ensures that the credentials are not exposed in logs or other outputs.
.github/workflows/release.yml
.github/workflows/release-dev-builder-images.yaml
.github/workflows/nightly-build.yml
.github/workflows/dev-build.yml
.github/workflows/nightly-ci.yml
No further action is required as the credentials are handled securely.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that AWS credentials are not exposed in logs. # Test: Search for accidental exposure of AWS credentials in logs. Expect: No credentials found. rg --type yaml 'aws-access-key-id|aws-secret-access-key' .github/workflows/ --files-with-matchesLength of output: 279
Script:
#!/bin/bash # Description: Inspect the identified YAML files for exposure of AWS credentials. # Test: Ensure that credentials are not exposed in logs or other outputs. files=( ".github/workflows/release.yml" ".github/workflows/release-dev-builder-images.yaml" ".github/workflows/nightly-build.yml" ".github/workflows/dev-build.yml" ".github/workflows/nightly-ci.yml" ) for file in "${files[@]}"; do echo "Checking $file for AWS credentials exposure..." rg 'aws-access-key-id|aws-secret-access-key' "$file" doneLength of output: 3833
.github/workflows/nightly-build.yml (2)
Line range hint
331-356
: Add logging for debugging AWS ECR operationsWhile the steps to push images to AWS ECR are well-defined, adding verbose logging could help in debugging and tracking the operations performed, especially in a production environment where tracing issues might be critical.
- name: Set build image result id: set-build-image-result run: | + echo "Attempting to push images to AWS ECR..." + echo "Using AWS region: ${{ vars.AWS_ECR_REGION }}" + echo "Using image namespace: ${{ vars.IMAGE_NAMESPACE }}" echo "build-image-result=success" >> $GITHUB_OUTPUT
246-246
: Ensure consistency in job dependenciesThe job
release-cn-artifacts
has a dependency onrelease-images-to-ecr
, which is correctly specified. However, it's essential to ensure that all other jobs that should depend onrelease-images-to-ecr
are updated accordingly. This ensures that the workflow runs smoothly without any race conditions..github/workflows/dev-build.yml (3)
243-273
: Implementation of AWS ECR integration in dev buildThe integration of AWS ECR in the development build workflow is consistent with the changes in the nightly build. It's crucial to ensure that the AWS credentials and region settings are correctly configured and secured. Consider using GitHub secrets for sensitive data and environment variables for non-sensitive configuration.
280-280
: Dependency management in artifact releaseThe dependency on
release-images-to-ecr
for therelease-cn-artifacts
job is correctly set. This ensures that the images are available in AWS ECR before attempting to release artifacts to the CN region. Good job on maintaining the order of operations!
Line range hint
362-387
: Enhanced notification logic for development buildsThe updated notification logic to account for the success or failure of AWS ECR image pushes is a critical improvement. This ensures that the team is promptly informed about the build status, which is crucial for quick debugging and response.
.github/workflows/release.yml (3)
328-358
: Consistency in AWS ECR integration across workflowsThe addition of AWS ECR image pushing in the release workflow mirrors the changes in the nightly and development builds. It's essential to maintain consistency across all workflows to ensure a uniform CI/CD process. The steps used here are appropriate and follow best practices for cloud interactions.
369-369
: Check dependencies for CN artifact releaseThe
release-cn-artifacts
job correctly listsrelease-images-to-ecr
as a dependency. This is crucial for ensuring that the Docker images are available in AWS ECR before attempting to release artifacts to the CN region.
Line range hint
481-507
: Robust notification system for release workflowThe notification system in the release workflow has been enhanced to include the results from AWS ECR operations. This is a significant improvement, ensuring that stakeholders are kept informed about the success or failure of the release process, facilitating quicker response times in case of failures.
release-images-to-ecr: | ||
name: Build and push images to aws ecr | ||
if: ${{ inputs.release_images || github.event_name == 'push' || github.event_name == 'schedule' }} | ||
needs: [ | ||
allocate-runners, | ||
build-linux-amd64-artifacts, | ||
build-linux-arm64-artifacts, | ||
] | ||
runs-on: ubuntu-2004-16-cores | ||
outputs: | ||
nightly-build-result: ${{ steps.set-nightly-build-result.outputs.nightly-build-result }} | ||
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
fetch-depth: 0 | ||
|
||
- name: Build and push images to ecr | ||
uses: ./.github/actions/build-images | ||
with: | ||
aws-image-registry: ${{ vars.aws-image-registry }} | ||
image-namespace: ${{ vars.IMAGE_NAMESPACE }} | ||
aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} | ||
aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} | ||
version: ${{ needs.allocate-runners.outputs.version }} | ||
aws-ecr-region: ${{ vars.AWS_ECR_REGION }} | ||
|
||
- name: Set nightly build result | ||
id: set-nightly-build-result | ||
run: | | ||
echo "nightly-build-result=success" >> $GITHUB_OUTPUT |
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.
Add robust error handling for AWS ECR image push
The job for pushing images to AWS ECR lacks specific error handling steps. Given the potential for network issues or AWS-specific errors during the push process, it would be prudent to implement error handling mechanisms to ensure that the workflow can gracefully handle failures.
- name: Set nightly build result
id: set-nightly-build-result
+ continue-on-error: true
run: |
echo "nightly-build-result=success" >> $GITHUB_OUTPUT
Committable suggestion was skipped due to low confidence.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4214 +/- ##
==========================================
- Coverage 84.89% 84.57% -0.33%
==========================================
Files 1039 1040 +1
Lines 182758 183023 +265
==========================================
- Hits 155150 154783 -367
- Misses 27608 28240 +632 |
Suggestion: The original image can be built once and copied anywhere, it will make the CI code simple and easy to read. |
@@ -63,7 +105,22 @@ runs: | |||
IMAGE_NAMESPACE=${{ inputs.dockerhub-image-namespace }} \ | |||
IMAGE_TAG=${{ inputs.version }} | |||
|
|||
- name: Build and push dev-builder-android image # Only build image for amd64 platform. | |||
docker push ${{ inputs.dockerhub-image-registry }}/${{ inputs.dockerhub-image-namespace }}/dev-builder-centos:${{ inputs.version }} |
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.
It's no need to build the image again. You should use skopeo
to copy the image and push to ECR.
@@ -60,3 +72,35 @@ runs: | |||
amd64-artifact-name: greptime-linux-amd64-centos-${{ inputs.version }} | |||
platforms: linux/amd64 | |||
push-latest-tag: ${{ inputs.push-latest-tag }} | |||
|
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.
ditto
@daviderli614 any updates for this? |
No, plan to change code in next week. |
Plan to use copying necessary images to the target image repository, reduce our storage costs. |
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
Same as this: GreptimeTeam/greptimedb-operator#155
What's changed and what's your intention?
Add push image to aws container image registry.
Need to add two repositoriy variables:
aws-image-registry
:public.ecr.aws/i8k6a5e1
AWS_ECR_REGION
:us-east-1
Checklist
Summary by CodeRabbit
New Features
release-images-to-ecr
for building and pushing images to AWS ECR.Chores
Documentation