fix: skeleton packages drop non-host-arch components - #5020
Conversation
Signed-off-by: Jeff Rescignano <jeffr@defenseunicorns.com>
✅ Deploy Preview for zarf-docs canceled.
|
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
Signed-off-by: Jeff Rescignano <jeffr@defenseunicorns.com>
Signed-off-by: Jeff Rescignano <jeffr@defenseunicorns.com>
AustinAbro321
left a comment
There was a problem hiding this comment.
Thanks for taking this on, requesting an abstraction
| // SkipVersionCheck skips version requirement validation | ||
| SkipVersionCheck bool | ||
| // Skeleton retains all architecture variants instead of filtering to the build host arch | ||
| Skeleton bool |
There was a problem hiding this comment.
I'd like to avoid tying load.PackageDefinition to skeletons.
I think something like below would be a better abstraction. It would also aid the work that @chaospuppy is doing in #4783.
type VariantDimension string
const (
VariantArchitecture VariantDimension = "architecture"
VariantFlavor VariantDimension = "flavor"
)
// SkipVariantFilters lists dimensions to retain instead of filtering out.
SkipVariantFilters []VariantDimension
func AllVariantDimension() []VariantDimension
There was a problem hiding this comment.
Just pushed a commit to address this, let me know any feedback you have
Co-authored-by: Austin Abro <37223396+AustinAbro321@users.noreply.github.com> Signed-off-by: Jeff Rescignano <jeff@jeffresc.dev>
Signed-off-by: Jeff Rescignano <jeffr@defenseunicorns.com>
AustinAbro321
left a comment
There was a problem hiding this comment.
Few more separation of concern comments
| } | ||
| } | ||
|
|
||
| func TestResolveImportsSkeletonRetainsAllArches(t *testing.T) { |
There was a problem hiding this comment.
This test shouldn't be focused on skeletons (publish_test.go is handling that). Make this test ensure that the variant dimensions work separately and together
| } | ||
| pkg.Metadata.Architecture = config.GetArch(pkg.Metadata.Architecture) | ||
| if slices.Contains(opts.SkipVariantFilters, VariantArchitecture) { | ||
| pkg.Metadata.Architecture = v1alpha1.SkeletonArch |
There was a problem hiding this comment.
We still don't want the load package to know about skeletons, instead of setting the architecture, and checking for a skeleton architecture we should pass the variant dimensions into validate and import
Description
Skeleton publishing resolved the package definition using the build host's architecture, so compose filtered out components gated to other arches via
only.cluster.architecturebefore the package was marked arch-agnostic. The publishedzarf initskeleton therefore shipped only the amd64zarf-injector/k3s, and importing it to build for arm64 failed withno compatible component named zarf-injector found.This resolves the skeleton definition under
SkeletonArch(match-all), so all architecture variants are retained, mirroring howOnly.Flavoris already neutralized for skeletons. Consumers are unaffected: they import the (now complete) skeleton and their own load filters to the concrete target arch.Related Issue
Fixes #5019
Checklist before merging