feat: implement native Helm release lifecycle controls - #2847
Open
Mikhail Shirkov (shirkevich) wants to merge 21 commits into
Open
feat: implement native Helm release lifecycle controls#2847Mikhail Shirkov (shirkevich) wants to merge 21 commits into
Mikhail Shirkov (shirkevich) wants to merge 21 commits into
Conversation
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Mikhail Shirkov (shirkevich)
temporarily deployed
to
screengrabs
July 31, 2026 23:21 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Mikhail Shirkov (shirkevich)
changed the title
This was referenced Jul 31, 2026
Mikhail Shirkov (shirkevich)
added
the
minor
Mikhail Shirkov (shirkevich)
temporarily deployed
to
screengrabs
July 31, 2026 23:23 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
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:
📝 WalkthroughWalkthroughHelm now supports configurable release lifecycle policies across schemas, stack inheritance, CLI flags, lifecycle validation, and install, upgrade, and uninstall execution. Dry-run handling, lifecycle summaries, external-target validation, and related tests were added. ChangesHelm lifecycle support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant HelmCommand
participant HelmExecutor
participant HelmClient
participant Kubernetes
HelmCommand->>HelmExecutor: lifecycle flags and operation
HelmExecutor->>HelmClient: resolved chartSpec and dryRun
HelmClient->>Kubernetes: install, upgrade, or uninstall release
Kubernetes-->>HelmClient: releaseActionResult
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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: 2
🧹 Nitpick comments (2)
pkg/datafetcher/schema/stacks/stack-config/1.0.json (1)
855-862: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared
wait_strategyenum definition.This schema repeats the
wait_strategyenum (watcher,hookOnly,legacy) in the "helm" defaults block and in "helm_component_manifest".pkg/datafetcher/schema/atmos/manifest/1.0.jsonavoids this by defininghelm_wait_strategyonce and referencing it with$refin both places. Apply the same pattern here to keep the two copies from diverging.♻️ Proposed refactor
"definitions": { + "helm_wait_strategy": { + "title": "helm_wait_strategy", + "description": "Helm 4 release wait strategy", + "type": "string", + "enum": ["watcher", "hookOnly", "legacy"] + }, "helm_repository": {Then, in both the "helm" and "helm_component_manifest" definitions:
- "wait_strategy": { - "type": "string", - "enum": [ - "watcher", - "hookOnly", - "legacy" - ] - }, + "wait_strategy": { + "$ref": "`#/definitions/helm_wait_strategy`" + },Also applies to: 998-1005
🤖 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/datafetcher/schema/stacks/stack-config/1.0.json` around lines 855 - 862, Define a shared `helm_wait_strategy` enum schema containing `watcher`, `hookOnly`, and `legacy`, then replace the duplicated `wait_strategy` enum definitions in both the `helm` defaults block and `helm_component_manifest` with `$ref` references to it, matching the pattern used in the atmos manifest schema.cmd/helm/helm.go (1)
165-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep Helm history defaults in one place.
cmd/helm/helm.gousesdefaultHelmHistoryMax = 10, whilepkg/component/helm/lifecycle.gouses a separatedefaultHelmMaxHistory = 10. Use a shared constant for the CLI flag and lifecycle default so future changes do not drift between help text,--history-maxbehavior, and the lifecycle resolver’s applied policy.🤖 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 `@cmd/helm/helm.go` at line 165, Consolidate the duplicate Helm history defaults by defining one shared constant and using it for both the cmd/helm/helm.go history-max flag and the pkg/component/helm/lifecycle.go defaultHelmMaxHistory resolver behavior. Remove the separate defaultHelmHistoryMax and defaultHelmMaxHistory definitions while preserving the current value and CLI/help behavior.
🤖 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 `@pkg/component/helm/client_test.go`:
- Around line 59-97: Upgrade the pinned helm.sh/helm/v4 dependency to a release
containing the RollbackOnFailure WaitStrategy fix, ensuring
configureInstallLifecycle and configureUpgradeLifecycle no longer allow the
internal rollback/uninstall path to fail with “unknown wait strategy.”
In `@pkg/component/helm/executor.go`:
- Around line 164-172: Update the lifecycle warning loop in the apply/delete
operation path to send each warning through the command UI’s stderr/status
output mechanism instead of log.Warn. Preserve the warning message and its
field/code context using the established ui.Write* or equivalent UI symbol
available in the surrounding executor flow.
---
Nitpick comments:
In `@cmd/helm/helm.go`:
- Line 165: Consolidate the duplicate Helm history defaults by defining one
shared constant and using it for both the cmd/helm/helm.go history-max flag and
the pkg/component/helm/lifecycle.go defaultHelmMaxHistory resolver behavior.
Remove the separate defaultHelmHistoryMax and defaultHelmMaxHistory definitions
while preserving the current value and CLI/help behavior.
In `@pkg/datafetcher/schema/stacks/stack-config/1.0.json`:
- Around line 855-862: Define a shared `helm_wait_strategy` enum schema
containing `watcher`, `hookOnly`, and `legacy`, then replace the duplicated
`wait_strategy` enum definitions in both the `helm` defaults block and
`helm_component_manifest` with `$ref` references to it, matching the pattern
used in the atmos manifest schema.
🪄 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: 2dcfc3bd-8fdb-4470-b0cc-9df9fd9be979
📒 Files selected for processing (27)
cmd/helm/helm.gocmd/helm/helm_test.goerrors/errors.gointernal/exec/stack_processor_merge.gointernal/exec/stack_processor_merge_test.gointernal/exec/stack_processor_process_stacks.gointernal/exec/stack_processor_process_stacks_helpers.gointernal/exec/stack_processor_process_stacks_helpers_extraction.gointernal/exec/stack_processor_process_stacks_helpers_test.gointernal/exec/stack_processor_process_stacks_test.gopkg/component/helm/chart.gopkg/component/helm/client.gopkg/component/helm/client_lifecycle_test.gopkg/component/helm/client_test.gopkg/component/helm/executor.gopkg/component/helm/executor_extra_test.gopkg/component/helm/executor_test.gopkg/component/helm/lifecycle.gopkg/component/helm/lifecycle_test.gopkg/component/helm/provision.gopkg/component/helm/provision_test.gopkg/component/helm/values.gopkg/config/const.gopkg/datafetcher/schema/atmos/manifest/1.0.jsonpkg/datafetcher/schema/stacks/stack-config/1.0.jsonpkg/datafetcher/schema_helm_lifecycle_validation_test.gopkg/datafetcher/schema_section_coverage_test.go
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/helm-lifecycle-actions
branch
from
August 1, 2026 00:07
fc4bb7e to
3fbac27
Compare
Mikhail Shirkov (shirkevich)
temporarily deployed
to
screengrabs
August 1, 2026 00:07 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Mikhail Shirkov (shirkevich)
temporarily deployed
to
screengrabs
August 1, 2026 00:07 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
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 34 minutes. |
Collaborator
Author
|
CodeRabbit (@coderabbitai) full review |
Contributor
✅ Action performedFull review finished. |
Mikhail Shirkov (shirkevich)
temporarily deployed
to
screengrabs
August 1, 2026 01:38 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
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 48 minutes. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pkg/component/helm/lifecycle.go (1)
97-105: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a warning for the silent rollback-driven wait strategy override.
validateAndDeriveLifecyclechangesWaitStrategyfromHookOnlyStrategytoStatusWatcherStrategywheneverRollbackOnFailureis true. This change has no observable trace: nolifecycleWarningis appended, unlikewarningAtomicDeprecated,warningWaitIgnored,warningWaitBoolean, andwarningTimeoutMigration.A user who sets
wait_strategy: hookOnlyexplicitly, together withrollback_on_failure: true, gets a different wait strategy than requested, with no signal inresolution.Warningsto explain why. Add a warning code for this derivation so the change is visible in lifecycle summaries and logs.♻️ Proposed fix to surface the derived override
+const warningWaitStrategyDerived lifecycleWarningCode = "wait_strategy_derived_from_rollback" + func validateAndDeriveLifecycle(resolution *releaseLifecycleResolution) error { if resolution.Policy.RollbackOnFailure && resolution.Policy.WaitStrategy == kube.HookOnlyStrategy { resolution.Policy.WaitStrategy = kube.StatusWatcherStrategy + resolution.Warnings = append(resolution.Warnings, lifecycleWarning{ + Code: warningWaitStrategyDerived, + Field: cfg.HelmWaitStrategySectionName, + Message: "helm wait_strategy was changed to 'watcher' because rollback_on_failure requires monitoring release status", + }) } if resolution.Policy.WaitForJobs && resolution.Policy.WaitStrategy == kube.HookOnlyStrategy { return errUtils.ErrHelmWaitForJobsRequiresWait } return nil }🤖 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/lifecycle.go` around lines 97 - 105, Add a dedicated lifecycle warning code for the rollback-driven wait-strategy override, following the existing warning symbols such as warningAtomicDeprecated and warningTimeoutMigration. In validateAndDeriveLifecycle, append that warning to resolution.Warnings whenever RollbackOnFailure changes HookOnlyStrategy to StatusWatcherStrategy, while preserving the existing wait-for-jobs validation behavior.
🤖 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 `@cmd/helm/helm.go`:
- Around line 157-175: Update the duplicated --wait help descriptions in
addLifecycleOperationFlags for both apply and delete to document the accepted
legacy boolean values true and false alongside watcher, hookOnly, and legacy;
preserve the existing resolver behavior and default watcher selection.
In `@pkg/component/helm/client.go`:
- Around line 74-75: Update the Helm action failure returns in the history
inspection branch and the corresponding lines around the later Helm action
handling to wrap distinct static sentinel errors from errors/errors.go, while
preserving the Helm error as the wrapped cause and existing contextual messages.
Reuse an existing sentinel where appropriate or add clearly named static errors,
ensuring callers can use errors.Is() to identify each failure category.
---
Nitpick comments:
In `@pkg/component/helm/lifecycle.go`:
- Around line 97-105: Add a dedicated lifecycle warning code for the
rollback-driven wait-strategy override, following the existing warning symbols
such as warningAtomicDeprecated and warningTimeoutMigration. In
validateAndDeriveLifecycle, append that warning to resolution.Warnings whenever
RollbackOnFailure changes HookOnlyStrategy to StatusWatcherStrategy, while
preserving the existing wait-for-jobs validation behavior.
🪄 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: 3769a387-c520-4d4f-bf22-166e16bf3f3f
📒 Files selected for processing (27)
cmd/helm/helm.gocmd/helm/helm_test.goerrors/errors.gointernal/exec/stack_processor_merge.gointernal/exec/stack_processor_merge_test.gointernal/exec/stack_processor_process_stacks.gointernal/exec/stack_processor_process_stacks_helpers.gointernal/exec/stack_processor_process_stacks_helpers_extraction.gointernal/exec/stack_processor_process_stacks_helpers_test.gointernal/exec/stack_processor_process_stacks_test.gopkg/component/helm/chart.gopkg/component/helm/client.gopkg/component/helm/client_lifecycle_test.gopkg/component/helm/client_test.gopkg/component/helm/executor.gopkg/component/helm/executor_extra_test.gopkg/component/helm/executor_test.gopkg/component/helm/lifecycle.gopkg/component/helm/lifecycle_test.gopkg/component/helm/provision.gopkg/component/helm/provision_test.gopkg/component/helm/values.gopkg/config/const.gopkg/datafetcher/schema/atmos/manifest/1.0.jsonpkg/datafetcher/schema/stacks/stack-config/1.0.jsonpkg/datafetcher/schema_helm_lifecycle_validation_test.gopkg/datafetcher/schema_section_coverage_test.go
Mikhail Shirkov (shirkevich)
temporarily deployed
to
screengrabs
August 1, 2026 02:14 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Mikhail Shirkov (shirkevich)
temporarily deployed
to
screengrabs
August 4, 2026 13:57 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/helm-lifecycle-actions
branch
from
August 4, 2026 15:25
0d1cc24 to
b65ee6b
Compare
Mikhail Shirkov (shirkevich)
temporarily deployed
to
screengrabs
August 4, 2026 15:26 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Mikhail Shirkov (shirkevich)
temporarily deployed
to
screengrabs
August 4, 2026 15:26 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/helm-lifecycle-actions
branch
from
August 5, 2026 13:24
b65ee6b to
d10d4cd
Compare
Mikhail Shirkov (shirkevich)
temporarily deployed
to
screengrabs
August 5, 2026 13:24 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Mikhail Shirkov (shirkevich)
temporarily deployed
to
screengrabs
August 5, 2026 13:24 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Mikhail Shirkov (shirkevich)
temporarily deployed
to
screengrabs
August 5, 2026 21:10 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
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
2 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
releasepolicy with release-wide defaults and operation-specificinstall,upgrade, anddeleteoverlays.values:key decodes to null, keep inline andvalues_filestemplate strings consistent, propagate top-level secrets, and honor command-local--mask=false.This is 2 of 4 in the native Helm lifecycle stack and is based on #2846:
why
validation
releasetree, including distinct install and upgrade timeouts and upgrade rollback plus cleanup policy.git diff --check, and generic fixture-name audit pass.references
docs/prd/native-helm-release-lifecycle.md