chore: Feature deploy with ArgoCD #17610
Conversation
WalkthroughThe pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant PC as Pre-Checks
participant HB as Helm-Docker-Build
participant DF as Deploy-Feature
participant D as Deploy
PC->>PC: Check Deployment Type
PC-->>HB: Trigger Build
HB-->>DF: Conditional Execution
alt Feature Deployment
DF->>DF: Generate ArgoCD Charts
DF->>DF: Commit and Push
else Standard Deployment
D->>D: Standard Deployment Process
end
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/push.yml (1)
779-781: Consider using environment variables for git configuration.Instead of hardcoding the git user and email, consider using environment variables:
- git config user.name andes-it - git config user.email [email protected] + git config user.name "${GIT_AUTHOR_NAME:-andes-it}" + git config user.email "${GIT_AUTHOR_EMAIL:[email protected]}"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/push.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/push.yml
724-724: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
754-754: property "infra-node-modules-hash" is not defined in object type {build_chunks: string; cache_keys: string; docker_tag: string; images: string; last_good_build_docker_tag: string; node-modules-hash: string; node_image_tag: string; test_chunks: string; unaffected: string}
(expression)
767-767: shellcheck reported issue in this script: SC2086:info:1:68: Double quote to prevent globbing and word splitting
(shellcheck)
767-767: shellcheck reported issue in this script: SC2086:info:1:217: Double quote to prevent globbing and word splitting
(shellcheck)
767-767: shellcheck reported issue in this script: SC2086:info:2:165: Double quote to prevent globbing and word splitting
(shellcheck)
785-785: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🔇 Additional comments (2)
.github/workflows/push.yml (2)
723-737: Review job execution conditions.The job conditions are complex but well-structured, ensuring the job only runs when:
- Pre-checks are successful
- It's a feature deployment
- Not a pre-release
- Helm docker build hasn't failed
- There are images to deploy
🧰 Tools
🪛 actionlint (1.7.4)
724-724: label "ec2-runners" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
788-788: LGTM! Well-structured deployment conditions.The condition correctly ensures that the regular deploy job doesn't run when feature deployment is active, preventing potential conflicts.
| - name: Checkout feature-values | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: island-is/feature-values | ||
| token: ${{ secrets.DIRTY_FIX_BOT_TOKEN }} | ||
| path: feature-values |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider replacing DIRTY_FIX_BOT_TOKEN with a properly scoped token.
The token name suggests this might be a temporary solution. Consider:
- Creating a dedicated GitHub App or service account
- Using the minimum required permissions
- Renaming the secret to reflect its purpose (e.g., FEATURE_VALUES_REPO_TOKEN)
| run: | | ||
| ./monorepo/infra/scripts/generate-feature-values.sh jobs --feature $FEATURE_NAME --images "$IMAGES" --chart islandis --jobImage "821090935708.dkr.ecr.eu-west-1.amazonaws.com/helm-config:$DOCKER_TAG" > feature-values/$FEATURE_NAME-job-manifest.yaml | ||
| ./monorepo/infra/scripts/generate-feature-values.sh values --chart islandis --feature "$FEATURE_NAME" --images "$IMAGES" --dockertag "$DOCKER_TAG" > feature-values/$FEATURE_NAME.yaml |
There was a problem hiding this comment.
Fix shell script quoting issues.
The generate-feature-values.sh calls have potential word splitting issues.
Apply this diff to fix the quoting:
-./monorepo/infra/scripts/generate-feature-values.sh jobs --feature $FEATURE_NAME --images "$IMAGES" --chart islandis --jobImage "821090935708.dkr.ecr.eu-west-1.amazonaws.com/helm-config:$DOCKER_TAG" > feature-values/$FEATURE_NAME-job-manifest.yaml
-./monorepo/infra/scripts/generate-feature-values.sh values --chart islandis --feature "$FEATURE_NAME" --images "$IMAGES" --dockertag "$DOCKER_TAG" > feature-values/$FEATURE_NAME.yaml
+./monorepo/infra/scripts/generate-feature-values.sh jobs --feature "$FEATURE_NAME" --images "$IMAGES" --chart "islandis" --jobImage "821090935708.dkr.ecr.eu-west-1.amazonaws.com/helm-config:$DOCKER_TAG" > "feature-values/$FEATURE_NAME-job-manifest.yaml"
+./monorepo/infra/scripts/generate-feature-values.sh values --chart "islandis" --feature "$FEATURE_NAME" --images "$IMAGES" --dockertag "$DOCKER_TAG" > "feature-values/$FEATURE_NAME.yaml"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run: | | |
| ./monorepo/infra/scripts/generate-feature-values.sh jobs --feature $FEATURE_NAME --images "$IMAGES" --chart islandis --jobImage "821090935708.dkr.ecr.eu-west-1.amazonaws.com/helm-config:$DOCKER_TAG" > feature-values/$FEATURE_NAME-job-manifest.yaml | |
| ./monorepo/infra/scripts/generate-feature-values.sh values --chart islandis --feature "$FEATURE_NAME" --images "$IMAGES" --dockertag "$DOCKER_TAG" > feature-values/$FEATURE_NAME.yaml | |
| run: | | |
| ./monorepo/infra/scripts/generate-feature-values.sh jobs --feature "$FEATURE_NAME" --images "$IMAGES" --chart "islandis" --jobImage "821090935708.dkr.ecr.eu-west-1.amazonaws.com/helm-config:$DOCKER_TAG" > "feature-values/$FEATURE_NAME-job-manifest.yaml" | |
| ./monorepo/infra/scripts/generate-feature-values.sh values --chart "islandis" --feature "$FEATURE_NAME" --images "$IMAGES" --dockertag "$DOCKER_TAG" > "feature-values/$FEATURE_NAME.yaml" |
🧰 Tools
🪛 actionlint (1.7.4)
767-767: shellcheck reported issue in this script: SC2086:info:1:68: Double quote to prevent globbing and word splitting
(shellcheck)
767-767: shellcheck reported issue in this script: SC2086:info:1:217: Double quote to prevent globbing and word splitting
(shellcheck)
767-767: shellcheck reported issue in this script: SC2086:info:2:165: Double quote to prevent globbing and word splitting
(shellcheck)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17610 +/- ##
==========================================
+ Coverage 35.59% 35.61% +0.01%
==========================================
Files 7027 7030 +3
Lines 150376 150997 +621
Branches 42920 43173 +253
==========================================
+ Hits 53522 53773 +251
- Misses 96854 97224 +370 Flags with carried forward coverage won't be shown. Click here to find out more. see 170 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 9 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (4) |
peturgq
added
deploy-feature
|
Affected services are: Deployed services: . |
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit