feat: skip existing helm chart releases

[benie-joy-possi]

Add skip_existing flag to prevent release conflicts

[Dericko681]

please make the following adjustments to optimize the workflow or you add the skip existing: true to the other branch and merge as we agreed in the meeting

Critical Issues:

1. Missing Chart Discovery Step

Problem: The workflow uses chart-releaser-action but doesn't have a step to identify which charts changed. The action needs to know which specific charts to process.

Fix: Add a chart discovery step before chart-releaser:

- name: Discover changed charts
  id: discover
  run: |
    changed_charts=$(ct list-changed --chart-dirs charts --target-branch origin/main)
    echo "changed_charts=${changed_charts}" >> $GITHUB_OUTPUT

2. Inefficient Branch Configuration

Problem: branches: ["**"] triggers on every branch push, but chart-releaser only runs on main. This wastes resources.

Fix: Restrict the trigger to main branch only:

on:
  push:
    branches:
      - main
    paths:
      - "charts/**"

3. Race Condition in Git Configuration

Problem: Git user config runs on every branch, but chart-releaser (which needs it) only runs on main.

Fix: Move Git configuration inside the conditional or make it conditional:

- name: Configure Git
  if: github.ref == 'refs/heads/main'
  run: |
    git config user.name "github-actions[bot]"
    git config user.email "41898282+github-actions[bot]@users.noreply.github.com"

4. Fragile Repository Name Generation

Problem: basename "$repo" | tr -cd '[:alnum:]-' can create duplicate names or invalid repository names.

Fix: Use a more robust naming strategy:

name=$(echo "$repo" | md5sum | cut -d' ' -f1)
# or
name="repo-$(echo "$repo" | shasum -a 256 | cut -c1-8)"

5. Missing Error Handling

Problem: No error handling for failed helm repo add or helm dependency build commands.

Fix: Add error handling:

helm repo add "$name" "$repo" || echo "Failed to add repo $name, continuing..."
helm dependency build "$dir" || exit 1

6. Inefficient Dependency Processing

Problem: Builds dependencies for ALL charts instead of only changed ones.

Fix: Integrate with chart discovery to only process changed charts.
7. Missing chart-releaser Input

Problem: chart-releaser-action isn't receiving the charts parameter that tells it which charts to process.

Fix: Use the discovered charts:

- name: Run chart-releaser
  if: github.ref == 'refs/heads/main'
  uses: helm/[email protected]
  with:
    charts_dir: charts
    charts: ${{ steps.discover.outputs.changed_charts }}
    skip_existing: true

8. No Chart Validation

Problem: No linting or testing before release, which could publish broken charts.

Fix: Add validation steps:

- name: Lint charts
  run: |
    for dir in charts/*; do
      if [ -f "$dir/Chart.yaml" ]; then
        helm lint "$dir"
      fi
    done

9. Potential OCI Registry Issues

Problem: Basic OCI detection might not cover all OCI registry formats.

Fix: More comprehensive OCI handling:

if [[ "$repo" =~ ^oci:// ]] || [[ "$repo" =~ \.azurecr\.io ]] || [[ "$repo" =~ \.ecr\. ]]; then

10. Unnecessary Repository Processing

Problem: Processes repositories for all charts on every run, even unchanged ones.

These issues range from critical (missing chart discovery) to inefficiencies that could cause failures or security concerns.

[benie-joy-possi]

Thanks for the detailed review, @Dericko681 !!

Important clarification: Adding skip_existing: true to the other PR wouldn't solve these issues - that workflow is identical to this one. The skip_existing flag only prevents release conflicts, not the optimization concerns you've raised here.

Here are my responses:

Points 1, 6, 7, 10: Chart Discovery & Efficiency

Without the charts parameter, chart-releaser automatically processes all charts and skips unchanged ones via skip_existing: true. This is documented behavior.

Given our current scale (few charts, <30 sec builds), adding chart discovery would add complexity for minimal gain (~10 sec savings). We can add this optimization when we hit 10+ charts.

Trade-off: Simplicity over premature optimization.

Point 2: Branch Configuration

This is intentional - workflow runs on all branches for validation, but only releases on main (via the if condition). This was discussed and resolved in previous review comments made by @stephane-segning . It's standard CI/CD: test everywhere, deploy from main.

Point 3: Git Config Race Condition

✅ Agreed! I'll move Git config inside the if: github.ref == 'refs/heads/main' conditional.

[benie-joy-possi]

Point 4: Repository Name Generation

Current approach works for all major Helm repos. Instead of hash-based names (unreadable for debugging), I'll add error handling:
Point 5: Error Handling
✅ Agreed! I'll add set -e and proper error messages throughout.
Point 8: Chart Validation
✅ Great point! I'll add helm linting:

Point 9: OCI Registry Detection
Current oci://* detection handles our needs. I'll add a comment noting we can expand this if we add Azure or other OCI registries. Adding unused logic now increases maintenance burden.

[Dericko681]

@benie-joy-possi so you could fix the other PR as we agreed in the meeting, we can not have two open PRs addressing the same issue @Koufan-De-King

and for this workflow we should not want to one day come back to it. adding chart-releaser will not bring so much complexity as u propose.

[benie-joy-possi]

@Dericko681 This new PR was created to replace the previous one, which included several unnecessary commits. This version has a cleaner commit history and is better suited for merging into main.

[Dericko681]

@Dericko681 This new PR was created to replace the previous one, which included several unnecessary commits. This version has a cleaner commit history and is better suited for merging into main.

you could force push to override all commits as u did to gh-pages, when a PR is transferred to you, you continue on the same branch. copy from the karpenter PR that was transferred to Koufan

[stephane-segning]

@codex please review this pr

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Breezy!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Breezy!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

[Franck-Sorel]

The changes in this PR are good, especially the added error handling and linting. I have one question about the change in the release strategy. The charts parameter was removed from the helm/chart-releaser-action and skip_existing: true was added. This means that on every push to main, the workflow will try to release all charts and skip the ones that already exist. The previous implementation only released the charts that were changed. Was this change in behavior intentional?

[Franck-Sorel]

The changes in this PR are good, especially the added error handling and linting. I have one question about the change in the release strategy. The charts parameter was removed from the helm/chart-releaser-action and skip_existing: true was added. This means that on every push to main, the workflow will try to release all charts and skip the ones that already exist. The previous implementation only released the charts that were changed. Was this change in behavior intentional?