Skip to content

Commit 146d5e5

Browse files
committed
test(e2e): read the opensearch release history through the pinned name
The opensearch suite wrote the history jsonpath out as a literal, so the contract test that runs HELMRELEASE_HISTORY_JSONPATH against the upstream Flux type never saw the expression this suite hands kubectl. A field rename upstream would have reached it only as an empty read. The suite now takes the shared expression, and the contract test reads every Chainsaw suite that calls the guard, so a suite copying the literal, or assigning the shared name after sourcing the library, fails in the unit lane instead of going unchecked. The read is matched inside the script step that calls the guard, because every step runs in a shell of its own and a read in another step never reaches the call. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent bf03c84 commit 146d5e5

2 files changed

Lines changed: 132 additions & 8 deletions

File tree

‎hack/e2e-chainsaw/opensearch/chainsaw-test.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,7 @@ spec:
551551
cd ../../..
552552
. hack/e2e-chainsaw/_lib/remediation-guard.sh
553553
statuses=$(kubectl -n "$NAMESPACE" get helmrelease opensearch-test \
554-
-o jsonpath='{range .status.history[*]}{.status}{"\n"}{end}')
554+
-o "jsonpath=${HELMRELEASE_HISTORY_JSONPATH}")
555555
# Always emit the raw value so a silent future-Flux field rename shows
556556
# up as "empty history on a Ready HR" in CI logs rather than vanishing.
557557
# The helper reads empty input as "no cycle observed", so without this

‎internal/fluxcontract/snapshot_shape_test.go‎

Lines changed: 131 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,18 @@ import (
3535

3636
helmv2 "github.com/fluxcd/helm-controller/api/v2"
3737
"k8s.io/client-go/util/jsonpath"
38+
"sigs.k8s.io/yaml"
3839
)
3940

4041
const (
4142
// The shell library that owns the expression, and the script that runs it.
4243
guardLibrary = "../../hack/e2e-chainsaw/_lib/remediation-guard.sh"
4344
guardCaller = "../../hack/e2e-chainsaw/_lib/run-kubernetes.sh"
4445

46+
// The Chainsaw suites, one directory each, which may call the guard from a
47+
// script step.
48+
chainsawSuites = "../../hack/e2e-chainsaw"
49+
4550
sharedName = "HELMRELEASE_HISTORY_JSONPATH"
4651
)
4752

@@ -73,11 +78,12 @@ var nameFollowedByEquals = regexp.MustCompile(regexp.QuoteMeta(sharedName) + `=`
7378
// free, because this script mixes forms that behave identically. Each is a
7479
// case in the accept table below.
7580
//
76-
// The reach is one named caller and calls that pass a local. A second script
77-
// calling the guard, or this one passing a substitution written out in the
78-
// argument, is outside what this sees. Neither exists today, and the rule for
79-
// authors is prose rather than a pattern: docs/agents/e2e-testing.md tells them
80-
// to take the expression from the shared name.
81+
// The reach is one named caller, the Chainsaw suites, and calls that pass a
82+
// local. Any other script calling the guard, or a caller passing a substitution
83+
// written out in the argument, is outside what this sees. Neither exists today,
84+
// and the rule for authors is prose rather than a pattern:
85+
// docs/agents/e2e-testing.md tells them to take the expression from the shared
86+
// name.
8187
var guardConsumers = regexp.MustCompile(
8288
`(?m)^[ \t]*(?:if[ \t]+)?(?:![ \t]*)?helmrelease_has_remediation_cycle[ \t]+"?\$\{?([A-Za-z_][A-Za-z0-9_]*)`)
8389

@@ -478,6 +484,11 @@ func TestGuardCallerReferencesTheSharedExpansion(t *testing.T) {
478484
"calls it in a shape %s cannot see.", guardCaller, guardConsumers)
479485
}
480486

