Use experimental instead of unsafe and add vault token

requirements to helm template

Signed-off-by: jaellio <[email protected]>

Author: jaellio

charts/osm/README.md

@@ -93,6 +93,8 @@ The following table lists the configurable parameters of the osm chart and their
 | osm.enableReconciler | bool | `false` | Enable reconciler for OSM's CRDs and mutating webhook |
 | osm.enforceSingleMesh | bool | `true` | Enforce only deploying one mesh in the cluster |
 | osm.envoyLogLevel | string | `"error"` | Log level for the Envoy proxy sidecar. Non developers should generally never set this value. In production environments the LogLevel should be set to `error` |
+| osm.experimental | object | `{"enableMeshRootCertificate":false}` | Experimental values. Behavior is not supported. |
+| osm.experimental.enableMeshRootCertificate | bool | `false` | Enable the MeshRootCertificate to configure the OSM certificate provider. |
 | osm.featureFlags.enableAsyncProxyServiceMapping | bool | `false` | Enable async proxy-service mapping |
 | osm.featureFlags.enableEgressPolicy | bool | `true` | Enable OSM's Egress policy API. When enabled, fine grained control over Egress (external) traffic is enforced |
 | osm.featureFlags.enableEnvoyActiveHealthChecks | bool | `false` | Enable Envoy active health checks |
@@ -265,14 +267,12 @@ The following table lists the configurable parameters of the osm chart and their
 | osm.tracing.port | int | `9411` | Port of the tracing collector service |
 | osm.tracing.tolerations | list | `[]` | Node tolerations applied to control plane pods. The specified tolerations allow pods to schedule onto nodes with matching taints. |
 | osm.trustDomain | string | `"cluster.local"` | The trust domain to use as part of the common name when requesting new certificates. |
-| osm.unsafe | object | `{"enableMeshRootCertificate":false}` | Unsafe values. Behavior is not supported. |
-| osm.unsafe.enableMeshRootCertificate | bool | `false` | Enable the MeshRootCertificate to configure the OSM certificate provider. |
 | osm.validatorWebhook.webhookConfigurationName | string | `""` | Name of the ValidatingWebhookConfiguration |
 | osm.vault.host | string | `""` | Hashicorp Vault host/service - where Vault is installed |
 | osm.vault.port | int | `8200` | port to use to connect to Vault |
 | osm.vault.protocol | string | `"http"` | protocol to use to connect to Vault |
 | osm.vault.role | string | `"openservicemesh"` | Vault role to be used by Open Service Mesh |
-| osm.vault.secret | object | `{"key":"","name":""}` | The Kubernetes secret storing the Vault token used in OSM |
+| osm.vault.secret | object | `{"key":"","name":""}` | The Kubernetes secret storing the Vault token used in OSM. The secret must be located in the namespace of the OSM installation |
 | osm.vault.secret.key | string | `""` | The Kubernetes secret key with the value bring the Vault token |
 | osm.vault.secret.name | string | `""` | The Kubernetes secret name storing the Vault token used in OSM |
 | osm.vault.token | string | `""` | token that should be used to connect to Vault |

charts/osm/templates/osm-bootstrap-deployment.yaml

@@ -61,7 +61,7 @@ spec:
             "--osm-version", "{{ .Chart.AppVersion }}",
             "--ca-bundle-secret-name", "{{.Values.osm.caBundleSecretName}}",
             "--certificate-manager", "{{.Values.osm.certificateProvider.kind}}",
-            "--enable-mesh-root-certificate={{.Values.osm.unsafe.enableMeshRootCertificate}}",
+            "--enable-mesh-root-certificate={{.Values.osm.experimental.enableMeshRootCertificate}}",
             {{ if eq .Values.osm.certificateProvider.kind "vault" }}
             "--vault-host", "{{.Values.osm.vault.host}}",
             "--vault-port", "{{.Values.osm.vault.port}}",

charts/osm/templates/osm-deployment.yaml

@@ -61,14 +61,18 @@ spec:
             "--validator-webhook-config", "{{ include "osm.validatorWebhookConfigName" . }}",
             "--ca-bundle-secret-name", "{{.Values.osm.caBundleSecretName}}",
             "--certificate-manager", "{{.Values.osm.certificateProvider.kind}}",
-            "--enable-mesh-root-certificate={{.Values.osm.unsafe.enableMeshRootCertificate}}",
+            "--enable-mesh-root-certificate={{.Values.osm.experimental.enableMeshRootCertificate}}",
             {{ if eq .Values.osm.certificateProvider.kind "vault" }}
             "--vault-host", "{{ required "osm.vault.host is required when osm.certificateProvider.kind==vault" .Values.osm.vault.host }}",
             "--vault-port", "{{.Values.osm.vault.port}}",
             "--vault-protocol", "{{.Values.osm.vault.protocol}}",
