From c37817a0a759817719244161cce52206f7242b78 Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Wed, 14 Feb 2024 11:12:04 -0700 Subject: [PATCH 1/7] feat: import all vars exported from package --- src/pkg/bundle/common.go | 18 +++++++++++++++--- src/pkg/bundle/deploy.go | 5 +++++ .../import-all-bad-name/uds-bundle.yaml | 19 +++++++++++++++++++ .../02-simple-vars/import-all/uds-bundle.yaml | 19 +++++++++++++++++++ src/test/e2e/commands_test.go | 6 ++++++ src/test/e2e/variable_test.go | 18 ++++++++++++++++++ src/types/bundle.go | 2 +- uds.schema.json | 1 - 8 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 src/test/bundles/02-simple-vars/import-all-bad-name/uds-bundle.yaml create mode 100644 src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml diff --git a/src/pkg/bundle/common.go b/src/pkg/bundle/common.go index f4648779..4ee7d72b 100644 --- a/src/pkg/bundle/common.go +++ b/src/pkg/bundle/common.go @@ -327,10 +327,22 @@ func validateBundleVars(packages []types.Package) error { // ensure imports have a matching export if pkg.Imports != nil { for _, v := range pkg.Imports { - if _, ok := exports[v.Name]; ok && v.Package == exports[v.Name] { - continue + // check import names if they are set, otherwise importing all the variables for the given package + if v.Name != "" { + if _, ok := exports[v.Name]; ok && v.Package == exports[v.Name] { + continue + } + return fmt.Errorf("import var %s does not have a matching export", v.Name) + // Check that the import package is valid + } else { + for _, pkgName := range exports { + if pkgName == v.Package { + break + } else { + return fmt.Errorf("import package %s does not match any exporting package", v.Package) + } + } } - return fmt.Errorf("import var %s does not have a matching export", v.Name) } } } diff --git a/src/pkg/bundle/deploy.go b/src/pkg/bundle/deploy.go index 45c8cb5d..b4a05ee6 100644 --- a/src/pkg/bundle/deploy.go +++ b/src/pkg/bundle/deploy.go @@ -215,6 +215,11 @@ func (b *Bundle) loadVariables(pkg types.Package, bundleExportedVars map[string] // Set variables in order or precendence (least specific to most specific) // imported vars for _, imp := range pkg.Imports { + if imp.Name == "" { + for key, value := range bundleExportedVars[imp.Package] { + pkgVars[strings.ToUpper(key)] = value + } + } pkgVars[strings.ToUpper(imp.Name)] = bundleExportedVars[imp.Package][imp.Name] } // shared vars diff --git a/src/test/bundles/02-simple-vars/import-all-bad-name/uds-bundle.yaml b/src/test/bundles/02-simple-vars/import-all-bad-name/uds-bundle.yaml new file mode 100644 index 00000000..a1db362e --- /dev/null +++ b/src/test/bundles/02-simple-vars/import-all-bad-name/uds-bundle.yaml @@ -0,0 +1,19 @@ +kind: UDSBundle +metadata: + name: import-all-bad-name + description: show how import all works + version: 0.0.1 + +packages: + - name: output-var + repository: localhost:888/output-var + ref: 0.0.1 + exports: + - name: OUTPUT + - name: PRECEDENCE + + - name: receive-var + repository: localhost:888/receive-var + ref: 0.0.1 + imports: + - package: output-varz diff --git a/src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml b/src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml new file mode 100644 index 00000000..c622bf20 --- /dev/null +++ b/src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml @@ -0,0 +1,19 @@ +kind: UDSBundle +metadata: + name: import-all + description: show how import all works + version: 0.0.1 + +packages: + - name: output-var + repository: localhost:888/output-var + ref: 0.0.1 + exports: + - name: OUTPUT + - name: PRECEDENCE + + - name: receive-var + repository: localhost:888/receive-var + ref: 0.0.1 + imports: + - package: output-var diff --git a/src/test/e2e/commands_test.go b/src/test/e2e/commands_test.go index f52c5c06..1cf9e501 100644 --- a/src/test/e2e/commands_test.go +++ b/src/test/e2e/commands_test.go @@ -30,6 +30,12 @@ func createLocal(t *testing.T, bundlePath string, arch string) { require.NoError(t, err) } +func createLocalError(t *testing.T, bundlePath string, arch string) (stderr string) { + cmd := strings.Split(fmt.Sprintf("create %s --insecure --confirm -a %s", bundlePath, arch), " ") + _, stderr, _ = e2e.UDS(cmd...) + return stderr +} + func createRemoteInsecure(t *testing.T, bundlePath, registry, arch string) { cmd := strings.Split(fmt.Sprintf("create %s -o %s --confirm --insecure -a %s", bundlePath, registry, arch), " ") _, _, err := e2e.UDS(cmd...) diff --git a/src/test/e2e/variable_test.go b/src/test/e2e/variable_test.go index 95bbdffa..a7ec5ae1 100644 --- a/src/test/e2e/variable_test.go +++ b/src/test/e2e/variable_test.go @@ -39,7 +39,25 @@ func TestBundleVariables(t *testing.T) { os.Setenv("UDS_CONFIG", filepath.Join("src/test/bundles/02-simple-vars", "uds-config.yaml")) _, stderr := deploy(t, bundleTarballPath) + bunleVariablesTestChecks(t, stderr, bundleTarballPath) + remove(t, bundleTarballPath) + // Run same test checks but with package that is importing all of the exported variables by just providing the packageName in the import + bundleDir = "src/test/bundles/02-simple-vars/import-all" + bundleTarballPath = filepath.Join(bundleDir, fmt.Sprintf("uds-bundle-import-all-%s-0.0.1.tar.zst", e2e.Arch)) + createLocal(t, bundleDir, e2e.Arch) + createRemoteInsecure(t, bundleDir, "localhost:888", e2e.Arch) + _, stderr = deploy(t, bundleTarballPath) + bunleVariablesTestChecks(t, stderr, bundleTarballPath) + + // Test with bad package name in import + bundleDir = "src/test/bundles/02-simple-vars/import-all-bad-name" + // bundleTarballPath = filepath.Join(bundleDir, fmt.Sprintf("uds-bundle-import-all-bad-name-%s-0.0.1.tar.zst", e2e.Arch)) + stderr = createLocalError(t, bundleDir, e2e.Arch) + require.Contains(t, stderr, "does not match any exporting package") +} + +func bunleVariablesTestChecks(t *testing.T, stderr string, bundleTarballPath string) { require.NotContains(t, stderr, "CLIVersion is set to 'unset' which can cause issues with package creation and deployment") require.Contains(t, stderr, "This fun-fact was imported: Unicorns are the national animal of Scotland") require.Contains(t, stderr, "This fun-fact demonstrates precedence: The Red Dragon is the national symbol of Wales") diff --git a/src/types/bundle.go b/src/types/bundle.go index f9413f64..efdff9d1 100644 --- a/src/types/bundle.go +++ b/src/types/bundle.go @@ -56,7 +56,7 @@ type BundleChartVariable struct { // BundleVariableImport represents variables in the bundle type BundleVariableImport struct { - Name string `json:"name" jsonschema:"name=Name of the variable"` + Name string `json:"name,omitempty" jsonschema:"name=Name of the variable"` Package string `json:"package" jsonschema:"name=Name of the Zarf package to get the variable from"` Description string `json:"description,omitempty" jsonschema:"name=Description of the variable"` } diff --git a/uds.schema.json b/uds.schema.json index a1628ea6..a7cb8451 100644 --- a/uds.schema.json +++ b/uds.schema.json @@ -79,7 +79,6 @@ }, "BundleVariableImport": { "required": [ - "name", "package" ], "properties": { From 0e72dbbe6aa353ec306c4367bff09c0bba6b7f30 Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Wed, 14 Feb 2024 11:16:44 -0700 Subject: [PATCH 2/7] chore: update docs --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 6985ea85..cff3ceaa 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,7 @@ packages: package: output-var ``` -Variables that you want to make available to other package are in the `export` block of the Zarf package to export a variable from. To have another package ingest an exported variable, use the `imports` key to name both the `variable` and `package` that the variable is exported from. +Variables that you want to make available to other package are in the `export` block of the Zarf package to export a variable from. To have another package ingest an exported variable, use the `imports` key to name both the `variable` and `package` that the variable is exported from. You can also import all of the variables for a package by omitting the variable name and just providing the package name. In the example above, the `OUTPUT` variable is created as part of a Zarf Action in the [output-var](src/test/packages/zarf/no-cluster/output-var) package, and the [receive-var](src/test/packages/zarf/no-cluster/receive-var) package expects a variable called `OUTPUT`. From 1b2890a725892fdb924f9cb638db5829aa9e28ce Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Tue, 20 Feb 2024 16:37:43 -0700 Subject: [PATCH 3/7] feat: exported variables available by default --- README.md | 2 +- src/pkg/bundle/common.go | 18 ++---------- src/pkg/bundle/deploy.go | 12 ++++---- .../import-all-bad-name/uds-bundle.yaml | 3 +- .../02-simple-vars/import-all/uds-bundle.yaml | 2 -- src/test/e2e/variable_test.go | 28 ++++++++++++++----- src/types/bundle.go | 2 +- uds.schema.json | 1 + 8 files changed, 36 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index cff3ceaa..b9aede7f 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,7 @@ packages: package: output-var ``` -Variables that you want to make available to other package are in the `export` block of the Zarf package to export a variable from. To have another package ingest an exported variable, use the `imports` key to name both the `variable` and `package` that the variable is exported from. You can also import all of the variables for a package by omitting the variable name and just providing the package name. +Variables that you want to make available to other package are in the `export` block of the Zarf package to export a variable from. By default, all exported variables are available to all of the packages in a bundle. To have another package ingest a specific exported variable, use the `imports` key to name both the `variable` and `package` that the variable is exported from, like in the example above. In the example above, the `OUTPUT` variable is created as part of a Zarf Action in the [output-var](src/test/packages/zarf/no-cluster/output-var) package, and the [receive-var](src/test/packages/zarf/no-cluster/receive-var) package expects a variable called `OUTPUT`. diff --git a/src/pkg/bundle/common.go b/src/pkg/bundle/common.go index 4ee7d72b..f4648779 100644 --- a/src/pkg/bundle/common.go +++ b/src/pkg/bundle/common.go @@ -327,22 +327,10 @@ func validateBundleVars(packages []types.Package) error { // ensure imports have a matching export if pkg.Imports != nil { for _, v := range pkg.Imports { - // check import names if they are set, otherwise importing all the variables for the given package - if v.Name != "" { - if _, ok := exports[v.Name]; ok && v.Package == exports[v.Name] { - continue - } - return fmt.Errorf("import var %s does not have a matching export", v.Name) - // Check that the import package is valid - } else { - for _, pkgName := range exports { - if pkgName == v.Package { - break - } else { - return fmt.Errorf("import package %s does not match any exporting package", v.Package) - } - } + if _, ok := exports[v.Name]; ok && v.Package == exports[v.Name] { + continue } + return fmt.Errorf("import var %s does not have a matching export", v.Name) } } } diff --git a/src/pkg/bundle/deploy.go b/src/pkg/bundle/deploy.go index b4a05ee6..333688db 100644 --- a/src/pkg/bundle/deploy.go +++ b/src/pkg/bundle/deploy.go @@ -212,14 +212,16 @@ func deployPackages(packages []types.Package, resume bool, b *Bundle, zarfPackag func (b *Bundle) loadVariables(pkg types.Package, bundleExportedVars map[string]map[string]string) map[string]string { pkgVars := make(map[string]string) + // load all exported variables + for _, exportedVarMap := range bundleExportedVars { + for varName, varValue := range exportedVarMap { + pkgVars[strings.ToUpper(varName)] = varValue + } + } + // Set variables in order or precendence (least specific to most specific) // imported vars for _, imp := range pkg.Imports { - if imp.Name == "" { - for key, value := range bundleExportedVars[imp.Package] { - pkgVars[strings.ToUpper(key)] = value - } - } pkgVars[strings.ToUpper(imp.Name)] = bundleExportedVars[imp.Package][imp.Name] } // shared vars diff --git a/src/test/bundles/02-simple-vars/import-all-bad-name/uds-bundle.yaml b/src/test/bundles/02-simple-vars/import-all-bad-name/uds-bundle.yaml index a1db362e..34671511 100644 --- a/src/test/bundles/02-simple-vars/import-all-bad-name/uds-bundle.yaml +++ b/src/test/bundles/02-simple-vars/import-all-bad-name/uds-bundle.yaml @@ -1,7 +1,7 @@ kind: UDSBundle metadata: name: import-all-bad-name - description: show how import all works + description: show errors for bad imports version: 0.0.1 packages: @@ -17,3 +17,4 @@ packages: ref: 0.0.1 imports: - package: output-varz + name: OUTPUTZ diff --git a/src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml b/src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml index c622bf20..088522f7 100644 --- a/src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml +++ b/src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml @@ -15,5 +15,3 @@ packages: - name: receive-var repository: localhost:888/receive-var ref: 0.0.1 - imports: - - package: output-var diff --git a/src/test/e2e/variable_test.go b/src/test/e2e/variable_test.go index a7ec5ae1..1f9e56d4 100644 --- a/src/test/e2e/variable_test.go +++ b/src/test/e2e/variable_test.go @@ -39,25 +39,39 @@ func TestBundleVariables(t *testing.T) { os.Setenv("UDS_CONFIG", filepath.Join("src/test/bundles/02-simple-vars", "uds-config.yaml")) _, stderr := deploy(t, bundleTarballPath) - bunleVariablesTestChecks(t, stderr, bundleTarballPath) + bundleVariablesTestChecks(t, stderr, bundleTarballPath) remove(t, bundleTarballPath) - // Run same test checks but with package that is importing all of the exported variables by just providing the packageName in the import + // Run same test checks but with package that isn't explicitly importing vars bundleDir = "src/test/bundles/02-simple-vars/import-all" bundleTarballPath = filepath.Join(bundleDir, fmt.Sprintf("uds-bundle-import-all-%s-0.0.1.tar.zst", e2e.Arch)) createLocal(t, bundleDir, e2e.Arch) createRemoteInsecure(t, bundleDir, "localhost:888", e2e.Arch) _, stderr = deploy(t, bundleTarballPath) - bunleVariablesTestChecks(t, stderr, bundleTarballPath) + bundleVariablesTestChecks(t, stderr, bundleTarballPath) - // Test with bad package name in import + // Test with bad variable name in import bundleDir = "src/test/bundles/02-simple-vars/import-all-bad-name" - // bundleTarballPath = filepath.Join(bundleDir, fmt.Sprintf("uds-bundle-import-all-bad-name-%s-0.0.1.tar.zst", e2e.Arch)) stderr = createLocalError(t, bundleDir, e2e.Arch) - require.Contains(t, stderr, "does not match any exporting package") + require.Contains(t, stderr, "does not have a matching export") + + // Test name collisions with exported variables + zarfPkgPath3 := "src/test/packages/no-cluster/output-var-collision" + e2e.CreateZarfPkg(t, zarfPkgPath3, false) + + pkg = filepath.Join(zarfPkgPath3, fmt.Sprintf("zarf-package-output-var-collision-%s-0.0.1.tar.zst", e2e.Arch)) + zarfPublish(t, pkg, "localhost:888") + + bundleDir = "src/test/bundles/02-simple-vars/export-name-collision" + bundleTarballPath = filepath.Join(bundleDir, fmt.Sprintf("uds-bundle-export-name-collision-%s-0.0.1.tar.zst", e2e.Arch)) + createLocal(t, bundleDir, e2e.Arch) + createRemoteInsecure(t, bundleDir, "localhost:888", e2e.Arch) + _, stderr = deploy(t, bundleTarballPath) + require.Contains(t, stderr, "This fun-fact was imported: Daffodils are the national flower of Wales") + require.NotContains(t, stderr, "This fun-fact was imported: Unicorns are the national animal of Scotland") } -func bunleVariablesTestChecks(t *testing.T, stderr string, bundleTarballPath string) { +func bundleVariablesTestChecks(t *testing.T, stderr string, bundleTarballPath string) { require.NotContains(t, stderr, "CLIVersion is set to 'unset' which can cause issues with package creation and deployment") require.Contains(t, stderr, "This fun-fact was imported: Unicorns are the national animal of Scotland") require.Contains(t, stderr, "This fun-fact demonstrates precedence: The Red Dragon is the national symbol of Wales") diff --git a/src/types/bundle.go b/src/types/bundle.go index efdff9d1..f9413f64 100644 --- a/src/types/bundle.go +++ b/src/types/bundle.go @@ -56,7 +56,7 @@ type BundleChartVariable struct { // BundleVariableImport represents variables in the bundle type BundleVariableImport struct { - Name string `json:"name,omitempty" jsonschema:"name=Name of the variable"` + Name string `json:"name" jsonschema:"name=Name of the variable"` Package string `json:"package" jsonschema:"name=Name of the Zarf package to get the variable from"` Description string `json:"description,omitempty" jsonschema:"name=Description of the variable"` } diff --git a/uds.schema.json b/uds.schema.json index a7cb8451..a1628ea6 100644 --- a/uds.schema.json +++ b/uds.schema.json @@ -79,6 +79,7 @@ }, "BundleVariableImport": { "required": [ + "name", "package" ], "properties": { From 53c4e411f9941c5a04e1a04fea2827e388b37ad0 Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Tue, 20 Feb 2024 16:50:37 -0700 Subject: [PATCH 4/7] oops forgot to add new test package --- .../export-name-collision/uds-bundle.yaml | 26 +++++++++++++++++++ .../no-cluster/output-var-collision/zarf.yaml | 22 ++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 src/test/bundles/02-simple-vars/export-name-collision/uds-bundle.yaml create mode 100644 src/test/packages/no-cluster/output-var-collision/zarf.yaml diff --git a/src/test/bundles/02-simple-vars/export-name-collision/uds-bundle.yaml b/src/test/bundles/02-simple-vars/export-name-collision/uds-bundle.yaml new file mode 100644 index 00000000..ad15a689 --- /dev/null +++ b/src/test/bundles/02-simple-vars/export-name-collision/uds-bundle.yaml @@ -0,0 +1,26 @@ +kind: UDSBundle +metadata: + name: export-name-collision + description: show how to specify import vars in case of name collisions + version: 0.0.1 + +packages: + - name: output-var + repository: localhost:888/output-var + ref: 0.0.1 + exports: + - name: OUTPUT + - name: PRECEDENCE + + - name: output-var-collision + repository: localhost:888/output-var-collision + ref: 0.0.1 + exports: + - name: OUTPUT + + - name: receive-var + repository: localhost:888/receive-var + ref: 0.0.1 + imports: + - package: output-var-collision + name: OUTPUT diff --git a/src/test/packages/no-cluster/output-var-collision/zarf.yaml b/src/test/packages/no-cluster/output-var-collision/zarf.yaml new file mode 100644 index 00000000..db6b6f1c --- /dev/null +++ b/src/test/packages/no-cluster/output-var-collision/zarf.yaml @@ -0,0 +1,22 @@ +kind: ZarfPackageConfig +metadata: + name: output-var-collision + description: | + Exports variable with same name as variable exported from output-var package + version: 0.0.1 + +variables: + - name: COUNTRY + default: Wales + - name: FLOWER + default: Daffodils + +components: + - name: echo + required: true + actions: + onDeploy: + after: + - cmd: echo ""${ZARF_VAR_FLOWER}" are the national flower of "${ZARF_VAR_COUNTRY}"" + setVariables: + - name: OUTPUT From 908ce7fa0e28d29acc18c932ae413e0fd96fd44b Mon Sep 17 00:00:00 2001 From: decleaver <85503726+decleaver@users.noreply.github.com> Date: Wed, 21 Feb 2024 11:59:24 -0700 Subject: [PATCH 5/7] Update README.md Co-authored-by: UncleGedd <42304551+UncleGedd@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b9aede7f..7543b2b7 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,7 @@ packages: package: output-var ``` -Variables that you want to make available to other package are in the `export` block of the Zarf package to export a variable from. By default, all exported variables are available to all of the packages in a bundle. To have another package ingest a specific exported variable, use the `imports` key to name both the `variable` and `package` that the variable is exported from, like in the example above. +Variables that you want to make available to other packages are in the `export` block of the Zarf package to export a variable from. By default, all exported variables are available to all of the packages in a bundle. To have another package ingest a specific exported variable, like in the case of variable name collisions, use the `imports` key to name both the `variable` and `package` that the variable is exported from, like in the example above. In the example above, the `OUTPUT` variable is created as part of a Zarf Action in the [output-var](src/test/packages/zarf/no-cluster/output-var) package, and the [receive-var](src/test/packages/zarf/no-cluster/receive-var) package expects a variable called `OUTPUT`. From 55dd7f81f3fa77ba83789ce64615ddcab933eb28 Mon Sep 17 00:00:00 2001 From: decleaver <85503726+decleaver@users.noreply.github.com> Date: Wed, 21 Feb 2024 11:59:44 -0700 Subject: [PATCH 6/7] Update src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml Co-authored-by: UncleGedd <42304551+UncleGedd@users.noreply.github.com> --- src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml b/src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml index 088522f7..7c2e28fa 100644 --- a/src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml +++ b/src/test/bundles/02-simple-vars/import-all/uds-bundle.yaml @@ -1,7 +1,7 @@ kind: UDSBundle metadata: name: import-all - description: show how import all works + description: show how global exports work version: 0.0.1 packages: From ab3b8aa74fbe38dddaef193bed4cb302c936ea89 Mon Sep 17 00:00:00 2001 From: Darcy Cleaver Date: Wed, 21 Feb 2024 13:57:10 -0700 Subject: [PATCH 7/7] cleanup tests per pr comment --- src/test/e2e/variable_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/e2e/variable_test.go b/src/test/e2e/variable_test.go index 1f9e56d4..a8edf1ef 100644 --- a/src/test/e2e/variable_test.go +++ b/src/test/e2e/variable_test.go @@ -46,7 +46,6 @@ func TestBundleVariables(t *testing.T) { bundleDir = "src/test/bundles/02-simple-vars/import-all" bundleTarballPath = filepath.Join(bundleDir, fmt.Sprintf("uds-bundle-import-all-%s-0.0.1.tar.zst", e2e.Arch)) createLocal(t, bundleDir, e2e.Arch) - createRemoteInsecure(t, bundleDir, "localhost:888", e2e.Arch) _, stderr = deploy(t, bundleTarballPath) bundleVariablesTestChecks(t, stderr, bundleTarballPath)