diff --git a/charts/osm/README.md b/charts/osm/README.md index 2a97f7a33a..6b1e528c7a 100644 --- a/charts/osm/README.md +++ b/charts/osm/README.md @@ -97,6 +97,7 @@ The following table lists the configurable parameters of the osm chart and their | 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 | | osm.featureFlags.enableIngressBackendPolicy | bool | `true` | Enables OSM's IngressBackend policy API. When enabled, OSM will use the IngressBackend API allow ingress traffic to mesh backends | +| osm.featureFlags.enableMeshRootCertificate | bool | `false` | Enable the MeshRootCertificate to configure the OSM certificate provider | | osm.featureFlags.enableRetryPolicy | bool | `false` | Enable Retry Policy for automatic request retries | | osm.featureFlags.enableSnapshotCacheMode | bool | `false` | Enables SnapshotCache feature for Envoy xDS server. | | osm.featureFlags.enableWASMStats | bool | `true` | Enable extra Envoy statistics generated by a custom WASM extension | @@ -270,7 +271,7 @@ The following table lists the configurable parameters of the osm chart and their | 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 | diff --git a/charts/osm/templates/osm-bootstrap-deployment.yaml b/charts/osm/templates/osm-bootstrap-deployment.yaml index 80583ca29d..1e2d7aea2d 100644 --- a/charts/osm/templates/osm-bootstrap-deployment.yaml +++ b/charts/osm/templates/osm-bootstrap-deployment.yaml @@ -61,11 +61,14 @@ spec: "--osm-version", "{{ .Chart.AppVersion }}", "--ca-bundle-secret-name", "{{.Values.osm.caBundleSecretName}}", "--certificate-manager", "{{.Values.osm.certificateProvider.kind}}", + "--enable-mesh-root-certificate={{.Values.osm.featureFlags.enableMeshRootCertificate}}", {{ if eq .Values.osm.certificateProvider.kind "vault" }} "--vault-host", "{{.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 }}", {{- end }} "--cert-manager-issuer-name", "{{.Values.osm.certmanager.issuerName}}", "--cert-manager-issuer-kind", "{{.Values.osm.certmanager.issuerKind}}", diff --git a/charts/osm/templates/osm-deployment.yaml b/charts/osm/templates/osm-deployment.yaml index a32855f8ae..b05c1edb52 100644 --- a/charts/osm/templates/osm-deployment.yaml +++ b/charts/osm/templates/osm-deployment.yaml @@ -61,11 +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.featureFlags.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", "{{ required "osm.vault.token is required when osm.certificateProvider.kind==vault" .Values.osm.vault.token }}", + {{ 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}}", diff --git a/charts/osm/templates/osm-injector-deployment.yaml b/charts/osm/templates/osm-injector-deployment.yaml index 44afe094df..fd25c4c946 100644 --- a/charts/osm/templates/osm-injector-deployment.yaml +++ b/charts/osm/templates/osm-injector-deployment.yaml @@ -58,11 +58,14 @@ 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.featureFlags.enableMeshRootCertificate}}", {{ if eq .Values.osm.certificateProvider.kind "vault" }} "--vault-host", "{{.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 }}", {{- end }} "--cert-manager-issuer-name", "{{.Values.osm.certmanager.issuerName}}", "--cert-manager-issuer-kind", "{{.Values.osm.certmanager.issuerKind}}", diff --git a/charts/osm/templates/preset-mesh-root-certificate.yaml b/charts/osm/templates/preset-mesh-root-certificate.yaml index ab97a53ac6..c034ec3531 100644 --- a/charts/osm/templates/preset-mesh-root-certificate.yaml +++ b/charts/osm/templates/preset-mesh-root-certificate.yaml @@ -1,3 +1,4 @@ +{{- if .Values.osm.featureFlags.enableMeshRootCertificate }} apiVersion: v1 kind: ConfigMap metadata: @@ -42,3 +43,4 @@ data: {{- end}} } } +{{- end}} diff --git a/charts/osm/values.schema.json b/charts/osm/values.schema.json index 7ed332e2cb..2c1e7f6be7 100644 --- a/charts/osm/values.schema.json +++ b/charts/osm/values.schema.json @@ -983,7 +983,8 @@ "enableIngressBackendPolicy", "enableEnvoyActiveHealthChecks", "enableSnapshotCacheMode", - "enableRetryPolicy" + "enableRetryPolicy", + "enableMeshRootCertificate" ], "properties": { "enableWASMStats": { @@ -1048,6 +1049,15 @@ "examples": [ true ] + }, + "enableMeshRootCertificate": { + "$id": "#/properties/osm/properties/featureFlags/properties/enableMeshRootCertificate", + "type": "boolean", + "title": "Enable the MeshRootCertificate", + "description": "Enable the MeshRootCertificate to configure the OSM certificate provider.", + "examples": [ + false + ] } }, "additionalProperties": false diff --git a/charts/osm/values.yaml b/charts/osm/values.yaml index fcef163188..eedf29a2a9 100644 --- a/charts/osm/values.yaml +++ b/charts/osm/values.yaml @@ -191,7 +191,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: "" @@ -482,6 +482,8 @@ osm: enableSnapshotCacheMode: false # -- Enable Retry Policy for automatic request retries enableRetryPolicy: false + # -- Enable the MeshRootCertificate to configure the OSM certificate provider + enableMeshRootCertificate: false # -- Node tolerations applied to control plane pods. # The specified tolerations allow pods to schedule onto nodes with matching taints. @@ -547,7 +549,7 @@ osm: # # -- OSM's preinstall hook parameters - + preinstall: ## Affinity settings for pod assignment ## Ref: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/ @@ -580,7 +582,7 @@ osm: ## Affinity settings for pod assignment ## Ref: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/ - affinity: + affinity: nodeAffinity: requiredDuringSchedulingIgnoredDuringExecution: nodeSelectorTerms: diff --git a/cmd/cli/install_test.go b/cmd/cli/install_test.go index d67800e698..ed9014c194 100644 --- a/cmd/cli/install_test.go +++ b/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 @@ -291,11 +374,32 @@ var _ = Describe("Running the install command", func() { Expect(err.Error()).To(ContainSubstring("osm.vault.host is required")) }) - It("should error when token isn't set", func() { + 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")) }) }) diff --git a/cmd/osm-bootstrap/osm-bootstrap.go b/cmd/osm-bootstrap/osm-bootstrap.go index 3ece04d99b..6eb419f70f 100644 --- a/cmd/osm-bootstrap/osm-bootstrap.go +++ b/cmd/osm-bootstrap/osm-bootstrap.go @@ -28,6 +28,7 @@ import ( "k8s.io/kubectl/pkg/util" configv1alpha2 "github.com/openservicemesh/osm/pkg/apis/config/v1alpha2" + "github.com/openservicemesh/osm/pkg/certificate" configClientset "github.com/openservicemesh/osm/pkg/gen/client/config/clientset/versioned" "github.com/openservicemesh/osm/pkg/health" @@ -63,7 +64,8 @@ var ( meshName string osmVersion string - certProviderKind string + certProviderKind string + enableMeshRootCertificate bool tresorOptions providers.TresorOptions vaultOptions providers.VaultOptions @@ -94,6 +96,7 @@ func init() { // Generic certificate manager/provider options flags.StringVar(&certProviderKind, "certificate-manager", providers.TresorKind.String(), fmt.Sprintf("Certificate manager, one of [%v]", providers.ValidCertificateProviders)) + flags.BoolVar(&enableMeshRootCertificate, "enable-mesh-root-certificate", false, "Enable unsupported MeshRootCertificate to create the OSM Certificate Manager") flags.StringVar(&caBundleSecretName, "ca-bundle-secret-name", "", "Name of the Kubernetes Secret for the OSM CA bundle") // Vault certificate manager/provider options @@ -102,6 +105,8 @@ func init() { flags.StringVar(&vaultOptions.VaultToken, "vault-token", "", "Secret token for the the Hashi Vault") flags.StringVar(&vaultOptions.VaultRole, "vault-role", "openservicemesh", "Name of the Vault role dedicated to Open Service Mesh") flags.IntVar(&vaultOptions.VaultPort, "vault-port", 8200, "Port of the Hashi Vault") + flags.StringVar(&vaultOptions.VaultTokenSecretName, "vault-token-secret-name", "", "Name of the secret storing the Vault token used in OSM") + flags.StringVar(&vaultOptions.VaultTokenSecretKey, "vault-token-secret-key", "", "Key for the vault token used in OSM") // Cert-manager certificate manager/provider options flags.StringVar(&certManagerOptions.IssuerName, "cert-manager-issuer-name", "osm-ca", "cert-manager issuer name") @@ -122,6 +127,7 @@ func getCertOptions() (providers.Options, error) { tresorOptions.SecretName = caBundleSecretName return tresorOptions, nil case providers.VaultKind: + vaultOptions.VaultTokenSecretNamespace = osmNamespace return vaultOptions, nil case providers.CertManagerKind: return certManagerOptions, nil @@ -171,10 +177,12 @@ func main() { return } - err = bootstrap.ensureMeshRootCertificate() - if err != nil { - log.Fatal().Err(err).Msgf("Error setting up default MeshRootCertificate %s from ConfigMap %s", meshRootCertificateName, presetMeshRootCertificateName) - return + if enableMeshRootCertificate { + err = bootstrap.ensureMeshRootCertificate() + if err != nil { + log.Fatal().Err(err).Msgf("Error setting up default MeshRootCertificate %s from ConfigMap %s", meshRootCertificateName, presetMeshRootCertificateName) + return + } } err = bootstrap.initiatilizeKubernetesEventsRecorder() @@ -214,10 +222,19 @@ func main() { log.Fatal().Err(err).Msg("Error getting certificate options") } - certManager, err := providers.NewCertificateManager(ctx, kubeClient, kubeConfig, cfg, osmNamespace, certOpts, msgBroker, informerCollection, 5*time.Second) - if err != nil { - events.GenericEventRecorder().FatalEvent(err, events.InvalidCertificateManager, - "Error initializing certificate manager of kind %s", certProviderKind) + var certManager *certificate.Manager + if enableMeshRootCertificate { + certManager, err = providers.NewCertificateManagerFromMRC(ctx, kubeClient, kubeConfig, cfg, osmNamespace, certOpts, msgBroker, informerCollection, 5*time.Second) + if err != nil { + events.GenericEventRecorder().FatalEvent(err, events.InvalidCertificateManager, + "Error initializing certificate manager of kind %s from MRC", certProviderKind) + } + } else { + certManager, err = providers.NewCertificateManager(ctx, kubeClient, kubeConfig, cfg, osmNamespace, certOpts, msgBroker, 5*time.Second) + if err != nil { + events.GenericEventRecorder().FatalEvent(err, events.InvalidCertificateManager, + "Error initializing certificate manager of kind %s", certProviderKind) + } } // Initialize the crd conversion webhook server to support the conversion of OSM's CRDs @@ -414,7 +431,7 @@ func (b *bootstrap) createMeshRootCertificate() error { _, err = b.configClient.ConfigV1alpha2().MeshRootCertificates(b.namespace).UpdateStatus(context.Background(), createdMRC, metav1.UpdateOptions{}) if apierrors.IsAlreadyExists(err) { - log.Info().Msgf("MeshRootCertificate statys already exists in %s. Skip creating.", b.namespace) + log.Info().Msgf("MeshRootCertificate status already exists in %s. Skip creating.", b.namespace) } if err != nil { @@ -440,9 +457,6 @@ func buildMeshRootCertificate(presetMeshRootCertificateConfigMap *corev1.ConfigM }, ObjectMeta: metav1.ObjectMeta{ Name: meshRootCertificateName, - Annotations: map[string]string{ - constants.MRCVersionAnnotation: "0", - }, }, Spec: presetMeshRootCertificateSpec, } diff --git a/cmd/osm-controller/osm-controller.go b/cmd/osm-controller/osm-controller.go index cf69340b76..b23966de0e 100644 --- a/cmd/osm-controller/osm-controller.go +++ b/cmd/osm-controller/osm-controller.go @@ -74,7 +74,8 @@ var ( osmMeshConfigName string osmVersion string - certProviderKind string + certProviderKind string + enableMeshRootCertificate bool tresorOptions providers.TresorOptions vaultOptions providers.VaultOptions @@ -102,6 +103,7 @@ func init() { // Generic certificate manager/provider options flags.StringVar(&certProviderKind, "certificate-manager", providers.TresorKind.String(), fmt.Sprintf("Certificate manager, one of [%v]", providers.ValidCertificateProviders)) + flags.BoolVar(&enableMeshRootCertificate, "enable-mesh-root-certificate", false, "Enable unsupported MeshRootCertificate to create the OSM Certificate Manager") flags.StringVar(&caBundleSecretName, "ca-bundle-secret-name", "", "Name of the Kubernetes Secret for the OSM CA bundle") // Vault certificate manager/provider options @@ -110,6 +112,8 @@ func init() { flags.StringVar(&vaultOptions.VaultToken, "vault-token", "", "Secret token for the the Hashi Vault") flags.StringVar(&vaultOptions.VaultRole, "vault-role", "openservicemesh", "Name of the Vault role dedicated to Open Service Mesh") flags.IntVar(&vaultOptions.VaultPort, "vault-port", 8200, "Port of the Hashi Vault") + flags.StringVar(&vaultOptions.VaultTokenSecretName, "vault-token-secret-name", "", "Name of the secret storing the Vault token used in OSM") + flags.StringVar(&vaultOptions.VaultTokenSecretKey, "vault-token-secret-key", "", "Key for the vault token used in OSM") // Cert-manager certificate manager/provider options flags.StringVar(&certManagerOptions.IssuerName, "cert-manager-issuer-name", "osm-ca", "cert-manager issuer name") @@ -131,6 +135,7 @@ func getCertOptions() (providers.Options, error) { tresorOptions.SecretName = caBundleSecretName return tresorOptions, nil case providers.VaultKind: + vaultOptions.VaultTokenSecretNamespace = osmNamespace return vaultOptions, nil case providers.CertManagerKind: return certManagerOptions, nil @@ -207,12 +212,21 @@ func main() { } // Intitialize certificate manager/provider - certManager, err := providers.NewCertificateManager(ctx, kubeClient, kubeConfig, cfg, osmNamespace, - certOpts, msgBroker, informerCollection, 5*time.Second) - - if err != nil { - events.GenericEventRecorder().FatalEvent(err, events.InvalidCertificateManager, - "Error fetching certificate manager of kind %s", certProviderKind) + var certManager *certificate.Manager + if enableMeshRootCertificate { + certManager, err = providers.NewCertificateManagerFromMRC(ctx, kubeClient, kubeConfig, cfg, osmNamespace, + certOpts, msgBroker, informerCollection, 5*time.Second) + if err != nil { + events.GenericEventRecorder().FatalEvent(err, events.InvalidCertificateManager, + "Error fetching certificate manager of kind %s from MRC", certProviderKind) + } + } else { + certManager, err = providers.NewCertificateManager(ctx, kubeClient, kubeConfig, cfg, osmNamespace, + certOpts, msgBroker, 5*time.Second) + if err != nil { + events.GenericEventRecorder().FatalEvent(err, events.InvalidCertificateManager, + "Error fetching certificate manager of kind %s", certProviderKind) + } } kubeProvider := kube.NewClient(k8sClient, cfg) diff --git a/cmd/osm-injector/osm-injector.go b/cmd/osm-injector/osm-injector.go index 5e67f7afeb..b950a0f67f 100644 --- a/cmd/osm-injector/osm-injector.go +++ b/cmd/osm-injector/osm-injector.go @@ -25,6 +25,7 @@ import ( _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" "k8s.io/client-go/tools/clientcmd" + "github.com/openservicemesh/osm/pkg/certificate" configClientset "github.com/openservicemesh/osm/pkg/gen/client/config/clientset/versioned" policyClientset "github.com/openservicemesh/osm/pkg/gen/client/policy/clientset/versioned" "github.com/openservicemesh/osm/pkg/health" @@ -57,7 +58,8 @@ var ( webhookTimeout int32 osmVersion string - certProviderKind string + certProviderKind string + enableMeshRootCertificate bool enableReconciler bool @@ -87,6 +89,7 @@ func init() { // Generic certificate manager/provider options flags.StringVar(&certProviderKind, "certificate-manager", providers.TresorKind.String(), fmt.Sprintf("Certificate manager, one of [%v]", providers.ValidCertificateProviders)) + flags.BoolVar(&enableMeshRootCertificate, "enable-mesh-root-certificate", false, "Enable unsupported MeshRootCertificate to create the OSM Certificate Manager") flags.StringVar(&caBundleSecretName, "ca-bundle-secret-name", "", "Name of the Kubernetes Secret for the OSM CA bundle") // Vault certificate manager/provider options @@ -95,6 +98,8 @@ func init() { flags.StringVar(&vaultOptions.VaultToken, "vault-token", "", "Secret token for the the Hashi Vault") flags.StringVar(&vaultOptions.VaultRole, "vault-role", "openservicemesh", "Name of the Vault role dedicated to Open Service Mesh") flags.IntVar(&vaultOptions.VaultPort, "vault-port", 8200, "Port of the Hashi Vault") + flags.StringVar(&vaultOptions.VaultTokenSecretName, "vault-token-secret-name", "", "Name of the secret storing the Vault token used in OSM") + flags.StringVar(&vaultOptions.VaultTokenSecretKey, "vault-token-secret-key", "", "Key for the vault token used in OSM") // Cert-manager certificate manager/provider options flags.StringVar(&certManagerOptions.IssuerName, "cert-manager-issuer-name", "osm-ca", "cert-manager issuer name") @@ -117,6 +122,7 @@ func getCertOptions() (providers.Options, error) { tresorOptions.SecretName = caBundleSecretName return tresorOptions, nil case providers.VaultKind: + vaultOptions.VaultTokenSecretNamespace = osmNamespace return vaultOptions, nil case providers.CertManagerKind: return certManagerOptions, nil @@ -200,11 +206,21 @@ func main() { log.Fatal().Err(err).Msg("Error getting certificate options") } // Intitialize certificate manager/provider - certManager, err := providers.NewCertificateManager(ctx, kubeClient, kubeConfig, cfg, osmNamespace, - certOpts, msgBroker, informerCollection, 5*time.Second) - if err != nil { - events.GenericEventRecorder().FatalEvent(err, events.InvalidCertificateManager, - "Error initializing certificate manager of kind %s", certProviderKind) + var certManager *certificate.Manager + if enableMeshRootCertificate { + certManager, err = providers.NewCertificateManagerFromMRC(ctx, kubeClient, kubeConfig, cfg, osmNamespace, + certOpts, msgBroker, informerCollection, 5*time.Second) + if err != nil { + events.GenericEventRecorder().FatalEvent(err, events.InvalidCertificateManager, + "Error initializing certificate manager of kind %s from MRC", certProviderKind) + } + } else { + certManager, err = providers.NewCertificateManager(ctx, kubeClient, kubeConfig, cfg, osmNamespace, + certOpts, msgBroker, 5*time.Second) + if err != nil { + events.GenericEventRecorder().FatalEvent(err, events.InvalidCertificateManager, + "Error initializing certificate manager of kind %s", certProviderKind) + } } // Initialize the sidecar injector webhook diff --git a/pkg/certificate/fake_manager.go b/pkg/certificate/fake_manager.go index a622dce654..711496fd58 100644 --- a/pkg/certificate/fake_manager.go +++ b/pkg/certificate/fake_manager.go @@ -20,8 +20,8 @@ var ( type fakeMRCClient struct{} -func (c *fakeMRCClient) GetCertIssuerForMRC(mrc *v1alpha2.MeshRootCertificate) (Issuer, pem.RootCertificate, string, error) { - return &fakeIssuer{}, pem.RootCertificate("rootCA"), "fake-issuer-1", nil +func (c *fakeMRCClient) GetCertIssuerForMRC(mrc *v1alpha2.MeshRootCertificate) (Issuer, pem.RootCertificate, error) { + return &fakeIssuer{}, pem.RootCertificate("rootCA"), nil } // List returns the single, pre-generated MRC. It is intended to implement the certificate.MRCClient interface. @@ -43,9 +43,6 @@ func (c *fakeMRCClient) Watch(ctx context.Context) (<-chan MRCEvent, error) { ObjectMeta: metav1.ObjectMeta{ Name: "osm-mesh-root-certificate", Namespace: "osm-system", - Annotations: map[string]string{ - constants.MRCVersionAnnotation: "0", - }, }, Spec: v1alpha2.MeshRootCertificateSpec{ Provider: v1alpha2.ProviderSpec{ diff --git a/pkg/certificate/manager.go b/pkg/certificate/manager.go index cb0492aa7f..073d296954 100644 --- a/pkg/certificate/manager.go +++ b/pkg/certificate/manager.go @@ -121,12 +121,12 @@ func (m *Manager) handleMRCEvent(mrcClient MRCClient, event MRCEvent) error { return nil } - client, ca, clientID, err := mrcClient.GetCertIssuerForMRC(mrc) + client, ca, err := mrcClient.GetCertIssuerForMRC(mrc) if err != nil { return err } - c := &issuer{Issuer: client, ID: clientID, CertificateAuthority: ca} + c := &issuer{Issuer: client, ID: mrc.Name, CertificateAuthority: ca} switch { case mrc.Status.State == constants.MRCStateActive: m.mu.Lock() diff --git a/pkg/certificate/providers/config.go b/pkg/certificate/providers/config.go index f76c1f0161..c64fb6d6c5 100644 --- a/pkg/certificate/providers/config.go +++ b/pkg/certificate/providers/config.go @@ -41,106 +41,97 @@ var getCA func(certificate.Issuer) (pem.RootCertificate, error) = func(i certifi return cert.GetIssuingCA(), nil } -// NewCertificateManager returns a new certificate manager, with an MRC compat client. -// TODO(4713): Use an informer behind a feature flag. +// NewCertificateManager returns a new certificate manager with a MRC compat client. +// TODO(4713): Remove and use NewCertificateManagerFromMRC func NewCertificateManager(ctx context.Context, kubeClient kubernetes.Interface, kubeConfig *rest.Config, cfg configurator.Configurator, - providerNamespace string, options Options, msgBroker *messaging.Broker, ic *informers.InformerCollection, checkInterval time.Duration) (*certificate.Manager, error) { - if err := options.Validate(); err != nil { + providerNamespace string, option Options, msgBroker *messaging.Broker, checkInterval time.Duration) (*certificate.Manager, error) { + if err := option.Validate(); err != nil { return nil, err } - var mrcClient certificate.MRCClient - if ic == nil || len(ic.List(informers.InformerKeyMeshRootCertificate)) == 0 { - // no MRCs detected; use the compat client - c := &MRCCompatClient{ - MRCProviderGenerator: MRCProviderGenerator{ - kubeClient: kubeClient, - kubeConfig: kubeConfig, - KeyBitSize: cfg.GetCertKeyBitSize(), - caExtractorFunc: getCA, + mrcClient := &MRCCompatClient{ + MRCProviderGenerator: MRCProviderGenerator{ + kubeClient: kubeClient, + kubeConfig: kubeConfig, + KeyBitSize: cfg.GetCertKeyBitSize(), + caExtractorFunc: getCA, + }, + mrc: &v1alpha2.MeshRootCertificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "legacy-compat", + Namespace: providerNamespace, }, - mrc: &v1alpha2.MeshRootCertificate{ - ObjectMeta: metav1.ObjectMeta{ - Name: "legacy-compat", - Namespace: providerNamespace, - Annotations: map[string]string{ - constants.MRCVersionAnnotation: "legacy-compat", - }, - }, - Spec: v1alpha2.MeshRootCertificateSpec{ - Provider: options.AsProviderSpec(), - TrustDomain: "cluster.local", - }, - Status: v1alpha2.MeshRootCertificateStatus{ - State: constants.MRCStateActive, - }, + Spec: v1alpha2.MeshRootCertificateSpec{ + Provider: option.AsProviderSpec(), + TrustDomain: "cluster.local", }, - } - // TODO(#4745): Remove after deprecating the osm.vault.token option. - if vaultOption, ok := options.(VaultOptions); ok { - c.MRCProviderGenerator.DefaultVaultToken = vaultOption.VaultToken - } - mrcClient = c - } else { - // we have MRCs; use the MRC Client - c := &MRCComposer{ - MRCProviderGenerator: MRCProviderGenerator{ - kubeClient: kubeClient, - kubeConfig: kubeConfig, - KeyBitSize: cfg.GetCertKeyBitSize(), - caExtractorFunc: getCA, + Status: v1alpha2.MeshRootCertificateStatus{ + State: constants.MRCStateActive, }, - informerCollection: ic, - } - // TODO(#4745): Remove after deprecating the osm.vault.token option. - if vaultOption, ok := options.(VaultOptions); ok { - c.MRCProviderGenerator.DefaultVaultToken = vaultOption.VaultToken - } + }, + } + // TODO(#4745): Remove after deprecating the osm.vault.token option. + if vaultOption, ok := option.(VaultOptions); ok { + mrcClient.MRCProviderGenerator.DefaultVaultToken = vaultOption.VaultToken + } + + return certificate.NewManager(ctx, mrcClient, cfg.GetServiceCertValidityPeriod, cfg.GetIngressGatewayCertValidityPeriod, msgBroker, checkInterval) +} - mrcClient = c +// NewCertificateManagerFromMRC returns a new certificate manager. +func NewCertificateManagerFromMRC(ctx context.Context, kubeClient kubernetes.Interface, kubeConfig *rest.Config, cfg configurator.Configurator, + providerNamespace string, option Options, msgBroker *messaging.Broker, ic *informers.InformerCollection, checkInterval time.Duration) (*certificate.Manager, error) { + if err := option.Validate(); err != nil { + return nil, err + } + + mrcClient := &MRCComposer{ + MRCProviderGenerator: MRCProviderGenerator{ + kubeClient: kubeClient, + kubeConfig: kubeConfig, + KeyBitSize: cfg.GetCertKeyBitSize(), + caExtractorFunc: getCA, + }, + informerCollection: ic, + } + // TODO(#4745): Remove after deprecating the osm.vault.token option. + if vaultOption, ok := option.(VaultOptions); ok { + mrcClient.MRCProviderGenerator.DefaultVaultToken = vaultOption.VaultToken } return certificate.NewManager(ctx, mrcClient, cfg.GetServiceCertValidityPeriod, cfg.GetIngressGatewayCertValidityPeriod, msgBroker, checkInterval) } // GetCertIssuerForMRC returns a certificate.Issuer generated from the provided MRC. -func (c *MRCProviderGenerator) GetCertIssuerForMRC(mrc *v1alpha2.MeshRootCertificate) (certificate.Issuer, pem.RootCertificate, string, error) { +func (c *MRCProviderGenerator) GetCertIssuerForMRC(mrc *v1alpha2.MeshRootCertificate) (certificate.Issuer, pem.RootCertificate, error) { p := mrc.Spec.Provider var issuer certificate.Issuer - var id string var err error switch { case p.Tresor != nil: - issuer, id, err = c.getTresorOSMCertificateManager(mrc) + issuer, err = c.getTresorOSMCertificateManager(mrc) case p.Vault != nil: - issuer, id, err = c.getHashiVaultOSMCertificateManager(mrc) + issuer, err = c.getHashiVaultOSMCertificateManager(mrc) case p.CertManager != nil: - issuer, id, err = c.getCertManagerOSMCertificateManager(mrc) + issuer, err = c.getCertManagerOSMCertificateManager(mrc) default: - return nil, nil, "", fmt.Errorf("Unknown certificate provider: %+v", p) + return nil, nil, fmt.Errorf("Unknown certificate provider: %+v", p) } if err != nil { - return nil, nil, "", err + return nil, nil, err } ca, err := c.caExtractorFunc(issuer) if err != nil { - return nil, nil, "", fmt.Errorf("error generating init cert: %w", err) + return nil, nil, fmt.Errorf("error generating init cert: %w", err) } - return issuer, ca, id, nil -} - -func getMRCID(mrc *v1alpha2.MeshRootCertificate) (string, error) { - if mrc.Annotations == nil || mrc.Annotations[constants.MRCVersionAnnotation] == "" { - return "", fmt.Errorf("no annotation found for MRC %s/%s, expected annotation %s", mrc.Namespace, mrc.Name, constants.MRCVersionAnnotation) - } - return mrc.Annotations[constants.MRCVersionAnnotation], nil + return issuer, ca, nil } // getTresorOSMCertificateManager returns a certificate manager instance with Tresor as the certificate provider -func (c *MRCProviderGenerator) getTresorOSMCertificateManager(mrc *v1alpha2.MeshRootCertificate) (certificate.Issuer, string, error) { +func (c *MRCProviderGenerator) getTresorOSMCertificateManager(mrc *v1alpha2.MeshRootCertificate) (certificate.Issuer, error) { var err error var rootCert *certificate.Certificate @@ -150,20 +141,20 @@ func (c *MRCProviderGenerator) getTresorOSMCertificateManager(mrc *v1alpha2.Mesh // Regardless of success or failure, all instances can proceed to load the same CA. rootCert, err = tresor.NewCA(constants.CertificationAuthorityCommonName, constants.CertificationAuthorityRootValidityPeriod, rootCertCountry, rootCertLocality, rootCertOrganization) if err != nil { - return nil, "", errors.New("Failed to create new Certificate Authority with cert issuer tresor") + return nil, errors.New("Failed to create new Certificate Authority with cert issuer tresor") } if rootCert.GetPrivateKey() == nil { - return nil, "", errors.New("Root cert does not have a private key") + return nil, errors.New("Root cert does not have a private key") } rootCert, err = k8s.GetCertificateFromSecret(mrc.Namespace, mrc.Spec.Provider.Tresor.CA.SecretRef.Name, rootCert, c.kubeClient) if err != nil { - return nil, "", fmt.Errorf("Failed to synchronize certificate on Secrets API : %w", err) + return nil, fmt.Errorf("Failed to synchronize certificate on Secrets API : %w", err) } if rootCert.GetPrivateKey() == nil { - return nil, "", fmt.Errorf("Root cert does not have a private key: %w", certificate.ErrInvalidCertSecret) + return nil, fmt.Errorf("Root cert does not have a private key: %w", certificate.ErrInvalidCertSecret) } tresorClient, err := tresor.New( @@ -172,18 +163,14 @@ func (c *MRCProviderGenerator) getTresorOSMCertificateManager(mrc *v1alpha2.Mesh c.KeyBitSize, ) if err != nil { - return nil, "", fmt.Errorf("failed to instantiate Tresor as a Certificate Manager: %w", err) + return nil, fmt.Errorf("failed to instantiate Tresor as a Certificate Manager: %w", err) } - id, err := getMRCID(mrc) - if err != nil { - return nil, "", err - } - return tresorClient, id, nil + return tresorClient, nil } // getHashiVaultOSMCertificateManager returns a certificate manager instance with Hashi Vault as the certificate provider -func (c *MRCProviderGenerator) getHashiVaultOSMCertificateManager(mrc *v1alpha2.MeshRootCertificate) (certificate.Issuer, string, error) { +func (c *MRCProviderGenerator) getHashiVaultOSMCertificateManager(mrc *v1alpha2.MeshRootCertificate) (certificate.Issuer, error) { provider := mrc.Spec.Provider.Vault // A Vault address would have the following shape: "http://vault.default.svc.cluster.local:8200" @@ -196,7 +183,7 @@ func (c *MRCProviderGenerator) getHashiVaultOSMCertificateManager(mrc *v1alpha2. log.Debug().Msgf("Attempting to get Vault token from secret %s", provider.Token.SecretKeyRef.Name) vaultToken, err = getHashiVaultOSMToken(&provider.Token.SecretKeyRef, c.kubeClient) if err != nil { - return nil, "", err + return nil, err } } @@ -206,13 +193,10 @@ func (c *MRCProviderGenerator) getHashiVaultOSMCertificateManager(mrc *v1alpha2. provider.Role, ) if err != nil { - return nil, "", fmt.Errorf("error instantiating Hashicorp Vault as a Certificate Manager: %w", err) + return nil, fmt.Errorf("error instantiating Hashicorp Vault as a Certificate Manager: %w", err) } - id, err := getMRCID(mrc) - if err != nil { - return nil, "", err - } - return vaultClient, id, nil + + return vaultClient, nil } // getHashiVaultOSMToken returns the Hashi Vault token from the secret specified in the provided secret key reference @@ -231,11 +215,11 @@ func getHashiVaultOSMToken(secretKeyRef *v1alpha2.SecretKeyReferenceSpec, kubeCl } // getCertManagerOSMCertificateManager returns a certificate manager instance with cert-manager as the certificate provider -func (c *MRCProviderGenerator) getCertManagerOSMCertificateManager(mrc *v1alpha2.MeshRootCertificate) (certificate.Issuer, string, error) { +func (c *MRCProviderGenerator) getCertManagerOSMCertificateManager(mrc *v1alpha2.MeshRootCertificate) (certificate.Issuer, error) { provider := mrc.Spec.Provider.CertManager client, err := cmversionedclient.NewForConfig(c.kubeConfig) if err != nil { - return nil, "", fmt.Errorf("Failed to build cert-manager client set: %s", err) + return nil, fmt.Errorf("Failed to build cert-manager client set: %s", err) } cmClient, err := certmanager.New( @@ -249,11 +233,8 @@ func (c *MRCProviderGenerator) getCertManagerOSMCertificateManager(mrc *v1alpha2 c.KeyBitSize, ) if err != nil { - return nil, "", fmt.Errorf("error instantiating Jetstack cert-manager client: %w", err) + return nil, fmt.Errorf("error instantiating Jetstack cert-manager client: %w", err) } - id, err := getMRCID(mrc) - if err != nil { - return nil, "", err - } - return cmClient, id, nil + + return cmClient, nil } diff --git a/pkg/certificate/providers/config_test.go b/pkg/certificate/providers/config_test.go index 80673e90ea..74e9e0dd41 100644 --- a/pkg/certificate/providers/config_test.go +++ b/pkg/certificate/providers/config_test.go @@ -8,7 +8,6 @@ import ( "github.com/golang/mock/gomock" tassert "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -40,14 +39,12 @@ func TestGetCertificateManager(t *testing.T) { expectError bool // params - kubeClient kubernetes.Interface - configClient configClientset.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{ { @@ -90,16 +87,254 @@ func TestGetCertificateManager(t *testing.T) { cfg: mockConfigurator, }, { - name: "Valid Vault protocol using vault secret defined in MRC", + name: "Valid Vault protocol using vault secret", + options: VaultOptions{ + VaultHost: "vault.default.svc.cluster.local", + VaultRole: "role", + VaultPort: 8200, + VaultProtocol: "http", + VaultTokenSecretName: "secret", + VaultTokenSecretKey: "token", + VaultTokenSecretNamespace: "osm-system", + }, + kubeClient: fake.NewSimpleClientset(&v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret", + Namespace: "osm-system", + }, + Data: map[string][]byte{ + "token": []byte("secret"), + }, + }), + cfg: mockConfigurator, + }, + { + name: "Not a valid Vault protocol", + options: VaultOptions{ + VaultHost: "vault.default.svc.cluster.local", + VaultToken: "vault-token", + VaultRole: "role", + VaultPort: 8200, + VaultProtocol: "hi", + }, + expectError: true, + }, + { + name: "Invalid cert manager options", + options: CertManagerOptions{ + IssuerKind: "test-kind", + IssuerGroup: "cert-manager.io", + }, + cfg: mockConfigurator, + expectError: true, + }, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf(tc.name), func(t *testing.T) { + assert := tassert.New(t) + + oldCA := getCA + getCA = func(i certificate.Issuer) (pem.RootCertificate, error) { + return pem.RootCertificate("id2"), nil + } + + defer func() { + getCA = oldCA + }() + + manager, err := NewCertificateManager(context.Background(), tc.kubeClient, tc.restConfig, tc.cfg, tc.providerNamespace, tc.options, tc.msgBroker, 1*time.Hour) + if tc.expectError { + assert.Empty(manager) + assert.Error(err) + } else { + assert.NotEmpty(manager) + assert.NoError(err) + } + + if opt, ok := tc.options.(TresorOptions); ok && !tc.expectError { + _, err := tc.kubeClient.CoreV1().Secrets(tc.providerNamespace).Get(context.TODO(), opt.SecretName, metav1.GetOptions{}) + assert.NoError(err) + } + }) + } +} + +func TestGetCertificateManagerFromMRC(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockConfigurator := configurator.NewMockConfigurator(mockCtrl) + + mockConfigurator.EXPECT().IsDebugServerEnabled().Return(false).AnyTimes() + mockConfigurator.EXPECT().GetCertKeyBitSize().Return(2048).AnyTimes() + mockConfigurator.EXPECT().GetServiceCertValidityPeriod().Return(1 * time.Hour).AnyTimes() + + type testCase struct { + name string + expectError bool + + // params + kubeClient kubernetes.Interface + configClient configClientset.Interface + restConfig *rest.Config + cfg configurator.Configurator + providerNamespace string + options Options + msgBroker *messaging.Broker + } + testCases := []testCase{ + { + name: "tresor as the certificate manager", + options: TresorOptions{SecretName: "osm-ca-bundle"}, + providerNamespace: "osm-system", + cfg: mockConfigurator, + kubeClient: fake.NewSimpleClientset(), + configClient: fakeConfigClientset.NewSimpleClientset(&v1alpha2.MeshRootCertificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osm-mesh-root-certificate", + Namespace: "osm-system", + }, + Spec: v1alpha2.MeshRootCertificateSpec{ + Provider: v1alpha2.ProviderSpec{ + Tresor: &v1alpha2.TresorProviderSpec{ + CA: v1alpha2.TresorCASpec{ + SecretRef: v1.SecretReference{ + Name: "osm-ca-bundle", + Namespace: "osm-system", + }, + }, + }, + }, + }, + Status: v1alpha2.MeshRootCertificateStatus{ + State: constants.MRCStateActive, + }, + }), + }, + { + name: "tresor with no secret", + options: TresorOptions{}, + providerNamespace: "osm-system", + cfg: mockConfigurator, + kubeClient: fake.NewSimpleClientset(), + expectError: true, + configClient: fakeConfigClientset.NewSimpleClientset(&v1alpha2.MeshRootCertificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osm-mesh-root-certificate", + Namespace: "osm-system", + }, + Spec: v1alpha2.MeshRootCertificateSpec{ + Provider: v1alpha2.ProviderSpec{ + Tresor: &v1alpha2.TresorProviderSpec{ + CA: v1alpha2.TresorCASpec{ + SecretRef: v1.SecretReference{ + Name: "", + Namespace: "", + }, + }, + }, + }, + }, + Status: v1alpha2.MeshRootCertificateStatus{ + State: constants.MRCStateActive, + }, + }), + }, + { + name: "certManager as the certificate manager", + kubeClient: fake.NewSimpleClientset(), + restConfig: &rest.Config{}, + cfg: mockConfigurator, + providerNamespace: "osm-system", + options: CertManagerOptions{IssuerName: "test-name", IssuerKind: "ClusterIssuer", IssuerGroup: "cert-manager.io"}, + configClient: fakeConfigClientset.NewSimpleClientset(&v1alpha2.MeshRootCertificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osm-mesh-root-certificate", + Namespace: "osm-system", + }, + Spec: v1alpha2.MeshRootCertificateSpec{ + Provider: v1alpha2.ProviderSpec{ + CertManager: &v1alpha2.CertManagerProviderSpec{ + IssuerName: "test-name", + IssuerKind: "ClusterIssuer", + IssuerGroup: "cert-manager.io", + }, + }, + }, + Status: v1alpha2.MeshRootCertificateStatus{ + State: constants.MRCStateActive, + }, + }), + }, + { + name: "Fail to validate Config", + options: VaultOptions{}, + kubeClient: fake.NewSimpleClientset(), + expectError: true, + configClient: fakeConfigClientset.NewSimpleClientset(&v1alpha2.MeshRootCertificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osm-mesh-root-certificate", + Namespace: "osm-system", + }, + Spec: v1alpha2.MeshRootCertificateSpec{ + Provider: v1alpha2.ProviderSpec{ + Vault: &v1alpha2.VaultProviderSpec{ + Host: "", + Port: 0, + Role: "", + Protocol: "", + }, + }, + }, + Status: v1alpha2.MeshRootCertificateStatus{ + State: constants.MRCStateActive, + }, + }), + }, + { + name: "Valid Vault protocol", options: VaultOptions{ VaultHost: "vault.default.svc.cluster.local", VaultRole: "role", VaultPort: 8200, VaultProtocol: "http", + VaultToken: "vault-token", + }, + cfg: mockConfigurator, + kubeClient: fake.NewSimpleClientset(), + configClient: fakeConfigClientset.NewSimpleClientset(&v1alpha2.MeshRootCertificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osm-mesh-root-certificate", + Namespace: "osm-system", + }, + Spec: v1alpha2.MeshRootCertificateSpec{ + Provider: v1alpha2.ProviderSpec{ + Vault: &v1alpha2.VaultProviderSpec{ + Host: "vault.default.svs.cluster.local", + Port: 8200, + Role: "role", + Protocol: "http", + }, + }, + }, + Status: v1alpha2.MeshRootCertificateStatus{ + State: constants.MRCStateActive, + }, + }), + }, + { + name: "Valid Vault protocol using vault secret", + options: VaultOptions{ + VaultHost: "vault.default.svc.cluster.local", + VaultRole: "role", + VaultPort: 8200, + VaultProtocol: "http", + VaultTokenSecretName: "secret", + VaultTokenSecretKey: "token", + VaultTokenSecretNamespace: "osm-system", }, kubeClient: fake.NewSimpleClientset(&v1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "vault-token", + Name: "secret", Namespace: "osm-system", }, Data: map[string][]byte{ @@ -110,9 +345,6 @@ func TestGetCertificateManager(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "osm-mesh-root-certificate", Namespace: "osm-system", - Annotations: map[string]string{ - constants.MRCVersionAnnotation: "0", - }, }, Spec: v1alpha2.MeshRootCertificateSpec{ Provider: v1alpha2.ProviderSpec{ @@ -123,7 +355,7 @@ func TestGetCertificateManager(t *testing.T) { Protocol: "http", Token: v1alpha2.VaultTokenSpec{ SecretKeyRef: v1alpha2.SecretKeyReferenceSpec{ - Name: "vault-token", + Name: "secret", Namespace: "osm-system", Key: "token", }, @@ -135,14 +367,6 @@ func TestGetCertificateManager(t *testing.T) { State: constants.MRCStateActive, }, }), - informerCollectionFunc: func(tc testCase) (*informers.InformerCollection, error) { - ic, err := informers.NewInformerCollection("osm", nil, informers.WithKubeClient(tc.kubeClient), informers.WithConfigClient(tc.configClient, "", "osm-system")) - if err != nil { - return nil, err - } - - return ic, nil - }, cfg: mockConfigurator, }, { @@ -155,6 +379,27 @@ func TestGetCertificateManager(t *testing.T) { VaultProtocol: "hi", }, expectError: true, + cfg: mockConfigurator, + kubeClient: fake.NewSimpleClientset(), + configClient: fakeConfigClientset.NewSimpleClientset(&v1alpha2.MeshRootCertificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "osm-mesh-root-certificate", + Namespace: "osm-system", + }, + Spec: v1alpha2.MeshRootCertificateSpec{ + Provider: v1alpha2.ProviderSpec{ + Vault: &v1alpha2.VaultProviderSpec{ + Host: "vault.default.svs.cluster.local", + Port: 8200, + Role: "role", + Protocol: "hi", + }, + }, + }, + Status: v1alpha2.MeshRootCertificateStatus{ + State: constants.MRCStateActive, + }, + }), }, { name: "Invalid cert manager options", @@ -162,32 +407,19 @@ func TestGetCertificateManager(t *testing.T) { IssuerKind: "test-kind", IssuerGroup: "cert-manager.io", }, - cfg: mockConfigurator, - expectError: true, - }, - { - name: "Reads MRC from informer collection", - cfg: mockConfigurator, - kubeClient: fake.NewSimpleClientset(), - options: TresorOptions{SecretName: "osm-ca-bundle"}, - providerNamespace: "osm-system", + cfg: mockConfigurator, + kubeClient: fake.NewSimpleClientset(), configClient: fakeConfigClientset.NewSimpleClientset(&v1alpha2.MeshRootCertificate{ ObjectMeta: metav1.ObjectMeta{ Name: "osm-mesh-root-certificate", Namespace: "osm-system", - Annotations: map[string]string{ - constants.MRCVersionAnnotation: "0", - }, }, Spec: v1alpha2.MeshRootCertificateSpec{ Provider: v1alpha2.ProviderSpec{ - Tresor: &v1alpha2.TresorProviderSpec{ - CA: v1alpha2.TresorCASpec{ - SecretRef: v1.SecretReference{ - Name: "osm-ca-bundle", - Namespace: "osm-system", - }, - }, + CertManager: &v1alpha2.CertManagerProviderSpec{ + IssuerName: "", + IssuerKind: "test-kind", + IssuerGroup: "cert-manager.io", }, }, }, @@ -195,14 +427,7 @@ func TestGetCertificateManager(t *testing.T) { State: constants.MRCStateActive, }, }), - informerCollectionFunc: func(tc testCase) (*informers.InformerCollection, error) { - ic, err := informers.NewInformerCollection("osm", nil, informers.WithKubeClient(tc.kubeClient), informers.WithConfigClient(tc.configClient, "", "osm-system")) - if err != nil { - return nil, err - } - - return ic, nil - }, + expectError: true, }, } @@ -219,14 +444,11 @@ func TestGetCertificateManager(t *testing.T) { getCA = oldCA }() - var ic *informers.InformerCollection - var err error - if tc.informerCollectionFunc != nil { - ic, err = tc.informerCollectionFunc(tc) - require.NoError(t, err) - } + ic, err := informers.NewInformerCollection("osm", nil, informers.WithKubeClient(tc.kubeClient), informers.WithConfigClient(tc.configClient, "", "osm-system")) + assert.NoError(err) + assert.NotNil(ic) - manager, err := NewCertificateManager(context.Background(), tc.kubeClient, tc.restConfig, tc.cfg, tc.providerNamespace, tc.options, tc.msgBroker, ic, 1*time.Hour) + manager, err := NewCertificateManagerFromMRC(context.Background(), tc.kubeClient, tc.restConfig, tc.cfg, tc.providerNamespace, tc.options, tc.msgBroker, ic, 1*time.Hour) if tc.expectError { assert.Empty(manager) assert.Error(err) diff --git a/pkg/certificate/providers/options.go b/pkg/certificate/providers/options.go index 17c39a1509..5fccf78bd2 100644 --- a/pkg/certificate/providers/options.go +++ b/pkg/certificate/providers/options.go @@ -36,8 +36,8 @@ func (options VaultOptions) Validate() error { return errors.New("VaultHost not specified in Hashi Vault options") } - if options.VaultToken == "" { - log.Warn().Msg("VaultToken is not specified in Hashi Vault options. The token secret reference option must be specified.") + if options.VaultToken == "" && (options.VaultTokenSecretKey == "" || options.VaultTokenSecretName == "") { + return errors.New("VaultTokenSecretKey and VaultTokenSecretName must both specified if VaultToken is not specified in Hashi Vault options") } if options.VaultRole == "" { diff --git a/pkg/certificate/providers/options_test.go b/pkg/certificate/providers/options_test.go index 9dcda6b87a..920e8bbfe9 100644 --- a/pkg/certificate/providers/options_test.go +++ b/pkg/certificate/providers/options_test.go @@ -91,14 +91,38 @@ func TestValidateVaultOptions(t *testing.T) { expectErr: true, }, { - testName: "Empty token", + testName: "Empty token, valid token secret", + options: VaultOptions{ + VaultProtocol: "https", + VaultHost: "vault-host", + VaultToken: "", + VaultRole: "vault-role", + VaultTokenSecretName: "secret", + VaultTokenSecretKey: "key", + }, + expectErr: false, + }, + { + testName: "Empty token, empty token secret", options: VaultOptions{ VaultProtocol: "https", VaultHost: "vault-host", VaultToken: "", VaultRole: "vault-role", }, - expectErr: false, + expectErr: true, + }, + { + testName: "Empty token secret key", + options: VaultOptions{ + VaultProtocol: "https", + VaultHost: "vault-host", + VaultToken: "", + VaultRole: "vault-role", + VaultTokenSecretName: "secret", + VaultTokenSecretKey: "", + }, + expectErr: true, }, { testName: "Empty role", diff --git a/pkg/certificate/providers/tresor/fake/fake.go b/pkg/certificate/providers/tresor/fake/fake.go index 26af745568..1bb64be8bd 100644 --- a/pkg/certificate/providers/tresor/fake/fake.go +++ b/pkg/certificate/providers/tresor/fake/fake.go @@ -22,15 +22,15 @@ const ( type fakeMRCClient struct{} -func (c *fakeMRCClient) GetCertIssuerForMRC(mrc *v1alpha2.MeshRootCertificate) (certificate.Issuer, pem.RootCertificate, string, error) { +func (c *fakeMRCClient) GetCertIssuerForMRC(mrc *v1alpha2.MeshRootCertificate) (certificate.Issuer, pem.RootCertificate, error) { rootCertCountry := "US" rootCertLocality := "CA" ca, err := tresor.NewCA("Fake Tresor CN", 1*time.Hour, rootCertCountry, rootCertLocality, rootCertOrganization) if err != nil { - return nil, nil, "", err + return nil, nil, err } issuer, err := tresor.New(ca, rootCertOrganization, 2048) - return issuer, pem.RootCertificate("rootCA"), "issuer-1", err + return issuer, pem.RootCertificate("rootCA"), err } // List returns the single, pre-generated MRC. It is intended to implement the certificate.MRCClient interface. @@ -48,9 +48,6 @@ func (c *fakeMRCClient) Watch(ctx context.Context) (<-chan certificate.MRCEvent, ObjectMeta: metav1.ObjectMeta{ Name: "osm-mesh-root-certificate", Namespace: "osm-system", - Annotations: map[string]string{ - constants.MRCVersionAnnotation: "0", - }, }, Spec: v1alpha2.MeshRootCertificateSpec{ Provider: v1alpha2.ProviderSpec{ diff --git a/pkg/certificate/types.go b/pkg/certificate/types.go index 98ba78b4b6..210065d83d 100644 --- a/pkg/certificate/types.go +++ b/pkg/certificate/types.go @@ -123,7 +123,7 @@ type MRCClient interface { MRCEventBroker // GetCertIssuerForMRC returns an Issuer based on the provided MRC. - GetCertIssuerForMRC(mrc *v1alpha2.MeshRootCertificate) (Issuer, pem.RootCertificate, string, error) + GetCertIssuerForMRC(mrc *v1alpha2.MeshRootCertificate) (Issuer, pem.RootCertificate, error) } // MRCEventType is a type alias for a string describing the type of MRC event diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 09f7d7fbe6..65781bc057 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -190,9 +190,6 @@ const ( // Annotations and labels used by the MeshRootCertificate const ( - // MRCVersionAnnotation is the annotation used for the version of the MeshRootCertificate - MRCVersionAnnotation = "openservicemesh.io/mrc-version" - // MRCStateValidatingRollout is the validating rollout status option for the State of the MeshRootCertificate MRCStateValidatingRollout = "validatingRollout"