Ensure all mount errors are covered (#2289)

* CI: Test against vault enterprise 1.17.1 and bump other versions
* Build: Add support running tests using gotestsum
* CI: Drop 1.11.12-ent

.github/workflows/build.yml

@@ -63,12 +63,12 @@ jobs:
       fail-fast: false
       matrix:
         image:
-        - "vault-enterprise:1.11.12-ent"
         - "vault-enterprise:1.12.11-ent"
         - "vault-enterprise:1.13.13-ent"
-        - "vault-enterprise:1.14.12-ent"
-        - "vault-enterprise:1.15.8-ent"
-        - "vault-enterprise:1.16.2-ent"
+        - "vault-enterprise:1.14.13-ent"
+        - "vault-enterprise:1.15.11-ent"
+        - "vault-enterprise:1.16.5-ent"
+        - "vault-enterprise:1.17.1-ent"
         - "vault:latest"
     services:
       vault:

Makefile

@@ -17,12 +17,21 @@ build: go-version-check fmtcheck
 test: go-version-check fmtcheck
 	TF_ACC= VAULT_TOKEN= go test $(TESTARGS) -timeout 10m $(TEST_PATH)
 
+testsum: go-version-check fmtcheck
+	TF_ACC= VAULT_TOKEN= gotestsum $(TEST_PATH) $(TESTARGS) -test.timeout 10m
+
 testacc: fmtcheck
 	TF_ACC=1 go test $(TESTARGS) -timeout 30m $(TEST_PATH)
 
+testaccsum: fmtcheck
+	TF_ACC=1 gotestsum $(TEST_PATH) $(TESTARGS) -timeout 30m
+
 testacc-ent:
 	make testacc TF_ACC_ENTERPRISE=1
 
+testaccsum-ent:
+	make testaccsum TF_ACC_ENTERPRISE=1
+
 dev: go-version-check fmtcheck
 	go build -o terraform-provider-vault
 	mv terraform-provider-vault ~/.terraform.d/plugins/
@@ -71,4 +80,4 @@ ifeq (,$(wildcard $(GOPATH)/src/$(WEBSITE_REPO)))
 endif
 	@$(MAKE) -C $(GOPATH)/src/$(WEBSITE_REPO) website-provider-test PROVIDER_PATH=$(shell pwd) PROVIDER_NAME=$(PKG_NAME)
 
-.PHONY: build test testacc testacc-ent vet fmt fmtcheck errcheck test-compile website website-test go-version-check
+.PHONY: build test testacc testacc-ent vet fmt fmtcheck errcheck test-compile website website-test go-version-check testaccsum testaccsum-ent

go.mod

@@ -34,6 +34,7 @@ require (
 	github.com/jcmturner/gokrb5/v8 v8.4.4
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/mitchellh/mapstructure v1.5.0
+	github.com/stretchr/testify v1.9.0
 	golang.org/x/crypto v0.23.0
 	golang.org/x/net v0.25.0
 	golang.org/x/oauth2 v0.18.0
@@ -148,7 +149,6 @@ require (
 	github.com/sasha-s/go-deadlock v0.2.0 // indirect
 	github.com/sirupsen/logrus v1.9.3 // indirect
 	github.com/sony/gobreaker v0.5.0 // indirect
-	github.com/stretchr/testify v1.9.0 // indirect
 	github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect
 	github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect
 	github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect

util/mountutil/mountutil.go

@@ -6,17 +6,18 @@ package mountutil
 import (
 	"context"
 	"errors"
-	"fmt"
+	"net/http"
 	"strings"
 
-	"github.com/hashicorp/terraform-provider-vault/internal/consts"
 	"github.com/hashicorp/vault/api"
+
+	"github.com/hashicorp/terraform-provider-vault/internal/consts"
 )
 
 // Error strings that are returned by the Vault API.
 const (
-	ErrVaultSecretMountNotFound = "No secret engine mount at"
-	ErrVaultAuthMountNotFound   = "No auth engine at"
+	VaultSecretMountNotFoundErrMsg = "No secret engine mount at"
+	VaultAuthMountNotFoundErrMsg   = "No auth engine at"
 )
 
 // Error strings that are used internally by TFVP
@@ -28,40 +29,24 @@ var (
 
 // GetMount will fetch the secret mount at the given path.
 func GetMount(ctx context.Context, client *api.Client, path string) (*api.MountOutput, error) {
-	mount, err := client.Sys().GetMountWithContext(ctx, path)
-	// Hardcoding the error string check is not ideal, but Vault does not
-	// return 404 in this case
-	if err != nil && strings.Contains(err.Error(), ErrVaultSecretMountNotFound) {
-		return nil, fmt.Errorf("%w: %s", ErrMountNotFound, err)
-	}
-	// some other error occured, like 403, etc.
-	if err != nil {
-		return nil, fmt.Errorf("error reading from Vault: %s", err)
-	}
-	// no error but no mount either, so return not found
-	if mount == nil {
-		return nil, fmt.Errorf("%w: %s", ErrMountNotFound, err)
+	if resp, err := client.Sys().GetMountWithContext(ctx, path); err != nil {
+		return nil, err
+	} else if resp == nil {
+		return nil, ErrMountNotFound
+	} else {
+		return resp, nil
 	}
-	return mount, nil
 }
 
 // GetAuthMount will fetch the auth mount at the given path.
 func GetAuthMount(ctx context.Context, client *api.Client, path string) (*api.MountOutput, error) {
-	mount, err := client.Sys().GetAuthWithContext(ctx, path)
-	// Hardcoding the error string check is not ideal, but Vault does not
-	// return 404 in this case
-	if err != nil && strings.Contains(err.Error(), ErrVaultAuthMountNotFound) {
-		return nil, fmt.Errorf("%w: %s", ErrMountNotFound, err)
+	if resp, err := client.Sys().GetAuthWithContext(ctx, path); err != nil {
+		return nil, err
+	} else if resp == nil {
+		return nil, ErrMountNotFound
+	} else {
+		return resp, nil
 	}
-	// some other error occured, like 403, etc.
-	if err != nil {
-		return nil, fmt.Errorf("error reading from Vault: %s", err)
-	}
-	// no error but no mount either, so return not found
-	if mount == nil {
-		return nil, fmt.Errorf("%w: %s", ErrMountNotFound, err)
-	}
-	return mount, nil
 }
 
 // NormalizeMountPath to be in a form valid for accessing values from api.MountOutput
@@ -74,21 +59,40 @@ func TrimSlashes(path string) string {
 	return strings.Trim(path, consts.PathDelim)
 }
 
-// CheckMountEnabledWithContext in Vault
-func CheckMountEnabledWithContext(ctx context.Context, client *api.Client, path string) (bool, error) {
-	_, err := GetMount(ctx, client, path)
-	if errors.Is(err, ErrMountNotFound) {
-		return false, err
-	}
-
-	if err != nil {
+// CheckMountEnabled in Vault
+func CheckMountEnabled(ctx context.Context, client *api.Client, path string) (bool, error) {
+	if _, err := GetMount(ctx, client, path); err != nil {
+		if IsMountNotFoundError(err) {
+			return false, nil
+		}
 		return false, err
 	}
 
 	return true, nil
 }
 
-// CheckMountEnabled in Vault
-func CheckMountEnabled(client *api.Client, path string) (bool, error) {
-	return CheckMountEnabledWithContext(context.Background(), client, path)
+// IsMountNotFoundError returns true if error is a mount not found error.
+func IsMountNotFoundError(err error) bool {
+	var respErr *api.ResponseError
+	if errors.As(err, &respErr) && respErr != nil {
+		if respErr.StatusCode == http.StatusNotFound {
+			return true
+		}
+		if respErr.StatusCode == http.StatusBadRequest {
+			for _, e := range respErr.Errors {
+				if strings.Contains(e, VaultSecretMountNotFoundErrMsg) {
+					return true
+				}
+				if strings.Contains(e, VaultAuthMountNotFoundErrMsg) {
+					return true
+				}
+			}
+		}
+	}
+
+	if errors.Is(err, ErrMountNotFound) {
+		return true
+	}
+
+	return false
 }

util/mountutil/mountutil_test.go

@@ -0,0 +1,99 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package mountutil
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/hashicorp/vault/api"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsMountNotFoundError(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		err  error
+		want bool
+	}{
+		{
+			name: "with-err-mount-not-found",
+			err:  ErrMountNotFound,
+			want: true,
+		},
+		{
+			name: "with-response-error-no-secret-engine-mount",
+			err: &api.ResponseError{
+				StatusCode: http.StatusBadRequest,
+				Errors: []string{
+					"No secret engine mount at auth/operator/",
+				},
+			},
+			want: true,
+		},
+		{
+			name: "with-response-error-no-auth-engine-mount",
+			err: &api.ResponseError{
+				StatusCode: http.StatusBadRequest,
+				Errors: []string{
+					"No auth engine at auth/operator/",
+				},
+			},
+			want: true,
+		},
+		{
+			name: "with-response-error-both",
+			err: &api.ResponseError{
+				StatusCode: http.StatusBadRequest,
+				Errors: []string{
+					"No secret engine mount at auth/operator/",
+					"No auth engine at auth/operator/",
+				},
+			},
+			want: true,
+		},
+		{
+			name: "with-response-error-others",
+			err: &api.ResponseError{
+				StatusCode: http.StatusBadRequest,
+				Errors: []string{
+					"Some other error",
+					"No auth engine at auth/operator/",
+				},
+			},
+			want: true,
+		},
+		{
+			name: "with-not-found-status-code",
+			err: &api.ResponseError{
+				StatusCode: http.StatusNotFound,
+				Errors: []string{
+					"some error",
+				},
+			},
+			want: true,
+		},
+		{
+			name: "with-response-error-canary",
+			err: &api.ResponseError{
+				StatusCode: http.StatusBadRequest,
+				Errors: []string{
+					"secret engine mount",
+				},
+			},
+			want: false,
+		},
+		{
+			name: "with-nil-error",
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equalf(t, tt.want, IsMountNotFoundError(tt.err), "IsMountNotFoundError(%v)", tt.err)
+		})
+	}
+}

vault/resource_ad_secret_backend.go

@@ -5,7 +5,6 @@ package vault
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"log"
 	"strings"
@@ -344,14 +343,14 @@ func readConfigResource(d *schema.ResourceData, meta interface{}) error {
 	path := d.Id()
 	log.Printf("[DEBUG] Reading %q", path)
 
-	mount, err := mountutil.GetMount(context.Background(), client, path)
-	if errors.Is(err, mountutil.ErrMountNotFound) {
-		log.Printf("[WARN] Mount %q not found, removing from state.", path)
-		d.SetId("")
-		return nil
-	}
-
+	ctx := context.Background()
+	mount, err := mountutil.GetMount(ctx, client, path)
 	if err != nil {
+		if mountutil.IsMountNotFoundError(err) {
+			log.Printf("[WARN] Mount %q not found, removing from state.", path)
+			d.SetId("")
+			return nil
+		}
 		return err
 	}
 

vault/resource_auth_backend.go

@@ -5,17 +5,16 @@ package vault
 
 import (
 	"context"
-	"errors"
-
 	"log"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/hashicorp/vault/api"
+
 	"github.com/hashicorp/terraform-provider-vault/internal/consts"
 	"github.com/hashicorp/terraform-provider-vault/internal/provider"
 	"github.com/hashicorp/terraform-provider-vault/util"
 	"github.com/hashicorp/terraform-provider-vault/util/mountutil"
-	"github.com/hashicorp/vault/api"
 )
 
 func AuthBackendResource() *schema.Resource {
@@ -145,13 +144,12 @@ func authBackendRead(ctx context.Context, d *schema.ResourceData, meta interface
 	path := d.Id()
 
 	mount, err := mountutil.GetAuthMount(ctx, client, path)
-	if errors.Is(err, mountutil.ErrMountNotFound) {
-		log.Printf("[WARN] Mount %q not found, removing from state.", path)
-		d.SetId("")
-		return nil
-	}
-
 	if err != nil {
+		if mountutil.IsMountNotFoundError(err) {
+			log.Printf("[WARN] Mount %q not found, removing from state.", path)
+			d.SetId("")
+			return nil
+		}
 		return diag.FromErr(err)
 	}
 
@@ -171,9 +169,9 @@ func authBackendRead(ctx context.Context, d *schema.ResourceData, meta interface
 		return diag.FromErr(err)
 	}
 	// TODO: uncomment when identity token key is being returned on the read mount endpoint
-	//if err := d.Set(consts.FieldIdentityTokenKey, mount.Config.IdentityTokenKey); err != nil {
+	// if err := d.Set(consts.FieldIdentityTokenKey, mount.Config.IdentityTokenKey); err != nil {
 	//	return diag.FromErr(err)
-	//}
+	// }
 
 	return nil
 }

vault/resource_aws_secret_backend.go

@@ -5,7 +5,6 @@ package vault
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"log"
 	"strings"
@@ -256,13 +255,12 @@ func awsSecretBackendRead(ctx context.Context, d *schema.ResourceData, meta inte
 	log.Printf("[DEBUG] Reading AWS backend mount %q from Vault", path)
 
 	mount, err := mountutil.GetMount(ctx, client, path)
-	if errors.Is(err, mountutil.ErrMountNotFound) {
-		log.Printf("[WARN] Mount %q not found, removing from state.", path)
-		d.SetId("")
-		return nil
-	}
-
 	if err != nil {
+		if mountutil.IsMountNotFoundError(err) {
+			log.Printf("[WARN] Mount %q not found, removing from state.", path)
+			d.SetId("")
+			return nil
+		}
 		return diag.FromErr(err)
 	}