Repository navigation
feat(helm-lint): fix lint step id, make lint opt-in with registry login - #115
shimoncohen wants to merge 5 commits into
Conversation
The step id was list_changed while every condition and the changed output read steps.list-changed, so lint and install were always skipped. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
shimoncohen
changed the title
Lint (and install) run only when registry, username and password are set, since ct lint builds dependencies from the OCI registry. Repos that don't pass them keep passing as today. Bundle ct.yaml (no version-bump or maintainers check) and a lintconf that only warns on brackets, comments and trailing-spaces. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
69e6723 to
aaf644c
Compare
shimoncohen
changed the title
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Tested the action at
MapColonies/qgis-server#7 and MapColonies/nginx-s3-gateway#39 were throwaway PRs, now closed. |
There was a problem hiding this comment.
[nitpick]: this prints "linted" even when lint was skipped because there were no credentials (fork PRs, Dependabot). Maybe "Charts were changed." or check the lint result instead.
There was a problem hiding this comment.
Done in 09a69d9: now "Charts were changed."
| if: steps.list-changed.outputs.changed == 'true' | ||
| run: ct lint --chart-dirs "${{ inputs.chartPaths }}" --target-branch "${{ github.base_ref || github.event.repository.default_branch }}" | ||
| if: steps.login.outcome == 'success' | ||
| run: ct lint --config "${{ github.action_path }}/ct.yaml" --lint-conf "${{ github.action_path }}/lintconf.yaml" --chart-dirs "${{ inputs.chartPaths }}" --target-branch "${{ github.base_ref || github.event.repository.default_branch }}" |
There was a problem hiding this comment.
[nitpick] (pre-existing, but these lines are rewritten here): the credentials nicely go through env:. Could inputs.chartPaths and the target branch do the same, per GitHub's script-injection guidance? update-artifacts-file already follows this pattern.
env:
CHART_DIRS: ${{ inputs.chartPaths }}
TARGET_BRANCH: ${{ github.base_ref || github.event.repository.default_branch }}
run: ct lint --config "$GITHUB_ACTION_PATH/ct.yaml" --lint-conf "$GITHUB_ACTION_PATH/lintconf.yaml" --chart-dirs "$CHART_DIRS" --target-branch "$TARGET_BRANCH"Same applies to list-changed (line 47) and install (line 80).
There was a problem hiding this comment.
Done in 09a69d9: list-changed, lint, install read CHART_DIRS/TARGET_BRANCH from env: and use $GITHUB_ACTION_PATH.
- lintconf.yaml: ct's etc/lintconf.yaml verbatim; only brackets, comments and trailing-spaces downgraded to warning. - Pass chartPaths and target branch via env, log out of the registry after a successful login. - test-helm-lint: trigger on test/helm, touch the test chart so list-changed always picks it up, restore fixture templates mangled by prettier, add missing secret values. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
[nitpick] install: true can't be tested with this fixture: Kubernetes rejects restartPolicy: Never on a Deployment (Unsupported value: "Never": supported values: "Always", found with act + kind). Dropping the line, or a sleep command instead of echo, would make the fixture installable if we want a self-test for install later.
There was a problem hiding this comment.
Done in cb9e827: removed restartPolicy: Never.
Deployments only accept restartPolicy Always, so the fixture could not be installed with install: true. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2 participants
Lint never ran: the lint/install steps referenced
steps.list-changedwhile the step id waslist_changed.Fixing only the id would make lint fail in most consumers (
ct lintbuilds dependencies from the OCI registry), so lint is now opt-in:registry,username,password. When all are set, the action logs in to the registry and lints changed charts; otherwise it only lists changed charts, as today.installinput (defaultfalse) gates the kind cluster andct install.ct.yaml(no version-bump or maintainers check) andlintconf.yaml(brackets,comments,trailing-spacesonly warn).test-helm-lint.yml:chartPaths: test(./test/helmmade ct find no charts), removed the nonexistentrepositoryinput, passes the org ACR secrets.To opt in, add
registry,username,passwordfrom the org secretsACR_URL,ACR_PUSH_USER,ACR_PUSH_TOKEN(see README).🤖 Generated with Claude Code