487+
assertConsumedLocalsTakeTheSharedRead(t, guardCaller, src, consumers)
488+
}
489+
490+
func assertConsumedLocalsTakeTheSharedRead(t *testing.T, path string, src []byte, consumers [][][]byte) {
491+
t.Helper()
481492
for _, m := range consumers {
482493
local := string(m[1])
483494
n := len(localFollowedByEquals(local).FindAll(src, -1))
@@ -486,16 +497,129 @@ func TestGuardCallerReferencesTheSharedExpansion(t *testing.T) {
486497
"exactly one. The count is of occurrences, so a comment mentioning one counts too; "+
487498
"what it looks for is a second assignment, which the shell would take instead of "+
488499
"the one this pin matched, putting an unpinned expression in front of the guard.",
489-
guardCaller, local, n)
500+
path, local, n)
490501
continue
491502
}
492503
if !sharedReadBy(local).Match(src) {
493504
t.Errorf("%s fills %s, the local it hands the guard, in a shape that does not take its "+
494505
"expression from %s. The pattern is the exact form accepted: %s. Either the read "+
495506
"stopped using the shared assignment, or it uses it in a shape the pattern cannot "+
496-
"see.", guardCaller, local, sharedName, sharedReadBy(local))
507+
"see.", path, local, sharedName, sharedReadBy(local))
508+
}
509+
}
510+
}
511+
512+
// A Chainsaw suite that sources the guard library from a script step is a
513+
// caller too, and one this file would otherwise never read: the pin above names
514+
// one script, so a suite writing the jsonpath out as a literal runs an
515+
// expression nothing here checked.
516+
//
517+
// Every suite is read rather than a named one, because the next suite to adopt
518+
// the guard is the likeliest place for a copied literal and naming suites here
519+
// would need the same edit that introduces it. A suite cannot assign the shared
520+
// name either: its script sources the library first, so its own assignment
521+
// lands afterwards and replaces the value the upstream test resolved.
522+
//
523+
// Both sets are asserted non-empty. A glob that stops matching, or suites that
524+
// all stop calling the guard, leave the loop with nothing to check, and that
525+
// reads exactly like a pass.
526+
func TestChainsawSuitesReadHistoryThroughTheSharedName(t *testing.T) {
527+
var suites []string
528+
for _, pattern := range []string{"chainsaw-test.yaml", "chainsaw-test.yml"} {
529+
found, err := filepath.Glob(filepath.Join(chainsawSuites, "*", pattern))
530+
if err != nil {
531+
t.Fatalf("globbing the Chainsaw suites: %v", err)
532+
}
533+
suites = append(suites, found...)
534+
}
535+
if len(suites) == 0 {
536+
t.Fatalf("no Chainsaw suites found under %s, so this test asserted nothing", chainsawSuites)
537+
}
538+
539+
// Each script step runs in a shell of its own, so the read the guard sees is
540+
// the one in its own step. Checked against the whole file, a read in one
541+
// step would satisfy a call in another, and an unrelated assignment of the
542+
// same local elsewhere would fail a correct one.
543+
t.Run("a read in another step does not count", func(t *testing.T) {
544+
steps := scriptSteps(t, "fixture", []byte(`spec:
545+
steps:
546+
- try:
547+
- script:
548+
content: |
549+
statuses=$(kubectl get hr -o "jsonpath=${HELMRELEASE_HISTORY_JSONPATH}")
550+
---
551+
spec:
552+
steps:
553+
- try:
554+
- script:
555+
content: |
556+
helmrelease_has_remediation_cycle "$statuses"
557+
`))
558+
if len(steps) != 2 {
559+
t.Fatalf("split the fixture into %d script steps, want 2", len(steps))
560+
}
561+
if n := len(localFollowedByEquals("statuses").FindAll([]byte(steps[1]), -1)); n != 0 {
562+
t.Errorf("the calling step holds %d assignments of the local, want 0", n)
563+
}
564+
})
565+
566+
callers := 0
567+
for _, path := range suites {
568+
src := readGuardFile(t, path)
569+
if n := len(nameFollowedByEquals.FindAll(src, -1)); n != 0 {
570+
t.Errorf("%s writes %s followed by = %d times; only %s may assign it. A suite sources "+
571+
"the library before its own lines run, so its assignment replaces the value the "+
572+
"upstream test checked.", path, sharedName, n, guardLibrary)
573+
}
574+
for _, step := range scriptSteps(t, path, src) {
575+
consumers := guardConsumers.FindAllSubmatch([]byte(step), -1)
576+
if len(consumers) == 0 {
577+
continue
578+
}
579+
callers++
580+
assertConsumedLocalsTakeTheSharedRead(t, path, []byte(step), consumers)
581+
}
582+
}
583+
if callers == 0 {
584+
t.Fatalf("no Chainsaw suite under %s calls helmrelease_has_remediation_cycle with an "+
585+
"expanded local, so this test asserted nothing. If no suite runs the guard any more, "+
586+
"this test goes with it.", chainsawSuites)
587+
}
588+
}
589+
590+
var yamlDocumentSeparator = regexp.MustCompile(`(?m)^---[ \t]*$`)
591+
592+
// The content of every script step in a Chainsaw file, wherever it sits: try,
593+
// catch, finally, or a nested operation.
594+
func scriptSteps(t *testing.T, path string, src []byte) []string {
595+
t.Helper()
596+
var steps []string
597+
var walk func(any)
598+
walk = func(node any) {
599+
switch v := node.(type) {
600+
case map[string]any:
601+
if script, ok := v["script"].(map[string]any); ok {
602+
if content, ok := script["content"].(string); ok {
603+
steps = append(steps, content)
604+
}
605+
}
606+
for _, child := range v {
607+
walk(child)
608+
}
609+
case []any:
610+
for _, child := range v {
611+
walk(child)
612+
}
613+
}
614+
}
615+
for _, doc := range yamlDocumentSeparator.Split(string(src), -1) {
616+
var tree any
617+
if err := yaml.Unmarshal([]byte(doc), &tree); err != nil {
618+
t.Fatalf("parsing %s: %v", path, err)
497619
}
620+
walk(tree)
498621
}
622+
return steps
499623
}
500624

501625
// No shell library but the one that owns the name assigns it. Sourcing the

0 commit comments

Comments
 (0)