-            "--vault-token", "{{ .Values.osm.vault.token }}",
-            "--vault-token-secret-name",  "{{ .Values.osm.vault.secret.name }}",
-            "--vault-token-secret-key",  "{{ .Values.osm.vault.secret.key }}",
+            {{ if and (empty .Values.osm.vault.secret.name) (empty .Values.osm.vault.secret.key) }}
+            "--vault-token", "{{ required "osm.vault.token is required when osm.certificateProvider.kind==vault and osm.vault.secret.name and osm.vault.secret.key are empty" .Values.osm.vault.token }}",
+            {{- end }}
+            {{ if empty .Values.osm.vault.token }}
+            "--vault-token-secret-name",  "{{ required "osm.vault.secret.name is required when osm.certificateProvider.kind==vault and osm.vault.token is empty" .Values.osm.vault.secret.name }}",
+            "--vault-token-secret-key",  "{{ required "osm.vault.secret.key is required when osm.certificateProvider.kind==vault and osm.vault.token is empty" .Values.osm.vault.secret.key }}",
+            {{- end }}
             {{- end }}
             "--cert-manager-issuer-name", "{{.Values.osm.certmanager.issuerName}}",
             "--cert-manager-issuer-kind", "{{.Values.osm.certmanager.issuerKind}}",

charts/osm/templates/osm-injector-deployment.yaml

@@ -58,7 +58,7 @@ spec:
             "--webhook-timeout", "{{.Values.osm.injector.webhookTimeoutSeconds}}",
             "--ca-bundle-secret-name", "{{.Values.osm.caBundleSecretName}}",
             "--certificate-manager", "{{.Values.osm.certificateProvider.kind}}",
-            "--enable-mesh-root-certificate={{.Values.osm.unsafe.enableMeshRootCertificate}}",
+            "--enable-mesh-root-certificate={{.Values.osm.experimental.enableMeshRootCertificate}}",
             {{ if eq .Values.osm.certificateProvider.kind "vault" }}
             "--vault-host", "{{.Values.osm.vault.host}}",
             "--vault-port", "{{.Values.osm.vault.port}}",

charts/osm/templates/preset-mesh-root-certificate.yaml

@@ -1,4 +1,4 @@
-{{- if .Values.osm.unsafe.enableMeshRootCertificate }}
+{{- if .Values.osm.experimental.enableMeshRootCertificate }}
 apiVersion: v1
 kind: ConfigMap
 metadata:

charts/osm/values.schema.json

@@ -438,15 +438,15 @@
                         "envoyproxy/envoy-windows:v1.22.1@sha256:92733f8e5beae5c45df204a0e13edbd29e99adf962d1b1c7869b197d85c64bd0"
                     ]
                 },
