Skip to content

fix(ci): build helm deps natively to avoid bind-mount permission error#41935

Open
sebastianiv21 wants to merge 3 commits into
releasefrom
fix/helm-unittest-deps-permission
Open

fix(ci): build helm deps natively to avoid bind-mount permission error#41935
sebastianiv21 wants to merge 3 commits into
releasefrom
fix/helm-unittest-deps-permission

Conversation

@sebastianiv21

@sebastianiv21 sebastianiv21 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Description

Tip

TL;DR: Follow-up to #41926. The dependency-build step it added ran inside the unittest container, where the non-root container user can't write to the bind-mounted workdir on Linux CI (mkdir charts: permission denied). This runs helm dependency build natively on the runner via azure/setup-helm, matching the pattern already used in helm-release.yml / helm-docs.yml.

#41926 added a helm dependency update step so the Helm unit tests have their subcharts available (the .tgz files under deploy/helm/charts/ are gitignored). That step ran helm inside the helmunittest/helm-unittest container:

docker run --rm -v $(pwd):/apps --entrypoint helm helmunittest/helm-unittest dependency update .

On the Linux CI runner the container runs as a non-root user that cannot write into the bind-mounted workdir (owned by the runner user), so it failed:

Error: mkdir charts: permission denied

It passed locally only because Docker Desktop on macOS ignores bind-mount ownership.

Fix: install helm on the runner with azure/setup-helm@v4 (pinned v4.1.4) and run helm dependency build natively — the same approach already used in helm-release.yml and helm-docs.yml. The charts/ dir is then created as the runner user, and the existing unittest container reads it without issue.

- name: Setup Helm
  uses: azure/setup-helm@v4
  with:
    version: v4.1.4

- name: Build chart dependencies
  run: helm dependency build

Verification

Ran the new sequence locally against the chart:

  • helm dependency build (native) → all 5 subcharts resolved (redis, mongodb, postgresql, prometheus, mongodb-kubernetes)
  • helm-unittest . (docker) → 13 suites, 67 tests, 7 snapshots — all passing

Fixes N/A — CI infrastructure fix; follow-up to #41926.

Automation

/ok-to-test tags="@tag.Sanity"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/28547318249
Commit: 2b49886
Cypress dashboard.
Tags: @tag.Sanity
Spec:


Wed, 01 Jul 2026 21:31:26 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Updated the Helm unit test workflow to install a pinned Helm binary before running tests.
    • Simplified chart dependency preparation in CI to use a runner-native build, improving consistency.
  • Tests
    • Helm unit tests now run using the pinned Helm version and updated dependency build flow for more reliable chart validation.

The `helm dependency update` step added in #41926 ran inside the
`helmunittest/helm-unittest` container. On the Linux CI runner the
container's non-root user can't write to the bind-mounted workdir, so
`helm dependency build` failed with `mkdir charts: permission denied`
(it passed locally only because Docker Desktop on macOS ignores mount
ownership).

Run the dependency build natively on the runner via `azure/setup-helm`,
matching the pattern already used in helm-release.yml and helm-docs.yml.
The charts/ dir is then created as the runner user and the existing
unittest container reads it without issue.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR updates the Helm unit test workflow to install Helm v4.1.4 with azure/setup-helm@v4 and run helm dependency build directly on the GitHub runner instead of using Docker.

Changes

Helm Workflow Dependency Setup

Layer / File(s) Summary
Setup Helm and switch dependency build to runner-native execution
.github/workflows/helm-unittest.yml
Adds a pinned Helm setup step and replaces the container-based dependency update with a host-run helm dependency build, with comments adjusted accordingly.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • appsmithorg/appsmith#41926: Also changes .github/workflows/helm-unittest.yml to adjust Helm dependency handling before unit tests.

Suggested labels

ok-to-test

Suggested reviewers

  • wyattwalter

Poem

A Helm chart once needed Docker’s air,
Now the runner builds it right there.
v4.1.4 takes the stage,
Dependencies settle on the page.
CI sails on with steady cheer.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main CI fix: moving Helm dependency builds onto the runner to avoid bind-mount permission errors.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description covers the required sections with TL;DR, motivation, verification, automation, Cypress results, and communication, with only the issue reference being nonstandard.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/helm-unittest-deps-permission

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
.github/workflows/helm-unittest.yml (1)

24-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Pin the helmunittest/helm-unittest image tag too. The workflow now builds dependencies with a fixed Helm 4.1.4, but the Unittest container still floats on latest, so the Helm/plugin version used by helm-unittest can drift independently and change CI behavior over time.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/helm-unittest.yml around lines 24 - 40, The workflow’s
Unittest step is still using a floating helmunittest/helm-unittest image tag, so
update that docker run invocation to use a fixed version instead of latest. Keep
the pin consistent with the existing Setup Helm versioning approach and make the
change in the Unittest step so helm-unittest runs with a stable container image
across CI runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.github/workflows/helm-unittest.yml:
- Around line 24-40: The workflow’s Unittest step is still using a floating
helmunittest/helm-unittest image tag, so update that docker run invocation to
use a fixed version instead of latest. Keep the pin consistent with the existing
Setup Helm versioning approach and make the change in the Unittest step so
helm-unittest runs with a stable container image across CI runs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 06197aae-0a3a-4cc1-998a-3922e4352ad5

📥 Commits

Reviewing files that changed from the base of the PR and between 4163960 and 7a482ad.

📒 Files selected for processing (1)
  • .github/workflows/helm-unittest.yml

sebastianiv21 and others added 2 commits June 30, 2026 15:47
The comment claimed the unit tests derive the redis master host from the
redis subchart fullname; on release that host is hardcoded in the parent
templates, so the tests don't currently require the built subcharts. Reword
to state the actual rationale: resolve subchart templates when present and
keep CI dependency resolution in parity with the publish job (helm-release.yml).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@sebastianiv21 sebastianiv21 added the ok-to-test Required label for CI label Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Required label for CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant