fix: cache: serialize legacy import field

[mikemccracken]

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.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.44%. Comparing base (b585bfb) to head (2a8ada6).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #603      +/-   ##
==========================================
+ Coverage   57.38%   57.44%   +0.06%     
==========================================
  Files          64       65       +1     
  Lines        7566     7706     +140     
==========================================
+ Hits         4342     4427      +85     
- Misses       2481     2522      +41     
- Partials      743      757      +14     

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

[rchincha]

lgtm

[mikemccracken]

Some clarification, in part because I wasn't sure until after this was merged, why the field needed to be serialized to fix the build case - it doesn't:

There are two ways this ends up failing: for the single multi file run of stacker build, the cache package makes a copy of the Layer struct when the build checks the cache during the second file's build - copying the Layer out of the cache results in all non-exported fields being set to the zero value, so wasLegacy ends up as false, which conflicts with its original value, which is what it is being compared with in the second stage.

To fix this we only need to make it an exported field.

In the build then publish case, it is going to read in from the cache , and set the field to the zero value because it isn't serialized. To fix that, we also need to add the serialization annotation to write the field out to the cache.

It might be worth having a publish test case that also exercises this- I'll look into it.

[mikemccracken]

My clarification was wrong yesterday. The behavior of copying in go structs with non-exported values is not quite that crazy - there are still some sharp edges there but if you just assign a struct, you will get the identical contents.

My confusion was that I didn't realize that stacker re-read the cache from disk in between files of multi-file builds. So in fact adding the serialization annotation itself is the fix in both cases - and we have to make the field exported to serialize it. So the fix here is correct and minimal, and as a bonus now I can explain it correctly.