ref(chart): push vault validation to chart (#4513)

This change moves the validation for the `osm.vault.host` and
`osm.vault.token` values from the CLI to the chart so the same
validation is performed when using `helm` directly.

Part of #2147

Signed-off-by: Jon Huhn <johuhn@microsoft.com>

charts/osm/templates/osm-deployment.yaml

@@ -61,9 +61,9 @@ spec:
             "--ca-bundle-secret-name", "{{.Values.osm.caBundleSecretName}}",
             "--certificate-manager", "{{.Values.osm.certificateProvider.kind}}",
             {{ if eq .Values.osm.certificateProvider.kind "vault" }}
-            "--vault-host", "{{.Values.osm.vault.host}}",
+            "--vault-host", "{{ required "osm.vault.host is required when osm.certificateProvider.kind==vault" .Values.osm.vault.host }}",
             "--vault-protocol", "{{.Values.osm.vault.protocol}}",
-            "--vault-token", "{{.Values.osm.vault.token}}",
+            "--vault-token", "{{ required "osm.vault.token is required when osm.certificateProvider.kind==vault" .Values.osm.vault.token }}",
             {{- end }}
             "--cert-manager-issuer-name", "{{.Values.osm.certmanager.issuerName}}",
             "--cert-manager-issuer-kind", "{{.Values.osm.certmanager.issuerKind}}",

cmd/cli/install.go

@@ -249,28 +249,6 @@ func (i *installCmd) validateOptions() error {
 		return errors.Wrap(err, "invalid format for --set")
 	}
 
-	if setOptions, ok := s["osm"].(map[string]interface{}); ok {
-		// if the certificate provider kind is vault, ensure all relevant information (vault-host, vault-token) is available
-		if certProvider, ok := setOptions["certificateProvider"].(map[string]interface{}); ok && certProvider["kind"] == "vault" {
-			var missingFields []string
-			vaultOptions, ok := setOptions["vault"].(map[string]interface{})
-			if !ok {
-				missingFields = append(missingFields, "osm.vault.host", "osm.vault.token")
-			} else {
-				if vaultOptions["host"] == nil || vaultOptions["host"] == "" {
-					missingFields = append(missingFields, "osm.vault.host")
-				}
-				if vaultOptions["token"] == nil || vaultOptions["token"] == "" {
-					missingFields = append(missingFields, "osm.vault.token")
-				}
-			}
-
-			if len(missingFields) != 0 {
-				return errors.Errorf("Missing arguments for certificate-manager vault: %v", missingFields)
-			}
-		}
-	}
-
 	return nil
 }
 

cmd/cli/install_test.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"path/filepath"
 	"testing"
 
 	. "github.com/onsi/ginkgo"
@@ -240,15 +241,13 @@ var _ = Describe("Running the install command", func() {
 
 	Describe("without required vault parameters", func() {
 		var (
-			out    *bytes.Buffer
-			store  *storage.Storage
-			config *helm.Configuration
-			err    error
+			installCmd installCmd
+			config     *helm.Configuration
 		)
 
 		BeforeEach(func() {
-			out = new(bytes.Buffer)
-			store = storage.Init(driver.NewMemory())
+			out := new(bytes.Buffer)
+			store := storage.Init(driver.NewMemory())
 			if mem, ok := store.Driver.(*driver.Memory); ok {
 				mem.SetNamespace(settings.Namespace())
 			}
@@ -261,15 +260,24 @@ var _ = Describe("Running the install command", func() {
 				Log:          func(format string, v ...interface{}) {},
 			}
 
-			installCmd := getDefaultInstallCmd(out)
+			installCmd = getDefaultInstallCmd(out)
+			installCmd.chartPath = filepath.FromSlash("../../charts/osm")
 			installCmd.setOptions = []string{
 				"osm.certificateProvider.kind=vault",
 			}
-			err = installCmd.run(config)
 		})
 
-		It("should error", func() {
-			Expect(err).To(MatchError("Missing arguments for certificate-manager vault: [osm.vault.host osm.vault.token]"))
+		It("should error when host isn't set", func() {
+			err := installCmd.run(config)
+			Expect(err.Error()).To(ContainSubstring("osm.vault.host is required"))
+		})
+
+		It("should error when token isn't set", func() {
+			installCmd.setOptions = append(installCmd.setOptions,
+				"osm.vault.host=my-host",
+			)
+			err := installCmd.run(config)
+			Expect(err.Error()).To(ContainSubstring("osm.vault.token is required"))
 		})
 	})