-                "unsafe": {
-                    "$id": "#/properties/osm/properties/unsafe",
+                "experimental": {
+                    "$id": "#/properties/osm/properties/experimental",
                     "type": "object",
-                    "title": "The unsafe schema",
+                    "title": "The experimental schema",
                     "description": "Parameters that are unsupported by OSM",
                     "additionalProperties": false,
                     "properties": {
                         "enableMeshRootCertificate": {
-                            "$id": "#/properties/osm/properties/unsafe/properties/enableMeshRootCertificate",
+                            "$id": "#/properties/osm/properties/experimental/properties/enableMeshRootCertificate",
                             "type": "boolean",
                             "title": "Enable the MeshRootCertificate",
                             "description": "Using the MeshRootCertificate to configure the OSM certificate provider is not supported",

charts/osm/values.yaml

@@ -167,8 +167,8 @@ osm:
     # The specified tolerations allow pods to schedule onto nodes with matching taints.
     tolerations: []
 
-  # -- Unsafe values. Behavior is not supported.
-  unsafe:
+  # -- Experimental values. Behavior is not supported.
+  experimental:
     # -- Enable the MeshRootCertificate to configure the OSM certificate provider.
     enableMeshRootCertificate: false
 
@@ -196,7 +196,7 @@ osm:
     token: ""
     # -- Vault role to be used by Open Service Mesh
     role: openservicemesh
-    # -- The Kubernetes secret storing the Vault token used in OSM
+    # -- The Kubernetes secret storing the Vault token used in OSM. The secret must be located in the namespace of the OSM installation
     secret:
       # -- The Kubernetes secret name storing the Vault token used in OSM
       name: ""

cmd/cli/install_test.go

@@ -28,13 +28,15 @@ import (
 )
 
 const (
-	testRegistrySecret = "test-registry-secret"
-	testVaultHost      = "vault.osm.svc.cluster.local"
-	testVaultToken     = "token"
-	testChartPath      = "testdata/test-chart"
-	kubeVersionMajor   = 1
-	kubeVersionMinor   = 22
-	kubeVersionPatch   = 9
+	testRegistrySecret  = "test-registry-secret"
+	testVaultHost       = "vault.osm.svc.cluster.local"
+	testVaultToken      = "token"
+	testVaultSecretName = "secret"
+	testVaultSecretKey  = "key"
+	testChartPath       = "testdata/test-chart"
+	kubeVersionMajor    = 1
+	kubeVersionMinor    = 22
+	kubeVersionPatch    = 9
 )
 
 func helmCapabilities() *chartutil.Capabilities {
@@ -181,7 +183,7 @@ var _ = Describe("Running the install command", func() {
 		})
 	})
 
-	Describe("with the vault cert manager", func() {
+	Describe("with the vault cert manager using vault token", func() {
 		var (
 			out    *bytes.Buffer
 			store  *storage.Storage
@@ -258,6 +260,87 @@ var _ = Describe("Running the install command", func() {
 		})
 	})
 
+	Describe("with the vault cert manager using token secret ref", func() {
+		var (
+			out    *bytes.Buffer
+			store  *storage.Storage
+			config *helm.Configuration
+			err    error
+		)
+
+		BeforeEach(func() {
+			out = new(bytes.Buffer)
+			store = storage.Init(driver.NewMemory())
+			if mem, ok := store.Driver.(*driver.Memory); ok {
+				mem.SetNamespace(settings.Namespace())
+			}
+
+			config = &helm.Configuration{
+				Releases: store,
+				KubeClient: &kubefake.PrintingKubeClient{
+					Out: ioutil.Discard},
+				Capabilities: helmCapabilities(),
+				Log:          func(format string, v ...interface{}) {},
+			}
+
+			installCmd := getDefaultInstallCmd(out)
+
+			installCmd.setOptions = []string{
+				"osm.certificateProvider.kind=vault",
+				fmt.Sprintf("osm.vault.host=%s", testVaultHost),
+				"osm.vault.token=",
+				fmt.Sprintf("osm.vault.secret.name=%s", testVaultSecretName),
+				fmt.Sprintf("osm.vault.secret.key=%s", testVaultSecretKey),
+			}
+			err = installCmd.run(config)
+		})
+
+		It("should not error", func() {
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		It("should give a message confirming the successful install", func() {
+			Expect(out.String()).To(Equal("OSM installed successfully in namespace [osm-system] with mesh name [osm]\n"))
+		})
+
+		Context("the Helm release", func() {
+			var (
+				rel *release.Release
+				err error
+			)
+
+			BeforeEach(func() {
+				rel, err = config.Releases.Get(defaultMeshName, 1)
+			})
+
+			It("should not error when retrieved", func() {
+				Expect(err).NotTo(HaveOccurred())
+			})
+
+			It("should have the correct values", func() {
+				expectedValues := getDefaultValues()
+				valuesConfig := []string{
+					fmt.Sprintf("osm.certificateProvider.kind=%s", "vault"),
+					fmt.Sprintf("osm.vault.host=%s", testVaultHost),
+					"osm.vault.token=",
+					fmt.Sprintf("osm.vault.secret.name=%s", testVaultSecretName),
+					fmt.Sprintf("osm.vault.secret.key=%s", testVaultSecretKey),
+				}
+				for _, val := range valuesConfig {
+					// parses Helm strvals line and merges into a map
+					err := strvals.ParseInto(val, expectedValues)
+					Expect(err).NotTo(HaveOccurred())
+				}
+
+				Expect(rel.Config).To(BeEquivalentTo(expectedValues))
+			})
+
+			It("should be installed in the correct namespace", func() {
+				Expect(rel.Namespace).To(Equal(settings.Namespace()))
+			})
+		})
+	})
+
 	Describe("without required vault parameters", func() {
 		var (
 			installCmd installCmd
@@ -290,6 +373,35 @@ var _ = Describe("Running the install command", func() {
 			err := installCmd.run(config)
 			Expect(err.Error()).To(ContainSubstring("osm.vault.host is required"))
 		})
+
+		It("should error when token and token secret key are not set", func() {
+			installCmd.setOptions = append(installCmd.setOptions,
+				"osm.vault.host=my-host",
+				"osm.vault.secret.name=secret",
+			)
+			err := installCmd.run(config)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("osm.vault.secret.key is required"))
+		})
+
+		It("should error when token and token secret name are not set", func() {
+			installCmd.setOptions = append(installCmd.setOptions,
+				"osm.vault.host=my-host",
+				"osm.vault.secret.key=key",
+			)
+			err := installCmd.run(config)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("osm.vault.secret.name is required"))
+		})
+
+		It("should error when token and token secret name and key are not set", func() {
+			installCmd.setOptions = append(installCmd.setOptions,
+				"osm.vault.host=my-host",
+			)
+			err := installCmd.run(config)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("osm.vault.token is required"))
+		})
 	})
 
 	Describe("with the cert-manager certificate manager", func() {

pkg/certificate/providers/config_test.go

@@ -39,13 +39,12 @@ func TestGetCertificateManager(t *testing.T) {
 		expectError bool
 
 		// params
-		kubeClient             kubernetes.Interface
-		restConfig             *rest.Config
-		cfg                    configurator.Configurator
-		providerNamespace      string
-		options                Options
-		msgBroker              *messaging.Broker
-		informerCollectionFunc func(testCase) (*informers.InformerCollection, error)
+		kubeClient        kubernetes.Interface
+		restConfig        *rest.Config
+		cfg               configurator.Configurator
+		providerNamespace string
+		options           Options
+		msgBroker         *messaging.Broker
 	}
 	testCases := []testCase{
 		{