Set the Read permission state as Unknown if the response from the list calls does not find the expected key

Instead of returning the error right away, append the error when `ListObject` is successful but the key expected if not found.

This can be caused if the credentials do not have `s3:PutObject` but have `s3:ListBucket`. Without `s3:PutObject`, the object will not be written to S3 so other calls to find the object will not be successful even if they have valid permissions.

This prevents us from returning a `write` error on Boundary because the list call will always fail. Without this, the  Permissions are not wrapped and the SBC states are all set to `unknown` even though we know certain permission failed.

internal/credential/attributes.go

@@ -37,7 +37,7 @@ type CredentialAttributes struct {
 // CredentialsConfig used for configuring an AWS session. An status error is returned
 // with an InvalidArgument code if any unrecognized fields are found in the protobuf
 // struct input.
-func GetCredentialsConfig(secrets *structpb.Struct, attrs *CredentialAttributes) (*awsutil.CredentialsConfig, error) {
+func GetCredentialsConfig(secrets *structpb.Struct, attrs *CredentialAttributes, required bool) (*awsutil.CredentialsConfig, error) {
 	// initialize secrets if it is nil
 	// secrets can be nil because static credentials are optional
 	if secrets == nil {
@@ -49,13 +49,13 @@ func GetCredentialsConfig(secrets *structpb.Struct, attrs *CredentialAttributes)
 	unknownFields := values.StructFields(secrets)
 	badFields := make(map[string]string)
 
-	accessKey, err := values.GetStringValue(secrets, ConstAccessKeyId, false)
+	accessKey, err := values.GetStringValue(secrets, ConstAccessKeyId, required)
 	if err != nil {
 		badFields[fmt.Sprintf("secrets.%s", ConstAccessKeyId)] = err.Error()
 	}
 	delete(unknownFields, ConstAccessKeyId)
 
-	secretKey, err := values.GetStringValue(secrets, ConstSecretAccessKey, false)
+	secretKey, err := values.GetStringValue(secrets, ConstSecretAccessKey, required)
 	if err != nil {
 		badFields[fmt.Sprintf("secrets.%s", ConstSecretAccessKey)] = err.Error()
 	}

internal/credential/attributes_test.go

@@ -105,6 +105,7 @@ func TestGetCredentialsConfig(t *testing.T) {
 	cases := []struct {
 		name                string
 		secrets             *structpb.Struct
+		required            bool
 		attrs               *CredentialAttributes
 		expected            *awsutil.CredentialsConfig
 		expectedErrContains string
@@ -135,6 +136,32 @@ func TestGetCredentialsConfig(t *testing.T) {
 				Region:    "us-west-2",
 			},
 		},
+		{
+			name: "missing access key",
+			secrets: &structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					ConstSecretAccessKey: structpb.NewStringValue("bazqux"),
+				},
+			},
+			attrs: &CredentialAttributes{
+				Region: "us-west-2",
+			},
+			required:            true,
+			expectedErrContains: "secrets.access_key_id: missing required value",
+		},
+		{
+			name: "missing secret key",
+			secrets: &structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					ConstAccessKeyId: structpb.NewStringValue("AKIAfoobar"),
+				},
+			},
+			required: true,
+			attrs: &CredentialAttributes{
+				Region: "us-west-2",
+			},
+			expectedErrContains: "secrets.secret_access_key: missing required value",
+		},
 		{
 			name: "unknown fields",
 			secrets: &structpb.Struct{
@@ -233,7 +260,7 @@ func TestGetCredentialsConfig(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			require := require.New(t)
 
-			actual, err := GetCredentialsConfig(tc.secrets, tc.attrs)
+			actual, err := GetCredentialsConfig(tc.secrets, tc.attrs, tc.required)
 			if tc.expectedErrContains != "" {
 				require.Error(err)
 				require.Contains(err.Error(), tc.expectedErrContains)

internal/errors/error_test.go

@@ -1,6 +1,3 @@
-// Copyright (c) HashiCorp, Inc.
-// SPDX-License-Identifier: MPL-2.0
-
 package errors
 
 import (

internal/errors/testing.go

@@ -1,6 +1,3 @@
-// Copyright (c) HashiCorp, Inc.
-// SPDX-License-Identifier: MPL-2.0
-
 package errors
 
 import (

plugin/service/host/plugin.go

@@ -5,19 +5,22 @@ package host
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/ec2"
 	"github.com/aws/aws-sdk-go-v2/service/ec2/types"
-	"github.com/hashicorp/boundary-plugin-aws/internal/credential"
-	"github.com/hashicorp/boundary-plugin-aws/internal/errors"
+	"github.com/aws/smithy-go"
 	pb "github.com/hashicorp/boundary/sdk/pbs/plugin"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/structpb"
+
+	"github.com/hashicorp/boundary-plugin-aws/internal/credential"
+	cred "github.com/hashicorp/boundary-plugin-aws/internal/credential"
 )
 
 // HostPlugin implements the HostPluginServiceServer interface for the
@@ -39,19 +42,24 @@ var _ pb.HostPluginServiceServer = (*HostPlugin)(nil)
 func (p *HostPlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalogRequest) (*pb.OnCreateCatalogResponse, error) {
 	catalog := req.GetCatalog()
 	if catalog == nil {
-		return nil, errors.BadRequestStatus("catalog is required")
+		return nil, status.Error(codes.InvalidArgument, "catalog is nil")
+	}
+
+	secrets := catalog.GetSecrets()
+	if secrets == nil {
+		return nil, status.Error(codes.InvalidArgument, "secrets are required")
 	}
 
 	attrs := catalog.GetAttributes()
 	if attrs == nil {
-		return nil, errors.BadRequestStatus("attributes are required")
+		return nil, status.Error(codes.InvalidArgument, "attributes are required")
 	}
 
 	catalogAttributes, err := getCatalogAttributes(attrs)
 	if err != nil {
 		return nil, err
 	}
-	credConfig, err := credential.GetCredentialsConfig(catalog.GetSecrets(), catalogAttributes.CredentialAttributes)
+	credConfig, err := cred.GetCredentialsConfig(secrets, catalogAttributes.CredentialAttributes, true)
 	if err != nil {
 		return nil, err
 	}
@@ -63,14 +71,7 @@ func (p *HostPlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalo
 		}, p.testCredStateOpts...)...,
 	)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error setting up persisted state: %s", err)
-	}
-
-	// Try to rotate AWS static credentials
-	if credential.GetCredentialType(credState.CredentialsConfig) == credential.StaticAWS && !catalogAttributes.DisableCredentialRotation {
-		if err := credState.RotateCreds(ctx); err != nil {
-			return nil, err
-		}
+		return nil, status.Errorf(codes.InvalidArgument, "error setting up persisted state: %s", err)
 	}
 
 	catalogState, err := newAwsCatalogPersistedState(
@@ -79,17 +80,25 @@ func (p *HostPlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalo
 		}, p.testCatalogStateOpts...)...,
 	)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error setting up persisted state: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error setting up persisted state: %s", err)
 	}
 
-	// perform dry run to ensure we can interact with AWS as expected.
-	if st := dryRunValidation(ctx, catalogState); st != nil {
-		return nil, st.Err()
+	if cred.GetCredentialType(credState.CredentialsConfig) == cred.StaticAWS {
+		if !catalogAttributes.DisableCredentialRotation {
+			if err := credState.RotateCreds(ctx); err != nil {
+				return nil, status.Errorf(codes.InvalidArgument, "error during credential rotation: %s", err)
+			}
+		} else {
+			// Simply validate if we aren't rotating.
+			if err := credState.ValidateCreds(ctx); err != nil {
+				return nil, status.Errorf(codes.InvalidArgument, "error during credential validation: %s", err)
+			}
+		}
 	}
 
 	persistedProto, err := catalogState.toProto()
 	if err != nil {
-		return nil, errors.BadRequestStatus(err.Error())
+		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
 	return &pb.OnCreateCatalogResponse{
@@ -102,100 +111,43 @@ func (p *HostPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalo
 	// For host catalogs we only need to really validate what is
 	// currently in the new copy of the host catalog data, so skip
 	// fetching the new copy to keep things a little less messy.
-	currentCatalog := req.GetCurrentCatalog()
-	if currentCatalog == nil {
-		return nil, errors.BadRequestStatus("current catalog is required")
-	}
-	newCatalog := req.GetNewCatalog()
-	if newCatalog == nil {
-		return nil, errors.BadRequestStatus("new catalog is required")
+	catalog := req.GetNewCatalog()
+	if catalog == nil {
+		return nil, status.Error(codes.InvalidArgument, "new catalog is nil")
 	}
 
-	oldAttrs := currentCatalog.GetAttributes()
-	if oldAttrs == nil {
-		return nil, errors.BadRequestStatus("current catalog attributes are required")
+	var updateSecrets bool
+	secrets := catalog.GetSecrets()
+	if secrets != nil {
+		// We will be updating secrets this run, but what exactly that
+		// means will be determined later.
+		updateSecrets = true
 	}
-	oldCatalogAttributes, err := getCatalogAttributes(oldAttrs)
-	if err != nil {
-		return nil, err
+
+	attrs := catalog.GetAttributes()
+	if attrs == nil {
+		return nil, status.Error(codes.InvalidArgument, "new catalog missing attributes")
 	}
-	newAttrs := newCatalog.GetAttributes()
-	if newAttrs == nil {
-		return nil, errors.BadRequestStatus("new catalog attributes are required")
-	}
-	newCatalogAttributes, err := getCatalogAttributes(newAttrs)
+
+	catalogAttributes, err := getCatalogAttributes(attrs)
 	if err != nil {
 		return nil, err
 	}
 
+	// Get the persisted data.
+	// NOTE: We might need to change this at a later time. I'm not too
+	// sure *exactly* what scenarios we might encounter that would
+	// ultimately mean that we would have to handle an empty or missing
+	// state in update, but what we are ultimately assuming here
+	// (implicitly through awsCatalogPersistedStateFromProto) is that
+	// the state will exist and be populated. Personally I think this
+	// is fine and important, but this may change in the future.
 	credState, err := credential.AwsCredentialPersistedStateFromProto(
 		req.GetPersisted().GetSecrets(),
-		oldCatalogAttributes.CredentialAttributes,
+		catalogAttributes.CredentialAttributes,
 		p.testCredStateOpts...)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error loading persisted state: %s", err)
-	}
-
-	// Verify the incoming credentials are valid and return any errors to the
-	// user if they're not. Note this doesn't validate the credentials against
-	// AWS - it only does logical validation on the fields.
-	updatedCredentials, err := credential.GetCredentialsConfig(newCatalog.GetSecrets(), newCatalogAttributes.CredentialAttributes)
-	if err != nil {
-		return nil, err
-	}
-
-	// We will be updating credentials when one of two changes occur:
-	// 1. the new catalog has secrets for static credentials, in which case we need to delete the old static credentials
-	// 2. the new catalog has a different roleARN value for dynamic credentials
-	if newCatalog.GetSecrets() != nil || newCatalogAttributes.RoleArn != oldCatalogAttributes.RoleArn {
-		// Ensure the incoming credentials are valid for interaction with the S3
-		// Bucket before replacing them.
-		newCredState, err := credential.NewAwsCredentialPersistedState(
-			append([]credential.AwsCredentialPersistedStateOption{
-				credential.WithCredentialsConfig(updatedCredentials),
-			}, p.testCredStateOpts...)...,
-		)
-		if err != nil {
-			return nil, errors.BadRequestStatus("error setting up new credential persisted state: %s", err)
-		}
-		newCatalogState, err := newAwsCatalogPersistedState(
-			append([]awsCatalogPersistedStateOption{
-				withCredentials(newCredState),
-			}, p.testCatalogStateOpts...)...,
-		)
-		if err != nil {
-			return nil, errors.BadRequestStatus("error loading persisted state: %s", err)
-		}
-		if st := dryRunValidation(ctx, newCatalogState); st != nil {
-			return nil, st.Err()
-		}
-
-		// Replace the existing credential state.
-		// This checks the timestamp on the last rotation time as well
-		// and deletes the credentials if we are managing them
-		// (ie: if we've rotated them before).
-		if err := credState.ReplaceCreds(ctx, updatedCredentials); err != nil {
-			return nil, err
-		}
-	}
-
-	if credential.GetCredentialType(credState.CredentialsConfig) == credential.StaticAWS {
-		// This is a validate check to make sure that we aren't disabling
-		// rotation for credentials currently being managed by rotation.
-		// This is not allowed.
-		if newCatalogAttributes.DisableCredentialRotation && newCatalog.GetSecrets() == nil {
-			if !credState.CredsLastRotatedTime.IsZero() {
-				return nil, errors.BadRequestStatus("cannot disable rotation for already-rotated credentials")
-			}
-		}
-
-		// If we're enabling rotation now but didn't before, or have
-		// freshly replaced credentials, we can rotate here.
-		if !newCatalogAttributes.DisableCredentialRotation && credState.CredsLastRotatedTime.IsZero() {
-			if err := credState.RotateCreds(ctx); err != nil {
-				return nil, err
-			}
-		}
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
 	}
 
 	catalogState, err := newAwsCatalogPersistedState(
@@ -204,17 +156,45 @@ func (p *HostPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalo
 		}, p.testCatalogStateOpts...)...,
 	)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error loading persisted state: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
 	}
 
-	// perform dry run to ensure we can interact with AWS as expected.
-	if st := dryRunValidation(ctx, catalogState); st != nil {
-		return nil, st.Err()
+	if cred.GetCredentialType(credState.CredentialsConfig) == cred.StaticAWS {
+		if catalogAttributes.DisableCredentialRotation && !updateSecrets {
+			// This is a validate check to make sure that we aren't disabling
+			// rotation for credentials currently being managed by rotation.
+			// This is not allowed.
+			if !credState.CredsLastRotatedTime.IsZero() {
+				return nil, status.Error(codes.FailedPrecondition, "cannot disable rotation for already-rotated credentials")
+			}
+		}
+
+		if updateSecrets {
+			catalogSecrets, err := credential.GetCredentialsConfig(secrets, catalogAttributes.CredentialAttributes, true)
+			if err != nil {
+				return nil, err
+			}
+
+			// Replace the credentials. This checks the timestamp on the last
+			// rotation time as well and deletes the credentials if we are
+			// managing them (ie: if we've rotated them before).
+			if err := credState.ReplaceCreds(ctx, catalogSecrets); err != nil {
+				return nil, status.Errorf(codes.InvalidArgument, "error attempting to replace credentials: %s", err)
+			}
+		}
+
+		if !catalogAttributes.DisableCredentialRotation && credState.CredsLastRotatedTime.IsZero() {
+			// If we're enabling rotation now but didn't before, or have
+			// freshly replaced credentials, we can rotate here.
+			if err := credState.RotateCreds(ctx); err != nil {
+				return nil, status.Errorf(codes.InvalidArgument, "error during credential rotation: %s", err)
+			}
+		}
 	}
 
 	persistedProto, err := catalogState.toProto()
 	if err != nil {
-		return nil, errors.BadRequestStatus(err.Error())
+		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
 	return &pb.OnUpdateCatalogResponse{
@@ -226,12 +206,12 @@ func (p *HostPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalo
 func (p *HostPlugin) OnDeleteCatalog(ctx context.Context, req *pb.OnDeleteCatalogRequest) (*pb.OnDeleteCatalogResponse, error) {
 	catalog := req.GetCatalog()
 	if catalog == nil {
-		return nil, errors.BadRequestStatus("catalog is required")
+		return nil, status.Error(codes.InvalidArgument, "new catalog is nil")
 	}
 
 	attrs := catalog.GetAttributes()
 	if attrs == nil {
-		return nil, errors.BadRequestStatus("catalog attributes are required")
+		return nil, status.Error(codes.InvalidArgument, "new catalog missing attributes")
 	}
 
 	catalogAttributes, err := getCatalogAttributes(attrs)
@@ -250,7 +230,7 @@ func (p *HostPlugin) OnDeleteCatalog(ctx context.Context, req *pb.OnDeleteCatalo
 		catalogAttributes.CredentialAttributes,
 		p.testCredStateOpts...)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error loading persisted state: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
 	}
 
 	_, err = newAwsCatalogPersistedState(
@@ -259,17 +239,17 @@ func (p *HostPlugin) OnDeleteCatalog(ctx context.Context, req *pb.OnDeleteCatalo
 		}, p.testCatalogStateOpts...)...,
 	)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error loading persisted state: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
 	}
 
 	// try to delete static credentials
-	if credential.GetCredentialType(credState.CredentialsConfig) == credential.StaticAWS {
+	if cred.GetCredentialType(credState.CredentialsConfig) == cred.StaticAWS {
 		if !credState.CredsLastRotatedTime.IsZero() {
 			// Delete old/existing credentials. This is done with the same
 			// credentials to ensure that it has the proper permissions to do
 			// it.
 			if err := credState.DeleteCreds(ctx); err != nil {
-				return nil, err
+				return nil, status.Errorf(codes.Aborted, "error removing rotated credentials during catalog deletion: %s", err)
 			}
 		}
 	}
@@ -308,12 +288,12 @@ func (p *HostPlugin) NormalizeSetData(ctx context.Context, req *pb.NormalizeSetD
 func (p *HostPlugin) OnCreateSet(ctx context.Context, req *pb.OnCreateSetRequest) (*pb.OnCreateSetResponse, error) {
 	catalog := req.GetCatalog()
 	if catalog == nil {
-		return nil, errors.BadRequestStatus("catalog is required")
+		return nil, status.Error(codes.InvalidArgument, "catalog is nil")
 	}
 
 	catalogAttrsRaw := catalog.GetAttributes()
 	if catalogAttrsRaw == nil {
-		return nil, errors.BadRequestStatus("catalog attributes are required")
+		return nil, status.Error(codes.InvalidArgument, "catalog missing attributes")
 	}
 
 	catalogAttributes, err := getCatalogAttributes(catalogAttrsRaw)
@@ -326,7 +306,7 @@ func (p *HostPlugin) OnCreateSet(ctx context.Context, req *pb.OnCreateSetRequest
 		catalogAttributes.CredentialAttributes,
 		p.testCredStateOpts...)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error loading persisted state: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
 	}
 
 	catalogState, err := newAwsCatalogPersistedState(
@@ -335,43 +315,56 @@ func (p *HostPlugin) OnCreateSet(ctx context.Context, req *pb.OnCreateSetRequest
 		}, p.testCatalogStateOpts...)...,
 	)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error loading persisted state: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
 	}
 
 	set := req.GetSet()
 	if set == nil {
-		return nil, errors.BadRequestStatus("set is required")
+		return nil, status.Error(codes.InvalidArgument, "set is nil")
 	}
 
 	if set.GetAttributes() == nil {
-		return nil, errors.BadRequestStatus("set attributes are required")
+		return nil, status.Error(codes.InvalidArgument, "set missing attributes")
 	}
 	setAttrs, err := getSetAttributes(set.GetAttributes())
 	if err != nil {
 		return nil, err
 	}
 
-	describeInstanceFilters, err := buildFilters(setAttrs)
+	ec2Client, err := catalogState.EC2Client(ctx)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error building set filters: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error getting EC2 client: %s", err)
 	}
 
-	if st := dryRunValidation(ctx, catalogState, describeInstanceFilters...); st != nil {
-		return nil, st.Err()
+	input, err := buildDescribeInstancesInput(setAttrs, true)
+	if err != nil {
+		return nil, status.Errorf(codes.InvalidArgument, "error building DescribeInstances parameters: %s", err)
 	}
-	return &pb.OnCreateSetResponse{}, nil
+
+	_, err = ec2Client.DescribeInstances(ctx, input)
+	if err == nil {
+		return nil, status.Error(codes.FailedPrecondition, "query error: DescribeInstances DryRun should have returned error, but none was found")
+	}
+
+	var apiErr smithy.APIError
+	if errors.As(err, &apiErr) && apiErr.ErrorCode() == "DryRunOperation" {
+		// Success
+		return &pb.OnCreateSetResponse{}, nil
+	}
+
+	return nil, status.Errorf(codes.InvalidArgument, "error performing dry run of DescribeInstances: %s", err)
 }
 
 // OnUpdateSet is called when a dynamic host set is updated.
 func (p *HostPlugin) OnUpdateSet(ctx context.Context, req *pb.OnUpdateSetRequest) (*pb.OnUpdateSetResponse, error) {
 	catalog := req.GetCatalog()
 	if catalog == nil {
-		return nil, errors.BadRequestStatus("catalog is required")
+		return nil, status.Error(codes.InvalidArgument, "catalog is nil")
 	}
 
 	catalogAttrsRaw := catalog.GetAttributes()
 	if catalogAttrsRaw == nil {
-		return nil, errors.BadRequestStatus("catalog attributes are required")
+		return nil, status.Error(codes.InvalidArgument, "catalog missing attributes")
 	}
 
 	catalogAttributes, err := getCatalogAttributes(catalogAttrsRaw)
@@ -384,7 +377,7 @@ func (p *HostPlugin) OnUpdateSet(ctx context.Context, req *pb.OnUpdateSetRequest
 		catalogAttributes.CredentialAttributes,
 		p.testCredStateOpts...)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error loading persisted state: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
 	}
 
 	catalogState, err := newAwsCatalogPersistedState(
@@ -393,34 +386,46 @@ func (p *HostPlugin) OnUpdateSet(ctx context.Context, req *pb.OnUpdateSetRequest
 		}, p.testCatalogStateOpts...)...,
 	)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error loading persisted state: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
 	}
 
 	// As with catalog, we don't need to really look at the old host
 	// set here, just need to work off of/validate the new set config
 	set := req.GetNewSet()
 	if set == nil {
-		return nil, errors.BadRequestStatus("new set is required")
+		return nil, status.Error(codes.InvalidArgument, "new set is nil")
 	}
 
 	if set.GetAttributes() == nil {
-		return nil, errors.BadRequestStatus("new set attributes are required")
+		return nil, status.Error(codes.InvalidArgument, "new set missing attributes")
 	}
 	setAttrs, err := getSetAttributes(set.GetAttributes())
 	if err != nil {
 		return nil, err
 	}
 
-	describeInstanceFilters, err := buildFilters(setAttrs)
+	ec2Client, err := catalogState.EC2Client(ctx)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error building set filters: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error getting EC2 client: %s", err)
 	}
 
-	if st := dryRunValidation(ctx, catalogState, describeInstanceFilters...); st != nil {
-		return nil, st.Err()
+	input, err := buildDescribeInstancesInput(setAttrs, true)
+	if err != nil {
+		return nil, status.Errorf(codes.InvalidArgument, "error building DescribeInstances parameters: %s", err)
 	}
 
-	return &pb.OnUpdateSetResponse{}, nil
+	_, err = ec2Client.DescribeInstances(ctx, input)
+	if err == nil {
+		return nil, status.Error(codes.FailedPrecondition, "query error: DescribeInstances DryRun should have returned error, but none was found")
+	}
+
+	var apiErr smithy.APIError
+	if errors.As(err, &apiErr) && apiErr.ErrorCode() == "DryRunOperation" {
+		// Success
+		return &pb.OnUpdateSetResponse{}, nil
+	}
+
+	return nil, status.Errorf(codes.InvalidArgument, "error performing dry run of DescribeInstances: %s", err)
 }
 
 // OnDeleteSet is called when a dynamic host set is deleted.
@@ -434,12 +439,12 @@ func (p *HostPlugin) OnDeleteSet(ctx context.Context, req *pb.OnDeleteSetRequest
 func (p *HostPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*pb.ListHostsResponse, error) {
 	catalog := req.GetCatalog()
 	if catalog == nil {
-		return nil, errors.BadRequestStatus("catalog is required")
+		return nil, status.Error(codes.InvalidArgument, "catalog is nil")
 	}
 
 	catalogAttrsRaw := catalog.GetAttributes()
 	if catalogAttrsRaw == nil {
-		return nil, errors.BadRequestStatus("catalog attributes are required")
+		return nil, status.Error(codes.InvalidArgument, "catalog missing attributes")
 	}
 
 	catalogAttributes, err := getCatalogAttributes(catalogAttrsRaw)
@@ -452,7 +457,7 @@ func (p *HostPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*
 		catalogAttributes.CredentialAttributes,
 		p.testCredStateOpts...)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error loading persisted state: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
 	}
 
 	catalogState, err := newAwsCatalogPersistedState(
@@ -461,12 +466,12 @@ func (p *HostPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*
 		}, p.testCatalogStateOpts...)...,
 	)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error loading persisted state: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
 	}
 
 	sets := req.GetSets()
 	if sets == nil {
-		return nil, errors.BadRequestStatus("sets are required")
+		return nil, status.Error(codes.InvalidArgument, "sets is nil")
 	}
 
 	// Build all the queries in advance.
@@ -481,11 +486,11 @@ func (p *HostPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*
 	for i, set := range sets {
 		// Validate Id since we use it in output
 		if set.GetId() == "" {
-			return nil, errors.BadRequestStatus("set id is required")
+			return nil, status.Error(codes.InvalidArgument, "set missing id")
 		}
 
 		if set.GetAttributes() == nil {
-			return nil, errors.BadRequestStatus("set %s attributes are required", set.GetId())
+			return nil, status.Error(codes.InvalidArgument, "set missing attributes")
 		}
 		setAttrs, err := getSetAttributes(set.GetAttributes())
 		if err != nil {
@@ -494,7 +499,7 @@ func (p *HostPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*
 
 		input, err := buildDescribeInstancesInput(setAttrs, false)
 		if err != nil {
-			return nil, errors.BadRequestStatus("error building DescribeInstances parameters for host set id %q: %s", set.GetId(), err)
+			return nil, status.Errorf(codes.InvalidArgument, "error building DescribeInstances parameters for host set id %q: %s", set.GetId(), err)
 		}
 		queries[i] = hostSetQuery{
 			Id:    set.GetId(),
@@ -504,7 +509,7 @@ func (p *HostPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*
 
 	ec2Client, err := catalogState.EC2Client(ctx)
 	if err != nil {
-		return nil, errors.BadRequestStatus("error getting EC2 client: %s", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error getting EC2 client: %s", err)
 	}
 
 	// Run all queries now and assemble output.
@@ -512,7 +517,7 @@ func (p *HostPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*
 	for i, query := range queries {
 		output, err := ec2Client.DescribeInstances(ctx, query.Input)
 		if err != nil {
-			return nil, errors.BadRequestStatus("error running DescribeInstances for host set id %q: %s", query.Id, err)
+			return nil, status.Errorf(codes.InvalidArgument, "error running DescribeInstances for host set id %q: %s", query.Id, err)
 		}
 
 		queries[i].Output = output
@@ -523,7 +528,7 @@ func (p *HostPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*
 			for _, instance := range reservation.Instances {
 				host, err := awsInstanceToHost(instance)
 				if err != nil {
-					return nil, errors.BadRequestStatus("error processing host results for host set id %q: %s", query.Id, err)
+					return nil, status.Errorf(codes.InvalidArgument, "error processing host results for host set id %q: %s", query.Id, err)
 				}
 
 				queries[i].OutputHosts = append(queries[i].OutputHosts, host)
@@ -621,7 +626,7 @@ func buildDescribeInstancesInput(attrs *SetAttributes, dryRun bool) (*ec2.Descri
 // populated.
 func awsInstanceToHost(instance types.Instance) (*pb.ListHostsResponseHost, error) {
 	if aws.ToString(instance.InstanceId) == "" {
-		return nil, fmt.Errorf("response integrity error: missing instance id")
+		return nil, errors.New("response integrity error: missing instance id")
 	}
 
 	result := new(pb.ListHostsResponseHost)
@@ -669,28 +674,6 @@ func awsInstanceToHost(instance types.Instance) (*pb.ListHostsResponseHost, erro
 	return result, nil
 }
 
-// dryRunValidation performs an AWS DescribeInstances call to verify the state's
-// credentials, the host listing functionality as well as the filters, if any
-// are passed in. This function can therefore be used for both host catalog and
-// host set validation.
-func dryRunValidation(ctx context.Context, state *awsCatalogPersistedState, filters ...types.Filter) *status.Status {
-	if state == nil {
-		return status.New(codes.InvalidArgument, "persisted state is required")
-	}
-
-	ec2Client, err := state.EC2Client(ctx)
-	if err != nil {
-		return status.New(codes.InvalidArgument, fmt.Sprintf("error getting EC2 client: %s", err))
-	}
-
-	_, err = ec2Client.DescribeInstances(ctx, &ec2.DescribeInstancesInput{Filters: filters})
-	if err != nil {
-		return status.New(codes.FailedPrecondition, fmt.Sprintf("aws describe instances failed: %s", err))
-	}
-
-	return nil
-}
-
 // appendDistinct will append the elements to the slice
 // if an element is not nil, empty, and does not exist in slice.
 func appendDistinct(slice []string, elems ...*string) []string {

plugin/service/storage/error.go

@@ -1,6 +1,3 @@
-// Copyright (c) HashiCorp, Inc.
-// SPDX-License-Identifier: MPL-2.0
-
 package storage
 
 import (

plugin/service/storage/error_test.go

@@ -1,6 +1,3 @@
-// Copyright (c) HashiCorp, Inc.
-// SPDX-License-Identifier: MPL-2.0
-
 package storage
 
 import (

plugin/service/storage/plugin.go

@@ -64,7 +64,7 @@ func (p *StoragePlugin) OnCreateStorageBucket(ctx context.Context, req *pb.OnCre
 		return nil, err
 	}
 
-	credConfig, err := cred.GetCredentialsConfig(bucket.GetSecrets(), storageAttributes.CredentialAttributes)
+	credConfig, err := cred.GetCredentialsConfig(bucket.GetSecrets(), storageAttributes.CredentialAttributes, false)
 	if err != nil {
 		return nil, err
 	}
@@ -155,7 +155,7 @@ func (p *StoragePlugin) OnUpdateStorageBucket(ctx context.Context, req *pb.OnUpd
 	// Verify the incoming credentials are valid and return any errors to the
 	// user if they're not. Note this doesn't validate the credentials against
 	// AWS - it only does logical validation on the fields.
-	updatedCredentials, err := cred.GetCredentialsConfig(newBucket.GetSecrets(), newStorageAttributes.CredentialAttributes)
+	updatedCredentials, err := cred.GetCredentialsConfig(newBucket.GetSecrets(), newStorageAttributes.CredentialAttributes, false)
 	if err != nil {
 		return nil, err
 	}
@@ -318,7 +318,7 @@ func (p *StoragePlugin) HeadObject(ctx context.Context, req *pb.HeadObjectReques
 		return nil, err
 	}
 
-	credConfig, err := cred.GetCredentialsConfig(bucket.GetSecrets(), storageAttributes.CredentialAttributes)
+	credConfig, err := cred.GetCredentialsConfig(bucket.GetSecrets(), storageAttributes.CredentialAttributes, false)
 	if err != nil {
 		return nil, err
 	}
@@ -386,7 +386,7 @@ func (p *StoragePlugin) ValidatePermissions(ctx context.Context, req *pb.Validat
 		return nil, err
 	}
 
-	credConfig, err := cred.GetCredentialsConfig(bucket.GetSecrets(), storageAttributes.CredentialAttributes)
+	credConfig, err := cred.GetCredentialsConfig(bucket.GetSecrets(), storageAttributes.CredentialAttributes, false)
 	if err != nil {
 		return nil, err
 	}
@@ -444,7 +444,7 @@ func (p *StoragePlugin) GetObject(req *pb.GetObjectRequest, stream pb.StoragePlu
 		return err
 	}
 
-	credConfig, err := cred.GetCredentialsConfig(bucket.GetSecrets(), storageAttributes.CredentialAttributes)
+	credConfig, err := cred.GetCredentialsConfig(bucket.GetSecrets(), storageAttributes.CredentialAttributes, false)
 	if err != nil {
 		return err
 	}
@@ -555,7 +555,7 @@ func (p *StoragePlugin) PutObject(ctx context.Context, req *pb.PutObjectRequest)
 		return nil, err
 	}
 
-	credConfig, err := cred.GetCredentialsConfig(bucket.GetSecrets(), storageAttributes.CredentialAttributes)
+	credConfig, err := cred.GetCredentialsConfig(bucket.GetSecrets(), storageAttributes.CredentialAttributes, false)
 	if err != nil {
 		return nil, err
 	}
@@ -650,7 +650,7 @@ func (p *StoragePlugin) DeleteObjects(ctx context.Context, req *pb.DeleteObjects
 		return nil, err
 	}
 
-	credConfig, err := cred.GetCredentialsConfig(bucket.GetSecrets(), storageAttributes.CredentialAttributes)
+	credConfig, err := cred.GetCredentialsConfig(bucket.GetSecrets(), storageAttributes.CredentialAttributes, false)
 	if err != nil {
 		return nil, err
 	}