mirror of https://github.com/helm/helm
Lint validation for subchart and resource-group dependency cycles, orphan group references, and the both-or-neither readiness annotation rule. Refs: HIP-0025 Signed-off-by: Rohit Gudi <50377477+caretak3r@users.noreply.github.com>pull/32314/head
parent
442566aa32
commit
9ec03f3cbb
@ -0,0 +1,256 @@
|
|||||||
|
/*
|
||||||
|
Copyright The Helm Authors.
|
||||||
|
|
||||||
|
Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
you may not use this file except in compliance with the License.
|
||||||
|
You may obtain a copy of the License at
|
||||||
|
|
||||||
|
http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
|
||||||
|
Unless required by applicable law or agreed to in writing, software
|
||||||
|
distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
See the License for the specific language governing permissions and
|
||||||
|
limitations under the License.
|
||||||
|
*/
|
||||||
|
|
||||||
|
package rules
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"path"
|
||||||
|
"strings"
|
||||||
|
|
||||||
|
"sigs.k8s.io/yaml"
|
||||||
|
|
||||||
|
"helm.sh/helm/v4/pkg/chart/common"
|
||||||
|
commonutil "helm.sh/helm/v4/pkg/chart/common/util"
|
||||||
|
chart "helm.sh/helm/v4/pkg/chart/v2"
|
||||||
|
"helm.sh/helm/v4/pkg/chart/v2/lint/support"
|
||||||
|
"helm.sh/helm/v4/pkg/chart/v2/loader"
|
||||||
|
chartutil "helm.sh/helm/v4/pkg/chart/v2/util"
|
||||||
|
"helm.sh/helm/v4/pkg/engine"
|
||||||
|
"helm.sh/helm/v4/pkg/kube"
|
||||||
|
release "helm.sh/helm/v4/pkg/release/v1"
|
||||||
|
releaseutil "helm.sh/helm/v4/pkg/release/v1/util"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Sequencing runs lint rules for HIP-0025 sequencing annotations.
|
||||||
|
func Sequencing(linter *support.Linter, namespace string, values map[string]any) {
|
||||||
|
c, err := loader.LoadDir(linter.ChartDir)
|
||||||
|
if err != nil {
|
||||||
|
// Chart load errors are already reported by other lint rules.
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// ProcessDependencies must run before validateSubchartSequencing:
|
||||||
|
// it prunes disabled subcharts from c.Dependencies() and applies
|
||||||
|
// alias renames, which BuildSubchartDAG relies on.
|
||||||
|
if err := chartutil.ProcessDependencies(c, values); err != nil {
|
||||||
|
// ProcessDependencies rejects invalid depends-on references (e.g.
|
||||||
|
// ambiguous alias references); surface that instead of silently
|
||||||
|
// skipping sequencing validation.
|
||||||
|
linter.RunLinterRule(support.ErrorSev, linter.ChartDir, err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
linter.RunLinterRule(support.ErrorSev, linter.ChartDir, validateSubchartSequencing(c))
|
||||||
|
validateRenderedSequencingAnnotations(linter, c, namespace, values)
|
||||||
|
}
|
||||||
|
|
||||||
|
func validateSubchartSequencing(c *chart.Chart) error {
|
||||||
|
// Note: we must NOT early-return on len(Dependencies) == 0. A chart with
|
||||||
|
// zero dependencies can still carry a helm.sh/depends-on/subcharts
|
||||||
|
// annotation that references a non-existent subchart — the orphan case
|
||||||
|
// BuildSubchartDAG's validateParentSubchartDependencies is meant to catch.
|
||||||
|
if c.Metadata == nil {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
dag, err := chartutil.BuildSubchartDAG(c)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if _, err := dag.GetBatches(); err != nil {
|
||||||
|
return fmt.Errorf("subchart circular dependency detected: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
func validateRenderedSequencingAnnotations(linter *support.Linter, c *chart.Chart, namespace string, values map[string]any) {
|
||||||
|
if err := chartutil.ProcessDependencies(c, values); err != nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
options := common.ReleaseOptions{
|
||||||
|
Name: "test-release",
|
||||||
|
Namespace: namespace,
|
||||||
|
}
|
||||||
|
caps := common.DefaultCapabilities.Copy()
|
||||||
|
|
||||||
|
coalescedValues, err := commonutil.CoalesceValues(c, values)
|
||||||
|
if err != nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
valuesToRender, err := commonutil.ToRenderValues(c, coalescedValues, options, caps)
|
||||||
|
if err != nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
var renderEngine engine.Engine
|
||||||
|
renderEngine.LintMode = true
|
||||||
|
|
||||||
|
renderedContentMap, err := renderEngine.Render(c, valuesToRender)
|
||||||
|
if err != nil {
|
||||||
|
// Template rendering errors are already reported by the Templates lint rule.
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
manifestsByChart := make(map[string][]releaseutil.Manifest)
|
||||||
|
for templatePath, content := range renderedContentMap {
|
||||||
|
if strings.TrimSpace(content) == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
chartPath := renderedTemplateChartPath(templatePath)
|
||||||
|
for _, manifest := range parseRenderedManifests(content) {
|
||||||
|
// HIP-0025 explicitly excludes hooks from sequencing: at install
|
||||||
|
// time SortManifests routes hook resources out before resource-group
|
||||||
|
// parsing runs, so their sequencing annotations are ignored. Mirror
|
||||||
|
// that here — otherwise lint reads a hook's leftover
|
||||||
|
// resource-group/depends-on annotations into the DAG and can report
|
||||||
|
// spurious cycles or orphan errors that never occur at runtime.
|
||||||
|
if isHookManifest(manifest) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
validateReadinessAnnotations(linter, templatePath, manifest)
|
||||||
|
manifestsByChart[chartPath] = append(manifestsByChart[chartPath], manifest)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for chartPath, manifests := range manifestsByChart {
|
||||||
|
validateResourceGroupAnnotations(linter, chartPath, manifests)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// isHookManifest reports whether a rendered manifest is a Helm hook. Hooks
|
||||||
|
// carry the helm.sh/hook annotation and are excluded from HIP-0025 sequencing
|
||||||
|
// (the install path separates them via SortManifests), so the sequencing lint
|
||||||
|
// rules must skip them too.
|
||||||
|
func isHookManifest(manifest releaseutil.Manifest) bool {
|
||||||
|
if manifest.Head == nil || manifest.Head.Metadata == nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
return strings.TrimSpace(manifest.Head.Metadata.Annotations[release.HookAnnotation]) != ""
|
||||||
|
}
|
||||||
|
|
||||||
|
func renderedTemplateChartPath(templatePath string) string {
|
||||||
|
if chartPath, _, ok := strings.Cut(templatePath, "/templates/"); ok {
|
||||||
|
return chartPath
|
||||||
|
}
|
||||||
|
|
||||||
|
return path.Dir(templatePath)
|
||||||
|
}
|
||||||
|
|
||||||
|
func parseRenderedManifests(content string) []releaseutil.Manifest {
|
||||||
|
rawManifests := releaseutil.SplitManifests(content)
|
||||||
|
manifests := make([]releaseutil.Manifest, 0, len(rawManifests))
|
||||||
|
|
||||||
|
for manifestName, raw := range rawManifests {
|
||||||
|
if strings.TrimSpace(raw) == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
var head releaseutil.SimpleHead
|
||||||
|
if err := yaml.Unmarshal([]byte(raw), &head); err != nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
if head.Metadata != nil && head.Metadata.Name != "" {
|
||||||
|
manifestName = head.Metadata.Name
|
||||||
|
}
|
||||||
|
|
||||||
|
manifests = append(manifests, releaseutil.Manifest{
|
||||||
|
Name: manifestName,
|
||||||
|
Content: raw,
|
||||||
|
Head: &head,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
return manifests
|
||||||
|
}
|
||||||
|
|
||||||
|
func validateReadinessAnnotations(linter *support.Linter, templatePath string, manifest releaseutil.Manifest) {
|
||||||
|
if manifest.Head == nil || manifest.Head.Metadata == nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
annotations := manifest.Head.Metadata.Annotations
|
||||||
|
successRaw := strings.TrimSpace(annotations[kube.AnnotationReadinessSuccess])
|
||||||
|
failureRaw := strings.TrimSpace(annotations[kube.AnnotationReadinessFailure])
|
||||||
|
hasSuccess := successRaw != ""
|
||||||
|
hasFailure := failureRaw != ""
|
||||||
|
|
||||||
|
if hasSuccess != hasFailure {
|
||||||
|
linter.RunLinterRule(support.ErrorSev, templatePath, fmt.Errorf(
|
||||||
|
"resource %q has only one of %q / %q annotations; both must be present or absent together",
|
||||||
|
resourceDisplayName(manifest),
|
||||||
|
kube.AnnotationReadinessSuccess,
|
||||||
|
kube.AnnotationReadinessFailure,
|
||||||
|
))
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// Both present (or both absent). When present, the JSONPath expressions
|
||||||
|
// must still be well-formed — the presence-symmetry check alone let
|
||||||
|
// malformed expressions through at lint time.
|
||||||
|
for key, raw := range map[string]string{
|
||||||
|
kube.AnnotationReadinessSuccess: successRaw,
|
||||||
|
kube.AnnotationReadinessFailure: failureRaw,
|
||||||
|
} {
|
||||||
|
if err := kube.ValidateReadinessExpressions(raw); err != nil {
|
||||||
|
linter.RunLinterRule(support.ErrorSev, templatePath, fmt.Errorf(
|
||||||
|
"resource %q has malformed %q annotation: %w",
|
||||||
|
resourceDisplayName(manifest), key, err,
|
||||||
|
))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func validateResourceGroupAnnotations(linter *support.Linter, chartPath string, manifests []releaseutil.Manifest) {
|
||||||
|
result, warnings, err := releaseutil.ParseResourceGroups(manifests)
|
||||||
|
// HIP-0025: lint must fail on orphan resource-group dependencies and
|
||||||
|
// malformed annotation JSON. Runtime falls back to the unsequenced batch
|
||||||
|
// for graceful recovery, but the chart author should fix these at lint time.
|
||||||
|
for _, warning := range warnings {
|
||||||
|
linter.RunLinterRule(support.ErrorSev, chartPath, fmt.Errorf("%s", warning))
|
||||||
|
}
|
||||||
|
if err != nil {
|
||||||
|
linter.RunLinterRule(support.ErrorSev, chartPath, err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
dag, err := releaseutil.BuildResourceGroupDAG(result)
|
||||||
|
if err != nil {
|
||||||
|
linter.RunLinterRule(support.ErrorSev, chartPath, err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, err := dag.GetBatches(); err != nil {
|
||||||
|
linter.RunLinterRule(support.ErrorSev, chartPath, fmt.Errorf("resource-group circular dependency detected: %w", err))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func resourceDisplayName(manifest releaseutil.Manifest) string {
|
||||||
|
if manifest.Head == nil || manifest.Head.Metadata == nil {
|
||||||
|
return manifest.Name
|
||||||
|
}
|
||||||
|
|
||||||
|
if manifest.Head.Kind == "" {
|
||||||
|
return manifest.Head.Metadata.Name
|
||||||
|
}
|
||||||
|
|
||||||
|
return fmt.Sprintf("%s/%s", manifest.Head.Kind, manifest.Head.Metadata.Name)
|
||||||
|
}
|
||||||
@ -0,0 +1,348 @@
|
|||||||
|
/*
|
||||||
|
Copyright The Helm Authors.
|
||||||
|
|
||||||
|
Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
you may not use this file except in compliance with the License.
|
||||||
|
You may obtain a copy of the License at
|
||||||
|
|
||||||
|
http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
|
||||||
|
Unless required by applicable law or agreed to in writing, software
|
||||||
|
distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
See the License for the specific language governing permissions and
|
||||||
|
limitations under the License.
|
||||||
|
*/
|
||||||
|
|
||||||
|
package rules
|
||||||
|
|
||||||
|
import (
|
||||||
|
"fmt"
|
||||||
|
"path/filepath"
|
||||||
|
"sort"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
|
"helm.sh/helm/v4/pkg/chart/common"
|
||||||
|
chart "helm.sh/helm/v4/pkg/chart/v2"
|
||||||
|
"helm.sh/helm/v4/pkg/chart/v2/lint/support"
|
||||||
|
chartutil "helm.sh/helm/v4/pkg/chart/v2/util"
|
||||||
|
releaseutil "helm.sh/helm/v4/pkg/release/v1/util"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestSequencing_SubchartCircularDependency(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
root := newChart("testchart", nil)
|
||||||
|
root.Metadata.Dependencies = []*chart.Dependency{
|
||||||
|
{Name: "subchart-a", Version: "0.1.0", Repository: "file://charts/subchart-a", DependsOn: []string{"subchart-b"}},
|
||||||
|
{Name: "subchart-b", Version: "0.1.0", Repository: "file://charts/subchart-b", DependsOn: []string{"subchart-a"}},
|
||||||
|
}
|
||||||
|
root.SetDependencies(newChart("subchart-a", nil), newChart("subchart-b", nil))
|
||||||
|
|
||||||
|
messages := runSequencingLint(t, root)
|
||||||
|
requireMessage(t, messages, support.ErrorSev, "subchart circular dependency detected")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSequencing_AliasedSubchartDependsOnOriginalName(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// Full loader + ProcessDependencies pipeline: an aliased subchart may be
|
||||||
|
// referenced by its original chart name (HIP-0025: "names or aliases").
|
||||||
|
// Before the fix this reported a spurious "unknown or disabled subchart".
|
||||||
|
root := newChart("testchart", nil)
|
||||||
|
root.Metadata.Dependencies = []*chart.Dependency{
|
||||||
|
{Name: "subchart-a", Version: "0.1.0", Repository: "file://charts/subchart-a", Alias: "aliased-a"},
|
||||||
|
{Name: "subchart-b", Version: "0.1.0", Repository: "file://charts/subchart-b", DependsOn: []string{"subchart-a"}},
|
||||||
|
}
|
||||||
|
root.SetDependencies(newChart("subchart-a", nil), newChart("subchart-b", nil))
|
||||||
|
|
||||||
|
messages := runSequencingLint(t, root)
|
||||||
|
assert.Empty(t, messages)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSequencing_AmbiguousDependsOnReported(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// Ambiguity is now rejected by ProcessDependencies; the lint rule must
|
||||||
|
// surface that instead of silently skipping sequencing validation.
|
||||||
|
root := newChart("testchart", nil)
|
||||||
|
root.Metadata.Dependencies = []*chart.Dependency{
|
||||||
|
{Name: "subchart-a", Version: "0.1.0", Repository: "file://charts/subchart-a", Alias: "first"},
|
||||||
|
{Name: "subchart-a", Version: "0.1.0", Repository: "file://charts/subchart-a", Alias: "second"},
|
||||||
|
{Name: "subchart-b", Version: "0.1.0", Repository: "file://charts/subchart-b", DependsOn: []string{"subchart-a"}},
|
||||||
|
}
|
||||||
|
root.SetDependencies(newChart("subchart-a", nil), newChart("subchart-b", nil))
|
||||||
|
|
||||||
|
messages := runSequencingLint(t, root)
|
||||||
|
requireMessage(t, messages, support.ErrorSev, `ambiguous subchart reference "subchart-a"`)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSequencing_SubchartAnnotationRequiresHIPListSyntax(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
root := newChart("testchart", nil)
|
||||||
|
root.Metadata.Dependencies = []*chart.Dependency{
|
||||||
|
{Name: "subchart-a", Version: "0.1.0", Repository: "file://charts/subchart-a"},
|
||||||
|
{Name: "subchart-b", Version: "0.1.0", Repository: "file://charts/subchart-b"},
|
||||||
|
}
|
||||||
|
root.Metadata.Annotations = map[string]string{
|
||||||
|
chartutil.AnnotationDependsOnSubcharts: `{"subchart-a":["subchart-b"],"subchart-b":["subchart-a"]}`,
|
||||||
|
}
|
||||||
|
root.SetDependencies(newChart("subchart-a", nil), newChart("subchart-b", nil))
|
||||||
|
|
||||||
|
messages := runSequencingLint(t, root)
|
||||||
|
requireMessage(t, messages, support.ErrorSev, "JSON string array")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSequencing_SubchartAnnotationOrphanWithNoDependencies(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// A chart with ZERO dependencies can still carry a
|
||||||
|
// helm.sh/depends-on/subcharts annotation referencing a subchart that
|
||||||
|
// doesn't exist. The lint rule previously early-returned on
|
||||||
|
// len(Dependencies)==0 and missed this orphan reference entirely.
|
||||||
|
root := newChart("testchart", nil)
|
||||||
|
root.Metadata.Annotations = map[string]string{
|
||||||
|
chartutil.AnnotationDependsOnSubcharts: `["does-not-exist"]`,
|
||||||
|
}
|
||||||
|
|
||||||
|
messages := runSequencingLint(t, root)
|
||||||
|
requireMessage(t, messages, support.ErrorSev, "unknown or disabled subchart")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSequencing_RenderedAnnotationRules(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
templates map[string]string
|
||||||
|
wantSeverity int
|
||||||
|
wantSubstring string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "partial readiness annotations",
|
||||||
|
templates: map[string]string{
|
||||||
|
"templates/configmap.yaml": manifestYAML("ConfigMap", "partial-readiness", map[string]string{
|
||||||
|
releaseutil.AnnotationResourceGroup: "bootstrap",
|
||||||
|
"helm.sh/readiness-success": `'["{.phase} == \"Ready\""]'`,
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
wantSeverity: support.ErrorSev,
|
||||||
|
wantSubstring: "both must be present or absent together",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "duplicate group assignment",
|
||||||
|
templates: map[string]string{
|
||||||
|
"templates/first.yaml": manifestYAML("ConfigMap", "shared-resource", map[string]string{releaseutil.AnnotationResourceGroup: "bootstrap"}),
|
||||||
|
"templates/second.yaml": manifestYAML("ConfigMap", "shared-resource", map[string]string{releaseutil.AnnotationResourceGroup: "app"}),
|
||||||
|
},
|
||||||
|
wantSeverity: support.ErrorSev,
|
||||||
|
wantSubstring: "assigned to multiple resource groups",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "circular resource-group dependencies",
|
||||||
|
templates: map[string]string{
|
||||||
|
"templates/group-a.yaml": manifestYAML("ConfigMap", "group-a", map[string]string{
|
||||||
|
releaseutil.AnnotationResourceGroup: "a",
|
||||||
|
releaseutil.AnnotationDependsOnResourceGroups: `'["c"]'`,
|
||||||
|
}),
|
||||||
|
"templates/group-b.yaml": manifestYAML("ConfigMap", "group-b", map[string]string{
|
||||||
|
releaseutil.AnnotationResourceGroup: "b",
|
||||||
|
releaseutil.AnnotationDependsOnResourceGroups: `'["a"]'`,
|
||||||
|
}),
|
||||||
|
"templates/group-c.yaml": manifestYAML("ConfigMap", "group-c", map[string]string{
|
||||||
|
releaseutil.AnnotationResourceGroup: "c",
|
||||||
|
releaseutil.AnnotationDependsOnResourceGroups: `'["b"]'`,
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
wantSeverity: support.ErrorSev,
|
||||||
|
wantSubstring: "resource-group circular dependency detected",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "orphan resource-group reference",
|
||||||
|
templates: map[string]string{
|
||||||
|
"templates/configmap.yaml": manifestYAML("ConfigMap", "orphaned-group", map[string]string{
|
||||||
|
releaseutil.AnnotationResourceGroup: "app",
|
||||||
|
releaseutil.AnnotationDependsOnResourceGroups: `'["database"]'`,
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
wantSeverity: support.ErrorSev,
|
||||||
|
wantSubstring: `depends-on non-existent group "database"`,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "malformed depends-on-resource-groups annotation",
|
||||||
|
templates: map[string]string{
|
||||||
|
"templates/configmap.yaml": manifestYAML("ConfigMap", "bad-json", map[string]string{
|
||||||
|
releaseutil.AnnotationResourceGroup: "app",
|
||||||
|
releaseutil.AnnotationDependsOnResourceGroups: `not-a-json-array`,
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
wantSeverity: support.ErrorSev,
|
||||||
|
wantSubstring: "depends-on",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
// Both readiness annotations present (so the presence-symmetry
|
||||||
|
// check passes), but the JSONPath in readiness-success is
|
||||||
|
// malformed. Presence symmetry alone let this through; the rule
|
||||||
|
// must validate the expressions compile.
|
||||||
|
name: "malformed readiness JSONPath with both annotations present",
|
||||||
|
templates: map[string]string{
|
||||||
|
"templates/job.yaml": manifestYAML("Job", "bad-jsonpath", map[string]string{
|
||||||
|
"helm.sh/readiness-success": `'["{.status.succeeded >= 1"]'`,
|
||||||
|
"helm.sh/readiness-failure": `'["{.status.failed} >= 1"]'`,
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
wantSeverity: support.ErrorSev,
|
||||||
|
wantSubstring: "malformed",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
// The comparison value's type IS statically known: an ordering
|
||||||
|
// operator with a non-numeric literal can never evaluate at
|
||||||
|
// runtime, so lint flags it as a definite authoring error.
|
||||||
|
name: "ordering operator with non-numeric readiness value",
|
||||||
|
templates: map[string]string{
|
||||||
|
"templates/job.yaml": manifestYAML("Job", "string-ordering", map[string]string{
|
||||||
|
"helm.sh/readiness-success": `'["{.status.phase} > \"Running\""]'`,
|
||||||
|
"helm.sh/readiness-failure": `'["{.status.failed} >= 1"]'`,
|
||||||
|
}),
|
||||||
|
},
|
||||||
|
wantSeverity: support.ErrorSev,
|
||||||
|
wantSubstring: "requires a numeric comparison value",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
messages := runSequencingLint(t, newChart("testchart", tc.templates))
|
||||||
|
requireMessage(t, messages, tc.wantSeverity, tc.wantSubstring)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSequencing_HookResourcesExcludedFromResourceGroupDAG(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// A hook resource carrying sequencing annotations that WOULD form a cycle
|
||||||
|
// with the non-hook resources must be ignored by the sequencing lint rules,
|
||||||
|
// exactly as the install path ignores it (SortManifests routes hooks out
|
||||||
|
// before resource-group parsing). group-b depends on group-a; the hook
|
||||||
|
// claims group-a depends-on group-b — a cycle only if the hook is counted.
|
||||||
|
templates := map[string]string{
|
||||||
|
"templates/group-a.yaml": manifestYAML("ConfigMap", "group-a", map[string]string{
|
||||||
|
releaseutil.AnnotationResourceGroup: "group-a",
|
||||||
|
}),
|
||||||
|
"templates/group-b.yaml": manifestYAML("ConfigMap", "group-b", map[string]string{
|
||||||
|
releaseutil.AnnotationResourceGroup: "group-b",
|
||||||
|
releaseutil.AnnotationDependsOnResourceGroups: `'["group-a"]'`,
|
||||||
|
}),
|
||||||
|
"templates/hook-job.yaml": manifestYAML("Job", "hook-job", map[string]string{
|
||||||
|
"helm.sh/hook": "pre-install",
|
||||||
|
releaseutil.AnnotationResourceGroup: "group-a",
|
||||||
|
releaseutil.AnnotationDependsOnResourceGroups: `'["group-b"]'`,
|
||||||
|
}),
|
||||||
|
}
|
||||||
|
|
||||||
|
messages := runSequencingLint(t, newChart("hook-chart", templates))
|
||||||
|
|
||||||
|
for _, m := range messages {
|
||||||
|
assert.NotContains(t, m.Err.Error(), "circular",
|
||||||
|
"hook resource must be excluded from the resource-group DAG; got: %v", m.Err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSequencing_NoMessagesWithoutSequencingAnnotations(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
messages := runSequencingLint(t, newChart("plain-chart", map[string]string{
|
||||||
|
"templates/configmap.yaml": manifestYAML("ConfigMap", "plain", nil),
|
||||||
|
}))
|
||||||
|
|
||||||
|
assert.Empty(t, messages)
|
||||||
|
}
|
||||||
|
|
||||||
|
func runSequencingLint(t *testing.T, c *chart.Chart) []support.Message {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
require.NoError(t, chartutil.SaveDir(c, tmpDir))
|
||||||
|
|
||||||
|
linter := support.Linter{ChartDir: filepath.Join(tmpDir, c.Name())}
|
||||||
|
Sequencing(&linter, "test-namespace", nil)
|
||||||
|
|
||||||
|
return linter.Messages
|
||||||
|
}
|
||||||
|
|
||||||
|
func requireMessage(t *testing.T, messages []support.Message, severity int, substring string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
for _, message := range messages {
|
||||||
|
if message.Severity == severity && strings.Contains(message.Err.Error(), substring) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Fatalf("expected severity %d message containing %q, got %#v", severity, substring, messages)
|
||||||
|
}
|
||||||
|
|
||||||
|
func newChart(name string, templates map[string]string) *chart.Chart {
|
||||||
|
files := make([]*common.File, 0, len(templates))
|
||||||
|
names := make([]string, 0, len(templates))
|
||||||
|
for templateName := range templates {
|
||||||
|
names = append(names, templateName)
|
||||||
|
}
|
||||||
|
sort.Strings(names)
|
||||||
|
|
||||||
|
for _, templateName := range names {
|
||||||
|
files = append(files, &common.File{
|
||||||
|
Name: templateName,
|
||||||
|
Data: []byte(templates[templateName]),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
return &chart.Chart{
|
||||||
|
Metadata: &chart.Metadata{
|
||||||
|
Name: name,
|
||||||
|
Version: "0.1.0",
|
||||||
|
APIVersion: chart.APIVersionV2,
|
||||||
|
},
|
||||||
|
Templates: files,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func manifestYAML(kind, name string, annotations map[string]string) string {
|
||||||
|
if len(annotations) == 0 {
|
||||||
|
return fmt.Sprintf(`apiVersion: v1
|
||||||
|
kind: %s
|
||||||
|
metadata:
|
||||||
|
name: %s
|
||||||
|
data:
|
||||||
|
key: value
|
||||||
|
`, kind, name)
|
||||||
|
}
|
||||||
|
|
||||||
|
keys := make([]string, 0, len(annotations))
|
||||||
|
for key := range annotations {
|
||||||
|
keys = append(keys, key)
|
||||||
|
}
|
||||||
|
sort.Strings(keys)
|
||||||
|
|
||||||
|
var annotationBlock strings.Builder
|
||||||
|
for _, key := range keys {
|
||||||
|
fmt.Fprintf(&annotationBlock, "\n %s: %s", key, annotations[key])
|
||||||
|
}
|
||||||
|
|
||||||
|
return fmt.Sprintf(`apiVersion: v1
|
||||||
|
kind: %s
|
||||||
|
metadata:
|
||||||
|
name: %s
|
||||||
|
annotations:%s
|
||||||
|
data:
|
||||||
|
key: value
|
||||||
|
`, kind, name, annotationBlock.String())
|
||||||
|
}
|
||||||
Loading…
Reference in new issue