From 6a17d6651b3ff5efa711c33036b4a61f6ac8de5d Mon Sep 17 00:00:00 2001 From: bdw617 Date: Wed, 18 Oct 2023 14:01:42 -0400 Subject: [PATCH 01/30] feat: moving cli config to --set for nodeport and storageclass --- .github/workflows/test-upgrade.yml | 2 +- .../100-cli-commands/zarf_init.md | 2 - packages/gitea/gitea-values.yaml | 2 +- packages/zarf-registry/registry-values.yaml | 4 +- src/cmd/initialize.go | 43 +++++++ src/cmd/initialize_test.go | 118 ++++++++++++++++++ src/config/lang/english.go | 14 ++- src/test/e2e/20_zarf_init_test.go | 2 +- 8 files changed, 178 insertions(+), 9 deletions(-) create mode 100644 src/cmd/initialize_test.go diff --git a/.github/workflows/test-upgrade.yml b/.github/workflows/test-upgrade.yml index b51ff401f6..524a7adaf6 100644 --- a/.github/workflows/test-upgrade.yml +++ b/.github/workflows/test-upgrade.yml @@ -74,7 +74,7 @@ jobs: # NOTE: "PATH=$PATH" preserves the default user $PATH. This is needed to maintain the version of zarf installed # in a previous step. This test run will the current release to create a K3s cluster. run: | - sudo env "PATH=$PATH" CI=true zarf init --components k3s,git-server,logging --nodeport 31337 --confirm + sudo env "PATH=$PATH" CI=true zarf init --components k3s,git-server,logging --set REGISTRY_NODEPORT=31337 --confirm # Before we run the regular tests we need to aggressively cleanup files to reduce disk pressure - name: Cleanup files diff --git a/docs/2-the-zarf-cli/100-cli-commands/zarf_init.md b/docs/2-the-zarf-cli/100-cli-commands/zarf_init.md index 3aabfdac50..f3b6776c76 100644 --- a/docs/2-the-zarf-cli/100-cli-commands/zarf_init.md +++ b/docs/2-the-zarf-cli/100-cli-commands/zarf_init.md @@ -61,7 +61,6 @@ zarf init [flags] --git-push-username string Username to access to the git server Zarf is configured to use. User must be able to create repositories via 'git push' (default "zarf-git-user") --git-url string External git server url to use for this Zarf cluster -h, --help help for init - --nodeport int Nodeport to access a registry internal to the k8s cluster. Between [30000-32767] --registry-pull-password string Password for the pull-only user to access the registry --registry-pull-username string Username for pull-only access to the registry --registry-push-password string Password for the push-user to connect to the registry @@ -69,7 +68,6 @@ zarf init [flags] --registry-secret string Registry secret value --registry-url string External registry url address to use for this Zarf cluster --set stringToString Specify deployment variables to set on the command line (KEY=value) (default []) - --storage-class string Specify the storage class to use for the registry and git server. E.g. --storage-class=standard ``` ## Options inherited from parent commands diff --git a/packages/gitea/gitea-values.yaml b/packages/gitea/gitea-values.yaml index 3ac892ba27..623b806e1d 100644 --- a/packages/gitea/gitea-values.yaml +++ b/packages/gitea/gitea-values.yaml @@ -1,5 +1,5 @@ persistence: - storageClass: "###ZARF_STORAGE_CLASS###" + storageClass: "###ZARF_VAR_REGISTRY_STORAGE_CLASS###" existingClaim: "###ZARF_VAR_GIT_SERVER_EXISTING_PVC###" size: "###ZARF_VAR_GIT_SERVER_PVC_SIZE###" accessModes: diff --git a/packages/zarf-registry/registry-values.yaml b/packages/zarf-registry/registry-values.yaml index 7f43da9c67..39a1c540a9 100644 --- a/packages/zarf-registry/registry-values.yaml +++ b/packages/zarf-registry/registry-values.yaml @@ -1,6 +1,6 @@ persistence: enabled: ###ZARF_VAR_REGISTRY_PVC_ENABLED### - storageClass: "###ZARF_STORAGE_CLASS###" + storageClass: "###ZARF_VAR_REGISTRY_STORAGE_CLASS###" size: "###ZARF_VAR_REGISTRY_PVC_SIZE###" existingClaim: "###ZARF_VAR_REGISTRY_EXISTING_PVC###" accessMode: "###ZARF_VAR_REGISTRY_PVC_ACCESS_MODE###" @@ -19,7 +19,7 @@ secrets: secret: "###ZARF_REGISTRY_SECRET###" service: - nodePort: "###ZARF_NODEPORT###" + nodePort: "###ZARF_VAR_REGISTRY_NODEPORT###" resources: requests: diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index 193ba0f0d4..d61463f737 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -10,6 +10,7 @@ import ( "os" "path" "path/filepath" + "strconv" "strings" "github.com/AlecAivazis/survey/v2" @@ -61,6 +62,10 @@ var initCmd = &cobra.Command{ pkgConfig.PkgOpts.SetVariables = helpers.TransformAndMergeMap( v.GetStringMapString(common.VPkgDeploySet), pkgConfig.PkgOpts.SetVariables, strings.ToUpper) + // DEPRECATED_V1.0.0: these functions will need cleanup + setRegistryStorageClass() + setRegistryNodePort() + // Configure the packager pkgClient := packager.NewOrDie(&pkgConfig, packager.WithSource(src)) defer pkgClient.ClearTempPaths() @@ -73,6 +78,42 @@ var initCmd = &cobra.Command{ }, } +// DEPRECATED_V1.0.0: do not check pkgConfig.InitOpts.RegistryInfo.NodePort, always overwrite it. +func setRegistryNodePort() { + if nodePortStr, ok := pkgConfig.PkgOpts.SetVariables["REGISTRY_NODEPORT"]; ok { + if pkgConfig.InitOpts.RegistryInfo.NodePort != 0 { + message.Warn(lang.WarnBothNodePortSchemesDeprecated) + } + nodePort, err := strconv.Atoi(nodePortStr) + if err != nil || nodePort < 30000 || nodePort > 32767 { + message.Fatal(err, lang.FatalNodeportInvalid) + } + pkgConfig.InitOpts.RegistryInfo.NodePort = nodePort + } else { + // if the old way is set, use it, or the default + if pkgConfig.InitOpts.RegistryInfo.NodePort > 0 { + message.Warn(lang.WarnNodePortDeprecated) + } else { + pkgConfig.InitOpts.RegistryInfo.NodePort = config.ZarfInClusterContainerRegistryNodePort + } + pkgConfig.PkgOpts.SetVariables["REGISTRY_NODEPORT"] = strconv.Itoa(pkgConfig.InitOpts.RegistryInfo.NodePort) + } +} + +// DEPRECATED_V1.0.0: do not check pkgConfig.InitOpts.StorageClass, always overwrite it. +func setRegistryStorageClass() { + if storageClass, ok := pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]; ok { + if pkgConfig.InitOpts.StorageClass != "" { + message.Warn(lang.WarnBothStorageClassesDeprecated) + } + pkgConfig.InitOpts.StorageClass = storageClass + } else { + // if the old way was set (or if the old way is empty) + pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] = pkgConfig.InitOpts.StorageClass + message.Warn(lang.WarnStorageClassDeprecated) + } +} + func findInitPackage(initPackageName string) (string, error) { // First, look for the init package in the current working directory if !utils.InvalidPath(initPackageName) { @@ -188,6 +229,7 @@ func init() { // Continue to require --confirm flag for init command to avoid accidental deployments initCmd.Flags().BoolVar(&config.CommonOptions.Confirm, "confirm", false, lang.CmdInitFlagConfirm) initCmd.Flags().StringVar(&pkgConfig.PkgOpts.OptionalComponents, "components", v.GetString(common.VInitComponents), lang.CmdInitFlagComponents) + // [Deprecated] --storage-class is deprecated in favor of --set REGISTRY_STORAGE_CLASS (to be removed in v1.0.0) initCmd.Flags().StringVar(&pkgConfig.InitOpts.StorageClass, "storage-class", v.GetString(common.VInitStorageClass), lang.CmdInitFlagStorageClass) // Flags for using an external Git server @@ -199,6 +241,7 @@ func init() { // Flags for using an external registry initCmd.Flags().StringVar(&pkgConfig.InitOpts.RegistryInfo.Address, "registry-url", v.GetString(common.VInitRegistryURL), lang.CmdInitFlagRegURL) + // [Deprecated] --nodeport is deprecated in favor of --set REGISTRY_NODEPORT initCmd.Flags().IntVar(&pkgConfig.InitOpts.RegistryInfo.NodePort, "nodeport", v.GetInt(common.VInitRegistryNodeport), lang.CmdInitFlagRegNodePort) initCmd.Flags().StringVar(&pkgConfig.InitOpts.RegistryInfo.PushUsername, "registry-push-username", v.GetString(common.VInitRegistryPushUser), lang.CmdInitFlagRegPushUser) initCmd.Flags().StringVar(&pkgConfig.InitOpts.RegistryInfo.PushPassword, "registry-push-password", v.GetString(common.VInitRegistryPushPass), lang.CmdInitFlagRegPushPass) diff --git a/src/cmd/initialize_test.go b/src/cmd/initialize_test.go new file mode 100644 index 0000000000..3f3865967d --- /dev/null +++ b/src/cmd/initialize_test.go @@ -0,0 +1,118 @@ +// DEPRECATED_V1.0.0: do not check pkgConfig.InitOpts.RegistryInfo.NodePort, always overwrite it. +package cmd + +import ( + "fmt" + "os" + "os/exec" + "syscall" + "testing" + + "github.com/defenseunicorns/zarf/src/config" +) + +func TestSetRegistryStorageClass(t *testing.T) { + // Test case 1: storageClass is set + pkgConfig.PkgOpts.SetVariables = map[string]string{"REGISTRY_STORAGE_CLASS": "test-storage-class"} + pkgConfig.InitOpts.StorageClass = "" + setRegistryStorageClass() + if pkgConfig.InitOpts.StorageClass != "test-storage-class" { + t.Errorf("Expected storage class to be set to 'test-storage-class', but got '%s'", pkgConfig.InitOpts.StorageClass) + } + + // Test case 2: storageClass is not set, use old way + pkgConfig.PkgOpts.SetVariables = map[string]string{} + pkgConfig.InitOpts.StorageClass = "old-storage-class" + setRegistryStorageClass() + if pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] != "old-storage-class" { + t.Errorf("Expected storage class to be set to old way value, but got '%s'", pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]) + } + + // Test case 3: neither is set, should be empty + pkgConfig.PkgOpts.SetVariables = map[string]string{} + pkgConfig.InitOpts.StorageClass = "" + setRegistryStorageClass() + if pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] != "" { + t.Errorf("Expected storage class to be set to empty string, but got '%s'", pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]) + } +} + +// convinence function to setup tests +func setupNodeportTests(r string, n int) { + pkgConfig.PkgOpts.SetVariables = map[string]string{} + if r != "" { + pkgConfig.PkgOpts.SetVariables["REGISTRY_NODEPORT"] = r + } + pkgConfig.InitOpts.RegistryInfo.NodePort = n +} + +func TestSetRegistryNodePort(t *testing.T) { + // Test case 1: new way is set, old way is not + setupNodeportTests("30001", 0) + setRegistryNodePort() + if pkgConfig.InitOpts.RegistryInfo.NodePort != 30001 { + t.Errorf("Expected node port to be set to 30001, but got %d", pkgConfig.InitOpts.RegistryInfo.NodePort) + } + + // Test case 2: nothing is set, use the default + setupNodeportTests("", 0) + setRegistryNodePort() + if pkgConfig.InitOpts.RegistryInfo.NodePort != config.ZarfInClusterContainerRegistryNodePort { + t.Errorf("Expected node port to be set to default value, but got %d", pkgConfig.InitOpts.RegistryInfo.NodePort) + } + + // Test case 3: The old way is set, and the new way is not. We should set the new variable to the old method + setupNodeportTests("", 30001) + setRegistryNodePort() + if c, ok := pkgConfig.PkgOpts.SetVariables["REGISTRY_NODEPORT"]; !ok { + t.Error("Expected node port to be set to old way value, but it is not set") + } else { + if c != "30001" { + t.Errorf("Expected node port to be set to old way value, but got %s", c) + } + } +} + +// forked test which will fatal because the old way is invalid +func TestSetRegistryNodePortHelper(t *testing.T) { + if os.Getenv("TEST_SET_REGISTRY_NODE_PORT_HELPER") != "1" { + return + } + setupNodeportTests("invalid", 0) + err := func() (err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("Expected test case to fail with fatal error, but it panicked: %v", r) + } + }() + setRegistryNodePort() + return nil + }() + if err == nil { + os.Exit(0) + } + os.Exit(1) + +} +func TestSetRegistryNodePortExit(t *testing.T) { + // Calls the helper + cmd := exec.Command(os.Args[0], "-test.run=TestSetRegistryNodePortHelper") + cmd.Env = append(os.Environ(), "TEST_SET_REGISTRY_NODE_PORT_HELPER=1") + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + // make sure we got an exitStatus of 1, otherwise the test failed. + if err := cmd.Wait(); err != nil { + if exitErr, ok := err.(*exec.ExitError); ok { + if status, ok := exitErr.Sys().(syscall.WaitStatus); ok { + if status.ExitStatus() != 1 { + t.Errorf("Expected test case to fail with exit status 1, but got exit status %d", status.ExitStatus()) + } + } else { + t.Errorf("Expected syscall.WaitStatus, but got %T", exitErr.Sys()) + } + } else { + t.Error(err) + } + } +} diff --git a/src/config/lang/english.go b/src/config/lang/english.go index 7d1bd85688..56cbaaa575 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -31,6 +31,16 @@ const ( ErrFileNameExtract = "failed to extract filename from URL %s: %s" ) +// Zarf CLI deprecation warnings +const ( + // remove deprecation in v1.0.0 + WarnBothNodePortSchemesDeprecated = "--set REGISTRY_NODEPORT= will override --nodeport " + WarnStorageClassDeprecated = "--set REGISTRY_STORAGE_CLASS= will override --storage-class " + WarnBothStorageClassesDeprecated = "--set REGISTRY_STORAGE_CLASS= will override --storage-class " + WarnNodePortDeprecated = "--nodeport is deprecated, please use --set REGISTRY_NODEPORT= instead" + FatalNodeportInvalid = "Nodeport must be in the range [30000-32767]" +) + // Zarf CLI commands. const ( // common command language @@ -158,7 +168,7 @@ const ( CmdInitFlagConfirm = "Confirms package deployment without prompting. ONLY use with packages you trust. Skips prompts to review SBOM, configure variables, select optional components and review potential breaking changes." CmdInitFlagComponents = "Specify which optional components to install. E.g. --components=git-server,logging" - CmdInitFlagStorageClass = "Specify the storage class to use for the registry and git server. E.g. --storage-class=standard" + CmdInitFlagStorageClass = "[DEPRECATED] This command has been replaced by 'zarf init --set REGISTRY_STORAGE_CLASS=' and will be removed in Zarf v1.0.0" CmdInitFlagGitURL = "External git server url to use for this Zarf cluster" CmdInitFlagGitPushUser = "Username to access to the git server Zarf is configured to use. User must be able to create repositories via 'git push'" @@ -167,7 +177,7 @@ const ( CmdInitFlagGitPullPass = "Password for the pull-only user to access the git server" CmdInitFlagRegURL = "External registry url address to use for this Zarf cluster" - CmdInitFlagRegNodePort = "Nodeport to access a registry internal to the k8s cluster. Between [30000-32767]" + CmdInitFlagRegNodePort = "[DEPRECATED] Nodeport to access a registry internal to the k8s cluster. Between [30000-32767]" CmdInitFlagRegPushUser = "Username to access to the registry Zarf is configured to use" CmdInitFlagRegPushPass = "Password for the push-user to connect to the registry" CmdInitFlagRegPullUser = "Username for pull-only access to the registry" diff --git a/src/test/e2e/20_zarf_init_test.go b/src/test/e2e/20_zarf_init_test.go index fb1f46e0b5..dca32f6dfc 100644 --- a/src/test/e2e/20_zarf_init_test.go +++ b/src/test/e2e/20_zarf_init_test.go @@ -69,7 +69,7 @@ func TestZarfInit(t *testing.T) { } // run `zarf init` - _, initStdErr, err := e2e.Zarf("init", "--components="+initComponents, "--nodeport", "31337", "-l", "trace", "--confirm") + _, initStdErr, err := e2e.Zarf("init", "--components="+initComponents, "--set", "REGISTRY_NODEPORT=31337", "-l", "trace", "--confirm") require.NoError(t, err) require.Contains(t, initStdErr, "an inventory of all software contained in this package") From af54703307707b554f8983a325eff21b33387ed6 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Wed, 18 Oct 2023 16:05:14 -0400 Subject: [PATCH 02/30] updated docs to be consistent with deprecated CLI flags, which will still be there, just not in the output --- docs/2-the-zarf-cli/100-cli-commands/zarf_init.md | 2 +- src/cmd/initialize.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/2-the-zarf-cli/100-cli-commands/zarf_init.md b/docs/2-the-zarf-cli/100-cli-commands/zarf_init.md index f3b6776c76..53ac76a216 100644 --- a/docs/2-the-zarf-cli/100-cli-commands/zarf_init.md +++ b/docs/2-the-zarf-cli/100-cli-commands/zarf_init.md @@ -32,7 +32,7 @@ zarf init [flags] zarf init --components=git-server,logging # Initializing w/ an internal registry but with a different nodeport: - zarf init --nodeport=30333 + zarf init --set REGISTRY_NODEPORT=30333 # Initializing w/ an external registry: zarf init --registry-push-password={PASSWORD} --registry-push-username={USERNAME} --registry-url={URL} diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index d61463f737..d11b2d0506 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -229,8 +229,10 @@ func init() { // Continue to require --confirm flag for init command to avoid accidental deployments initCmd.Flags().BoolVar(&config.CommonOptions.Confirm, "confirm", false, lang.CmdInitFlagConfirm) initCmd.Flags().StringVar(&pkgConfig.PkgOpts.OptionalComponents, "components", v.GetString(common.VInitComponents), lang.CmdInitFlagComponents) + // [Deprecated] --storage-class is deprecated in favor of --set REGISTRY_STORAGE_CLASS (to be removed in v1.0.0) initCmd.Flags().StringVar(&pkgConfig.InitOpts.StorageClass, "storage-class", v.GetString(common.VInitStorageClass), lang.CmdInitFlagStorageClass) + initCmd.Flags().MarkHidden("storage-class") // Flags for using an external Git server initCmd.Flags().StringVar(&pkgConfig.InitOpts.GitServer.Address, "git-url", v.GetString(common.VInitGitURL), lang.CmdInitFlagGitURL) @@ -241,8 +243,11 @@ func init() { // Flags for using an external registry initCmd.Flags().StringVar(&pkgConfig.InitOpts.RegistryInfo.Address, "registry-url", v.GetString(common.VInitRegistryURL), lang.CmdInitFlagRegURL) + // [Deprecated] --nodeport is deprecated in favor of --set REGISTRY_NODEPORT initCmd.Flags().IntVar(&pkgConfig.InitOpts.RegistryInfo.NodePort, "nodeport", v.GetInt(common.VInitRegistryNodeport), lang.CmdInitFlagRegNodePort) + initCmd.Flags().MarkHidden("nodeport") + initCmd.Flags().StringVar(&pkgConfig.InitOpts.RegistryInfo.PushUsername, "registry-push-username", v.GetString(common.VInitRegistryPushUser), lang.CmdInitFlagRegPushUser) initCmd.Flags().StringVar(&pkgConfig.InitOpts.RegistryInfo.PushPassword, "registry-push-password", v.GetString(common.VInitRegistryPushPass), lang.CmdInitFlagRegPushPass) initCmd.Flags().StringVar(&pkgConfig.InitOpts.RegistryInfo.PullUsername, "registry-pull-username", v.GetString(common.VInitRegistryPullUser), lang.CmdInitFlagRegPullUser) From b79bae9bfb67c6e4335bddfc06abb7e2fcc6bed7 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Wed, 18 Oct 2023 16:17:32 -0400 Subject: [PATCH 03/30] updated fix docs test --- src/config/lang/english.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/lang/english.go b/src/config/lang/english.go index 56cbaaa575..179881fd1c 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -138,7 +138,7 @@ const ( zarf init --components=git-server,logging # Initializing w/ an internal registry but with a different nodeport: - zarf init --nodeport=30333 + zarf init --set REGISTRY_NODEPORT=30333 # Initializing w/ an external registry: zarf init --registry-push-password={PASSWORD} --registry-push-username={USERNAME} --registry-url={URL} From 3c0e9d59fb2b04def6eee9a421e184573dc76d6e Mon Sep 17 00:00:00 2001 From: bdw617 Date: Thu, 19 Oct 2023 09:30:44 -0400 Subject: [PATCH 04/30] removed a test for the existence of an init flag that is deprecated and removed from the zarf init --help --- src/test/e2e/30_config_file_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/e2e/30_config_file_test.go b/src/test/e2e/30_config_file_test.go index a03844fa5a..6fba440566 100644 --- a/src/test/e2e/30_config_file_test.go +++ b/src/test/e2e/30_config_file_test.go @@ -107,7 +107,6 @@ func configFileDefaultTests(t *testing.T) { initFlags := []string{ "components: 359049b9", - "storage_class: 9cae917f", "git.pull_password: 8522ccca", "git.pull_username: 36646dbe", "git.push_password: ba00d92d", From 3005fb616970bf1975f592cde6fe7b3eba7cfb5a Mon Sep 17 00:00:00 2001 From: bdw617 Date: Thu, 19 Oct 2023 11:31:46 -0400 Subject: [PATCH 05/30] we moved nodeport to --set REGISTRY_NODEPORT, and therefore not in the main init options --- src/test/e2e/30_config_file_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/e2e/30_config_file_test.go b/src/test/e2e/30_config_file_test.go index 6fba440566..d92298b14d 100644 --- a/src/test/e2e/30_config_file_test.go +++ b/src/test/e2e/30_config_file_test.go @@ -112,7 +112,6 @@ func configFileDefaultTests(t *testing.T) { "git.push_password: ba00d92d", "git.push_username: eb76dca8", "git.url: 7c63c1b9", - "Between [30000-32767] (default 186282)", "registry.pull_password: b8152e38", "registry.pull_username: d0961a97", "registry.push_password: 8f58ca41", From d67f9e86bfcdff1975b7919b5e42e31d3897afcb Mon Sep 17 00:00:00 2001 From: bdw617 Date: Thu, 19 Oct 2023 17:12:58 -0400 Subject: [PATCH 06/30] CI testing, not sure if necessary --- src/cmd/initialize.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index d11b2d0506..429e071d65 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -109,8 +109,10 @@ func setRegistryStorageClass() { pkgConfig.InitOpts.StorageClass = storageClass } else { // if the old way was set (or if the old way is empty) - pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] = pkgConfig.InitOpts.StorageClass - message.Warn(lang.WarnStorageClassDeprecated) + if pkgConfig.InitOpts.StorageClass != "" { + pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] = pkgConfig.InitOpts.StorageClass + message.Warn(lang.WarnStorageClassDeprecated) + } } } From 697554f6a0f57076ec3eb82314080059ec3afed5 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Fri, 20 Oct 2023 14:07:35 -0400 Subject: [PATCH 07/30] updated, using CI for testing --- packages/zarf-registry/zarf.yaml | 10 ++++++++++ src/cmd/initialize.go | 33 ++++++++++---------------------- src/test/zarf-config-test.toml | 1 - 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/packages/zarf-registry/zarf.yaml b/packages/zarf-registry/zarf.yaml index 79255b2538..e1675a35a3 100644 --- a/packages/zarf-registry/zarf.yaml +++ b/packages/zarf-registry/zarf.yaml @@ -52,6 +52,16 @@ variables: default: "" autoIndent: true + - name: REGISTRY_NODEPORT + description: NodePort for the registry + default: "" + autoIndent: true + + - name: REGISTRY_STORAGE_CLASS + description: StorageClass for the registry PVC + default: "" + autoIndent: true + constants: - name: REGISTRY_IMAGE value: "###ZARF_PKG_TMPL_REGISTRY_IMAGE###" diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index 429e071d65..d7073cf83b 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -80,39 +80,26 @@ var initCmd = &cobra.Command{ // DEPRECATED_V1.0.0: do not check pkgConfig.InitOpts.RegistryInfo.NodePort, always overwrite it. func setRegistryNodePort() { - if nodePortStr, ok := pkgConfig.PkgOpts.SetVariables["REGISTRY_NODEPORT"]; ok { - if pkgConfig.InitOpts.RegistryInfo.NodePort != 0 { - message.Warn(lang.WarnBothNodePortSchemesDeprecated) - } - nodePort, err := strconv.Atoi(nodePortStr) - if err != nil || nodePort < 30000 || nodePort > 32767 { - message.Fatal(err, lang.FatalNodeportInvalid) - } - pkgConfig.InitOpts.RegistryInfo.NodePort = nodePort + // TODO: this will always get overriden by the defautls + if pkgConfig.InitOpts.RegistryInfo.NodePort != 0 { + message.Warn(lang.WarnNodePortDeprecated) } else { - // if the old way is set, use it, or the default - if pkgConfig.InitOpts.RegistryInfo.NodePort > 0 { - message.Warn(lang.WarnNodePortDeprecated) + nodePort, err := strconv.Atoi(pkgConfig.PkgOpts.SetVariables["REGISTRY_NODEPORT"]) + if err == nil && (nodePort >= 30000 || nodePort <= 32767) { + // TODO: if the nodeport is out of range, warn that we're putting it back in range + pkgConfig.InitOpts.RegistryInfo.NodePort = nodePort } else { pkgConfig.InitOpts.RegistryInfo.NodePort = config.ZarfInClusterContainerRegistryNodePort } - pkgConfig.PkgOpts.SetVariables["REGISTRY_NODEPORT"] = strconv.Itoa(pkgConfig.InitOpts.RegistryInfo.NodePort) } } // DEPRECATED_V1.0.0: do not check pkgConfig.InitOpts.StorageClass, always overwrite it. func setRegistryStorageClass() { - if storageClass, ok := pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]; ok { - if pkgConfig.InitOpts.StorageClass != "" { - message.Warn(lang.WarnBothStorageClassesDeprecated) - } - pkgConfig.InitOpts.StorageClass = storageClass + if pkgConfig.InitOpts.StorageClass != "" { + message.Warn(lang.WarnBothStorageClassesDeprecated) } else { - // if the old way was set (or if the old way is empty) - if pkgConfig.InitOpts.StorageClass != "" { - pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] = pkgConfig.InitOpts.StorageClass - message.Warn(lang.WarnStorageClassDeprecated) - } + pkgConfig.InitOpts.StorageClass = pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] } } diff --git a/src/test/zarf-config-test.toml b/src/test/zarf-config-test.toml index 3585a45f3f..c7e4df1770 100644 --- a/src/test/zarf-config-test.toml +++ b/src/test/zarf-config-test.toml @@ -8,7 +8,6 @@ insecure = true [init] components = 'components: 359049b9' -storage_class = 'storage_class: 9cae917f' [init.git] pull_password = 'git.pull_password: 8522ccca' From 4fbadc0945cb55850ab8ba7c22a27046d13e16bb Mon Sep 17 00:00:00 2001 From: bdw617 Date: Mon, 23 Oct 2023 11:51:06 -0400 Subject: [PATCH 08/30] updated, ready for testing --- src/cmd/initialize.go | 33 +++++++++++++++++++++------------ src/config/lang/english.go | 7 ++----- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index d7073cf83b..add2258455 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -78,28 +78,37 @@ var initCmd = &cobra.Command{ }, } -// DEPRECATED_V1.0.0: do not check pkgConfig.InitOpts.RegistryInfo.NodePort, always overwrite it. +// DEPRECATED_V1.0.0: --nodeport should be removed from the cli in v1.0.0 func setRegistryNodePort() { - // TODO: this will always get overriden by the defautls + configVar := "REGISTRY_NODEPORT" if pkgConfig.InitOpts.RegistryInfo.NodePort != 0 { message.Warn(lang.WarnNodePortDeprecated) + } + nodePort, err := strconv.Atoi(pkgConfig.PkgOpts.SetVariables[configVar]) + if err != nil || nodePort < 30000 || nodePort > 32767 { + nodePort = pkgConfig.InitOpts.RegistryInfo.NodePort } else { - nodePort, err := strconv.Atoi(pkgConfig.PkgOpts.SetVariables["REGISTRY_NODEPORT"]) - if err == nil && (nodePort >= 30000 || nodePort <= 32767) { - // TODO: if the nodeport is out of range, warn that we're putting it back in range - pkgConfig.InitOpts.RegistryInfo.NodePort = nodePort - } else { - pkgConfig.InitOpts.RegistryInfo.NodePort = config.ZarfInClusterContainerRegistryNodePort - } + pkgConfig.InitOpts.RegistryInfo.NodePort = nodePort } + + if nodePort > 32767 || nodePort < 30000 { + nodePort = config.ZarfInClusterContainerRegistryNodePort + } + pkgConfig.PkgOpts.SetVariables[configVar] = strconv.Itoa(nodePort) + pkgConfig.InitOpts.RegistryInfo.NodePort = nodePort } -// DEPRECATED_V1.0.0: do not check pkgConfig.InitOpts.StorageClass, always overwrite it. +// DEPRECATED_V1.0.0: --storage-class should be removed from the CLI in v1.0.0 func setRegistryStorageClass() { + configVar := "REGISTRY_STORAGE_CLASS" + // there is no validation if this storage class is valid if pkgConfig.InitOpts.StorageClass != "" { - message.Warn(lang.WarnBothStorageClassesDeprecated) + message.Warn(lang.WarnStorageClassDeprecated) + } + if pkgConfig.PkgOpts.SetVariables[configVar] == "" { + pkgConfig.PkgOpts.SetVariables[configVar] = pkgConfig.InitOpts.StorageClass } else { - pkgConfig.InitOpts.StorageClass = pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] + pkgConfig.InitOpts.StorageClass = pkgConfig.PkgOpts.SetVariables[configVar] } } diff --git a/src/config/lang/english.go b/src/config/lang/english.go index 8506e2bb2f..a21905d61e 100644 --- a/src/config/lang/english.go +++ b/src/config/lang/english.go @@ -34,11 +34,8 @@ const ( // Zarf CLI deprecation warnings const ( // remove deprecation in v1.0.0 - WarnBothNodePortSchemesDeprecated = "--set REGISTRY_NODEPORT= will override --nodeport " - WarnStorageClassDeprecated = "--set REGISTRY_STORAGE_CLASS= will override --storage-class " - WarnBothStorageClassesDeprecated = "--set REGISTRY_STORAGE_CLASS= will override --storage-class " - WarnNodePortDeprecated = "--nodeport is deprecated, please use --set REGISTRY_NODEPORT= instead" - FatalNodeportInvalid = "Nodeport must be in the range [30000-32767]" + WarnStorageClassDeprecated = "--storage-class is deprecated, please use --set REGISTRY_STORAGE_CLASS=" + WarnNodePortDeprecated = "--nodeport is deprecated, please use --set REGISTRY_NODEPORT=" ) // Zarf CLI commands. From f49e4ad42e62708c46a50ba0a9aaa7f47a172c1b Mon Sep 17 00:00:00 2001 From: bdw617 Date: Mon, 23 Oct 2023 15:54:10 -0400 Subject: [PATCH 09/30] should work now, testing via CI --- src/cmd/initialize.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index add2258455..20a2e4558f 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -91,7 +91,8 @@ func setRegistryNodePort() { pkgConfig.InitOpts.RegistryInfo.NodePort = nodePort } - if nodePort > 32767 || nodePort < 30000 { + // 0 is unset, which will allow changing. + if nodePort > 32767 || nodePort < 30000 || nodePort != 0 { nodePort = config.ZarfInClusterContainerRegistryNodePort } pkgConfig.PkgOpts.SetVariables[configVar] = strconv.Itoa(nodePort) From 6576d6b2332b8fb8483633ae5f094640a712e8aa Mon Sep 17 00:00:00 2001 From: bdw617 Date: Tue, 24 Oct 2023 10:41:38 -0400 Subject: [PATCH 10/30] fix test in CI --- src/cmd/initialize.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index 20a2e4558f..c8071c8b08 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -92,8 +92,8 @@ func setRegistryNodePort() { } // 0 is unset, which will allow changing. - if nodePort > 32767 || nodePort < 30000 || nodePort != 0 { - nodePort = config.ZarfInClusterContainerRegistryNodePort + if nodePort > 32767 || nodePort < 30000 { + nodePort = 0 } pkgConfig.PkgOpts.SetVariables[configVar] = strconv.Itoa(nodePort) pkgConfig.InitOpts.RegistryInfo.NodePort = nodePort From d5f7d3506920ce7901f7ecc9e83d612c2e81a769 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Fri, 27 Oct 2023 10:56:25 -0400 Subject: [PATCH 11/30] updated for ci tests, some will break --- packages/zarf-registry/zarf.yaml | 2 +- src/cmd/initialize.go | 57 +++++++++---- src/cmd/initialize_test.go | 95 ++-------------------- src/internal/cluster/state.go | 35 +------- src/internal/packager/template/template.go | 16 +++- src/pkg/packager/deploy.go | 3 +- src/types/k8s.go | 28 +++++-- src/types/runtime.go | 3 +- 8 files changed, 82 insertions(+), 157 deletions(-) diff --git a/packages/zarf-registry/zarf.yaml b/packages/zarf-registry/zarf.yaml index e1675a35a3..34a7537ae5 100644 --- a/packages/zarf-registry/zarf.yaml +++ b/packages/zarf-registry/zarf.yaml @@ -54,7 +54,7 @@ variables: - name: REGISTRY_NODEPORT description: NodePort for the registry - default: "" + default: "31999" autoIndent: true - name: REGISTRY_STORAGE_CLASS diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index c8071c8b08..16906d02b3 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -27,6 +27,9 @@ import ( "github.com/spf13/cobra" ) +var nodePortArg = 0 +var storageClassArg = "" + // initCmd represents the init command. var initCmd = &cobra.Command{ Use: "init", @@ -81,35 +84,55 @@ var initCmd = &cobra.Command{ // DEPRECATED_V1.0.0: --nodeport should be removed from the cli in v1.0.0 func setRegistryNodePort() { configVar := "REGISTRY_NODEPORT" - if pkgConfig.InitOpts.RegistryInfo.NodePort != 0 { + + internalRegistry := pkgConfig.InitOpts.RegistryInfo.Address == "" + + // warn for deprecation + if nodePortArg != 0 { message.Warn(lang.WarnNodePortDeprecated) } - nodePort, err := strconv.Atoi(pkgConfig.PkgOpts.SetVariables[configVar]) - if err != nil || nodePort < 30000 || nodePort > 32767 { - nodePort = pkgConfig.InitOpts.RegistryInfo.NodePort - } else { - pkgConfig.InitOpts.RegistryInfo.NodePort = nodePort + + // check the --set REGISTRY_NODEPORT first + configuredNodePort, err := strconv.Atoi(pkgConfig.PkgOpts.SetVariables[configVar]) + if err != nil { + configuredNodePort = 0 + } + + // check the old --nodeport flag second + if configuredNodePort == 0 { + configuredNodePort = nodePortArg } - // 0 is unset, which will allow changing. - if nodePort > 32767 || nodePort < 30000 { - nodePort = 0 + // the user can't set both that this is an external registry and the nodeport stuff. + if !internalRegistry && configuredNodePort != 0 { + message.Fatal(nil, "both --registry-url and --nodeport are set, please only use one") } - pkgConfig.PkgOpts.SetVariables[configVar] = strconv.Itoa(nodePort) - pkgConfig.InitOpts.RegistryInfo.NodePort = nodePort + + if internalRegistry { + if configuredNodePort > 32767 || configuredNodePort < 30000 { + configuredNodePort = config.ZarfInClusterContainerRegistryNodePort + } + pkgConfig.PkgOpts.SetVariables[configVar] = strconv.Itoa(configuredNodePort) + pkgConfig.InitOpts.RegistryInfo.Address = fmt.Sprintf("%s:%d", helpers.IPV4Localhost, configuredNodePort) + } else { + // do not set the nodeport if this is an external registry + pkgConfig.PkgOpts.SetVariables[configVar] = "" + } + + pkgConfig.InitOpts.RegistryInfo.InternalRegistry = internalRegistry + + // TODO: test external registry and CLI. } // DEPRECATED_V1.0.0: --storage-class should be removed from the CLI in v1.0.0 func setRegistryStorageClass() { configVar := "REGISTRY_STORAGE_CLASS" // there is no validation if this storage class is valid - if pkgConfig.InitOpts.StorageClass != "" { + if storageClassArg != "" { message.Warn(lang.WarnStorageClassDeprecated) } if pkgConfig.PkgOpts.SetVariables[configVar] == "" { - pkgConfig.PkgOpts.SetVariables[configVar] = pkgConfig.InitOpts.StorageClass - } else { - pkgConfig.InitOpts.StorageClass = pkgConfig.PkgOpts.SetVariables[configVar] + pkgConfig.PkgOpts.SetVariables[configVar] = storageClassArg } } @@ -230,7 +253,7 @@ func init() { initCmd.Flags().StringVar(&pkgConfig.PkgOpts.OptionalComponents, "components", v.GetString(common.VInitComponents), lang.CmdInitFlagComponents) // [Deprecated] --storage-class is deprecated in favor of --set REGISTRY_STORAGE_CLASS (to be removed in v1.0.0) - initCmd.Flags().StringVar(&pkgConfig.InitOpts.StorageClass, "storage-class", v.GetString(common.VInitStorageClass), lang.CmdInitFlagStorageClass) + initCmd.Flags().StringVar(&storageClassArg, "storage-class", v.GetString(common.VInitStorageClass), lang.CmdInitFlagStorageClass) initCmd.Flags().MarkHidden("storage-class") // Flags for using an external Git server @@ -244,7 +267,7 @@ func init() { initCmd.Flags().StringVar(&pkgConfig.InitOpts.RegistryInfo.Address, "registry-url", v.GetString(common.VInitRegistryURL), lang.CmdInitFlagRegURL) // [Deprecated] --nodeport is deprecated in favor of --set REGISTRY_NODEPORT - initCmd.Flags().IntVar(&pkgConfig.InitOpts.RegistryInfo.NodePort, "nodeport", v.GetInt(common.VInitRegistryNodeport), lang.CmdInitFlagRegNodePort) + initCmd.Flags().IntVar(&nodePortArg, "nodeport", v.GetInt(common.VInitRegistryNodeport), lang.CmdInitFlagRegNodePort) initCmd.Flags().MarkHidden("nodeport") initCmd.Flags().StringVar(&pkgConfig.InitOpts.RegistryInfo.PushUsername, "registry-push-username", v.GetString(common.VInitRegistryPushUser), lang.CmdInitFlagRegPushUser) diff --git a/src/cmd/initialize_test.go b/src/cmd/initialize_test.go index 3f3865967d..af14400b2a 100644 --- a/src/cmd/initialize_test.go +++ b/src/cmd/initialize_test.go @@ -2,27 +2,21 @@ package cmd import ( - "fmt" - "os" - "os/exec" - "syscall" "testing" - - "github.com/defenseunicorns/zarf/src/config" ) func TestSetRegistryStorageClass(t *testing.T) { // Test case 1: storageClass is set pkgConfig.PkgOpts.SetVariables = map[string]string{"REGISTRY_STORAGE_CLASS": "test-storage-class"} - pkgConfig.InitOpts.StorageClass = "" + storageClassArg = "" setRegistryStorageClass() - if pkgConfig.InitOpts.StorageClass != "test-storage-class" { - t.Errorf("Expected storage class to be set to 'test-storage-class', but got '%s'", pkgConfig.InitOpts.StorageClass) + if storageClassArg != "test-storage-class" { + t.Errorf("Expected storage class to be set to 'test-storage-class', but got '%s'", storageClassArg) } // Test case 2: storageClass is not set, use old way pkgConfig.PkgOpts.SetVariables = map[string]string{} - pkgConfig.InitOpts.StorageClass = "old-storage-class" + localStorageClass = "old-storage-class" setRegistryStorageClass() if pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] != "old-storage-class" { t.Errorf("Expected storage class to be set to old way value, but got '%s'", pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]) @@ -30,89 +24,10 @@ func TestSetRegistryStorageClass(t *testing.T) { // Test case 3: neither is set, should be empty pkgConfig.PkgOpts.SetVariables = map[string]string{} - pkgConfig.InitOpts.StorageClass = "" + localStorageClass = "" setRegistryStorageClass() if pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] != "" { t.Errorf("Expected storage class to be set to empty string, but got '%s'", pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]) } -} - -// convinence function to setup tests -func setupNodeportTests(r string, n int) { - pkgConfig.PkgOpts.SetVariables = map[string]string{} - if r != "" { - pkgConfig.PkgOpts.SetVariables["REGISTRY_NODEPORT"] = r - } - pkgConfig.InitOpts.RegistryInfo.NodePort = n -} - -func TestSetRegistryNodePort(t *testing.T) { - // Test case 1: new way is set, old way is not - setupNodeportTests("30001", 0) - setRegistryNodePort() - if pkgConfig.InitOpts.RegistryInfo.NodePort != 30001 { - t.Errorf("Expected node port to be set to 30001, but got %d", pkgConfig.InitOpts.RegistryInfo.NodePort) - } - - // Test case 2: nothing is set, use the default - setupNodeportTests("", 0) - setRegistryNodePort() - if pkgConfig.InitOpts.RegistryInfo.NodePort != config.ZarfInClusterContainerRegistryNodePort { - t.Errorf("Expected node port to be set to default value, but got %d", pkgConfig.InitOpts.RegistryInfo.NodePort) - } - // Test case 3: The old way is set, and the new way is not. We should set the new variable to the old method - setupNodeportTests("", 30001) - setRegistryNodePort() - if c, ok := pkgConfig.PkgOpts.SetVariables["REGISTRY_NODEPORT"]; !ok { - t.Error("Expected node port to be set to old way value, but it is not set") - } else { - if c != "30001" { - t.Errorf("Expected node port to be set to old way value, but got %s", c) - } - } -} - -// forked test which will fatal because the old way is invalid -func TestSetRegistryNodePortHelper(t *testing.T) { - if os.Getenv("TEST_SET_REGISTRY_NODE_PORT_HELPER") != "1" { - return - } - setupNodeportTests("invalid", 0) - err := func() (err error) { - defer func() { - if r := recover(); r != nil { - err = fmt.Errorf("Expected test case to fail with fatal error, but it panicked: %v", r) - } - }() - setRegistryNodePort() - return nil - }() - if err == nil { - os.Exit(0) - } - os.Exit(1) - -} -func TestSetRegistryNodePortExit(t *testing.T) { - // Calls the helper - cmd := exec.Command(os.Args[0], "-test.run=TestSetRegistryNodePortHelper") - cmd.Env = append(os.Environ(), "TEST_SET_REGISTRY_NODE_PORT_HELPER=1") - if err := cmd.Start(); err != nil { - t.Fatal(err) - } - // make sure we got an exitStatus of 1, otherwise the test failed. - if err := cmd.Wait(); err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - if status, ok := exitErr.Sys().(syscall.WaitStatus); ok { - if status.ExitStatus() != 1 { - t.Errorf("Expected test case to fail with exit status 1, but got exit status %d", status.ExitStatus()) - } - } else { - t.Errorf("Expected syscall.WaitStatus, but got %T", exitErr.Sys()) - } - } else { - t.Error(err) - } - } } diff --git a/src/internal/cluster/state.go b/src/internal/cluster/state.go index ba9888c26f..d7c5b31176 100644 --- a/src/internal/cluster/state.go +++ b/src/internal/cluster/state.go @@ -120,11 +120,13 @@ func (c *Cluster) InitZarfState(initOptions types.ZarfInitOptions) error { state.RegistryInfo = c.fillInEmptyContainerRegistryValues(initOptions.RegistryInfo) state.ArtifactServer = c.fillInEmptyArtifactServerValues(initOptions.ArtifactServer) } else { + if helpers.IsNotZeroAndNotEqual(initOptions.GitServer, state.GitServer) { message.Warn("Detected a change in Git Server init options on a re-init. Ignoring... To update run:") message.ZarfCommand("tools update-creds git") } if helpers.IsNotZeroAndNotEqual(initOptions.RegistryInfo, state.RegistryInfo) { + message.Warn("Detected a change in Image Registry init options on a re-init. Ignoring... To update run:") message.ZarfCommand("tools update-creds registry") } @@ -138,21 +140,6 @@ func (c *Cluster) InitZarfState(initOptions types.ZarfInitOptions) error { return fmt.Errorf("cluster architecture %s does not match the Zarf state architecture %s", clusterArch, state.Architecture) } - switch state.Distro { - case k8s.DistroIsK3s, k8s.DistroIsK3d: - state.StorageClass = "local-path" - - case k8s.DistroIsKind, k8s.DistroIsGKE: - state.StorageClass = "standard" - - case k8s.DistroIsDockerDesktop: - state.StorageClass = "hostpath" - } - - if initOptions.StorageClass != "" { - state.StorageClass = initOptions.StorageClass - } - spinner.Success() // Save the state back to K8s @@ -260,13 +247,6 @@ func (c *Cluster) MergeZarfState(oldState *types.ZarfState, initOptions types.Za if slices.Contains(services, message.RegistryKey) { newState.RegistryInfo = helpers.MergeNonZero(newState.RegistryInfo, initOptions.RegistryInfo) - // Set the state of the internal registry if it has changed - if newState.RegistryInfo.Address == fmt.Sprintf("%s:%d", helpers.IPV4Localhost, newState.RegistryInfo.NodePort) { - newState.RegistryInfo.InternalRegistry = true - } else { - newState.RegistryInfo.InternalRegistry = false - } - // Set the new passwords if they should be autogenerated if newState.RegistryInfo.PushPassword == oldState.RegistryInfo.PushPassword && oldState.RegistryInfo.InternalRegistry { newState.RegistryInfo.PushPassword = utils.RandomString(config.ZarfGeneratedPasswordLen) @@ -316,17 +296,6 @@ func (c *Cluster) MergeZarfState(oldState *types.ZarfState, initOptions types.Za } func (c *Cluster) fillInEmptyContainerRegistryValues(containerRegistry types.RegistryInfo) types.RegistryInfo { - // Set default NodePort if none was provided - if containerRegistry.NodePort == 0 { - containerRegistry.NodePort = config.ZarfInClusterContainerRegistryNodePort - } - - // Set default url if an external registry was not provided - if containerRegistry.Address == "" { - containerRegistry.InternalRegistry = true - containerRegistry.Address = fmt.Sprintf("%s:%d", helpers.IPV4Localhost, containerRegistry.NodePort) - } - // Generate a push-user password if not provided by init flag if containerRegistry.PushPassword == "" { containerRegistry.PushPassword = utils.RandomString(config.ZarfGeneratedPasswordLen) diff --git a/src/internal/packager/template/template.go b/src/internal/packager/template/template.go index 7239947e8b..0adc900631 100644 --- a/src/internal/packager/template/template.go +++ b/src/internal/packager/template/template.go @@ -71,6 +71,15 @@ func (values *Values) GetRegistry() string { return values.registry } +func getPort(hostport string) string { + hp := strings.Split(hostport, ":") + if len(hp) != 2 { + message.Warn("hostport port mismatch, using the port in port") + return hostport + } + return hp[1] +} + // GetVariables returns the variables to be used in the template. func (values *Values) GetVariables(component types.ZarfComponent) (templateMap map[string]*utils.TextTemplate, deprecations map[string]string) { templateMap = make(map[string]*utils.TextTemplate) @@ -80,17 +89,16 @@ func (values *Values) GetVariables(component types.ZarfComponent) (templateMap m deprecations = map[string]string{ fmt.Sprintf("###ZARF_%s###", depMarkerOld): fmt.Sprintf("###ZARF_%s###", depMarkerNew), } - + // in the event there was a modification of this, having this misaligned will break the deployment + // TODO: test with yolo, and external registry. + values.config.PkgOpts.SetVariables["REGISTRY_NODEPORT"] = getPort(values.registry) if values.config.State != nil { regInfo := values.config.State.RegistryInfo gitInfo := values.config.State.GitServer builtinMap := map[string]string{ - "STORAGE_CLASS": values.config.State.StorageClass, - // Registry info "REGISTRY": values.registry, - "NODEPORT": fmt.Sprintf("%d", regInfo.NodePort), "REGISTRY_AUTH_PUSH": regInfo.PushPassword, "REGISTRY_AUTH_PULL": regInfo.PullPassword, diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index b2c350d40c..a0b3c1c085 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -213,7 +213,6 @@ func (p *Packager) deployComponents() (deployedComponents []types.DeployedCompon } func (p *Packager) deployInitComponent(component types.ZarfComponent) (charts []types.InstalledChart, err error) { - hasExternalRegistry := p.cfg.InitOpts.RegistryInfo.Address != "" isSeedRegistry := component.Name == "zarf-seed-registry" isRegistry := component.Name == "zarf-registry" isInjector := component.Name == "zarf-injector" @@ -232,7 +231,7 @@ func (p *Packager) deployInitComponent(component types.ZarfComponent) (charts [] } } - if hasExternalRegistry && (isSeedRegistry || isInjector || isRegistry) { + if !p.cfg.InitOpts.RegistryInfo.InternalRegistry && (isSeedRegistry || isInjector || isRegistry) { message.Notef("Not deploying the component (%s) since external registry information was provided during `zarf init`", component.Name) return charts, nil } diff --git a/src/types/k8s.go b/src/types/k8s.go index b01cea1848..bf5eb720e6 100644 --- a/src/types/k8s.go +++ b/src/types/k8s.go @@ -34,13 +34,20 @@ const ( // ZarfState is maintained as a secret in the Zarf namespace to track Zarf init data. type ZarfState struct { - ZarfAppliance bool `json:"zarfAppliance" jsonschema:"description=Indicates if Zarf was initialized while deploying its own k8s cluster"` - Distro string `json:"distro" jsonschema:"description=K8s distribution of the cluster Zarf was deployed to"` - Architecture string `json:"architecture" jsonschema:"description=Machine architecture of the k8s node(s)"` - StorageClass string `json:"storageClass" jsonschema:"Default StorageClass value Zarf uses for variable templating"` - AgentTLS k8s.GeneratedPKI `json:"agentTLS" jsonschema:"PKI certificate information for the agent pods Zarf manages"` + // TODO: do we need this long term? + ZarfAppliance bool `json:"zarfAppliance" jsonschema:"description=Indicates if Zarf was initialized while deploying its own k8s cluster"` - GitServer GitServerInfo `json:"gitServer" jsonschema:"description=Information about the repository Zarf is configured to use"` + // TODO: test how the default storage class behaves, validate if we get rid of it. Getting rid of this, might cause issues downstream with upgrades. + Distro string `json:"distro" jsonschema:"description=K8s distribution of the cluster Zarf was deployed to"` + + // TODO: validate if we need this, or can infer it from cluster. + Architecture string `json:"architecture" jsonschema:"description=Machine architecture of the k8s node(s)"` + + AgentTLS k8s.GeneratedPKI `json:"agentTLS" jsonschema:"PKI certificate information for the agent pods Zarf manages"` + + GitServer GitServerInfo `json:"gitServer" jsonschema:"description=Information about the repository Zarf is configured to use"` + + // TODO: this is where the refactoring needs to be. RegistryInfo RegistryInfo `json:"registryInfo" jsonschema:"description=Information about the container registry Zarf is configured to use"` ArtifactServer ArtifactServerInfo `json:"artifactServer" jsonschema:"description=Information about the artifact registry Zarf is configured to use"` LoggingSecret string `json:"loggingSecret" jsonschema:"description=Secret value that the internal Grafana server was seeded with"` @@ -107,9 +114,12 @@ type RegistryInfo struct { PullUsername string `json:"pullUsername" jsonschema:"description=Username of a user with pull-only access to the registry. If not provided for an external registry than the push-user is used"` PullPassword string `json:"pullPassword" jsonschema:"description=Password of a user with pull-only access to the registry. If not provided for an external registry than the push-user is used"` - Address string `json:"address" jsonschema:"description=URL address of the registry"` - NodePort int `json:"nodePort" jsonschema:"description=Nodeport of the registry. Only needed if the registry is running inside the kubernetes cluster"` - InternalRegistry bool `json:"internalRegistry" jsonschema:"description=Indicates if we are using a registry that Zarf is directly managing"` + // TODO: + Address string `json:"address" jsonschema:"description=URL address of the registry"` + + // TODO get rid of NodePort, it lives in Address + //NodePort int `json:"nodePort" jsonschema:"description=Nodeport of the registry. Only needed if the registry is running inside the kubernetes cluster"` + InternalRegistry bool `json:"internalRegistry" jsonschema:"description=Indicates if we are using a registry that Zarf is directly managing"` Secret string `json:"secret" jsonschema:"description=Secret value that the registry was seeded with"` } diff --git a/src/types/runtime.go b/src/types/runtime.go index 65c347b882..f6103ee305 100644 --- a/src/types/runtime.go +++ b/src/types/runtime.go @@ -78,7 +78,8 @@ type ZarfInitOptions struct { RegistryInfo RegistryInfo `json:"registryInfo" jsonschema:"description=Information about the container registry Zarf is going to be using"` ArtifactServer ArtifactServerInfo `json:"artifactServer" jsonschema:"description=Information about the artifact registry Zarf is going to be using"` - StorageClass string `json:"storageClass" jsonschema:"description=StorageClass of the k8s cluster Zarf is initializing"` + // removed + //StorageClass string `json:"storageClass" jsonschema:"description=StorageClass of the k8s cluster Zarf is initializing"` } // ZarfCreateOptions tracks the user-defined options used to create the package. From 4cd06a16f4dbefc5a4fea0c871b3269e054ba0c2 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Fri, 27 Oct 2023 11:10:18 -0400 Subject: [PATCH 12/30] fixed merge issue --- src/internal/cluster/state.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/internal/cluster/state.go b/src/internal/cluster/state.go index 8fa3228cc2..27d401ab55 100644 --- a/src/internal/cluster/state.go +++ b/src/internal/cluster/state.go @@ -129,10 +129,6 @@ func (c *Cluster) InitZarfState(initOptions types.ZarfInitOptions) error { } } - if clusterArch != state.Architecture { - return fmt.Errorf("cluster architecture %s does not match the Zarf state architecture %s", clusterArch, state.Architecture) - } - spinner.Success() // Save the state back to K8s From cf0d90a9ab1a50edc7a3fd127bff340d8da8eee1 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Fri, 27 Oct 2023 15:16:56 -0400 Subject: [PATCH 13/30] updated to fix an NPE in non-init --- src/internal/packager/template/template.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/internal/packager/template/template.go b/src/internal/packager/template/template.go index 0adc900631..5f8c3bd1dd 100644 --- a/src/internal/packager/template/template.go +++ b/src/internal/packager/template/template.go @@ -91,6 +91,10 @@ func (values *Values) GetVariables(component types.ZarfComponent) (templateMap m } // in the event there was a modification of this, having this misaligned will break the deployment // TODO: test with yolo, and external registry. + if values.config.PkgOpts.SetVariables == nil { + values.config.PkgOpts.SetVariables = map[string]string{} + } + values.config.PkgOpts.SetVariables["REGISTRY_NODEPORT"] = getPort(values.registry) if values.config.State != nil { regInfo := values.config.State.RegistryInfo From 83175e154d5825eb63e8f496a4f676037c10afc8 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Fri, 27 Oct 2023 15:26:53 -0400 Subject: [PATCH 14/30] updated --- src/cmd/initialize_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmd/initialize_test.go b/src/cmd/initialize_test.go index af14400b2a..72444fe7a5 100644 --- a/src/cmd/initialize_test.go +++ b/src/cmd/initialize_test.go @@ -16,7 +16,7 @@ func TestSetRegistryStorageClass(t *testing.T) { // Test case 2: storageClass is not set, use old way pkgConfig.PkgOpts.SetVariables = map[string]string{} - localStorageClass = "old-storage-class" + storageClassArg = "old-storage-class" setRegistryStorageClass() if pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] != "old-storage-class" { t.Errorf("Expected storage class to be set to old way value, but got '%s'", pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]) @@ -24,7 +24,7 @@ func TestSetRegistryStorageClass(t *testing.T) { // Test case 3: neither is set, should be empty pkgConfig.PkgOpts.SetVariables = map[string]string{} - localStorageClass = "" + storageClassArg = "" setRegistryStorageClass() if pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] != "" { t.Errorf("Expected storage class to be set to empty string, but got '%s'", pkgConfig.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]) From 7c2fde7074754367c354258d1cee5b4fe4a911a7 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Fri, 27 Oct 2023 16:02:08 -0400 Subject: [PATCH 15/30] moving to default storage class. makes us less distro complicated --- src/test/external/gitea-values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/external/gitea-values.yaml b/src/test/external/gitea-values.yaml index 08f0393688..9dbc338a84 100644 --- a/src/test/external/gitea-values.yaml +++ b/src/test/external/gitea-values.yaml @@ -1,5 +1,5 @@ persistence: - storageClass: "local-path" # 'local-path' is for k3d, 'standard' is for kind + storageClass: "" gitea: admin: username: "git-user" From e3551165e0bf3f260ea5b4ae13ea43fa4cdf6ab7 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Sat, 28 Oct 2023 10:29:46 -0400 Subject: [PATCH 16/30] fix upgrade test, will need to release notes this with --set STORAGE_CLASS=local-path --- src/test/e2e/20_zarf_init_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/e2e/20_zarf_init_test.go b/src/test/e2e/20_zarf_init_test.go index df4c555182..3fc08eacbb 100644 --- a/src/test/e2e/20_zarf_init_test.go +++ b/src/test/e2e/20_zarf_init_test.go @@ -69,7 +69,7 @@ func TestZarfInit(t *testing.T) { } // run `zarf init` - _, initStdErr, err := e2e.Zarf("init", "--components="+initComponents, "--set", "REGISTRY_NODEPORT=31337", "-l", "trace", "--confirm") + _, initStdErr, err := e2e.Zarf("init", "--components="+initComponents, "--set", "STORAGE_CLASS=local-path", "--set", "REGISTRY_NODEPORT=31337", "-l", "trace", "--confirm") require.NoError(t, err) require.Contains(t, initStdErr, "an inventory of all software contained in this package") From ae7eff169fb2d6105618ef8b207637f3bd811345 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Sat, 28 Oct 2023 16:18:01 -0400 Subject: [PATCH 17/30] fixed typo --- src/test/e2e/20_zarf_init_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/e2e/20_zarf_init_test.go b/src/test/e2e/20_zarf_init_test.go index 3fc08eacbb..10e2466455 100644 --- a/src/test/e2e/20_zarf_init_test.go +++ b/src/test/e2e/20_zarf_init_test.go @@ -69,7 +69,7 @@ func TestZarfInit(t *testing.T) { } // run `zarf init` - _, initStdErr, err := e2e.Zarf("init", "--components="+initComponents, "--set", "STORAGE_CLASS=local-path", "--set", "REGISTRY_NODEPORT=31337", "-l", "trace", "--confirm") + _, initStdErr, err := e2e.Zarf("init", "--components="+initComponents, "--set", "REGISTRY_STORAGE_CLASS=local-path", "--set", "REGISTRY_NODEPORT=31337", "-l", "trace", "--confirm") require.NoError(t, err) require.Contains(t, initStdErr, "an inventory of all software contained in this package") From c1ed1639733e361539f3e51266f536bf142924c2 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Sun, 29 Oct 2023 19:07:09 -0400 Subject: [PATCH 18/30] updated so if REGISTRY_STORAGE_CLASS isn't set, it will attempt to use the cluster's default storage class. This should be backwards compatible with any deployment that is using the default storage class, without having to put it into zarf state --- src/pkg/k8s/info.go | 18 ++++++++++++++++++ src/pkg/packager/deploy.go | 11 +++++++++++ src/test/e2e/20_zarf_init_test.go | 2 +- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/pkg/k8s/info.go b/src/pkg/k8s/info.go index 61effabdaa..9d0c72dd06 100644 --- a/src/pkg/k8s/info.go +++ b/src/pkg/k8s/info.go @@ -5,11 +5,14 @@ package k8s import ( + "context" "errors" "fmt" "regexp" "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // List of supported distros via distro detection. @@ -158,3 +161,18 @@ func MakeLabels(labels map[string]string) string { } return strings.Join(out, ",") } + +func (k *K8s) GetDefaultStorageClass() (string, error) { + storageV1Client := k.Clientset.StorageV1() + storageClasses, err := storageV1Client.StorageClasses().List(context.TODO(), metav1.ListOptions{}) + if err != nil { + return "", fmt.Errorf("unable to get Kubernetes default storage class from the cluster : %w", err) + } + + for _, sc := range storageClasses.Items { + if sc.Annotations["storageclass.kubernetes.io/is-default-class"] == "true" { + return sc.Name, nil + } + } + return "", fmt.Errorf("unable to get Kubernetes default storage class from the cluster (no default storage class found)") +} diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index 7d9e87f1f3..04ab338475 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -215,6 +215,17 @@ func (p *Packager) deployInitComponent(component types.ZarfComponent) (charts [] // Before deploying the seed registry, start the injector if isSeedRegistry { p.cluster.StartInjectionMadness(p.layout.Base, p.layout.Images.Base, component.Images) + + // Retrieve the default storage class from the cluster, it will be the default + if p.cfg.PkgOpts.SetVariables == nil { + p.cfg.PkgOpts.SetVariables = map[string]string{} + } + if c := p.cfg.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]; c == "" { + p.cfg.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"], err = p.cluster.GetDefaultStorageClass() + } + if err != nil { + message.WarnErr(err, "Unable to get the default storage class from the cluster") + } } charts, err = p.deployComponent(component, isAgent /* skip img checksum if isAgent */, isSeedRegistry /* skip image push if isSeedRegistry */) diff --git a/src/test/e2e/20_zarf_init_test.go b/src/test/e2e/20_zarf_init_test.go index 10e2466455..df4c555182 100644 --- a/src/test/e2e/20_zarf_init_test.go +++ b/src/test/e2e/20_zarf_init_test.go @@ -69,7 +69,7 @@ func TestZarfInit(t *testing.T) { } // run `zarf init` - _, initStdErr, err := e2e.Zarf("init", "--components="+initComponents, "--set", "REGISTRY_STORAGE_CLASS=local-path", "--set", "REGISTRY_NODEPORT=31337", "-l", "trace", "--confirm") + _, initStdErr, err := e2e.Zarf("init", "--components="+initComponents, "--set", "REGISTRY_NODEPORT=31337", "-l", "trace", "--confirm") require.NoError(t, err) require.Contains(t, initStdErr, "an inventory of all software contained in this package") From b12026d3757ef8291c984903e3e57c26cad4d877 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Mon, 30 Oct 2023 15:22:40 -0400 Subject: [PATCH 19/30] trying with just empty, fixing chart, need to update it. --- packages/zarf-registry/chart/templates/pvc.yaml | 8 ++------ src/pkg/packager/deploy.go | 16 +++++++++------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/zarf-registry/chart/templates/pvc.yaml b/packages/zarf-registry/chart/templates/pvc.yaml index dea05397a5..2dd70c1ac4 100644 --- a/packages/zarf-registry/chart/templates/pvc.yaml +++ b/packages/zarf-registry/chart/templates/pvc.yaml @@ -17,11 +17,7 @@ spec: requests: storage: {{ .Values.persistence.size | quote }} {{- if .Values.persistence.storageClass }} -{{- if (eq "-" .Values.persistence.storageClass) }} - storageClassName: "" -{{- else }} - storageClassName: "{{ .Values.persistence.storageClass }}" -{{- end }} -{{- end }} + storageClassName: {{ .Values.persistence.storageClass }} {{- end }} {{- end -}} +{{- end }} diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index 04ab338475..a6ad282e1e 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -216,13 +216,15 @@ func (p *Packager) deployInitComponent(component types.ZarfComponent) (charts [] if isSeedRegistry { p.cluster.StartInjectionMadness(p.layout.Base, p.layout.Images.Base, component.Images) - // Retrieve the default storage class from the cluster, it will be the default - if p.cfg.PkgOpts.SetVariables == nil { - p.cfg.PkgOpts.SetVariables = map[string]string{} - } - if c := p.cfg.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]; c == "" { - p.cfg.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"], err = p.cluster.GetDefaultStorageClass() - } + /* + // Retrieve the default storage class from the cluster, it will be the default + if p.cfg.PkgOpts.SetVariables == nil { + p.cfg.PkgOpts.SetVariables = map[string]string{} + } + if c := p.cfg.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]; c == "" { + p.cfg.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"], err = p.cluster.GetDefaultStorageClass() + } + */ if err != nil { message.WarnErr(err, "Unable to get the default storage class from the cluster") } From 5874c1ccb9931e782049a020262031e5d10761e6 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Mon, 30 Oct 2023 16:29:23 -0400 Subject: [PATCH 20/30] trying to always set the values for the storage class --- packages/zarf-registry/chart/templates/pvc.yaml | 2 -- src/pkg/packager/deploy.go | 17 ++++++++--------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/zarf-registry/chart/templates/pvc.yaml b/packages/zarf-registry/chart/templates/pvc.yaml index 2dd70c1ac4..56ad681de4 100644 --- a/packages/zarf-registry/chart/templates/pvc.yaml +++ b/packages/zarf-registry/chart/templates/pvc.yaml @@ -16,8 +16,6 @@ spec: resources: requests: storage: {{ .Values.persistence.size | quote }} -{{- if .Values.persistence.storageClass }} storageClassName: {{ .Values.persistence.storageClass }} -{{- end }} {{- end -}} {{- end }} diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index a6ad282e1e..d13e6553ba 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -216,15 +216,14 @@ func (p *Packager) deployInitComponent(component types.ZarfComponent) (charts [] if isSeedRegistry { p.cluster.StartInjectionMadness(p.layout.Base, p.layout.Images.Base, component.Images) - /* - // Retrieve the default storage class from the cluster, it will be the default - if p.cfg.PkgOpts.SetVariables == nil { - p.cfg.PkgOpts.SetVariables = map[string]string{} - } - if c := p.cfg.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]; c == "" { - p.cfg.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"], err = p.cluster.GetDefaultStorageClass() - } - */ + // Retrieve the default storage class from the cluster, it will be the default + if p.cfg.PkgOpts.SetVariables == nil { + p.cfg.PkgOpts.SetVariables = map[string]string{} + } + if c := p.cfg.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"]; c == "" { + p.cfg.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"], err = p.cluster.GetDefaultStorageClass() + } + if err != nil { message.WarnErr(err, "Unable to get the default storage class from the cluster") } From 824086760be912eca4122eb8438c35426cc274ca Mon Sep 17 00:00:00 2001 From: bdw617 Date: Mon, 30 Oct 2023 18:04:57 -0400 Subject: [PATCH 21/30] hack, will need a better way to template state into ###ZARF_VAR_...### --- src/internal/packager/template/template.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/internal/packager/template/template.go b/src/internal/packager/template/template.go index 5f8c3bd1dd..cff41a130d 100644 --- a/src/internal/packager/template/template.go +++ b/src/internal/packager/template/template.go @@ -153,6 +153,10 @@ func (values *Values) GetVariables(component types.ZarfComponent) (templateMap m } } + // TODO: order of operation hack. + values.config.SetVariableMap["REGISTRY_NODEPORT"].Value = values.config.PkgOpts.SetVariables["REGISTRY_NODEPORT"] + values.config.SetVariableMap["REGISTRY_STORAGE_CLASS"].Value = values.config.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] + for key, variable := range values.config.SetVariableMap { // Variable keys are always uppercase in the format ###ZARF_VAR_KEY### templateMap[strings.ToUpper(fmt.Sprintf("###ZARF_VAR_%s###", key))] = &utils.TextTemplate{ From 8954308b738ca44d4796ac16e8170436863b0d9f Mon Sep 17 00:00:00 2001 From: bdw617 Date: Tue, 31 Oct 2023 11:54:02 -0400 Subject: [PATCH 22/30] a bit of cleanup --- src/internal/packager/template/template.go | 23 +++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/internal/packager/template/template.go b/src/internal/packager/template/template.go index cff41a130d..130716a701 100644 --- a/src/internal/packager/template/template.go +++ b/src/internal/packager/template/template.go @@ -89,13 +89,7 @@ func (values *Values) GetVariables(component types.ZarfComponent) (templateMap m deprecations = map[string]string{ fmt.Sprintf("###ZARF_%s###", depMarkerOld): fmt.Sprintf("###ZARF_%s###", depMarkerNew), } - // in the event there was a modification of this, having this misaligned will break the deployment - // TODO: test with yolo, and external registry. - if values.config.PkgOpts.SetVariables == nil { - values.config.PkgOpts.SetVariables = map[string]string{} - } - values.config.PkgOpts.SetVariables["REGISTRY_NODEPORT"] = getPort(values.registry) if values.config.State != nil { regInfo := values.config.State.RegistryInfo gitInfo := values.config.State.GitServer @@ -153,9 +147,14 @@ func (values *Values) GetVariables(component types.ZarfComponent) (templateMap m } } - // TODO: order of operation hack. - values.config.SetVariableMap["REGISTRY_NODEPORT"].Value = values.config.PkgOpts.SetVariables["REGISTRY_NODEPORT"] - values.config.SetVariableMap["REGISTRY_STORAGE_CLASS"].Value = values.config.PkgOpts.SetVariables["REGISTRY_STORAGE_CLASS"] + if values.config.PkgOpts.SetVariables != nil { + // TODO: verify they are the same, if not, warn the user + values.config.PkgOpts.SetVariables["REGISTRY_NODEPORT"] = getPort(values.registry) + } + + // update the variable map based on things that might be set in the state of the cluster + values.updateVariableMap("REGISTRY_NODEPORT") + values.updateVariableMap("REGISTRY_STORAGE_CLASS") for key, variable := range values.config.SetVariableMap { // Variable keys are always uppercase in the format ###ZARF_VAR_KEY### @@ -181,6 +180,12 @@ func (values *Values) GetVariables(component types.ZarfComponent) (templateMap m return templateMap, deprecations } +func (values *Values) updateVariableMap(key string) { + if values.config.PkgOpts.SetVariables != nil && values.config.SetVariableMap[key] != nil && values.config.PkgOpts.SetVariables[key] != "" { + values.config.SetVariableMap[key].Value = values.config.PkgOpts.SetVariables[key] + } +} + // Apply renders the template and writes the result to the given path. func (values *Values) Apply(component types.ZarfComponent, path string, ignoreReady bool) error { // If Apply() is called before all values are loaded, fail unless ignoreReady is true From 5c32128368ecceb28df49a80ee6dc0cd2115a4ca Mon Sep 17 00:00:00 2001 From: bdw617 Date: Tue, 31 Oct 2023 12:50:33 -0400 Subject: [PATCH 23/30] immutable nodeport once created, need more testing --- src/test/e2e/20_zarf_init_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/e2e/20_zarf_init_test.go b/src/test/e2e/20_zarf_init_test.go index df4c555182..3f4199b3e3 100644 --- a/src/test/e2e/20_zarf_init_test.go +++ b/src/test/e2e/20_zarf_init_test.go @@ -100,7 +100,7 @@ func TestZarfInit(t *testing.T) { // Check that the registry is running on the correct NodePort stdOut, _, err = e2e.Kubectl("get", "service", "-n", "zarf", "zarf-docker-registry", "-o=jsonpath='{.spec.ports[*].nodePort}'") require.NoError(t, err) - require.Contains(t, stdOut, "31337") + require.Contains(t, stdOut, "31999") // Check that the registry is running with the correct scale down policy stdOut, _, err = e2e.Kubectl("get", "hpa", "-n", "zarf", "zarf-docker-registry", "-o=jsonpath='{.spec.behavior.scaleDown.selectPolicy}'") From 4fe956cec937dbb100fd70a0036c4ef676962ae4 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Tue, 31 Oct 2023 15:15:47 -0400 Subject: [PATCH 24/30] updated --- src/test/e2e/20_zarf_init_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/e2e/20_zarf_init_test.go b/src/test/e2e/20_zarf_init_test.go index 3f4199b3e3..df4c555182 100644 --- a/src/test/e2e/20_zarf_init_test.go +++ b/src/test/e2e/20_zarf_init_test.go @@ -100,7 +100,7 @@ func TestZarfInit(t *testing.T) { // Check that the registry is running on the correct NodePort stdOut, _, err = e2e.Kubectl("get", "service", "-n", "zarf", "zarf-docker-registry", "-o=jsonpath='{.spec.ports[*].nodePort}'") require.NoError(t, err) - require.Contains(t, stdOut, "31999") + require.Contains(t, stdOut, "31337") // Check that the registry is running with the correct scale down policy stdOut, _, err = e2e.Kubectl("get", "hpa", "-n", "zarf", "zarf-docker-registry", "-o=jsonpath='{.spec.behavior.scaleDown.selectPolicy}'") From 7a043a6a1efdec1f1a7f54cd9c38adc65fcb33ba Mon Sep 17 00:00:00 2001 From: bdw617 Date: Wed, 1 Nov 2023 13:33:07 -0400 Subject: [PATCH 25/30] tried to force the nodeport to always be the same, didn't work, this appears to work better --- src/internal/packager/template/template.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/internal/packager/template/template.go b/src/internal/packager/template/template.go index 130716a701..70455bbdcb 100644 --- a/src/internal/packager/template/template.go +++ b/src/internal/packager/template/template.go @@ -94,6 +94,11 @@ func (values *Values) GetVariables(component types.ZarfComponent) (templateMap m regInfo := values.config.State.RegistryInfo gitInfo := values.config.State.GitServer + // if the address has been updated from init, we'll update it here too. + if values.config.InitOpts.RegistryInfo.Address != "" { + values.registry = values.config.InitOpts.RegistryInfo.Address + } + builtinMap := map[string]string{ // Registry info "REGISTRY": values.registry, From 6698814241660eb358f4f9102158e060f199738e Mon Sep 17 00:00:00 2001 From: bdw617 Date: Wed, 1 Nov 2023 17:51:55 -0400 Subject: [PATCH 26/30] the address needs to change in this case --- src/internal/cluster/state.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/internal/cluster/state.go b/src/internal/cluster/state.go index 27d401ab55..fa8f546650 100644 --- a/src/internal/cluster/state.go +++ b/src/internal/cluster/state.go @@ -120,7 +120,11 @@ func (c *Cluster) InitZarfState(initOptions types.ZarfInitOptions) error { } if helpers.IsNotZeroAndNotEqual(initOptions.RegistryInfo, state.RegistryInfo) { + if initOptions.RegistryInfo.Address != "" { + state.RegistryInfo.Address = initOptions.RegistryInfo.Address + } message.Warn("Detected a change in Image Registry init options on a re-init. Ignoring... To update run:") + message.ZarfCommand("tools update-creds registry") } if helpers.IsNotZeroAndNotEqual(initOptions.ArtifactServer, state.ArtifactServer) { From 294ffbeacf70d69a2073c3f3537504de9e578d23 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Wed, 15 Nov 2023 17:20:10 -0500 Subject: [PATCH 27/30] updated to preserve the zarf state as it was, it simplifies config and needs a refactor later --- .../zarf-registry/chart/templates/pvc.yaml | 10 ++++-- src/cmd/initialize.go | 1 + src/internal/cluster/state.go | 11 +++--- src/internal/packager/template/template.go | 34 ++++--------------- src/types/k8s.go | 28 +++++---------- 5 files changed, 30 insertions(+), 54 deletions(-) diff --git a/packages/zarf-registry/chart/templates/pvc.yaml b/packages/zarf-registry/chart/templates/pvc.yaml index 56ad681de4..dea05397a5 100644 --- a/packages/zarf-registry/chart/templates/pvc.yaml +++ b/packages/zarf-registry/chart/templates/pvc.yaml @@ -16,6 +16,12 @@ spec: resources: requests: storage: {{ .Values.persistence.size | quote }} - storageClassName: {{ .Values.persistence.storageClass }} -{{- end -}} +{{- if .Values.persistence.storageClass }} +{{- if (eq "-" .Values.persistence.storageClass) }} + storageClassName: "" +{{- else }} + storageClassName: "{{ .Values.persistence.storageClass }}" +{{- end }} {{- end }} +{{- end }} +{{- end -}} diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index 16906d02b3..7ab42a2bfd 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -114,6 +114,7 @@ func setRegistryNodePort() { } pkgConfig.PkgOpts.SetVariables[configVar] = strconv.Itoa(configuredNodePort) pkgConfig.InitOpts.RegistryInfo.Address = fmt.Sprintf("%s:%d", helpers.IPV4Localhost, configuredNodePort) + pkgConfig.InitOpts.RegistryInfo.NodePort = configuredNodePort } else { // do not set the nodeport if this is an external registry pkgConfig.PkgOpts.SetVariables[configVar] = "" diff --git a/src/internal/cluster/state.go b/src/internal/cluster/state.go index fa8f546650..774123c186 100644 --- a/src/internal/cluster/state.go +++ b/src/internal/cluster/state.go @@ -113,18 +113,12 @@ func (c *Cluster) InitZarfState(initOptions types.ZarfInitOptions) error { state.RegistryInfo = c.fillInEmptyContainerRegistryValues(initOptions.RegistryInfo) state.ArtifactServer = c.fillInEmptyArtifactServerValues(initOptions.ArtifactServer) } else { - if helpers.IsNotZeroAndNotEqual(initOptions.GitServer, state.GitServer) { message.Warn("Detected a change in Git Server init options on a re-init. Ignoring... To update run:") message.ZarfCommand("tools update-creds git") } if helpers.IsNotZeroAndNotEqual(initOptions.RegistryInfo, state.RegistryInfo) { - - if initOptions.RegistryInfo.Address != "" { - state.RegistryInfo.Address = initOptions.RegistryInfo.Address - } message.Warn("Detected a change in Image Registry init options on a re-init. Ignoring... To update run:") - message.ZarfCommand("tools update-creds registry") } if helpers.IsNotZeroAndNotEqual(initOptions.ArtifactServer, state.ArtifactServer) { @@ -289,6 +283,11 @@ func (c *Cluster) MergeZarfState(oldState *types.ZarfState, initOptions types.Za } func (c *Cluster) fillInEmptyContainerRegistryValues(containerRegistry types.RegistryInfo) types.RegistryInfo { + // Set default NodePort if none was provided + if containerRegistry.NodePort == 0 { + containerRegistry.NodePort = config.ZarfInClusterContainerRegistryNodePort + } + // Generate a push-user password if not provided by init flag if containerRegistry.PushPassword == "" { containerRegistry.PushPassword = utils.RandomString(config.ZarfGeneratedPasswordLen) diff --git a/src/internal/packager/template/template.go b/src/internal/packager/template/template.go index 70455bbdcb..16828448d8 100644 --- a/src/internal/packager/template/template.go +++ b/src/internal/packager/template/template.go @@ -71,15 +71,6 @@ func (values *Values) GetRegistry() string { return values.registry } -func getPort(hostport string) string { - hp := strings.Split(hostport, ":") - if len(hp) != 2 { - message.Warn("hostport port mismatch, using the port in port") - return hostport - } - return hp[1] -} - // GetVariables returns the variables to be used in the template. func (values *Values) GetVariables(component types.ZarfComponent) (templateMap map[string]*utils.TextTemplate, deprecations map[string]string) { templateMap = make(map[string]*utils.TextTemplate) @@ -94,9 +85,13 @@ func (values *Values) GetVariables(component types.ZarfComponent) (templateMap m regInfo := values.config.State.RegistryInfo gitInfo := values.config.State.GitServer - // if the address has been updated from init, we'll update it here too. - if values.config.InitOpts.RegistryInfo.Address != "" { - values.registry = values.config.InitOpts.RegistryInfo.Address + if values.config.SetVariableMap != nil { + if regInfo.NodePort > 0 { + values.config.SetVariableMap["REGISTRY_NODEPORT"].Value = fmt.Sprintf("%d", regInfo.NodePort) + } + if values.config.State.StorageClass != "" { + values.config.SetVariableMap["REGISTRY_STORAGE_CLASS"].Value = values.config.State.StorageClass + } } builtinMap := map[string]string{ @@ -152,15 +147,6 @@ func (values *Values) GetVariables(component types.ZarfComponent) (templateMap m } } - if values.config.PkgOpts.SetVariables != nil { - // TODO: verify they are the same, if not, warn the user - values.config.PkgOpts.SetVariables["REGISTRY_NODEPORT"] = getPort(values.registry) - } - - // update the variable map based on things that might be set in the state of the cluster - values.updateVariableMap("REGISTRY_NODEPORT") - values.updateVariableMap("REGISTRY_STORAGE_CLASS") - for key, variable := range values.config.SetVariableMap { // Variable keys are always uppercase in the format ###ZARF_VAR_KEY### templateMap[strings.ToUpper(fmt.Sprintf("###ZARF_VAR_%s###", key))] = &utils.TextTemplate{ @@ -185,12 +171,6 @@ func (values *Values) GetVariables(component types.ZarfComponent) (templateMap m return templateMap, deprecations } -func (values *Values) updateVariableMap(key string) { - if values.config.PkgOpts.SetVariables != nil && values.config.SetVariableMap[key] != nil && values.config.PkgOpts.SetVariables[key] != "" { - values.config.SetVariableMap[key].Value = values.config.PkgOpts.SetVariables[key] - } -} - // Apply renders the template and writes the result to the given path. func (values *Values) Apply(component types.ZarfComponent, path string, ignoreReady bool) error { // If Apply() is called before all values are loaded, fail unless ignoreReady is true diff --git a/src/types/k8s.go b/src/types/k8s.go index bf5eb720e6..b01cea1848 100644 --- a/src/types/k8s.go +++ b/src/types/k8s.go @@ -34,20 +34,13 @@ const ( // ZarfState is maintained as a secret in the Zarf namespace to track Zarf init data. type ZarfState struct { - // TODO: do we need this long term? - ZarfAppliance bool `json:"zarfAppliance" jsonschema:"description=Indicates if Zarf was initialized while deploying its own k8s cluster"` + ZarfAppliance bool `json:"zarfAppliance" jsonschema:"description=Indicates if Zarf was initialized while deploying its own k8s cluster"` + Distro string `json:"distro" jsonschema:"description=K8s distribution of the cluster Zarf was deployed to"` + Architecture string `json:"architecture" jsonschema:"description=Machine architecture of the k8s node(s)"` + StorageClass string `json:"storageClass" jsonschema:"Default StorageClass value Zarf uses for variable templating"` + AgentTLS k8s.GeneratedPKI `json:"agentTLS" jsonschema:"PKI certificate information for the agent pods Zarf manages"` - // TODO: test how the default storage class behaves, validate if we get rid of it. Getting rid of this, might cause issues downstream with upgrades. - Distro string `json:"distro" jsonschema:"description=K8s distribution of the cluster Zarf was deployed to"` - - // TODO: validate if we need this, or can infer it from cluster. - Architecture string `json:"architecture" jsonschema:"description=Machine architecture of the k8s node(s)"` - - AgentTLS k8s.GeneratedPKI `json:"agentTLS" jsonschema:"PKI certificate information for the agent pods Zarf manages"` - - GitServer GitServerInfo `json:"gitServer" jsonschema:"description=Information about the repository Zarf is configured to use"` - - // TODO: this is where the refactoring needs to be. + GitServer GitServerInfo `json:"gitServer" jsonschema:"description=Information about the repository Zarf is configured to use"` RegistryInfo RegistryInfo `json:"registryInfo" jsonschema:"description=Information about the container registry Zarf is configured to use"` ArtifactServer ArtifactServerInfo `json:"artifactServer" jsonschema:"description=Information about the artifact registry Zarf is configured to use"` LoggingSecret string `json:"loggingSecret" jsonschema:"description=Secret value that the internal Grafana server was seeded with"` @@ -114,12 +107,9 @@ type RegistryInfo struct { PullUsername string `json:"pullUsername" jsonschema:"description=Username of a user with pull-only access to the registry. If not provided for an external registry than the push-user is used"` PullPassword string `json:"pullPassword" jsonschema:"description=Password of a user with pull-only access to the registry. If not provided for an external registry than the push-user is used"` - // TODO: - Address string `json:"address" jsonschema:"description=URL address of the registry"` - - // TODO get rid of NodePort, it lives in Address - //NodePort int `json:"nodePort" jsonschema:"description=Nodeport of the registry. Only needed if the registry is running inside the kubernetes cluster"` - InternalRegistry bool `json:"internalRegistry" jsonschema:"description=Indicates if we are using a registry that Zarf is directly managing"` + Address string `json:"address" jsonschema:"description=URL address of the registry"` + NodePort int `json:"nodePort" jsonschema:"description=Nodeport of the registry. Only needed if the registry is running inside the kubernetes cluster"` + InternalRegistry bool `json:"internalRegistry" jsonschema:"description=Indicates if we are using a registry that Zarf is directly managing"` Secret string `json:"secret" jsonschema:"description=Secret value that the registry was seeded with"` } From 4e977f14eaa0ea49681830a0009cedd7884d235f Mon Sep 17 00:00:00 2001 From: bdw617 Date: Wed, 15 Nov 2023 19:56:15 -0500 Subject: [PATCH 28/30] fixed NPE --- src/internal/packager/template/template.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/internal/packager/template/template.go b/src/internal/packager/template/template.go index 16828448d8..a69bcb8aba 100644 --- a/src/internal/packager/template/template.go +++ b/src/internal/packager/template/template.go @@ -86,10 +86,10 @@ func (values *Values) GetVariables(component types.ZarfComponent) (templateMap m gitInfo := values.config.State.GitServer if values.config.SetVariableMap != nil { - if regInfo.NodePort > 0 { + if regInfo.NodePort > 0 && values.config.SetVariableMap["REGISTRY_NODEPORT"] != nil { values.config.SetVariableMap["REGISTRY_NODEPORT"].Value = fmt.Sprintf("%d", regInfo.NodePort) } - if values.config.State.StorageClass != "" { + if values.config.State.StorageClass != "" && values.config.SetVariableMap["REGISTRY_STORAGE_CLASS"] != nil { values.config.SetVariableMap["REGISTRY_STORAGE_CLASS"].Value = values.config.State.StorageClass } } From 5d21c112b2954a7409fa1812b3eef7f1a66a6e0e Mon Sep 17 00:00:00 2001 From: bdw617 Date: Thu, 16 Nov 2023 09:34:13 -0500 Subject: [PATCH 29/30] must update this to the old way, since the release zarf won't have the config --- .github/workflows/test-upgrade.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-upgrade.yml b/.github/workflows/test-upgrade.yml index 23c10782c6..bf79bd3d71 100644 --- a/.github/workflows/test-upgrade.yml +++ b/.github/workflows/test-upgrade.yml @@ -71,7 +71,7 @@ jobs: # NOTE: "PATH=$PATH" preserves the default user $PATH. This is needed to maintain the version of zarf installed # in a previous step. This test run will the current release to create a K3s cluster. run: | - sudo env "PATH=$PATH" CI=true zarf init --components k3s,git-server,logging --set REGISTRY_NODEPORT=31337 --confirm + sudo env "PATH=$PATH" CI=true zarf init --components k3s,git-server,logging --nodeport 31337 --confirm # Before we run the regular tests we need to aggressively cleanup files to reduce disk pressure - name: Cleanup files From 43acd57ed5b1e0639c66b0bcf4384ef1374632b9 Mon Sep 17 00:00:00 2001 From: bdw617 Date: Thu, 16 Nov 2023 15:03:04 -0500 Subject: [PATCH 30/30] added a documentation line for an External function --- src/pkg/k8s/info.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pkg/k8s/info.go b/src/pkg/k8s/info.go index 9d0c72dd06..13d2074ca2 100644 --- a/src/pkg/k8s/info.go +++ b/src/pkg/k8s/info.go @@ -162,6 +162,7 @@ func MakeLabels(labels map[string]string) string { return strings.Join(out, ",") } +// GetDefaultStorageClass will query the cluster for the default storage class and return the name. func (k *K8s) GetDefaultStorageClass() (string, error) { storageV1Client := k.Clientset.StorageV1() storageClasses, err := storageV1Client.StorageClasses().List(context.TODO(), metav1.ListOptions{})