From 5ebf2fa5d010f274fbf9b6ce387a8115f9fb918c Mon Sep 17 00:00:00 2001 From: Haotian Date: Thu, 10 Mar 2022 11:00:57 -0800 Subject: [PATCH] refactor(errors): update plugin errors to include error codes --- plugin/attributes.go | 50 ++++++-- plugin/attributes_test.go | 14 ++- plugin/plugin.go | 110 ++++++++-------- plugin/plugin_test.go | 259 ++++++++++++++++++++++++-------------- 4 files changed, 269 insertions(+), 164 deletions(-) diff --git a/plugin/attributes.go b/plugin/attributes.go index 674be66..a887f7a 100644 --- a/plugin/attributes.go +++ b/plugin/attributes.go @@ -7,9 +7,23 @@ import ( "time" "github.com/mitchellh/mapstructure" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/structpb" ) +func invalidArgumentError(msg string, f map[string]string) error { + var fieldMsgs []string + for field, val := range f { + fieldMsgs = append(fieldMsgs, fmt.Sprintf("%s: %s", field, val)) + } + if len(fieldMsgs) > 0 { + sort.Strings(fieldMsgs) + msg = fmt.Sprintf("%s: [%s]", msg, strings.Join(fieldMsgs, ", ")) + } + return status.Error(codes.InvalidArgument, msg) +} + // CatalogAttributes is a Go-native representation of the Attributes // map. type CatalogAttributes struct { @@ -20,22 +34,27 @@ type CatalogAttributes struct { func getCatalogAttributes(in *structpb.Struct) (*CatalogAttributes, error) { unknownFields := structFields(in) result := new(CatalogAttributes) + badFields := make(map[string]string) var err error result.Region, err = getStringValue(in, constRegion, true) if err != nil { - return nil, err + badFields[fmt.Sprintf("attributes.%s", constRegion)] = err.Error() } delete(unknownFields, constRegion) result.DisableCredentialRotation, err = getBoolValue(in, constDisableCredentialRotation, false) if err != nil { - return nil, err + badFields[fmt.Sprintf("attributes.%s", constDisableCredentialRotation)] = err.Error() } delete(unknownFields, constDisableCredentialRotation) - if len(unknownFields) != 0 { - return nil, fmt.Errorf("unknown catalog attribute fields provided: %s", keysAsString(unknownFields)) + for s := range unknownFields { + badFields[fmt.Sprintf("attributes.%s", s)] = "unrecognized field" + } + + if len(badFields) > 0 { + return nil, invalidArgumentError("Invalid arguments in catalog attributes", badFields) } return result, nil @@ -50,22 +69,27 @@ type CatalogSecrets struct { func getCatalogSecrets(in *structpb.Struct) (*CatalogSecrets, error) { unknownFields := structFields(in) result := new(CatalogSecrets) + badFields := make(map[string]string) var err error result.AccessKeyId, err = getStringValue(in, constAccessKeyId, true) if err != nil { - return nil, err + badFields[fmt.Sprintf("secrets.%s", constAccessKeyId)] = err.Error() } delete(unknownFields, constAccessKeyId) result.SecretAccessKey, err = getStringValue(in, constSecretAccessKey, true) if err != nil { - return nil, err + badFields[fmt.Sprintf("secrets.%s", constSecretAccessKey)] = err.Error() } delete(unknownFields, constSecretAccessKey) - if len(unknownFields) != 0 { - return nil, fmt.Errorf("unknown catalog secret fields provided: %s", keysAsString(unknownFields)) + for s := range unknownFields { + badFields[fmt.Sprintf("secrets.%s", s)] = "unrecognized field" + } + + if len(badFields) > 0 { + return nil, invalidArgumentError("Error in the secrets provided", badFields) } return result, nil @@ -84,9 +108,13 @@ func getSetAttributes(in *structpb.Struct) (*SetAttributes, error) { // here for now. Make this more complex if we add more attributes // to host sets. unknownFields := structFields(in) + badFields := make(map[string]string) delete(unknownFields, constDescribeInstancesFilters) - if len(unknownFields) != 0 { - return nil, fmt.Errorf("unknown set attribute fields provided: %s", keysAsString(unknownFields)) + for a := range unknownFields { + badFields[fmt.Sprintf("attributes.%s", a)] = "unrecognized field" + } + if len(badFields) > 0 { + return nil, invalidArgumentError("Error in the attributes provided", badFields) } // Mapstructure complains if it expects a slice as output and sees a scalar @@ -101,7 +129,7 @@ func getSetAttributes(in *structpb.Struct) (*SetAttributes, error) { } if err := mapstructure.Decode(inMap, &setAttrs); err != nil { - return nil, fmt.Errorf("error decoding set attributes: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error decoding set attributes: %s", err) } return &setAttrs, nil diff --git a/plugin/attributes_test.go b/plugin/attributes_test.go index 18ccb66..650c50d 100644 --- a/plugin/attributes_test.go +++ b/plugin/attributes_test.go @@ -5,6 +5,8 @@ import ( "time" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/structpb" ) @@ -35,7 +37,7 @@ func TestGetCatalogAttributes(t *testing.T) { "foo": true, "bar": true, }), - expectedErrContains: "unknown catalog attribute fields provided: bar, foo", + expectedErrContains: "attributes.bar: Unrecognized field, attributes.foo: Unrecognized field", }, { name: "default", @@ -68,6 +70,7 @@ func TestGetCatalogAttributes(t *testing.T) { if tc.expectedErrContains != "" { require.Error(err) require.Contains(err.Error(), tc.expectedErrContains) + require.Equal(status.Code(err), codes.InvalidArgument) return } @@ -104,7 +107,7 @@ func TestGetCatalogSecrets(t *testing.T) { "foo": true, "bar": true, }), - expectedErrContains: "unknown catalog secret fields provided: bar, foo", + expectedErrContains: "secrets.bar: Unrecognized field, secrets.foo: Unrecognized field", }, { name: "good", @@ -127,6 +130,7 @@ func TestGetCatalogSecrets(t *testing.T) { if tc.expectedErrContains != "" { require.Error(err) require.Contains(err.Error(), tc.expectedErrContains) + require.Equal(status.Code(err), codes.InvalidArgument) return } @@ -170,7 +174,7 @@ func TestGetSetAttributes(t *testing.T) { "foo": true, "bar": true, }), - expectedErrContains: "unknown set attribute fields provided: bar, foo", + expectedErrContains: "attributes.bar: Unrecognized field, attributes.foo: Unrecognized field", }, { name: "good", @@ -194,6 +198,7 @@ func TestGetSetAttributes(t *testing.T) { if tc.expectedErrContains != "" { require.Error(err) require.Contains(err.Error(), tc.expectedErrContains) + require.Equal(status.Code(err), codes.InvalidArgument) return } @@ -413,7 +418,8 @@ func TestValidateRegion(t *testing.T) { require := require.New(t) err := validateRegion(tc.in) if tc.expectedErr != "" { - require.EqualError(err, tc.expectedErr) + require.Contains(err.Error(), tc.expectedErr) + require.Equal(status.Code(err), codes.InvalidArgument) return } diff --git a/plugin/plugin.go b/plugin/plugin.go index f484c8e..fedac13 100644 --- a/plugin/plugin.go +++ b/plugin/plugin.go @@ -12,6 +12,8 @@ import ( "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/service/ec2" pb "github.com/hashicorp/boundary/sdk/pbs/plugin" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) // rotationWaitTimeout controls the time we wait for credential rotation to @@ -36,17 +38,17 @@ var ( func (p *AwsPlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalogRequest) (*pb.OnCreateCatalogResponse, error) { catalog := req.GetCatalog() if catalog == nil { - return nil, errors.New("catalog is nil") + return nil, status.Error(codes.InvalidArgument, "catalog is nil") } secrets := catalog.GetSecrets() if secrets == nil { - return nil, errors.New("secrets are required") + return nil, status.Error(codes.InvalidArgument, "secrets are required") } attrs := catalog.GetAttributes() if attrs == nil { - return nil, errors.New("attributes are required") + return nil, status.Error(codes.InvalidArgument, "attributes are required") } catalogAttributes, err := getCatalogAttributes(attrs) @@ -68,24 +70,24 @@ func (p *AwsPlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalog withSecretAccessKey(catalogSecrets.SecretAccessKey), }, p.testStateOpts...)...) if err != nil { - return nil, fmt.Errorf("error setting up persisted state: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error setting up persisted state: %s", err) } // Try to rotate the credentials if we're not skipping them. if !catalogAttributes.DisableCredentialRotation { if err := state.RotateCreds(); err != nil { - return nil, fmt.Errorf("error during credential rotation: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error during credential rotation: %s", err) } } else { // Simply validate if we aren't rotating. if err := state.ValidateCreds(); err != nil { - return nil, fmt.Errorf("error during credential validation: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error during credential validation: %s", err) } } persistedProto, err := state.ToProto() if err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } return &pb.OnCreateCatalogResponse{ @@ -99,7 +101,7 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog // fetching the new copy to keep things a little less messy. catalog := req.GetNewCatalog() if catalog == nil { - return nil, errors.New("new catalog is nil") + return nil, status.Error(codes.InvalidArgument, "new catalog is nil") } var updateSecrets bool @@ -112,7 +114,7 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog attrs := catalog.GetAttributes() if attrs == nil { - return nil, errors.New("new catalog missing attributes") + return nil, status.Error(codes.InvalidArgument, "new catalog missing attributes") } catalogAttributes, err := getCatalogAttributes(attrs) @@ -134,7 +136,7 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog // is fine and important, but this may change in the future. state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...) if err != nil { - return nil, fmt.Errorf("error loading persisted state: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err) } if catalogAttributes.DisableCredentialRotation && !updateSecrets { @@ -142,7 +144,7 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog // rotation for credentials currently being managed by rotation. // This is not allowed. if !state.CredsLastRotatedTime.IsZero() { - return nil, errors.New("cannot disable rotation for already-rotated credentials") + return nil, status.Error(codes.FailedPrecondition, "cannot disable rotation for already-rotated credentials") } } @@ -156,7 +158,7 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog // rotation time as well and deletes the credentials if we are // managing them (ie: if we've rotated them before). if err := state.ReplaceCreds(catalogSecrets.AccessKeyId, catalogSecrets.SecretAccessKey); err != nil { - return nil, fmt.Errorf("error attempting to replace credentials: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error attempting to replace credentials: %s", err) } } @@ -164,14 +166,14 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog // If we're enabling rotation now but didn't before, or have // freshly replaced credentials, we can rotate here. if err := state.RotateCreds(); err != nil { - return nil, fmt.Errorf("error during credential rotation: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error during credential rotation: %s", err) } } // That's it! persistedProto, err := state.ToProto() if err != nil { - return nil, err + return nil, status.Error(codes.InvalidArgument, err.Error()) } return &pb.OnUpdateCatalogResponse{ @@ -188,7 +190,7 @@ func (p *AwsPlugin) OnDeleteCatalog(ctx context.Context, req *pb.OnDeleteCatalog // may be corrupt/and or legitimately missing. state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...) if err != nil { - return nil, fmt.Errorf("error loading persisted state: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err) } if !state.CredsLastRotatedTime.IsZero() { @@ -196,7 +198,7 @@ func (p *AwsPlugin) OnDeleteCatalog(ctx context.Context, req *pb.OnDeleteCatalog // credentials to ensure that it has the proper permissions to do // it. if err := state.DeleteCreds(); err != nil { - return nil, fmt.Errorf("error removing rotated credentials during catalog deletion: %w", err) + return nil, status.Errorf(codes.Aborted, "error removing rotated credentials during catalog deletion: %s", err) } } @@ -206,54 +208,54 @@ func (p *AwsPlugin) OnDeleteCatalog(ctx context.Context, req *pb.OnDeleteCatalog func (p *AwsPlugin) OnCreateSet(ctx context.Context, req *pb.OnCreateSetRequest) (*pb.OnCreateSetResponse, error) { catalog := req.GetCatalog() if catalog == nil { - return nil, errors.New("catalog is nil") + return nil, status.Error(codes.InvalidArgument, "catalog is nil") } catalogAttrsRaw := catalog.GetAttributes() if catalogAttrsRaw == nil { - return nil, errors.New("catalog missing attributes") + return nil, status.Error(codes.InvalidArgument, "catalog missing attributes") } catalogAttributes, err := getCatalogAttributes(catalogAttrsRaw) if err != nil { - return nil, fmt.Errorf("integrity error in host catalog attributes: %w", err) + return nil, err } if err := validateRegion(catalogAttributes.Region); err != nil { - return nil, fmt.Errorf("integrity error in host catalog attributes: %w", err) + return nil, err } state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...) if err != nil { - return nil, fmt.Errorf("error loading persisted state: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err) } set := req.GetSet() if set == nil { - return nil, errors.New("set is nil") + return nil, status.Error(codes.InvalidArgument, "set is nil") } if set.GetAttributes() == nil { - return nil, errors.New("set missing attributes") + return nil, status.Error(codes.InvalidArgument, "set missing attributes") } setAttrs, err := getSetAttributes(set.GetAttributes()) if err != nil { - return nil, fmt.Errorf("error parsing set attributes: %w", err) + return nil, err } ec2Client, err := state.EC2Client(catalogAttributes.Region) if err != nil { - return nil, fmt.Errorf("error getting EC2 client: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error getting EC2 client: %s", err) } input, err := buildDescribeInstancesInput(setAttrs, true) if err != nil { - return nil, fmt.Errorf("error building DescribeInstances parameters: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error building DescribeInstances parameters: %s", err) } _, err = ec2Client.DescribeInstances(input) if err == nil { - return nil, errors.New("query error: DescribeInstances DryRun should have returned error, but none was found") + return nil, status.Error(codes.FailedPrecondition, "query error: DescribeInstances DryRun should have returned error, but none was found") } if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "DryRunOperation" { @@ -261,62 +263,62 @@ func (p *AwsPlugin) OnCreateSet(ctx context.Context, req *pb.OnCreateSetRequest) return &pb.OnCreateSetResponse{}, nil } - return nil, fmt.Errorf("error performing dry run of DescribeInstances: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error performing dry run of DescribeInstances: %s", err) } func (p *AwsPlugin) OnUpdateSet(ctx context.Context, req *pb.OnUpdateSetRequest) (*pb.OnUpdateSetResponse, error) { catalog := req.GetCatalog() if catalog == nil { - return nil, errors.New("catalog is nil") + return nil, status.Error(codes.InvalidArgument, "catalog is nil") } catalogAttrsRaw := catalog.GetAttributes() if catalogAttrsRaw == nil { - return nil, errors.New("catalog missing attributes") + return nil, status.Error(codes.InvalidArgument, "catalog missing attributes") } catalogAttributes, err := getCatalogAttributes(catalogAttrsRaw) if err != nil { - return nil, fmt.Errorf("integrity error in host catalog attributes: %w", err) + return nil, err } if err := validateRegion(catalogAttributes.Region); err != nil { - return nil, fmt.Errorf("integrity error in host catalog attributes: %w", err) + return nil, err } state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...) if err != nil { - return nil, fmt.Errorf("error loading persisted state: %w", 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.New("new set is nil") + return nil, status.Error(codes.InvalidArgument, "new set is nil") } if set.GetAttributes() == nil { - return nil, errors.New("new set missing attributes") + return nil, status.Error(codes.InvalidArgument, "new set missing attributes") } setAttrs, err := getSetAttributes(set.GetAttributes()) if err != nil { - return nil, fmt.Errorf("error parsing set attributes: %w", err) + return nil, err } ec2Client, err := state.EC2Client(catalogAttributes.Region) if err != nil { - return nil, fmt.Errorf("error getting EC2 client: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error getting EC2 client: %s", err) } input, err := buildDescribeInstancesInput(setAttrs, true) if err != nil { - return nil, fmt.Errorf("error building DescribeInstances parameters: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error building DescribeInstances parameters: %s", err) } _, err = ec2Client.DescribeInstances(input) if err == nil { - return nil, errors.New("query error: DescribeInstances DryRun should have returned error, but none was found") + return nil, status.Error(codes.FailedPrecondition, "query error: DescribeInstances DryRun should have returned error, but none was found") } if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "DryRunOperation" { @@ -324,7 +326,7 @@ func (p *AwsPlugin) OnUpdateSet(ctx context.Context, req *pb.OnUpdateSetRequest) return &pb.OnUpdateSetResponse{}, nil } - return nil, fmt.Errorf("error performing dry run of DescribeInstances: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error performing dry run of DescribeInstances: %s", err) } func (p *AwsPlugin) OnDeleteSet(ctx context.Context, req *pb.OnDeleteSetRequest) (*pb.OnDeleteSetResponse, error) { @@ -336,31 +338,31 @@ func (p *AwsPlugin) OnDeleteSet(ctx context.Context, req *pb.OnDeleteSetRequest) func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*pb.ListHostsResponse, error) { catalog := req.GetCatalog() if catalog == nil { - return nil, errors.New("catalog is nil") + return nil, status.Error(codes.InvalidArgument, "catalog is nil") } catalogAttrsRaw := catalog.GetAttributes() if catalogAttrsRaw == nil { - return nil, errors.New("catalog missing attributes") + return nil, status.Error(codes.InvalidArgument, "catalog missing attributes") } catalogAttributes, err := getCatalogAttributes(catalogAttrsRaw) if err != nil { - return nil, fmt.Errorf("integrity error in host catalog attributes: %w", err) + return nil, err } if err := validateRegion(catalogAttributes.Region); err != nil { - return nil, fmt.Errorf("integrity error in host catalog attributes: %w", err) + return nil, err } state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...) if err != nil { - return nil, fmt.Errorf("error loading persisted state: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err) } sets := req.GetSets() if sets == nil { - return nil, errors.New("sets is nil") + return nil, status.Error(codes.InvalidArgument, "sets is nil") } // Build all the queries in advance. @@ -375,20 +377,20 @@ func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*p for i, set := range sets { // Validate Id since we use it in output if set.GetId() == "" { - return nil, errors.New("set missing id") + return nil, status.Error(codes.InvalidArgument, "set missing id") } if set.GetAttributes() == nil { - return nil, errors.New("set missing attributes") + return nil, status.Error(codes.InvalidArgument, "set missing attributes") } setAttrs, err := getSetAttributes(set.GetAttributes()) if err != nil { - return nil, fmt.Errorf("error parsing set attributes: %w", err) + return nil, err } input, err := buildDescribeInstancesInput(setAttrs, false) if err != nil { - return nil, fmt.Errorf("error building DescribeInstances parameters for host set id %q: %w", 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(), @@ -398,7 +400,7 @@ func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*p ec2Client, err := state.EC2Client(catalogAttributes.Region) if err != nil { - return nil, fmt.Errorf("error getting EC2 client: %w", err) + return nil, status.Errorf(codes.InvalidArgument, "error getting EC2 client: %s", err) } // Run all queries now and assemble output. @@ -406,7 +408,7 @@ func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*p for i, query := range queries { output, err := ec2Client.DescribeInstances(query.Input) if err != nil { - return nil, fmt.Errorf("error running DescribeInstances for host set id %q: %w", 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 @@ -417,7 +419,7 @@ func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*p for _, instance := range reservation.Instances { host, err := awsInstanceToHost(instance) if err != nil { - return nil, fmt.Errorf("error processing host results for host set id %q: %w", 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) @@ -461,7 +463,7 @@ func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*p func validateRegion(region string) error { _, found := endpoints.PartitionForRegion(endpoints.DefaultPartitions(), region) if !found { - return fmt.Errorf("not a valid region: %s", region) + return status.Errorf(codes.InvalidArgument, "not a valid region: %s", region) } return nil diff --git a/plugin/plugin_test.go b/plugin/plugin_test.go index 8fee701..7982f96 100644 --- a/plugin/plugin_test.go +++ b/plugin/plugin_test.go @@ -14,27 +14,32 @@ import ( pb "github.com/hashicorp/boundary/sdk/pbs/plugin" "github.com/hashicorp/go-secure-stdlib/awsutil" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/structpb" ) func TestPluginOnCreateCatalogErr(t *testing.T) { cases := []struct { - name string - req *pb.OnCreateCatalogRequest - opts []awsCatalogPersistedStateOption - expectedErr string + name string + req *pb.OnCreateCatalogRequest + opts []awsCatalogPersistedStateOption + expectedErrContains string + expectedErrCode codes.Code }{ { - name: "nil catalog", - req: &pb.OnCreateCatalogRequest{}, - expectedErr: "catalog is nil", + name: "nil catalog", + req: &pb.OnCreateCatalogRequest{}, + expectedErrContains: "catalog is nil", + expectedErrCode: codes.InvalidArgument, }, { name: "nil secrets", req: &pb.OnCreateCatalogRequest{ Catalog: &hostcatalogs.HostCatalog{}, }, - expectedErr: "secrets are required", + expectedErrContains: "secrets are required", + expectedErrCode: codes.InvalidArgument, }, { name: "nil attributes", @@ -43,7 +48,8 @@ func TestPluginOnCreateCatalogErr(t *testing.T) { Secrets: new(structpb.Struct), }, }, - expectedErr: "attributes are required", + expectedErrContains: "attributes are required", + expectedErrCode: codes.InvalidArgument, }, { name: "error reading attributes", @@ -53,7 +59,8 @@ func TestPluginOnCreateCatalogErr(t *testing.T) { Attributes: new(structpb.Struct), }, }, - expectedErr: "missing required value \"region\"", + expectedErrContains: "missing required value \"region\"", + expectedErrCode: codes.InvalidArgument, }, { name: "error reading secrets", @@ -65,7 +72,8 @@ func TestPluginOnCreateCatalogErr(t *testing.T) { }), }, }, - expectedErr: "missing required value \"access_key_id\"", + expectedErrContains: "missing required value \"access_key_id\"", + expectedErrCode: codes.InvalidArgument, }, { name: "invalid region", @@ -80,7 +88,8 @@ func TestPluginOnCreateCatalogErr(t *testing.T) { }), }, }, - expectedErr: "not a valid region: foobar", + expectedErrContains: "not a valid region: foobar", + expectedErrCode: codes.InvalidArgument, }, { name: "persisted state setup error", @@ -100,7 +109,8 @@ func TestPluginOnCreateCatalogErr(t *testing.T) { return errors.New(testOptionErr) }, }, - expectedErr: fmt.Sprintf("error setting up persisted state: %s", testOptionErr), + expectedErrContains: fmt.Sprintf("error setting up persisted state: %s", testOptionErr), + expectedErrCode: codes.InvalidArgument, }, { name: "rotation error", @@ -124,7 +134,8 @@ func TestPluginOnCreateCatalogErr(t *testing.T) { ), }), }, - expectedErr: fmt.Sprintf("error during credential rotation: error rotating credentials: error calling CreateAccessKey: error calling aws.GetUser: %s", testGetUserErr), + expectedErrContains: fmt.Sprintf("error during credential rotation: error rotating credentials: error calling CreateAccessKey: error calling aws.GetUser: %s", testGetUserErr), + expectedErrCode: codes.InvalidArgument, }, { name: "validation error", @@ -149,7 +160,8 @@ func TestPluginOnCreateCatalogErr(t *testing.T) { ), }), }, - expectedErr: fmt.Sprintf("error during credential validation: error validating credentials: %s", testGetCallerIdentityErr), + expectedErrContains: fmt.Sprintf("error during credential validation: error validating credentials: %s", testGetCallerIdentityErr), + expectedErrCode: codes.InvalidArgument, }, } @@ -161,22 +173,25 @@ func TestPluginOnCreateCatalogErr(t *testing.T) { testStateOpts: tc.opts, } _, err := p.OnCreateCatalog(context.Background(), tc.req) - require.EqualError(err, tc.expectedErr) + require.Contains(err.Error(), tc.expectedErrContains) + require.Equal(status.Code(err).String(), tc.expectedErrCode.String()) }) } } func TestPluginOnUpdateCatalogErr(t *testing.T) { cases := []struct { - name string - req *pb.OnUpdateCatalogRequest - opts []awsCatalogPersistedStateOption - expectedErr string + name string + req *pb.OnUpdateCatalogRequest + opts []awsCatalogPersistedStateOption + expectedErrContains string + expectedErrCode codes.Code }{ { - name: "nil new catalog", - req: &pb.OnUpdateCatalogRequest{}, - expectedErr: "new catalog is nil", + name: "nil new catalog", + req: &pb.OnUpdateCatalogRequest{}, + expectedErrContains: "new catalog is nil", + expectedErrCode: codes.InvalidArgument, }, { name: "nil new attributes", @@ -185,7 +200,8 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { Secrets: new(structpb.Struct), }, }, - expectedErr: "new catalog missing attributes", + expectedErrContains: "new catalog missing attributes", + expectedErrCode: codes.InvalidArgument, }, { name: "error reading attributes", @@ -195,7 +211,8 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { Attributes: new(structpb.Struct), }, }, - expectedErr: "missing required value \"region\"", + expectedErrContains: "missing required value \"region\"", + expectedErrCode: codes.InvalidArgument, }, { name: "invalid region", @@ -206,7 +223,8 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { }), }, }, - expectedErr: "not a valid region: foobar", + expectedErrContains: "not a valid region: foobar", + expectedErrCode: codes.InvalidArgument, }, { name: "persisted state setup error", @@ -229,7 +247,8 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { return errors.New(testOptionErr) }, }, - expectedErr: fmt.Sprintf("error loading persisted state: %s", testOptionErr), + expectedErrContains: fmt.Sprintf("error loading persisted state: %s", testOptionErr), + expectedErrCode: codes.InvalidArgument, }, { name: "cannot disable rotation for already rotated credentials", @@ -248,7 +267,8 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { }), }, }, - expectedErr: "cannot disable rotation for already-rotated credentials", + expectedErrContains: "cannot disable rotation for already-rotated credentials", + expectedErrCode: codes.FailedPrecondition, }, { name: "error reading secrets", @@ -267,7 +287,8 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { }), }, }, - expectedErr: "missing required value \"access_key_id\"", + expectedErrContains: "missing required value \"access_key_id\"", + expectedErrCode: codes.InvalidArgument, }, { name: "updating secrets, replace error", @@ -299,7 +320,8 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { ), }), }, - expectedErr: fmt.Sprintf("error attempting to replace credentials: error deleting old access key: %s", testDeleteAccessKeyErr), + expectedErrContains: fmt.Sprintf("error attempting to replace credentials: error deleting old access key: %s", testDeleteAccessKeyErr), + expectedErrCode: codes.InvalidArgument, }, { name: "rotation error", @@ -325,7 +347,8 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { ), }), }, - expectedErr: fmt.Sprintf("error during credential rotation: error rotating credentials: error calling CreateAccessKey: error calling aws.GetUser: %s", testGetUserErr), + expectedErrContains: fmt.Sprintf("error during credential rotation: error rotating credentials: error calling CreateAccessKey: error calling aws.GetUser: %s", testGetUserErr), + expectedErrCode: codes.InvalidArgument, }, } @@ -337,17 +360,19 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { testStateOpts: tc.opts, } _, err := p.OnUpdateCatalog(context.Background(), tc.req) - require.EqualError(err, tc.expectedErr) + require.Contains(err.Error(), tc.expectedErrContains) + require.Equal(status.Code(err).String(), tc.expectedErrCode.String()) }) } } func TestPluginOnDeleteCatalogErr(t *testing.T) { cases := []struct { - name string - req *pb.OnDeleteCatalogRequest - opts []awsCatalogPersistedStateOption - expectedErr string + name string + req *pb.OnDeleteCatalogRequest + opts []awsCatalogPersistedStateOption + expectedErrContains string + expectedErrCode codes.Code }{ { name: "persisted state setup error", @@ -370,7 +395,8 @@ func TestPluginOnDeleteCatalogErr(t *testing.T) { return errors.New(testOptionErr) }, }, - expectedErr: fmt.Sprintf("error loading persisted state: %s", testOptionErr), + expectedErrContains: fmt.Sprintf("error loading persisted state: %s", testOptionErr), + expectedErrCode: codes.InvalidArgument, }, { name: "delete error", @@ -397,7 +423,8 @@ func TestPluginOnDeleteCatalogErr(t *testing.T) { ), }), }, - expectedErr: fmt.Sprintf("error removing rotated credentials during catalog deletion: error deleting old access key: %s", testDeleteAccessKeyErr), + expectedErrContains: fmt.Sprintf("error removing rotated credentials during catalog deletion: error deleting old access key: %s", testDeleteAccessKeyErr), + expectedErrCode: codes.Aborted, }, } @@ -409,22 +436,25 @@ func TestPluginOnDeleteCatalogErr(t *testing.T) { testStateOpts: tc.opts, } _, err := p.OnDeleteCatalog(context.Background(), tc.req) - require.EqualError(err, tc.expectedErr) + require.Contains(err.Error(), tc.expectedErrContains) + require.Equal(status.Code(err).String(), tc.expectedErrCode.String()) }) } } func TestPluginOnCreateSetErr(t *testing.T) { cases := []struct { - name string - req *pb.OnCreateSetRequest - opts []awsCatalogPersistedStateOption - expectedErr string + name string + req *pb.OnCreateSetRequest + opts []awsCatalogPersistedStateOption + expectedErrContains string + expectedErrCode codes.Code }{ { - name: "nil catalog", - req: &pb.OnCreateSetRequest{}, - expectedErr: "catalog is nil", + name: "nil catalog", + req: &pb.OnCreateSetRequest{}, + expectedErrContains: "catalog is nil", + expectedErrCode: codes.InvalidArgument, }, { name: "nil catalog attributes", @@ -433,7 +463,8 @@ func TestPluginOnCreateSetErr(t *testing.T) { Secrets: new(structpb.Struct), }, }, - expectedErr: "catalog missing attributes", + expectedErrContains: "catalog missing attributes", + expectedErrCode: codes.InvalidArgument, }, { name: "error reading catalog attributes", @@ -443,7 +474,8 @@ func TestPluginOnCreateSetErr(t *testing.T) { Attributes: new(structpb.Struct), }, }, - expectedErr: "integrity error in host catalog attributes: missing required value \"region\"", + expectedErrContains: "missing required value \"region\"", + expectedErrCode: codes.InvalidArgument, }, { name: "invalid region", @@ -454,7 +486,8 @@ func TestPluginOnCreateSetErr(t *testing.T) { }), }, }, - expectedErr: "integrity error in host catalog attributes: not a valid region: foobar", + expectedErrContains: "not a valid region: foobar", + expectedErrCode: codes.InvalidArgument, }, { name: "persisted state setup error", @@ -477,7 +510,8 @@ func TestPluginOnCreateSetErr(t *testing.T) { return errors.New(testOptionErr) }, }, - expectedErr: fmt.Sprintf("error loading persisted state: %s", testOptionErr), + expectedErrContains: fmt.Sprintf("error loading persisted state: %s", testOptionErr), + expectedErrCode: codes.InvalidArgument, }, { name: "nil set", @@ -495,7 +529,8 @@ func TestPluginOnCreateSetErr(t *testing.T) { }), }, }, - expectedErr: "set is nil", + expectedErrContains: "set is nil", + expectedErrCode: codes.InvalidArgument, }, { name: "nil attributes in set", @@ -514,7 +549,8 @@ func TestPluginOnCreateSetErr(t *testing.T) { }, Set: &hostsets.HostSet{}, }, - expectedErr: "set missing attributes", + expectedErrContains: "set missing attributes", + expectedErrCode: codes.InvalidArgument, }, { name: "set attribute load error", @@ -538,7 +574,8 @@ func TestPluginOnCreateSetErr(t *testing.T) { }), }, }, - expectedErr: "error parsing set attributes: unknown set attribute fields provided: bar, foo", + expectedErrContains: "attributes.bar: Unrecognized field, attributes.foo: Unrecognized field", + expectedErrCode: codes.InvalidArgument, }, { name: "client load error", @@ -566,7 +603,8 @@ func TestPluginOnCreateSetErr(t *testing.T) { awsutil.MockOptionErr(errors.New(testOptionErr)), }), }, - expectedErr: fmt.Sprintf("error getting EC2 client: error getting AWS session: error reading options in NewCredentialsConfig: %s", testOptionErr), + expectedErrContains: fmt.Sprintf("error getting EC2 client: error getting AWS session: error reading options in NewCredentialsConfig: %s", testOptionErr), + expectedErrCode: codes.InvalidArgument, }, { name: "DescribeInstances error", @@ -595,7 +633,8 @@ func TestPluginOnCreateSetErr(t *testing.T) { testMockEC2WithDescribeInstancesError(errors.New(testDescribeInstancesError)), )), }, - expectedErr: fmt.Sprintf("error performing dry run of DescribeInstances: %s", testDescribeInstancesError), + expectedErrContains: fmt.Sprintf("error performing dry run of DescribeInstances: %s", testDescribeInstancesError), + expectedErrCode: codes.InvalidArgument, }, { name: "DescribeInstances non-error array filter", @@ -623,7 +662,8 @@ func TestPluginOnCreateSetErr(t *testing.T) { nil, )), }, - expectedErr: "query error: DescribeInstances DryRun should have returned error, but none was found", + expectedErrContains: "query error: DescribeInstances DryRun should have returned error, but none was found", + expectedErrCode: codes.FailedPrecondition, }, { name: "DescribeInstances non-error string filter", @@ -651,7 +691,8 @@ func TestPluginOnCreateSetErr(t *testing.T) { nil, )), }, - expectedErr: "query error: DescribeInstances DryRun should have returned error, but none was found", + expectedErrContains: "query error: DescribeInstances DryRun should have returned error, but none was found", + expectedErrCode: codes.FailedPrecondition, }, } @@ -663,22 +704,25 @@ func TestPluginOnCreateSetErr(t *testing.T) { testStateOpts: tc.opts, } _, err := p.OnCreateSet(context.Background(), tc.req) - require.EqualError(err, tc.expectedErr) + require.Contains(err.Error(), tc.expectedErrContains) + require.Equal(status.Code(err).String(), tc.expectedErrCode.String()) }) } } func TestPluginOnUpdateSetErr(t *testing.T) { cases := []struct { - name string - req *pb.OnUpdateSetRequest - opts []awsCatalogPersistedStateOption - expectedErr string + name string + req *pb.OnUpdateSetRequest + opts []awsCatalogPersistedStateOption + expectedErrContains string + expectedErrCode codes.Code }{ { - name: "nil catalog", - req: &pb.OnUpdateSetRequest{}, - expectedErr: "catalog is nil", + name: "nil catalog", + req: &pb.OnUpdateSetRequest{}, + expectedErrContains: "catalog is nil", + expectedErrCode: codes.InvalidArgument, }, { name: "nil catalog attributes", @@ -687,7 +731,8 @@ func TestPluginOnUpdateSetErr(t *testing.T) { Secrets: new(structpb.Struct), }, }, - expectedErr: "catalog missing attributes", + expectedErrContains: "catalog missing attributes", + expectedErrCode: codes.InvalidArgument, }, { name: "catalog attribute load error", @@ -697,7 +742,8 @@ func TestPluginOnUpdateSetErr(t *testing.T) { Attributes: new(structpb.Struct), }, }, - expectedErr: "integrity error in host catalog attributes: missing required value \"region\"", + expectedErrContains: "missing required value \"region\"", + expectedErrCode: codes.InvalidArgument, }, { name: "invalid region", @@ -709,7 +755,8 @@ func TestPluginOnUpdateSetErr(t *testing.T) { }), }, }, - expectedErr: "integrity error in host catalog attributes: not a valid region: foobar", + expectedErrContains: "not a valid region: foobar", + expectedErrCode: codes.InvalidArgument, }, { name: "persisted state setup error", @@ -732,7 +779,8 @@ func TestPluginOnUpdateSetErr(t *testing.T) { return errors.New(testOptionErr) }, }, - expectedErr: fmt.Sprintf("error loading persisted state: %s", testOptionErr), + expectedErrContains: fmt.Sprintf("error loading persisted state: %s", testOptionErr), + expectedErrCode: codes.InvalidArgument, }, { name: "nil set", @@ -750,7 +798,8 @@ func TestPluginOnUpdateSetErr(t *testing.T) { }), }, }, - expectedErr: "new set is nil", + expectedErrContains: "new set is nil", + expectedErrCode: codes.InvalidArgument, }, { name: "nil attributes in set", @@ -769,7 +818,8 @@ func TestPluginOnUpdateSetErr(t *testing.T) { }, NewSet: &hostsets.HostSet{}, }, - expectedErr: "new set missing attributes", + expectedErrContains: "new set missing attributes", + expectedErrCode: codes.InvalidArgument, }, { name: "set attribute load error", @@ -793,7 +843,8 @@ func TestPluginOnUpdateSetErr(t *testing.T) { }), }, }, - expectedErr: "error parsing set attributes: unknown set attribute fields provided: bar, foo", + expectedErrContains: "attributes.bar: Unrecognized field, attributes.foo: Unrecognized field", + expectedErrCode: codes.InvalidArgument, }, { name: "client load error", @@ -821,7 +872,8 @@ func TestPluginOnUpdateSetErr(t *testing.T) { awsutil.MockOptionErr(errors.New(testOptionErr)), }), }, - expectedErr: fmt.Sprintf("error getting EC2 client: error getting AWS session: error reading options in NewCredentialsConfig: %s", testOptionErr), + expectedErrContains: fmt.Sprintf("error getting EC2 client: error getting AWS session: error reading options in NewCredentialsConfig: %s", testOptionErr), + expectedErrCode: codes.InvalidArgument, }, { name: "DescribeInstances error", @@ -850,7 +902,8 @@ func TestPluginOnUpdateSetErr(t *testing.T) { testMockEC2WithDescribeInstancesError(errors.New(testDescribeInstancesError)), )), }, - expectedErr: fmt.Sprintf("error performing dry run of DescribeInstances: %s", testDescribeInstancesError), + expectedErrContains: fmt.Sprintf("error performing dry run of DescribeInstances: %s", testDescribeInstancesError), + expectedErrCode: codes.InvalidArgument, }, { name: "DescribeInstances non-error", @@ -878,7 +931,8 @@ func TestPluginOnUpdateSetErr(t *testing.T) { nil, )), }, - expectedErr: "query error: DescribeInstances DryRun should have returned error, but none was found", + expectedErrContains: "query error: DescribeInstances DryRun should have returned error, but none was found", + expectedErrCode: codes.FailedPrecondition, }, } @@ -890,22 +944,25 @@ func TestPluginOnUpdateSetErr(t *testing.T) { testStateOpts: tc.opts, } _, err := p.OnUpdateSet(context.Background(), tc.req) - require.EqualError(err, tc.expectedErr) + require.Contains(err.Error(), tc.expectedErrContains) + require.Equal(status.Code(err).String(), tc.expectedErrCode.String()) }) } } func TestPluginListHostsErr(t *testing.T) { cases := []struct { - name string - req *pb.ListHostsRequest - opts []awsCatalogPersistedStateOption - expectedErr string + name string + req *pb.ListHostsRequest + opts []awsCatalogPersistedStateOption + expectedErrContains string + expectedErrCode codes.Code }{ { - name: "nil catalog", - req: &pb.ListHostsRequest{}, - expectedErr: "catalog is nil", + name: "nil catalog", + req: &pb.ListHostsRequest{}, + expectedErrContains: "catalog is nil", + expectedErrCode: codes.InvalidArgument, }, { name: "nil catalog attributes", @@ -914,7 +971,8 @@ func TestPluginListHostsErr(t *testing.T) { Secrets: new(structpb.Struct), }, }, - expectedErr: "catalog missing attributes", + expectedErrContains: "catalog missing attributes", + expectedErrCode: codes.InvalidArgument, }, { name: "error reading catalog attributes", @@ -924,7 +982,8 @@ func TestPluginListHostsErr(t *testing.T) { Attributes: new(structpb.Struct), }, }, - expectedErr: "integrity error in host catalog attributes: missing required value \"region\"", + expectedErrContains: "attributes.region: missing required value \"region\"", + expectedErrCode: codes.InvalidArgument, }, { name: "invalid region", @@ -936,7 +995,8 @@ func TestPluginListHostsErr(t *testing.T) { }), }, }, - expectedErr: "integrity error in host catalog attributes: not a valid region: foobar", + expectedErrContains: "not a valid region: ", + expectedErrCode: codes.InvalidArgument, }, { name: "persisted state setup error", @@ -959,7 +1019,8 @@ func TestPluginListHostsErr(t *testing.T) { return errors.New(testOptionErr) }, }, - expectedErr: fmt.Sprintf("error loading persisted state: %s", testOptionErr), + expectedErrContains: fmt.Sprintf("error loading persisted state: %s", testOptionErr), + expectedErrCode: codes.InvalidArgument, }, { name: "nil sets", @@ -977,7 +1038,8 @@ func TestPluginListHostsErr(t *testing.T) { }), }, }, - expectedErr: "sets is nil", + expectedErrContains: "sets is nil", + expectedErrCode: codes.InvalidArgument, }, { name: "set missing id", @@ -996,7 +1058,8 @@ func TestPluginListHostsErr(t *testing.T) { }, Sets: []*hostsets.HostSet{&hostsets.HostSet{}}, }, - expectedErr: "set missing id", + expectedErrContains: "set missing id", + expectedErrCode: codes.InvalidArgument, }, { name: "set missing attributes", @@ -1019,7 +1082,8 @@ func TestPluginListHostsErr(t *testing.T) { }, }, }, - expectedErr: "set missing attributes", + expectedErrContains: "set missing attributes", + expectedErrCode: codes.InvalidArgument, }, { name: "set attribute load error", @@ -1046,7 +1110,8 @@ func TestPluginListHostsErr(t *testing.T) { }, }, }, - expectedErr: "error parsing set attributes: unknown set attribute fields provided: bar, foo", + expectedErrContains: "attributes.bar: Unrecognized field, attributes.foo: Unrecognized field", + expectedErrCode: codes.InvalidArgument, }, { name: "client load error", @@ -1077,7 +1142,8 @@ func TestPluginListHostsErr(t *testing.T) { awsutil.MockOptionErr(errors.New(testOptionErr)), }), }, - expectedErr: fmt.Sprintf("error getting EC2 client: error getting AWS session: error reading options in NewCredentialsConfig: %s", testOptionErr), + expectedErrContains: fmt.Sprintf("error getting EC2 client: error getting AWS session: error reading options in NewCredentialsConfig: %s", testOptionErr), + expectedErrCode: codes.InvalidArgument, }, { name: "DescribeInstances error", @@ -1109,7 +1175,8 @@ func TestPluginListHostsErr(t *testing.T) { testMockEC2WithDescribeInstancesError(errors.New(testDescribeInstancesError)), )), }, - expectedErr: fmt.Sprintf("error running DescribeInstances for host set id \"foobar\": %s", testDescribeInstancesError), + expectedErrContains: fmt.Sprintf("error running DescribeInstances for host set id \"foobar\": %s", testDescribeInstancesError), + expectedErrCode: codes.InvalidArgument, }, { name: "awsInstanceToHost error", @@ -1153,7 +1220,8 @@ func TestPluginListHostsErr(t *testing.T) { ), )), }, - expectedErr: "error processing host results for host set id \"foobar\": response integrity error: missing instance id", + expectedErrContains: "error processing host results for host set id \"foobar\": response integrity error: missing instance id", + expectedErrCode: codes.InvalidArgument, }, } @@ -1165,7 +1233,8 @@ func TestPluginListHostsErr(t *testing.T) { testStateOpts: tc.opts, } _, err := p.ListHosts(context.Background(), tc.req) - require.EqualError(err, tc.expectedErr) + require.Contains(err.Error(), tc.expectedErrContains) + require.Equal(status.Code(err).String(), tc.expectedErrCode.String()) }) } }