From 2a8ada640958d47e315589c6d478c250919001ef Mon Sep 17 00:00:00 2001 From: Michael McCracken Date: Wed, 20 Mar 2024 14:31:35 -0700 Subject: [PATCH] fix: cache: serialize legacy import field Fixes #592 by serializing the `wasLegacyImport` field to ensure that the cache records the correct value when an image has it set. Includes a test based on Serge's description in #592. The symptom is an error "cache miss because layer definition was changed" when the definition could not have changed since build. This happens during multi file builds in builds with dependencies, and in build-then-publish flows, where the build will be fine but the publish will fail with the same error. Using `--debug` to show the cache mismatch shows that somewhere along the line, the layer is being serialized with the wrong default value when writing to the cache. Needs to change currentCacheVersion to ensure compatibility, see the TestCacheEntryChanged function comment for more info. Signed-off-by: Michael McCracken --- pkg/stacker/build.go | 6 ++++-- pkg/stacker/cache.go | 2 +- pkg/stacker/cache_test.go | 2 +- pkg/types/layer.go | 9 ++------- pkg/types/stackerfile.go | 2 +- test/caching.bats | 33 +++++++++++++++++++++++++++++++++ 6 files changed, 42 insertions(+), 12 deletions(-) diff --git a/pkg/stacker/build.go b/pkg/stacker/build.go index 75764a4b..16ddc6c6 100644 --- a/pkg/stacker/build.go +++ b/pkg/stacker/build.go @@ -116,7 +116,7 @@ func (b *Builder) updateOCIConfigForOutput(sf *types.Stackerfile, s types.Storag } inDir := types.InternalStackerDir - if l.WasLegacyImport() { + if l.WasLegacyImport { inDir = types.LegacyInternalStackerDir } @@ -370,7 +370,9 @@ func (b *Builder) build(s types.Storage, file string) error { log.Infof("preparing image %s...", name) inDir := types.InternalStackerDir - if l.WasLegacyImport() { + if l.WasLegacyImport { + log.Debugf("image %s uses legacy import syntax, will also mount imports at %s", + name, types.LegacyInternalStackerDir) inDir = types.LegacyInternalStackerDir } diff --git a/pkg/stacker/cache.go b/pkg/stacker/cache.go index e7030cde..a75e13b1 100644 --- a/pkg/stacker/cache.go +++ b/pkg/stacker/cache.go @@ -22,7 +22,7 @@ import ( "stackerbuild.io/stacker/pkg/types" ) -const currentCacheVersion = 13 +const currentCacheVersion = 14 type ImportType int diff --git a/pkg/stacker/cache_test.go b/pkg/stacker/cache_test.go index 55bb15eb..451903f4 100644 --- a/pkg/stacker/cache_test.go +++ b/pkg/stacker/cache_test.go @@ -125,5 +125,5 @@ func TestCacheEntryChanged(t *testing.T) { // This test works because the type information is included in the // hashstructure hash above, so using a zero valued CacheEntry is // enough to capture changes in types. - assert.Equal(uint64(0x4ef432528243e356), h) + assert.Equal(uint64(0xa26696f335211127), h) } diff --git a/pkg/types/layer.go b/pkg/types/layer.go index 2f1d0b5d..84f35e29 100644 --- a/pkg/types/layer.go +++ b/pkg/types/layer.go @@ -260,7 +260,7 @@ type Layer struct { OS *string `yaml:"os" json:"os,omitempty"` Arch *string `yaml:"arch" json:"arch,omitempty"` Bom *Bom `yaml:"bom" json:"bom,omitempty"` - wasLegacyImport bool + WasLegacyImport bool `yaml:"was_legacy_import" json:"was_legacy_import,omitempty"` } func parseLayers(referenceDirectory string, lms yaml.MapSlice, requireHash bool) (map[string]Layer, error) { @@ -369,7 +369,7 @@ func parseLayers(referenceDirectory string, lms yaml.MapSlice, requireHash bool) if len(layer.LegacyImport) != 0 { layer.Imports = layer.LegacyImport layer.LegacyImport = nil - layer.wasLegacyImport = true + layer.WasLegacyImport = true } ret[name], err = layer.absolutify(referenceDirectory) @@ -442,11 +442,6 @@ func (l Layer) absolutify(referenceDirectory string) (Layer, error) { return ret, nil } -// WasLegacyImport - return true if this layer used legacy 'import' directive. -func (l Layer) WasLegacyImport() bool { - return l.wasLegacyImport -} - func requireImportHash(imports Imports) error { for _, imp := range imports { url, err := NewDockerishUrl(imp.Path) diff --git a/pkg/types/stackerfile.go b/pkg/types/stackerfile.go index 153afa93..e5dae0bc 100644 --- a/pkg/types/stackerfile.go +++ b/pkg/types/stackerfile.go @@ -269,7 +269,7 @@ func NewStackerfile(stackerfile string, validateHash bool, substitutions []strin for _, name := range sf.FileOrder { layer := sf.internal[name] - if layer.WasLegacyImport() { + if layer.WasLegacyImport { log.Warnf("'import' directive used in layer '%s' inside file '%s' is deprecated. "+ "Support for 'import' will be removed in releases after 2025-01-01. "+ "Migrate by changing 'import' to 'imports' and '/stacker' to '/stacker/imports'. "+ diff --git a/test/caching.bats b/test/caching.bats index 047f9452..eea5aa8e 100644 --- a/test/caching.bats +++ b/test/caching.bats @@ -225,3 +225,36 @@ EOF echo '{"version": 1, "cache": "lolnope"}' > .stacker/build.cache stacker build --substitute BUSYBOX_OCI=${BUSYBOX_OCI} } + +@test "cache comparison of legacy import works across prereqs" { + echo "well, hi!"> helloworld + + cat >> stacker-req.yaml << "EOF" +base: + from: + type: oci + url: ${{BUSYBOX_OCI}} + build_only: true + import: + - helloworld + run: | + cp /stacker/helloworld / +EOF + +cat >> stacker.yaml << "EOF" +config: + prerequisites: + - stacker-req.yaml + +buildenv: + from: + type: built + tag: base + import: + - stacker://base/helloworld + run: | + cp /stacker/helloworld /h3 + +EOF +stacker --debug build --substitute BUSYBOX_OCI=${BUSYBOX_OCI} +}