From 82faa370019f19bafc3c6ae165789118d633516b Mon Sep 17 00:00:00 2001 From: Wayne Starr Date: Wed, 6 Dec 2023 13:37:43 -0700 Subject: [PATCH] fix: compose dropping `only.localOS` in an only filter (#2173) ## Description This fixes an issue where compose could drop the `only.localOS` filter through compose. ## Related Issue Fixes #2158 ## Type of change - [X] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Other (security config, docs update, etc) ## Checklist before merging - [X] Test, docs, adr added or updated as needed - [X] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed --------- Co-authored-by: Lucas Rodriguez --- src/pkg/packager/compose.go | 2 +- src/pkg/packager/composer/list.go | 22 +++++++++++-------- src/pkg/packager/composer/list_test.go | 2 +- src/pkg/packager/composer/override.go | 14 +++++++++++- src/test/e2e/09_component_compose_test.go | 21 +++++++++++++----- .../bad-local-os/zarf.yaml | 14 ++++++++++++ .../sub-package/zarf.yaml | 2 ++ 7 files changed, 60 insertions(+), 17 deletions(-) create mode 100644 src/test/packages/09-composable-packages/bad-local-os/zarf.yaml diff --git a/src/pkg/packager/compose.go b/src/pkg/packager/compose.go index 6961e8b876..660659c5a2 100644 --- a/src/pkg/packager/compose.go +++ b/src/pkg/packager/compose.go @@ -44,7 +44,7 @@ func (p *Packager) composeComponents() error { if err != nil { return err } - components = append(components, composed) + components = append(components, *composed) // merge variables and constants pkgVars = chain.MergeVariables(pkgVars) diff --git a/src/pkg/packager/composer/list.go b/src/pkg/packager/composer/list.go index 89da2380c0..f38eeac49d 100644 --- a/src/pkg/packager/composer/list.go +++ b/src/pkg/packager/composer/list.go @@ -216,8 +216,8 @@ func (ic *ImportChain) Migrate(build types.ZarfBuildData) (warnings []string) { // Compose merges the import chain into a single component // fixing paths, overriding metadata, etc -func (ic *ImportChain) Compose() (composed types.ZarfComponent, err error) { - composed = ic.tail.ZarfComponent +func (ic *ImportChain) Compose() (composed *types.ZarfComponent, err error) { + composed = &ic.tail.ZarfComponent if ic.tail.prev == nil { // only had one component in the import chain @@ -225,11 +225,11 @@ func (ic *ImportChain) Compose() (composed types.ZarfComponent, err error) { } if err := ic.fetchOCISkeleton(); err != nil { - return composed, err + return nil, err } // start with an empty component to compose into - composed = types.ZarfComponent{} + composed = &types.ZarfComponent{} // start overriding with the tail node node := ic.tail @@ -237,12 +237,16 @@ func (ic *ImportChain) Compose() (composed types.ZarfComponent, err error) { fixPaths(&node.ZarfComponent, node.relativeToHead) // perform overrides here - overrideMetadata(&composed, node.ZarfComponent) - overrideDeprecated(&composed, node.ZarfComponent) - overrideResources(&composed, node.ZarfComponent) - overrideActions(&composed, node.ZarfComponent) + err := overrideMetadata(composed, node.ZarfComponent) + if err != nil { + return nil, err + } + + overrideDeprecated(composed, node.ZarfComponent) + overrideResources(composed, node.ZarfComponent) + overrideActions(composed, node.ZarfComponent) - composeExtensions(&composed, node.ZarfComponent, node.relativeToHead) + composeExtensions(composed, node.ZarfComponent, node.relativeToHead) node = node.prev } diff --git a/src/pkg/packager/composer/list_test.go b/src/pkg/packager/composer/list_test.go index bca4cd2fd5..f255803fde 100644 --- a/src/pkg/packager/composer/list_test.go +++ b/src/pkg/packager/composer/list_test.go @@ -253,7 +253,7 @@ func TestCompose(t *testing.T) { if testCase.returnError { require.Contains(t, err.Error(), testCase.expectedErrorMessage) } else { - require.EqualValues(t, testCase.expectedComposed, composed) + require.EqualValues(t, &testCase.expectedComposed, composed) } }) } diff --git a/src/pkg/packager/composer/override.go b/src/pkg/packager/composer/override.go index b45426dc96..8d44dce935 100644 --- a/src/pkg/packager/composer/override.go +++ b/src/pkg/packager/composer/override.go @@ -5,10 +5,12 @@ package composer import ( + "fmt" + "github.com/defenseunicorns/zarf/src/types" ) -func overrideMetadata(c *types.ZarfComponent, override types.ZarfComponent) { +func overrideMetadata(c *types.ZarfComponent, override types.ZarfComponent) error { c.Name = override.Name c.Default = override.Default c.Required = override.Required @@ -17,6 +19,16 @@ func overrideMetadata(c *types.ZarfComponent, override types.ZarfComponent) { if override.Description != "" { c.Description = override.Description } + + if override.Only.LocalOS != "" { + if c.Only.LocalOS != "" { + return fmt.Errorf("component %q \"only.localOS\" %q cannot be redefined as %q during compose", c.Name, c.Only.LocalOS, override.Only.LocalOS) + } + + c.Only.LocalOS = override.Only.LocalOS + } + + return nil } func overrideDeprecated(c *types.ZarfComponent, override types.ZarfComponent) { diff --git a/src/test/e2e/09_component_compose_test.go b/src/test/e2e/09_component_compose_test.go index 4dc73c340d..b996e6ea48 100644 --- a/src/test/e2e/09_component_compose_test.go +++ b/src/test/e2e/09_component_compose_test.go @@ -20,11 +20,12 @@ type CompositionSuite struct { } var ( - composeExample = filepath.Join("examples", "composable-packages") - composeExamplePath string - composeTest = filepath.Join("src", "test", "packages", "09-composable-packages") - composeTestPath string - relCacheDir string + composeExample = filepath.Join("examples", "composable-packages") + composeExamplePath string + composeTest = filepath.Join("src", "test", "packages", "09-composable-packages") + composeTestPath string + composeTestBadLocalOS = filepath.Join("src", "test", "packages", "09-composable-packages", "bad-local-os") + relCacheDir string ) func (suite *CompositionSuite) SetupSuite() { @@ -87,6 +88,8 @@ func (suite *CompositionSuite) Test_1_FullComposability() { - name: test-compose-package description: A contrived example for podinfo using many Zarf primitives for compose testing required: true + only: + localOS: linux `) // Check files @@ -184,6 +187,14 @@ func (suite *CompositionSuite) Test_1_FullComposability() { condition: available`) } +func (suite *CompositionSuite) Test_2_ComposabilityBadLocalOS() { + suite.T().Log("E2E: Package Compose Example") + + _, stdErr, err := e2e.Zarf("package", "create", composeTestBadLocalOS, "-o", "build", "--zarf-cache", relCacheDir, "--no-color", "--confirm") + suite.Error(err) + suite.Contains(stdErr, "\"only.localOS\" \"linux\" cannot be\n redefined as \"windows\" during compose") +} + func TestCompositionSuite(t *testing.T) { suite.Run(t, new(CompositionSuite)) } diff --git a/src/test/packages/09-composable-packages/bad-local-os/zarf.yaml b/src/test/packages/09-composable-packages/bad-local-os/zarf.yaml new file mode 100644 index 0000000000..d0772be32e --- /dev/null +++ b/src/test/packages/09-composable-packages/bad-local-os/zarf.yaml @@ -0,0 +1,14 @@ +kind: ZarfPackageConfig +metadata: + name: test-compose-package + description: A contrived example for localOS compose testing + +components: + - name: test-compose-package + description: A contrived example for localOS compose testing + only: + localOS: windows + required: true + import: + path: ../sub-package + name: test-compose-sub-package diff --git a/src/test/packages/09-composable-packages/sub-package/zarf.yaml b/src/test/packages/09-composable-packages/sub-package/zarf.yaml index b4ebd4511c..20b0a5b323 100644 --- a/src/test/packages/09-composable-packages/sub-package/zarf.yaml +++ b/src/test/packages/09-composable-packages/sub-package/zarf.yaml @@ -6,6 +6,8 @@ metadata: components: - name: test-compose-sub-package + only: + localOS: linux charts: - name: podinfo-compose releaseName: podinfo-compose