Skip to content

Commit f88e9b9

Browse files
committed
docs: minor corrections and adding linting and mutability considerations
Signed-off-by: Andrew Shoell <mrlunchbox777@gmail.com>
1 parent 78d4fe8 commit f88e9b9

1 file changed

Lines changed: 82 additions & 29 deletions

File tree

hips/hip-0027.md

Lines changed: 82 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,19 @@ The choice of `.DeployedChart` as the object name was made after considering sev
7272
- It works clearly for both upgrade and rollback scenarios
7373
- It follows the established pattern of top-level template objects like `.Chart`, `.Release`, and `.Values`
7474

75-
### Making `.DeployedChart` nil for Fresh Installs
75+
### Always Available as Template Object
7676

77-
When no deployed release exists (during `helm install`), `.DeployedChart` is set to `nil` rather than an empty struct or having default/zero values. This decision was made because:
77+
`.DeployedChart` is always present in the template context, even when it contains no data (nil). This design decision ensures:
78+
79+
- **Consistent template behavior**: Templates don't need conditional logic to check if the object exists vs. if it has data
80+
- **Follows Go template conventions**: nil is the idiomatic way to represent absence of data
81+
- **Simplifies template code**: `{{ if .DeployedChart }}` works in all scenarios
82+
- **Enables testing**: Templates using `.DeployedChart` can be tested with `helm template` without undefined variable errors
83+
- **Prevents template failures**: References to `.DeployedChart` don't cause rendering errors
84+
85+
### Making `.DeployedChart` nil When No Deployed Release Exists
86+
87+
When no deployed release exists, `.DeployedChart` is set to `nil` rather than an empty struct or having default/zero values. This decision was made because:
7888

7989
- It follows idiomatic Go and template patterns for representing absence of data
8090
- It enables simple, natural existence checks: `{{ if .DeployedChart }}`
@@ -93,7 +103,7 @@ This decision provides:
93103

94104
### Available During Upgrade and Rollback Operations
95105

96-
`.DeployedChart` is always available as a template object but is populated with chart metadata only during `helm upgrade` and `helm rollback` operations when a deployed release exists in the cluster. In all other scenarios, `.DeployedChart` is nil.
106+
`.DeployedChart` is populated with chart metadata only during `helm upgrade` and `helm rollback` operations when a deployed release exists in the cluster. In all other scenarios, `.DeployedChart` is nil.
97107

98108
**Populated with chart metadata:**
99109
- `helm upgrade` (when upgrading an existing release)
@@ -107,14 +117,6 @@ This decision provides:
107117
- `helm upgrade --dry-run` - No cluster access during dry-run
108118
- When `--disable-deployed-chart` flag is used - Explicitly disabled
109119

110-
The object is always present in the template context to ensure templates can be written consistently across all operations. Chart authors can rely on `.DeployedChart` being a valid object reference (though it may be nil) without worrying about undefined variable errors.
111-
112-
**Why always available but sometimes nil:**
113-
- Consistent template behavior - templates don't need to conditional logic to check if the object exists vs. if it has data
114-
- Follows Go template conventions - nil is the idiomatic way to represent absence of data
115-
- Simplifies template code - `{{ if .DeployedChart }}` works in all scenarios
116-
- Enables testing - templates using `.DeployedChart` can be tested with `helm template` without errors
117-
118120
### Chart Metadata Only (Not Full Release Data)
119121

120122
This proposal intentionally limits `.DeployedChart` to chart metadata and does not expose the full release object, which would include previous values, previous manifest, or detailed release metadata (revision number, timestamps, status, etc.).
@@ -178,7 +180,7 @@ Chart authors who need to inspect actual cluster state should use the `lookup()`
178180

179181
### Dry-Run and Template Operations
180182

181-
`.DeployedChart` is always available as a template object but is always nil during `helm template` and dry-run operations (`helm install --dry-run`, `helm upgrade --dry-run`) because these commands are intentionally cluster-agnostic and should remain deterministic without cluster context.
183+
`.DeployedChart` is always nil during `helm template` and dry-run operations (`helm install --dry-run`, `helm upgrade --dry-run`) because these commands are intentionally cluster-agnostic and should remain deterministic without cluster context.
182184

183185
The object is present (but nil) to ensure templates that reference `.DeployedChart` don't fail with undefined variable errors during templating and dry-run operations.
184186

@@ -338,7 +340,7 @@ When this flag is set, `.DeployedChart` will be nil regardless of whether a depl
338340
### Behavior During Different Operations
339341

340342
| Operation | `.DeployedChart` Availability | `.DeployedChart` Value |
341-
|-----------|------------------------------|----------------------|
343+
|-----------|-------------------------------|------------------------|
342344
| `helm install` | Always available | Always `nil` (no deployed release) |
343345
| `helm upgrade` (first install) | Always available | `nil` (no deployed release) |
344346
| `helm upgrade` (actual upgrade) | Always available | Populated with deployed chart metadata |
@@ -348,7 +350,7 @@ When this flag is set, `.DeployedChart` will be nil regardless of whether a depl
348350
| `helm template` | Always available | Always `nil` (no cluster context) |
349351
| `helm install --dry-run` | Always available | Always `nil` (no cluster context) |
350352

351-
**Note:** The object is always present to prevent template errors when referencing `.DeployedChart`, even when it contains no data.
353+
**Note:** The object is always present in the template context to prevent template errors when referencing `.DeployedChart`, even when it contains no data.
352354

353355
## Backwards Compatibility
354356

@@ -400,7 +402,7 @@ This proposal has been designed with security considerations in mind:
400402
The following documentation should be added or updated:
401403

