Binaries upload to internal s3 bucket #8896
Conversation
alessandrolomanto
added
V:state/compatible/backport
WalkthroughThe pull request modifies the Changes
Sequence Diagramflowchart TD
A[Start Release Process] --> B{GITHUB_TOKEN defined?}
B -->|No| C[Exit with Error]
B -->|Yes| D{S3_ENDPOINT defined?}
D -->|No| E[Print S3_ENDPOINT Error]
D -->|Yes| F{S3_REGION defined?}
F -->|No| G[Print S3_REGION Error]
F -->|Yes| H{AWS_ACCESS_KEY_ID defined?}
H -->|No| I[Print AWS_ACCESS_KEY_ID Error]
H -->|Yes| J{AWS_SECRET_ACCESS_KEY defined?}
J -->|No| K[Print AWS_SECRET_ACCESS_KEY Error]
J -->|Yes| L[Execute Release Command]
The sequence diagram illustrates the new hierarchical validation process for environment variables before executing the release command, ensuring all critical configurations are in place. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Makefile (1)
218-258: Consider simplifying the environment variable validation structureWhile the current implementation correctly validates the presence of required environment variables, consider simplifying the deeply nested structure for better maintainability.
Here's a suggested approach using a helper target:
+ check-release-env: + @if [ -z "$(GITHUB_TOKEN)" ]; then \ + echo "Error: GITHUB_TOKEN is not defined" >&2; exit 1; \ + fi + @if [ -z "$(S3_ENDPOINT)" ]; then \ + echo "Error: S3_ENDPOINT is not defined" >&2; exit 1; \ + fi + @if [ -z "$(S3_REGION)" ]; then \ + echo "Error: S3_REGION is not defined" >&2; exit 1; \ + fi + @if [ -z "$(AWS_ACCESS_KEY_ID)" ]; then \ + echo "Error: AWS_ACCESS_KEY_ID is not defined" >&2; exit 1; \ + fi + @if [ -z "$(AWS_SECRET_ACCESS_KEY)" ]; then \ + echo "Error: AWS_SECRET_ACCESS_KEY is not defined" >&2; exit 1; \ + fi - ifdef GITHUB_TOKEN - ifdef S3_ENDPOINT - ifdef S3_REGION - ifdef AWS_ACCESS_KEY_ID - ifdef AWS_SECRET_ACCESS_KEY - - release: + release: check-release-envAdditionally, consider adding basic format validation for the AWS credentials and S3 configuration:
+ check-release-env: + @if [ -z "$(GITHUB_TOKEN)" ]; then \ + echo "Error: GITHUB_TOKEN is not defined" >&2; exit 1; \ + fi + @if [ -z "$(S3_ENDPOINT)" ]; then \ + echo "Error: S3_ENDPOINT is not defined" >&2; exit 1; \ + elif ! echo "$(S3_ENDPOINT)" | grep -qE '^https?://'; then \ + echo "Error: S3_ENDPOINT must start with http:// or https://" >&2; exit 1; \ + fi + @if [ -z "$(S3_REGION)" ]; then \ + echo "Error: S3_REGION is not defined" >&2; exit 1; \ + elif ! echo "$(S3_REGION)" | grep -qE '^[a-z]{2}-[a-z]+-[0-9]$'; then \ + echo "Error: S3_REGION must be in format: xx-xxxx-n" >&2; exit 1; \ + fi + @if [ -z "$(AWS_ACCESS_KEY_ID)" ]; then \ + echo "Error: AWS_ACCESS_KEY_ID is not defined" >&2; exit 1; \ + elif ! echo "$(AWS_ACCESS_KEY_ID)" | grep -qE '^[A-Z0-9]{20}$'; then \ + echo "Error: AWS_ACCESS_KEY_ID format is invalid" >&2; exit 1; \ + fi + @if [ -z "$(AWS_SECRET_ACCESS_KEY)" ]; then \ + echo "Error: AWS_SECRET_ACCESS_KEY is not defined" >&2; exit 1; \ + elif ! echo "$(AWS_SECRET_ACCESS_KEY)" | grep -qE '^[A-Za-z0-9/+=]{40}$'; then \ + echo "Error: AWS_SECRET_ACCESS_KEY format is invalid" >&2; exit 1; \ + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
.github/workflows/release.ymlis excluded by!**/*.yml.goreleaser.yamlis excluded by!**/*.yaml
📒 Files selected for processing (1)
Makefile(1 hunks)
🔇 Additional comments (1)
Makefile (1)
218-258: Verify the integration with GitHub Actions workflow
Please ensure that:
- The S3 bucket has the correct IAM permissions configured
- The GitHub Actions workflow in
release.yamlcorrectly sets these environment variables - The S3 bucket's CORS configuration allows uploads from the GitHub Actions runners
Run the following script to verify the GitHub Actions workflow configuration:
✅ Verification successful
GitHub Actions workflow correctly integrates with the Makefile release target
The verification shows that:
- The GitHub Actions workflow
.github/workflows/release.yamlexists and correctly sets all required environment variables:GITHUB_TOKENS3_ENDPOINTS3_REGIONAWS_ACCESS_KEY_IDAWS_SECRET_ACCESS_KEY
- The workflow uses repository secrets to securely pass these values to the Makefile target
- The environment variables are properly passed to the Docker container in the release job
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify GitHub Actions workflow configuration
# Check if release.yaml exists and contains the required environment variables
if [ -d .github/workflows ]; then
echo "Checking release.yaml for required environment variables..."
rg -A 5 'env:|environment:' .github/workflows/release.yaml
fi
# Check if README or documentation contains S3 configuration instructions
echo "Checking for S3 configuration documentation..."
rg -i 's3|aws' README.md docs/
Length of output: 427
Script:
#!/bin/bash
# Let's check for any workflow files and their content
echo "Checking all workflow files:"
fd -e yml -e yaml . .github/workflows --exec cat
# Check for any release-related files or documentation
echo -e "\nChecking for release-related files:"
fd -g "*release*" . --exec cat
# Check for any deployment or CI configuration files
echo -e "\nChecking for deployment configs:"
fd -g "*deploy*" -g "*ci*" -g "*.toml" -g "*.yaml" -g "*.yml" . --exec cat
Length of output: 66980
|
|
||
| release: | ||
| docker run \ | ||
| --rm \ | ||
| -e GITHUB_TOKEN=$(GITHUB_TOKEN) \ | ||
| -e COSMWASM_VERSION=$(COSMWASM_VERSION) \ | ||
| -e S3_ENDPOINT=$(S3_ENDPOINT) \ | ||
| -e S3_REGION=$(S3_REGION) \ | ||
| -e AWS_ACCESS_KEY_ID=$(AWS_ACCESS_KEY_ID) \ | ||
| -e AWS_SECRET_ACCESS_KEY=$(AWS_SECRET_ACCESS_KEY) \ | ||
| -v /var/run/docker.sock:/var/run/docker.sock \ | ||
| -v `pwd`:/go/src/osmosisd \ | ||
| -w /go/src/osmosisd \ | ||
| $(GORELEASER_IMAGE) \ | ||
| release \ | ||
| --clean | ||
|
|
There was a problem hiding this comment.
Security: Consider using Docker secrets for sensitive credentials
The current implementation passes sensitive AWS credentials as environment variables to the container. While functional, this approach has security implications:
- Environment variables are visible in docker inspect output
- Credentials might be logged in container logs
- The docker socket mount gives the container full access to the docker daemon
Consider these security improvements:
- Use Docker secrets for sensitive data:
release:
+ @docker secret create aws_credentials - << EOF
+ AWS_ACCESS_KEY_ID=$(AWS_ACCESS_KEY_ID)
+ AWS_SECRET_ACCESS_KEY=$(AWS_SECRET_ACCESS_KEY)
+ EOF
docker run \
--rm \
+ --secret aws_credentials \
-e GITHUB_TOKEN=$(GITHUB_TOKEN) \
-e COSMWASM_VERSION=$(COSMWASM_VERSION) \
-e S3_ENDPOINT=$(S3_ENDPOINT) \
-e S3_REGION=$(S3_REGION) \
- -e AWS_ACCESS_KEY_ID=$(AWS_ACCESS_KEY_ID) \
- -e AWS_SECRET_ACCESS_KEY=$(AWS_SECRET_ACCESS_KEY) \
-v /var/run/docker.sock:/var/run/docker.sock \
-v `pwd`:/go/src/osmosisd \
-w /go/src/osmosisd \
$(GORELEASER_IMAGE) \
release \
- --clean
+ --clean && \
+ docker secret rm aws_credentials- Consider using buildx for better multi-arch support:
+PLATFORMS := linux/amd64,linux/arm64
+
release:
- docker run \
+ docker buildx build \
+ --platform=$(PLATFORMS) \
--rm \- Limit docker socket permissions or consider removing if not strictly necessary
Committable suggestion skipped: line range outside the PR's diff.
|
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you! |
What is the purpose of the change
This PR allows us to upload binaries to our internal S3 bucket.
S3_ENDPOINT,S3_REGION,AWS_ACCESS_KEY_ID, andAWS_SECRET_ACCESS_KEYdocker runcommand inMakefileand in therelease.yamlGitHub actionTesting and Verifying
Have tested the release workflow using the act tool and on my personal fork, and it is working.
Documentation and Release Note
Unreleasedsection ofCHANGELOG.md?Where is the change documented?
x/{module}/README.md)