[NopeCharon]

What type of PR is this?

bug

Which issue does this PR fix:

Fixes #807

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

parseLayers() checks the directives of each layer using unchecked type assertions on the raw YAML. Because of that, some malformed stacker files crash stacker with a Go panic instead of producing an error. The most common case is writing from as a string:

foo:
  from: docker://ubuntu:latest

An empty layer body (foo:), a layer that is a scalar (foo: bar), and non-string directive keys also panic. Separately, an empty string entry in imports (for example from - ${{EXTRA:}}) panicked in Layer.absolutify() with slice bounds out of range [-1:].

This PR:

• checks every type assertion in parseLayers() and returns a descriptive error that names the layer, e.g.
  stackerfile: layer 'foo': 'from' must be a map with a 'type' and a 'url' or 'tag', found: "docker://ubuntu:latest"
• rejects an empty import path in getImportFromInterface(). An import map without a path is already rejected there.

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

See #807.

Testing done on this change:

Added TestMalformedLayers (one case for each malformed input) and an empty-string case in TestGetImportFromInterface. Without the fix, both tests fail with the panics described in the issue:

--- FAIL: TestGetImportFromInterface (0.00s)
--- FAIL: TestMalformedLayers (0.00s)
    --- FAIL: TestMalformedLayers/layer_is_a_string (0.00s)
panic: interface conversion: interface {} is string, not yaml.MapSlice [recovered, repanicked]

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

--- PASS: TestGetImportFromInterface (0.00s)
--- PASS: TestMalformedLayers (0.00s)
...
ok  	stackerbuild.io/stacker/pkg/types	0.020s

Automation added to e2e:

No. The parsing is covered by Go unit tests in pkg/types.

Will this break upgrades or downgrades?

No. Stacker files that parsed before still parse the same way. Only inputs that used to panic now return an error.

Does this PR introduce any user-facing change?:

stacker now reports an error instead of panicking when a stacker file contains a malformed layer, such as `from:` given as a string or an empty `imports` entry.

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 87.50000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 13.28%. Comparing base (715bf05) to head (3c094e4).

Files with missing lines	Patch %	Lines
pkg/types/layer.go	87.50%	3 Missing and 1 partial ⚠️

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

HEAD has 36 uploads less than BASE

Flag	BASE (715bf05)	HEAD (3c094e4)
	40	4

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #808       +/-   ##
===========================================
- Coverage   55.06%   13.28%   -41.79%     
===========================================
  Files          55       55               
  Lines        5911     5932       +21     
===========================================
- Hits         3255      788     -2467     
- Misses       2085     5021     +2936     
+ Partials      571      123      -448     

☔ 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.