402404
**1. Template Objects Reference:**
403-
Add `.DeployedChart` to the list of built-in objects available in templates, with clear documentation about when it's available and when it's nil.
405+
Add `.DeployedChart` to the list of built-in objects available in templates, with clear documentation about when it's populated and when it's nil.
404406

405407
**2. Upgrade Guide:**
406408
Create a guide titled "Implementing Version-Aware Upgrades" that covers:
@@ -531,6 +533,14 @@ data:
531533
- How to test templates that use `.DeployedChart`
532534
- Common pitfalls and how to avoid them
533535

536+
**6. Chart Linting:**
537+
Consider updating `helm lint` to provide helpful warnings for common issues:
538+
- Warn if templates use `.DeployedChart` without nil checks (e.g., direct field access without `{{ if .DeployedChart }}`)
539+
- Suggest best practices for version comparison logic
540+
- Flag potential issues with upgrade-specific resources that might not be cleaned up
541+
542+
This would help chart authors catch potential issues early in development and promote correct usage patterns.
543+
534544
## Reference Implementation
535545

536546
A reference implementation will be provided in a future pull request to the Helm repository. The implementation will:
@@ -555,24 +565,67 @@ The following ideas were considered during the design of this HIP but were ultim
555565
- Complexity concerns (chart metadata is sufficient for most use cases)
556566
- Can be added later if compelling use cases emerge
557567

558-
### Making Available During `helm install`
568+
### Different Naming Conventions
569+
570+
**Ideas considered:**
571+
- `.PreviousChart` - Ambiguous for rollbacks ("previous" could mean different things)
572+
- `.InstalledChart` - Confusing with the chart currently being installed
573+
- `.CurrentChart` - Ambiguous (which chart is "current"?)
574+
- `.Release.Deployed.Chart` - Unnecessarily nested, breaks consistency with `.Chart`
575+
- `.OldChart` - Informal and ambiguous
576+
577+
**Rejection Reason:** `.DeployedChart` is the clearest and most consistent with Helm terminology. "Deployed" unambiguously means "what's currently running in the cluster."
578+
579+
### Exposing Only Version Strings
580+
581+
**Idea:** Only expose `.DeployedVersion` and `.DeployedAppVersion` as simple strings instead of the full chart object.
559582

560-
**Idea:** Make `.DeployedChart` unavailable (not present in template context) during fresh installs.
583+
**Rejection Reason:**
584+
- Inconsistent with `.Chart` object structure
585+
- Prevents access to other useful metadata (type, deprecated status, annotations)
586+
- Would need to be expanded later anyway if more fields are needed
587+
- Simpler to reuse existing Chart metadata structure
588+
589+
### Environment Variable or Config File for Opt-Out
590+
591+
**Idea:** Control the feature via environment variable (`HELM_DISABLE_DEPLOYED_CHART`) or Helm configuration file instead of CLI flag.
561592

562593
**Rejection Reason:**
563-
- Would cause template errors when templates reference `.DeployedChart` during install
564-
- Inconsistent behavior - templates would need to handle both "object doesn't exist" and "object is nil" cases
565-
- Makes testing harder - templates would fail during `helm template` with undefined variable errors
566-
- Having the object always present but nil is more idiomatic and user-friendly
567-
- Templates written with `{{ if .DeployedChart }}` work correctly in all scenarios
594+
- Less explicit than a CLI flag
595+
- Harder to see in scripts and documentation
596+
- Doesn't follow Helm's pattern of operation-specific flags
597+
- Could cause confusion if set globally but not desired for specific operations
598+
- Environment variables can be accidentally inherited in unexpected contexts
568599

569-
### Making Available During `helm template`
600+
### Querying Cluster During `helm template`
570601

571-
**Idea:** Make `.DeployedChart` unavailable (not present in template context) during `helm template`.
602+
**Idea:** Allow `.DeployedChart` to be populated during `helm template` by querying the cluster if kubectl context is available.
572603

573604
**Rejection Reason:**
574-
- Would cause template errors when templates reference `.DeployedChart`
575-
- Violates the design principle that `helm template` should work for all valid templates
576-
- Makes development harder - chart authors couldn't validate template syntax
577-
- Having the object present but nil allows templates to render successfully
578-
- A future mock/plugin system is a better solution for providing test data
605+
- Violates the design principle that `helm template` should be cluster-agnostic
606+
- Would make template rendering non-deterministic
607+
- Could cause security issues if templating in one environment with data from another
608+
- Would require cluster credentials during templating operations
609+
- A future mock/plugin system is a better solution for testing
610+
611+
### Making `.DeployedChart` Mutable
612+
613+
**Idea:** Allow templates to modify `.DeployedChart` values.
614+
615+
**Rejection Reason:**
616+
- Template objects should be read-only to maintain determinism
617+
- Would create confusion about source of truth (is it from the cluster or from template modifications?)
618+
- Could lead to complex bugs and security issues
619+
- Violates Helm's template rendering model where templates are purely declarative
620+
- No clear use case that couldn't be better solved with template variables
621+
622+
## Open Issues
623+
624+
None at this time. This HIP is ready for community review and feedback.
625+
626+
## References
627+
628+
- [Helm Built-in Objects Documentation](https://helm.sh/docs/chart_template_guide/builtin_objects/)
629+
- [Helm Template Functions and Pipelines](https://helm.sh/docs/chart_template_guide/functions_and_pipelines/)
630+
- [Go Template Documentation](https://pkg.go.dev/text/template)
631+
- [Semantic Versioning 2.0.0](https://semver.org/)

0 commit comments

Comments
 (0)