[NopeCharon]

What type of PR is this?

bug

Which issue does this PR fix:

Fixes #811

What does this PR do / Why do we need it:

NewStackerFiles() read a stacker file and then recursed into its config.prerequisites, without checking whether a prerequisite had already been read.

• Cycles: prerequisites that form a cycle (a.yaml → b.yaml → a.yaml, or a file listing itself) recursed until the goroutine stack limit was hit, and stacker died with fatal error: stack overflow. The cycle check in NewStackerFilesDAG() was never reached.
• Shared prerequisites: a file reachable through several paths was re-read and re-parsed once per path.

This PR loads stacker files into a single map and skips files that have already been read. It also tracks the chain of prerequisites being followed, and returns an error naming the cycle when a file appears in its own chain:

prerequisite cycle detected: /p/a.yaml -> /p/b.yaml -> /p/a.yaml

The resulting map is the same as before for acyclic inputs, so the DAG, build order and duplicate-layer-name check are unchanged.

If an issue # is not available please add repro steps and logs showing the issue:

See #811.

Testing done on this change:

Added pkg/types/stackerfiles_test.go:

• TestNewStackerFilesPrerequisites: c requires a and b, and b requires a. All three files are loaded and keyed by absolute path.
• TestNewStackerFilesPrerequisiteCycle: a two-file cycle and a self-reference each return the cycle error.

Without the fix, the cycle test crashes:

runtime: goroutine stack exceeds 1000000000-byte limit
fatal error: stack overflow
FAIL	stackerbuild.io/stacker/pkg/types	30.298s

With the fix, go test ./pkg/types/ passes (on linux/arm64, using the Makefile's go-test build tags):

--- PASS: TestNewStackerFilesPrerequisites (0.00s)
--- PASS: TestNewStackerFilesPrerequisiteCycle (0.00s)
...
ok  	stackerbuild.io/stacker/pkg/types	0.019s

Automation added to e2e:

No. The existing test/prerequisites.bats covers the acyclic build order, and the new behavior is covered by Go unit tests.

Will this break upgrades or downgrades?

No.

Does this PR introduce any user-facing change?:

stacker now reports "prerequisite cycle detected" instead of crashing with a stack overflow when stacker file prerequisites form a cycle.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov]

Codecov Report

❌ Patch coverage is 78.37838% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 13.57%. Comparing base (715bf05) to head (fe74319).

Files with missing lines	Patch %	Lines
pkg/types/stackerfiles.go	78.37%	6 Missing and 2 partials ⚠️

❗ There is a different number of reports uploaded between BASE (715bf05) and HEAD (fe74319). Click for more details.

HEAD has 36 uploads less than BASE

Flag	BASE (715bf05)	HEAD (fe74319)
	40	4

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #812       +/-   ##
===========================================
- Coverage   55.06%   13.57%   -41.49%     
===========================================
  Files          55       55               
  Lines        5911     5921       +10     
===========================================
- Hits         3255      804     -2451     
- Misses       2085     4982     +2897     
+ Partials      571      135      -436     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.