Merge pull request #6433 from vinny-sabatini/issue-5782

fix error message when setting up vault issuer

internal/vault/vault.go

@@ -497,15 +497,22 @@ func (v *Vault) IsVaultInitializedAndUnsealed() error {
 		defer healthResp.Body.Close()
 	}
 
+	// 200 = if initialized, unsealed, and active
 	// 429 = if unsealed and standby
 	// 472 = if disaster recovery mode replication secondary and active
 	// 473 = if performance standby
+	// 501 = if not initialized
+	// 503 = if sealed
 	if err != nil {
 		switch {
 		case healthResp == nil:
 			return err
 		case healthResp.StatusCode == 429, healthResp.StatusCode == 472, healthResp.StatusCode == 473:
 			return nil
+		case healthResp.StatusCode == 501:
+			return fmt.Errorf("Vault is not initialized")
+		case healthResp.StatusCode == 503:
+			return fmt.Errorf("Vault is sealed")
 		default:
 			return fmt.Errorf("error calling Vault %s: %w", healthURL, err)
 		}

internal/vault/vault_test.go

@@ -989,6 +989,7 @@ func TestNewConfig(t *testing.T) {
 		"a bad cert bundle should error": {
 			issuer: gen.Issuer("vault-issuer",
 				gen.SetIssuerVault(cmapi.VaultIssuer{
+					Server:   "https://vault.example.com",
 					CABundle: []byte("a bad cert bundle"),
 				}),
 			),
@@ -998,6 +999,7 @@ func TestNewConfig(t *testing.T) {
 		"a good cert bundle should be added to the config": {
 			issuer: gen.Issuer("vault-issuer",
 				gen.SetIssuerVault(cmapi.VaultIssuer{
+					Server:   "https://vault.example.com",
 					CABundle: []byte(testLeafCertificate),
 				}),
 			),
@@ -1025,6 +1027,7 @@ func TestNewConfig(t *testing.T) {
 		"a good bundle from a caBundleSecretRef should be added to the config": {
 			issuer: gen.Issuer("vault-issuer",
 				gen.SetIssuerVault(cmapi.VaultIssuer{
+					Server: "https://vault.example.com",
 					CABundleSecretRef: &cmmeta.SecretKeySelector{
 						Key: "my-bundle.crt",
 						LocalObjectReference: cmmeta.LocalObjectReference{
@@ -1060,6 +1063,7 @@ func TestNewConfig(t *testing.T) {
 		"a good bundle from a caBundleSecretRef with default key should be added to the config": {
 			issuer: gen.Issuer("vault-issuer",
 				gen.SetIssuerVault(cmapi.VaultIssuer{
+					Server: "https://vault.example.com",
 					CABundleSecretRef: &cmmeta.SecretKeySelector{
 						LocalObjectReference: cmmeta.LocalObjectReference{
 							Name: "bundle",
@@ -1094,6 +1098,7 @@ func TestNewConfig(t *testing.T) {
 		"a bad bundle from a caBundleSecretRef should error": {
 			issuer: gen.Issuer("vault-issuer",
 				gen.SetIssuerVault(cmapi.VaultIssuer{
+					Server: "https://vault.example.com",
 					CABundleSecretRef: &cmmeta.SecretKeySelector{
 						Key: "my-bundle.crt",
 						LocalObjectReference: cmmeta.LocalObjectReference{
@@ -1108,7 +1113,8 @@ func TestNewConfig(t *testing.T) {
 		"the tokenCreate func should be called with the correct namespace": {
 			issuer: gen.Issuer("vault-issuer",
 				gen.SetIssuerVault(cmapi.VaultIssuer{
-					Path: "my-path",
+					Server: "https://vault.example.com",
+					Path:   "my-path",
 					Auth: cmapi.VaultAuth{
 						Kubernetes: &cmapi.VaultKubernetesAuth{
 							Role: "my-role",
@@ -1320,6 +1326,7 @@ func TestNewWithVaultNamespaces(t *testing.T) {
 					Spec: v1.IssuerSpec{
 						IssuerConfig: v1.IssuerConfig{
 							Vault: &v1.VaultIssuer{
+								Server:    "https://vault.example.com",
 								Namespace: tc.vaultNS,
 								Auth: cmapi.VaultAuth{
 									TokenSecretRef: &cmmeta.SecretKeySelector{

pkg/controller/certificaterequests/vault/vault_test.go

@@ -88,7 +88,9 @@ func generateSelfSignedCertFromCR(cr *cmapi.CertificateRequest, key crypto.Signe
 func TestSign(t *testing.T) {
 	metaFixedClockStart := metav1.NewTime(fixedClockStart)
 	baseIssuer := gen.Issuer("vault-issuer",
-		gen.SetIssuerVault(cmapi.VaultIssuer{}),
+		gen.SetIssuerVault(cmapi.VaultIssuer{
+			Server: "https://example.vault.com",
+		}),
 		gen.AddIssuerCondition(cmapi.IssuerCondition{
 			Type:   cmapi.IssuerConditionReady,
 			Status: cmmeta.ConditionTrue,
@@ -234,6 +236,7 @@ func TestSign(t *testing.T) {
 								},
 							},
 						},
+						Server: "https://example.vault.com",
 					})),
 				},
 				ExpectedEvents: []string{
@@ -274,6 +277,7 @@ func TestSign(t *testing.T) {
 								},
 							},
 						},
+						Server: "https://example.vault.com",
 					}),
 				)},
 				ExpectedEvents: []string{

pkg/controller/certificatesigningrequests/vault/vault_test.go

@@ -70,6 +70,7 @@ func TestProcessItem(t *testing.T) {
 					},
 				},
 			},
+			Server: "https://example.vault.com",
 		}),
 		gen.AddIssuerCondition(cmapi.IssuerCondition{
 			Type:   cmapi.IssuerConditionReady,

pkg/issuer/vault/setup.go

@@ -18,7 +18,6 @@ package vault
 
 import (
 	"context"
-	"fmt"
 
 	vaultinternal "github.com/cert-manager/cert-manager/internal/vault"
 	apiutil "github.com/cert-manager/cert-manager/pkg/api/util"
@@ -33,12 +32,11 @@ const (
 
 	errorVault = "VaultError"
 
-	messageVaultClientInitFailed         = "Failed to initialize Vault client: "
-	messageVaultStatusVerificationFailed = "Vault is not initialized or is sealed"
-	messageVaultConfigRequired           = "Vault config cannot be empty"
-	messageServerAndPathRequired         = "Vault server and path are required fields"
-	messageAuthFieldsRequired            = "Vault tokenSecretRef, appRole, or kubernetes is required"
-	messageMultipleAuthFieldsSet         = "Multiple auth methods cannot be set on the same Vault issuer"
+	messageVaultClientInitFailed = "Failed to initialize Vault client: "
+	messageVaultConfigRequired   = "Vault config cannot be empty"
+	messageServerAndPathRequired = "Vault server and path are required fields"
+	messageAuthFieldsRequired    = "Vault tokenSecretRef, appRole, or kubernetes is required"
+	messageMultipleAuthFieldsSet = "Multiple auth methods cannot be set on the same Vault issuer"
 
 	messageKubeAuthRoleRequired      = "Vault Kubernetes auth requires a role to be set"
 	messageKubeAuthEitherRequired    = "Vault Kubernetes auth requires either secretRef.name or serviceAccountRef.name to be set"
@@ -135,9 +133,9 @@ func (v *Vault) Setup(ctx context.Context) error {
 	}
 
 	if err := client.IsVaultInitializedAndUnsealed(); err != nil {
-		logf.V(logf.WarnLevel).Infof("%s: %s: error: %s", v.issuer.GetObjectMeta().Name, messageVaultStatusVerificationFailed, err.Error())
-		apiutil.SetIssuerCondition(v.issuer, v.issuer.GetGeneration(), v1.IssuerConditionReady, cmmeta.ConditionFalse, errorVault, messageVaultStatusVerificationFailed)
-		return fmt.Errorf(messageVaultStatusVerificationFailed)
+		logf.V(logf.WarnLevel).Infof("%s: %s", v.issuer.GetObjectMeta().Name, err.Error())
+		apiutil.SetIssuerCondition(v.issuer, v.issuer.GetGeneration(), v1.IssuerConditionReady, cmmeta.ConditionFalse, errorVault, err.Error())
+		return err
 	}
 
 	logf.Log.V(logf.DebugLevel).Info(messageVaultVerified)

pkg/issuer/vault/setup_test.go

@@ -368,6 +368,42 @@ func TestVault_Setup(t *testing.T) {
 			},
 			expectCond: "Ready True: VaultVerified: Vault verified",
 		},
+		{
+			name: "server with invalid url should fail to setup",
+			givenIssuer: v1.IssuerConfig{
+				Vault: &v1.VaultIssuer{
+					Path:   "pki_int",
+					Server: "https:/vault.example.com",
+					Auth: v1.VaultAuth{
+						TokenSecretRef: &cmmeta.SecretKeySelector{
+							LocalObjectReference: cmmeta.LocalObjectReference{
+								Name: "cert-manager",
+							},
+							Key: "",
+						},
+					},
+				},
+			},
+			expectErr: "Get \"https:///vault.example.com/v1/sys/health\": http: no Host in request URL",
+		},
+		{
+			name: "server with leading whitespace should fail to parse",
+			givenIssuer: v1.IssuerConfig{
+				Vault: &v1.VaultIssuer{
+					Path:   "pki_int",
+					Server: " https://vault.example.com",
+					Auth: v1.VaultAuth{
+						TokenSecretRef: &cmmeta.SecretKeySelector{
+							LocalObjectReference: cmmeta.LocalObjectReference{
+								Name: "cert-manager",
+							},
+							Key: "",
+						},
+					},
+				},
+			},
+			expectErr: "error initializing Vault client: parse \" https://vault.example.com\": first path segment in URL cannot contain colon",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {