feat: add Helm lifecycle reporting and integration coverage - #2849
Open
Mikhail Shirkov (shirkevich) wants to merge 84 commits into
Open
feat: add Helm lifecycle reporting and integration coverage#2849Mikhail Shirkov (shirkevich) wants to merge 84 commits into
Mikhail Shirkov (shirkevich) wants to merge 84 commits into
Conversation
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Dependency Review✅ No vulnerabilities or license issues found.Scanned Files
|
Mikhail Shirkov (shirkevich)
changed the title
This was referenced Jul 31, 2026
Resource Changes Found for
|
Collaborator
Author
|
CodeRabbit (@coderabbitai) full review |
Contributor
✅ Action performedFull review finished. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (118)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (90)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThis change adds native Helm lifecycle execution, aggregate CI summaries, raw secret retrieval, published-content masking, reverse dependency deletion, affected-file detection, integration fixtures, and related documentation. ChangesHelm lifecycle, secrets, and CI reporting
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: ⚪ Minimal · up to No current merge-blocking behavior risk remains. Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 22.10% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 181 functions across 50 files. (65 skipped: 40 unsupported, 25 over the file limit.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/component/helm/client.go (1)
190-199: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not claim cancellable delete execution.
client.Run(spec.ReleaseName)does not receive acontext.Context, soctx.Err()after the call cannot stop an in-progress uninstall. Helm 4 only exposesRunWithContextforInstall/Upgrade;Uninstall.Runis the available API here.
pkg/component/helm/client.go#L190-L199: Document delete cancellation clearly only if it is supported; otherwise do not treatcontext.Canceledas propagated throughUninstall.Run.docs/fixes/2026-08-01-native-helm-release-lifecycle.md#L5-L9: Limit the cancellation claim to supported operations unless delete cancellation is implemented safely.🤖 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 `@pkg/component/helm/client.go` around lines 190 - 199, Remove the post-call ctx.Err() cancellation handling from the delete path around client.Run in pkg/component/helm/client.go, since Uninstall.Run is not context-aware; retain release-not-found handling and releaseOperationError propagation. Update docs/fixes/2026-08-01-native-helm-release-lifecycle.md to limit cancellation claims to operations that support context cancellation, without claiming cancellable delete execution.
🤖 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.
Inline comments:
In `@docs/fixes/2026-08-01-native-helm-release-lifecycle.md`:
- Line 3: Update the Date field in the release lifecycle document to use the
current release-note date, July 31, 2026, instead of the future date August 1,
2026.
In `@examples/helm/components/helm/demo/templates/job.yaml`:
- Line 1: Exclude the Helm templates from raw YAML validation, or configure
pre-commit to run check-yaml against manifests rendered by helm template
instead. Apply this to
examples/helm/components/helm/demo/templates/job.yaml#L1-L1,
deployment.yaml#L18-L18, extra-configmap.yaml#L1-L1,
hook-order-configmap.yaml#L1-L1, hook-order-job.yaml#L1-L1, and
failing-hook-job.yaml#L1-L1; each site requires exclusion from raw validation or
coverage through rendered-manifest validation.
In `@examples/helm/components/helm/demo/templates/ready-marker.yaml`:
- Around line 1-13: Exclude the Helm chart template directory containing
ready-marker.yaml from the repo-level pre-commit check-yaml hook, covering
sibling templates with leading Go-template directives as well. Preserve YAML
validation for non-template files and do not modify the rendered ConfigMap
template.
---
Outside diff comments:
In `@pkg/component/helm/client.go`:
- Around line 190-199: Remove the post-call ctx.Err() cancellation handling from
the delete path around client.Run in pkg/component/helm/client.go, since
Uninstall.Run is not context-aware; retain release-not-found handling and
releaseOperationError propagation. Update
docs/fixes/2026-08-01-native-helm-release-lifecycle.md to limit cancellation
claims to operations that support context cancellation, without claiming
cancellable delete execution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ca7db25-a005-47d1-8154-f031dbf38c66
📒 Files selected for processing (27)
docs/fixes/2026-08-01-native-helm-release-lifecycle.mderrors/errors.goexamples/helm/README.mdexamples/helm/atmos.yamlexamples/helm/components/helm/demo/crds/lifecycle.atmos.test_widgets.yamlexamples/helm/components/helm/demo/templates/deployment.yamlexamples/helm/components/helm/demo/templates/extra-configmap.yamlexamples/helm/components/helm/demo/templates/failing-hook-job.yamlexamples/helm/components/helm/demo/templates/hook-order-configmap.yamlexamples/helm/components/helm/demo/templates/hook-order-job.yamlexamples/helm/components/helm/demo/templates/job.yamlexamples/helm/components/helm/demo/templates/ready-marker.yamlexamples/helm/components/helm/demo/values.yamlexamples/helm/stacks/deploy/dev.yamlpkg/ci/plugins/helm/plugin.gopkg/ci/plugins/helm/plugin_test.gopkg/ci/plugins/helm/templates/apply.mdpkg/ci/plugins/helm/templates/delete.mdpkg/component/helm/client.gopkg/component/helm/client_lifecycle_test.gopkg/component/helm/client_test.gowebsite/docs/ci/job-summaries.mdxwebsite/docs/cli/commands/helm/helm-apply.mdxwebsite/docs/cli/commands/helm/helm-delete.mdxwebsite/docs/cli/commands/helm/usage.mdxwebsite/docs/cli/configuration/components/helm.mdxwebsite/docs/stacks/components/helm.mdx
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/helm-lifecycle-k3s
branch
from
August 1, 2026 00:07
e395efd to
dc58419
Compare
Collaborator
Author
|
CodeRabbit (@coderabbitai) full review |
Contributor
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 33 minutes. |
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/helm-lifecycle-k3s
branch
from
August 1, 2026 01:25
dc58419 to
b980fe4
Compare
Collaborator
Author
|
CodeRabbit (@coderabbitai) full review |
Contributor
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
pkg/component/helm/client_test.go (1)
100-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCover release operation error wrapping at each boundary.
Add behavior-focused tests for
installReleaseandupgradeReleaseerror paths that asserterrUtils.ErrHelmReleaseOperationand the underlying cause. Add expliciterrUtils.ErrHelmReleaseOperationcoverage for delete operations that fail, including delete not-found when that is not treated as success by the in-memory/stacked contract. Keep the existingapplyReleasecancellation coverage, or move the coverage focus to the relevant operation boundary.🤖 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 `@pkg/component/helm/client_test.go` around lines 100 - 120, Extend the tests around installRelease and upgradeRelease to verify operation errors wrap both errUtils.ErrHelmReleaseOperation and the original cause. Add deleteRelease failure tests, including the not-found case when the in-memory/stacked contract treats it as an error, while preserving existing applyRelease cancellation coverage or moving it to the appropriate operation boundary.Source: Coding guidelines
pkg/component/helm/client.go (1)
207-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the repository error builder for this contextual error.
releaseOperationErrorpreserves the Helm error cause and marks the operation sentinel, soBuild(errUtils.ErrHelmReleaseOperation).WithCause(cause)matches the existing builder contract. UseWithContextfor the lifecycle fields, or attach them via the standard message before the cause.🤖 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 `@pkg/component/helm/client.go` around lines 207 - 221, Update releaseOperationError to use the repository error builder: construct the error with Build(errUtils.ErrHelmReleaseOperation), attach cause via WithCause(cause), and preserve the operation, release, namespace, wait strategy, timeout, and component-field context through WithContext or the builder’s standard message mechanism.Source: Coding guidelines
🤖 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.
Inline comments:
In `@examples/helm/atmos.yaml`:
- Around line 100-103: Update the lifecycle-DAG verification around the helm
apply command to assert ordering, not just that dag-dependent exists: first wait
for or verify the foundation's post-readiness marker, then verify the dependent
hook or Deployment is created. Use the existing marker and resource checks so
the test fails when the dependent runs before foundation readiness.
- Around line 55-57: Extend the lifecycle assertions in the dry-run apply,
rollback, and dry-run delete scenarios around the existing kubectl checks.
Verify Helm release metadata and enumerate the complete expected Kubernetes
resource set: dry-run apply and dry-run delete must leave the release and
resources unchanged, while rollback must leave only the intentionally retained
hook ConfigMap and no other rendered resources.
- Around line 77-79: Update the Job-wait scenario around the `atmos helm apply`
and subsequent `kubectl wait` commands so it deterministically verifies that
apply waited for completion: configure a fixed Job duration and assert the Job
is already complete immediately after `atmos helm apply` returns, rather than
allowing `kubectl wait` to complete it afterward.
- Around line 80-82: Update the validation command following “Disabling chart
hooks” to check both weighted hook resources, including the hook Job and its
side effects or expected deletion policy, rather than only the
demo-no-hooks-hook-order ConfigMap. Keep the command failing when either hook
executes while preserving the existing Helm apply flow.
- Around line 95-99: Update the failed-upgrade verification around the demo
deployment commands to capture the pre-upgrade workload state before invoking
demo-upgrade-fail, then compare the restored Deployment/demo state against that
captured state after rollback. Preserve the existing assertion that cleanup
removes ConfigMap/demo-upgrade-only.
- Around line 83-86: Replace the one-second rollout timeout assertion in the
demo-hook-only validation with a deterministic lifecycle/readiness marker
configured for this scenario. Update the hookOnly setup and corresponding
kubectl check so the test verifies that the marker is absent before readiness
completes, while preserving the later successful Deployment readiness assertion.
In `@pkg/ci/plugins/helm/templates/apply.md`:
- Around line 28-46: Keep Helm template control directives from being
interpreted as Markdown table rows. In pkg/ci/plugins/helm/templates/apply.md
lines 28-46, restructure the operation-specific conditionals around complete
valid table blocks or apply the repository’s template-lint handling while
preserving rendered output; in pkg/ci/plugins/helm/templates/delete.md line 23,
apply the same handling to the closing conditional action. Ensure markdownlint
passes without disabling rendered-output validation.
---
Nitpick comments:
In `@pkg/component/helm/client_test.go`:
- Around line 100-120: Extend the tests around installRelease and upgradeRelease
to verify operation errors wrap both errUtils.ErrHelmReleaseOperation and the
original cause. Add deleteRelease failure tests, including the not-found case
when the in-memory/stacked contract treats it as an error, while preserving
existing applyRelease cancellation coverage or moving it to the appropriate
operation boundary.
In `@pkg/component/helm/client.go`:
- Around line 207-221: Update releaseOperationError to use the repository error
builder: construct the error with Build(errUtils.ErrHelmReleaseOperation),
attach cause via WithCause(cause), and preserve the operation, release,
namespace, wait strategy, timeout, and component-field context through
WithContext or the builder’s standard message mechanism.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e6e80cb-6511-4986-a279-4c3f2c49c52a
📒 Files selected for processing (28)
.pre-commit-config.yamldocs/fixes/2026-07-31-native-helm-release-lifecycle.mderrors/errors.goexamples/helm/README.mdexamples/helm/atmos.yamlexamples/helm/components/helm/demo/crds/lifecycle.atmos.test_widgets.yamlexamples/helm/components/helm/demo/templates/deployment.yamlexamples/helm/components/helm/demo/templates/extra-configmap.yamlexamples/helm/components/helm/demo/templates/failing-hook-job.yamlexamples/helm/components/helm/demo/templates/hook-order-configmap.yamlexamples/helm/components/helm/demo/templates/hook-order-job.yamlexamples/helm/components/helm/demo/templates/job.yamlexamples/helm/components/helm/demo/templates/ready-marker.yamlexamples/helm/components/helm/demo/values.yamlexamples/helm/stacks/deploy/dev.yamlpkg/ci/plugins/helm/plugin.gopkg/ci/plugins/helm/plugin_test.gopkg/ci/plugins/helm/templates/apply.mdpkg/ci/plugins/helm/templates/delete.mdpkg/component/helm/client.gopkg/component/helm/client_lifecycle_test.gopkg/component/helm/client_test.gowebsite/docs/ci/job-summaries.mdxwebsite/docs/cli/commands/helm/helm-apply.mdxwebsite/docs/cli/commands/helm/helm-delete.mdxwebsite/docs/cli/commands/helm/usage.mdxwebsite/docs/cli/configuration/components/helm.mdxwebsite/docs/stacks/components/helm.mdx
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/helm-lifecycle-k3s
branch
from
August 1, 2026 01:38
b980fe4 to
baa7579
Compare
|
Warning SHA Pin Verification Passed — with documented exceptionsAll 240 third-party action reference(s) are covered, but 2 rely on a documented allowlist entry in
See the action run for full details. |
|
Warning SHA Pin Verification Passed — with documented exceptionsAll 240 third-party action reference(s) are covered, but 2 rely on a documented allowlist entry in
See the action run for full details. |
|
Warning SHA Pin Verification Passed — with documented exceptionsAll 232 third-party action reference(s) are covered, but 2 rely on a documented allowlist entry in
See the action run for full details. |
The helm-lifecycle scenario installs the pinned public ingress-nginx chart (oss-ingress), whose chart index lives at kubernetes.github.io and whose workload images are served from registry.k8s.io (redirecting to Google Artifact Registry). harden-runner's egress block is enforced on Linux, so `atmos helm template oss-ingress` failed within ~26s fetching the chart index, and the later host `docker pull registry.k8s.io/...` step would fail next. macOS is unaffected (harden-runner cannot enforce egress there). Add the three endpoints to the k3s job allow-list so the Linux leg can fetch the chart and its images: - kubernetes.github.io:443 ingress-nginx chart index - registry.k8s.io:443 controller / kube-webhook-certgen manifests - *.pkg.dev:443 blob/manifest redirect target Verified locally: the full `atmos test` lifecycle (install, upgrade, rollback, cleanup, hooks, CRDs, DAG ordering, timeout, dry-run, masking, real ingress-nginx chart) passes end to end against the Kubernetes emulator.
|
Warning SHA Pin Verification Passed — with documented exceptionsAll 232 third-party action reference(s) are covered, but 2 rely on a documented allowlist entry in
See the action run for full details. |
The prior egress fix let the Linux leg fetch the ingress-nginx chart and start k3s, but the `docker pull registry.k8s.io/ingress-nginx/...` step then failed: error pulling image configuration: ... lookup cdn.registry.k8s.io ... operation not permitted registry.k8s.io serves manifests but redirects blob downloads to a CDN. On the Azure-hosted GitHub runners that host front is cdn.registry.k8s.io (the *.pkg.dev target only appears from GCP-adjacent networks), and harden-runner blocked its DNS lookup. Allow cdn.registry.k8s.io so the controller and kube-webhook-certgen image blobs can be pulled; keep *.pkg.dev for registry.k8s.io's other redirect targets.
|
Warning SHA Pin Verification Passed — with documented exceptionsAll 232 third-party action reference(s) are covered, but 2 rely on a documented allowlist entry in
See the action run for full details. |
The helm-lifecycle fixture's install/upgrade operations use Helm 4's status-watcher wait strategy. On the macos-15-intel Colima runner k3s is doubly nested (k3s -> Docker -> Colima VM), where the watcher's kube-apiserver watch is unreliable and every watcher-waited release burns its full timeout, so the job runs 30+ minutes and exhausts its attempt budget. The identical fixture passes in ~3m45s on the Linux runner and in ~6m on local macOS Docker Desktop, so this is a Colima nested-k3s limitation, not a lifecycle regression. Exclude the macOS/helm-lifecycle matrix combination; the demo-helmfile and helm fixtures continue to cover the macOS k3s path, and watcher-based lifecycle behavior remains fully covered on Linux.
|
Warning SHA Pin Verification Passed — with documented exceptionsAll 232 third-party action reference(s) are covered, but 2 rely on a documented allowlist entry in
See the action run for full details. |
|
Warning SHA Pin Verification Passed — with documented exceptionsAll 232 third-party action reference(s) are covered, but 2 rely on a documented allowlist entry in
See the action run for full details. |
Member
|
CodeRabbit (@coderabbitai) review |
Contributor
✅ Action performedReview finished.
|
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
releasedata to CI job summaries.hookOnlytowatcherpromotion.values_filestargets in--affectedselection, including files outside the chart directory.tests/fixtures/scenarios/helm-lifecycle.This is 4 of 4 in the native Helm lifecycle stack and is based on #2848:
why
--affectedexecution must react to the files whose values are rendered, not only to changes in thevalues_fileslist itself.validation
components/helmand an unrelated-file negative control.hookOnlytowatcherpromotion is reported before the action at the default log level with its field path and machine-readable code; Debug output distinguishes derived policy from a directly configuredwatcher.--dependency-update; six objects include both weighted hooks and the preserved inline Helm template expression, and its full k3s workflow passes end to end.describe componentconfirms deep-merged stack and component release policy.git diff --checkpass.references
docs/prd/native-helm-release-lifecycle.mdSummary by CodeRabbit
New Features
!secretoptions, global-scope secret management, and improved secret masking.Bug Fixes
Documentation