PR feedback

Signed-off-by: jaellio <jaellio@microsoft.com>
openservicemesh · May 24, 2022 · 1c3223b · 1c3223b
1 parent 9dd1784
commit 1c3223b
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 33 deletions.
diff --git a/charts/osm/README.md b/charts/osm/README.md
@@ -178,9 +178,10 @@ 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":"token","name":"osm-vault-token"}` | The Kubernetes secret storing the Vault token used in OSM |
+| osm.vault.secret.key | string | `"token"` | The Kubernetes secret key with the value bring the Vault token |
+| osm.vault.secret.name | string | `"osm-vault-token"` | The Kubernetes secret name storing the Vault token used in OSM |
 | osm.vault.token | string | `""` | token that should be used to connect to Vault |
-| osm.vault.tokenSecretKey | string | `"token"` | The Kubernetes secret key with the value bring the Vault token |
-| osm.vault.tokenSecretName | string | `"osm-vault-token"` | The Kubernetes secret name to store the Vault token used in OSM |
 | osm.webhookConfigNamePrefix | string | `"osm-webhook"` | Prefix used in name of the webhook configuration resources |
 | smi.validateTrafficTarget | bool | `true` | Enables validation of SMI Traffic Target |
 

diff --git a/charts/osm/templates/preset-mesh-root-certificate.yaml b/charts/osm/templates/preset-mesh-root-certificate.yaml
@@ -17,7 +17,7 @@ data:
           }
         }
         {{- end}}
-        {{- if eq (.Values.osm.certificateProvider.kind | lower) "certmanager"}}
+        {{- if eq (.Values.osm.certificateProvider.kind | lower) "cert-manager"}}
         "certManager": {
           "issuerName": {{.Values.osm.certManager.issuerName | mustToJson}},
           "issuerKind": {{.Values.osm.certManager.issuerKind | mustToJson}},
@@ -28,8 +28,8 @@ data:
         "vault": {
           "token": {
             "secretKeyRef": {
-              "name": {{.Values.osm.vault.tokenSecretName | mustToJson}},
-              "key": {{.Values.osm.vault.tokenSecretKey | mustToJson}},
+              "name": {{.Values.osm.vault.secret.name | mustToJson}},
+              "key": {{.Values.osm.vault.secret.key | mustToJson}},
               "namespace": "{{include "osm.namespace" .}}"
             }
           },

diff --git a/charts/osm/values.schema.json b/charts/osm/values.schema.json
@@ -1293,17 +1293,25 @@
                             "description": "Role to use with Vault",
                             "type": "string"
                         },
-                        "tokenSecretName": {
-                            "$id": "#/properties/osm/properties/vault/properties/tokenSecretName",
-                            "title": "Vault token secret name schema",
-                            "description": "Name of the Kubernetes Secret to store the vault token",
-                            "type": "string"
-                        },
-                        "tokenSecretKey": {
-                            "$id": "#/properties/osm/properties/vault/properties/tokenSecretKey",
-                            "title": "Vault token secret key schema",
-                            "description": "Name of the Kubernetes Secret key with the value of the vault token",
-                            "type": "string"
+                        "secret": {
+                            "$id": "#/properties/osm/properties/vault/properties/secret",
+                            "type": "object",
+                            "title": "Vault token secret schema",
+                            "description": "Vault token secret reference parameters",
+                            "properties": {
+                                "name": {
+                                    "$id": "#/properties/osm/properties/vault/properties/secret/properties/name",
+                                    "title": "Vault token secret name schema",
+                                    "description": "Name of the Kubernetes Secret to store the vault token",
+                                    "type": "string"
+                                },
+                                "key": {
+                                    "$id": "#/properties/osm/properties/vault/properties/secret/properties/key",
+                                    "title": "Vault token secret key schema",
+                                    "description": "Name of the Kubernetes Secret key with the value of the vault token",
+                                    "type": "string"
+                                }
+                            }
                         }
                     },
                     "examples": [

diff --git a/charts/osm/values.yaml b/charts/osm/values.yaml
@@ -129,10 +129,12 @@ osm:
     token: ""
     # -- Vault role to be used by Open Service Mesh
     role: openservicemesh
-    # -- The Kubernetes secret name to store the Vault token used in OSM
-    tokenSecretName: osm-vault-token
-    # -- The Kubernetes secret key with the value bring the Vault token
-    tokenSecretKey: token
+    # -- The Kubernetes secret storing the Vault token used in OSM
+    secret:
+      # -- The Kubernetes secret name storing the Vault token used in OSM
+      name: osm-vault-token
+      # -- The Kubernetes secret key with the value bring the Vault token
+      key: token
 
   #
   # -- cert-manager.io configuration

diff --git a/cmd/osm-bootstrap/osm-bootstrap.go b/cmd/osm-bootstrap/osm-bootstrap.go
@@ -371,13 +371,11 @@ func (b *bootstrap) ensureMeshRootCertificate() error {
 		return err
 	}
 
-	for _, mrc := range meshRootCertificateList.Items {
-		if mrc.Status.RotationStage == constants.MRCStageIssuing && mrc.Status.State == constants.MRCStateComplete {
-			return nil
-		}
+	if len(meshRootCertificateList.Items) != 0 {
+		return nil
 	}
 
-	// create a MeshRootCertificate since none were found in the complete state and issuing rotationStage
+	// create a MeshRootCertificate since none were found
 	return b.createMeshRootCertificate()
 }
 
@@ -393,17 +391,17 @@ func (b *bootstrap) createMeshRootCertificate() error {
 	if err != nil {
 		return err
 	}
-	if _, err := b.configClient.ConfigV1alpha2().MeshRootCertificates(b.namespace).Create(context.TODO(), defaultMeshRootCertificate, metav1.CreateOptions{}); err == nil {
-		log.Info().Msgf("MeshRootCertificate (%s) created in namespace %s", meshConfigName, b.namespace)
-		return nil
-	}
-
+	_, err = b.configClient.ConfigV1alpha2().MeshRootCertificates(b.namespace).Create(context.TODO(), defaultMeshRootCertificate, metav1.CreateOptions{})
 	if apierrors.IsAlreadyExists(err) {
 		log.Info().Msgf("MeshRootCertificate already exists in %s. Skip creating.", b.namespace)
 		return nil
 	}
+	if err != nil {
+		return err
+	}
 
-	return err
+	log.Info().Msgf("Successfully created MeshRootCertificate %s in %s.", meshRootCertificateName, b.namespace)
+	return nil
 }
 
 func buildMeshRootCertificate(presetMeshRootCertificateConfigMap *corev1.ConfigMap) (*configv1alpha2.MeshRootCertificate, error) {
@@ -414,7 +412,7 @@ func buildMeshRootCertificate(presetMeshRootCertificateConfigMap *corev1.ConfigM
 		log.Fatal().Err(err).Msgf("Error converting preset-mesh-root-certificate json string to MeshRootCertificate object")
 	}
 
-	config := &configv1alpha2.MeshRootCertificate{
+	mrc := &configv1alpha2.MeshRootCertificate{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "MeshRootCertificate",
 			APIVersion: "config.openservicemesh.io/configv1alpha2",
@@ -429,5 +427,5 @@ func buildMeshRootCertificate(presetMeshRootCertificateConfigMap *corev1.ConfigM
 		},
 	}
 
-	return config, util.CreateApplyAnnotation(config, unstructured.UnstructuredJSONScheme)
+	return mrc, util.CreateApplyAnnotation(mrc, unstructured.UnstructuredJSONScheme)
 }