-
Notifications
You must be signed in to change notification settings - Fork 71
feat(helm): publish Helm chart to OCI registry #267
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe pull request modifies the GitHub Actions release workflow to add OCI registry support. It introduces job permissions for contents and packages, then adds three new steps to authenticate with GHCR and push Helm chart packages to an OCI-compliant registry using Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Actions performedReview triggered.
|
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: 0
🧹 Nitpick comments (1)
.github/workflows/release-chart.yaml (1)
36-44: Simplify script by removing redundant nullglob check and add error handling.With
shopt -s nullglobenabled, theif [ -z "${pkg:-}" ]check is unnecessary—the loop simply won't execute if no files match. Additionally, ifhelm pushfails for any package, the workflow silently continues instead of failing the step. Consider adding error handling to fail fast on push failures.Apply this diff to streamline the script:
- - name: Push chart to GHCR - run: | - shopt -s nullglob - for pkg in .cr-release-packages/*.tgz; do - if [ -z "${pkg:-}" ]; then - break - fi - helm push "${pkg}" "oci://ghcr.io/${GITHUB_REPOSITORY_OWNER}/meilisearch-kubernetes" - done + - name: Push chart to GHCR + run: | + shopt -s nullglob + set -e + for pkg in .cr-release-packages/*.tgz; do + helm push "${pkg}" "oci://ghcr.io/${GITHUB_REPOSITORY_OWNER}/meilisearch-kubernetes" + done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release-chart.yaml(2 hunks)
🔇 Additional comments (3)
.github/workflows/release-chart.yaml (3)
9-11: Job permissions are correctly scoped.The permissions block appropriately grants
contents: writefor chart-releaser andpackages: writefor GHCR authentication, following the principle of least privilege.
29-34: GHCR login step looks good.Standard docker/login-action setup with GITHUB_TOKEN. The
packages: writepermission is sufficient for GHCR push operations.
36-44: No action needed. Theubuntu-latestrunner includes Helm 4.0.0 by default, which exceeds the Helm 3.8.0+ requirement for OCI registry operations. The workflow will work as written without an explicit Helm setup step.
brunoocasali
left a comment
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.
Hello there, thanks for this improvement!
| shopt -s nullglob | ||
| for pkg in .cr-release-packages/*.tgz; do | ||
| if [ -z "${pkg:-}" ]; then | ||
| break | ||
| fi | ||
| helm push "${pkg}" "oci://ghcr.io/${GITHUB_REPOSITORY_OWNER}/meilisearch-kubernetes" | ||
| done |
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.
I didn't know the shopt -s nullglob so I had to research a bit, and I learned that the conditional inside the loop is not relevant since the .cr-release-packages/*.tgz will expand []
| shopt -s nullglob | ||
| for pkg in .cr-release-packages/*.tgz; do | ||
| if [ -z "${pkg:-}" ]; then | ||
| break | ||
| fi | ||
| helm push "${pkg}" "oci://ghcr.io/${GITHUB_REPOSITORY_OWNER}/meilisearch-kubernetes" | ||
| done |
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.
| shopt -s nullglob | |
| for pkg in .cr-release-packages/*.tgz; do | |
| if [ -z "${pkg:-}" ]; then | |
| break | |
| fi | |
| helm push "${pkg}" "oci://ghcr.io/${GITHUB_REPOSITORY_OWNER}/meilisearch-kubernetes" | |
| done | |
| shopt -s nullglob | |
| for pkg in .cr-release-packages/*.tgz; do | |
| helm push "${pkg}" "oci://ghcr.io/${GITHUB_REPOSITORY_OWNER}/meilisearch-kubernetes" | |
| done |
Pull Request
Related issue
Fixes #266
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.