From 07270a7457031d198c61544e94fea1ff06fcc44c Mon Sep 17 00:00:00 2001 From: Chris Marchesi Date: Tue, 16 Nov 2021 17:29:17 -0800 Subject: [PATCH] plugin: add unknown attr checks, fix E2E test This adds checks for unknown attributes through the entire plugin lifecycle for all attributes, secrets, and persisted state. It also fixes the E2E test which seems to have drifted. --- plugin/attributes.go | 189 +++++++++++++ plugin/attributes_test.go | 423 ++++++++++++++++++++++++++++ plugin/e2e_test.go | 18 +- plugin/plugin.go | 211 ++++---------- plugin/plugin_test.go | 571 +++++++++++--------------------------- plugin/state.go | 9 + plugin/state_test.go | 14 + 7 files changed, 854 insertions(+), 581 deletions(-) create mode 100644 plugin/attributes.go create mode 100644 plugin/attributes_test.go diff --git a/plugin/attributes.go b/plugin/attributes.go new file mode 100644 index 0000000..02252ce --- /dev/null +++ b/plugin/attributes.go @@ -0,0 +1,189 @@ +package plugin + +import ( + "fmt" + "sort" + "strings" + "time" + + "github.com/mitchellh/mapstructure" + "google.golang.org/protobuf/types/known/structpb" +) + +// CatalogAttributes is a Go-native representation of the Attributes +// map. +type CatalogAttributes struct { + Region string + DisableCredentialRotation bool +} + +func getCatalogAttributes(in *structpb.Struct) (*CatalogAttributes, error) { + unknownFields := structFields(in) + result := new(CatalogAttributes) + + var err error + result.Region, err = getStringValue(in, constRegion, true) + if err != nil { + return nil, err + } + delete(unknownFields, constRegion) + + result.DisableCredentialRotation, err = getBoolValue(in, constDisableCredentialRotation, false) + if err != nil { + return nil, err + } + delete(unknownFields, constDisableCredentialRotation) + + if len(unknownFields) != 0 { + return nil, fmt.Errorf("unknown catalog attribute fields provided: %s", keysAsString(unknownFields)) + } + + return result, nil +} + +// CatalogSecrets is a Go-native representation of the Secrets map. +type CatalogSecrets struct { + AccessKeyId string + SecretAccessKey string +} + +func getCatalogSecrets(in *structpb.Struct) (*CatalogSecrets, error) { + unknownFields := structFields(in) + result := new(CatalogSecrets) + + var err error + result.AccessKeyId, err = getStringValue(in, constAccessKeyId, true) + if err != nil { + return nil, err + } + delete(unknownFields, constAccessKeyId) + + result.SecretAccessKey, err = getStringValue(in, constSecretAccessKey, true) + if err != nil { + return nil, err + } + delete(unknownFields, constSecretAccessKey) + + if len(unknownFields) != 0 { + return nil, fmt.Errorf("unknown catalog secret fields provided: %s", keysAsString(unknownFields)) + } + + return result, nil +} + +// SetAttributes is a Go-native representation of the Attributes map that can be +// used for decoding the incoming map via mapstructure. +type SetAttributes struct { + Filters []string +} + +func getSetAttributes(in *structpb.Struct) (*SetAttributes, error) { + var setAttrs SetAttributes + + // Quick validation to ensure that there's no non-filter attributes + // here for now. Make this more complex if we add more attributes + // to host sets. + unknownFields := structFields(in) + delete(unknownFields, constDescribeInstancesFilters) + if len(unknownFields) != 0 { + return nil, fmt.Errorf("unknown set attribute fields provided: %s", keysAsString(unknownFields)) + } + + // Mapstructure complains if it expects a slice as output and sees a scalar + // value. Rather than use WeakDecode and risk unintended consequences, I'm + // manually making this change if necessary. + inMap := in.AsMap() + if filtersRaw, ok := inMap[constDescribeInstancesFilters]; ok { + switch filterVal := filtersRaw.(type) { + case string: + inMap[constDescribeInstancesFilters] = []string{filterVal} + } + } + + if err := mapstructure.Decode(inMap, &setAttrs); err != nil { + return nil, fmt.Errorf("error decoding set attributes: %w", err) + } + + return &setAttrs, nil +} + +func getStringValue(in *structpb.Struct, k string, required bool) (string, error) { + mv := in.AsMap() + v, ok := mv[k] + if !ok { + if required { + return "", fmt.Errorf("missing required value %q", k) + } + + return "", nil + } + + s, ok := v.(string) + if !ok { + return "", fmt.Errorf("unexpected type for value %q: want string, got %T", k, v) + } + + if s == "" && required { + return "", fmt.Errorf("value %q cannot be empty", k) + } + + return s, nil +} + +func getBoolValue(in *structpb.Struct, k string, required bool) (bool, error) { + mv := in.AsMap() + v, ok := mv[k] + if !ok { + if required { + return false, fmt.Errorf("missing required value %q", k) + } + + return false, nil + } + + b, ok := v.(bool) + if !ok { + return false, fmt.Errorf("unexpected type for value %q: want bool, got %T", k, v) + } + + return b, nil +} + +func getTimeValue(in *structpb.Struct, k string) (time.Time, error) { + mv := in.AsMap() + v, ok := mv[k] + if !ok { + return time.Time{}, nil + } + + tRaw, ok := v.(string) + if !ok { + return time.Time{}, fmt.Errorf("unexpected type for value %q: want string, got %T", k, v) + } + + t, err := time.Parse(time.RFC3339Nano, tRaw) + if err != nil { + return time.Time{}, fmt.Errorf("could not parse time in value %q: %w", k, err) + } + + return t, nil +} + +func structFields(s *structpb.Struct) map[string]struct{} { + m := make(map[string]struct{}, len(s.GetFields())) + for k := range s.GetFields() { + m[k] = struct{}{} + } + + return m +} + +func keysAsString(m map[string]struct{}) string { + s := make([]string, 0, len(m)) + for k := range m { + s = append(s, k) + } + + sort.Strings(s) + return strings.Join(s, ", ") +} diff --git a/plugin/attributes_test.go b/plugin/attributes_test.go new file mode 100644 index 0000000..18ccb66 --- /dev/null +++ b/plugin/attributes_test.go @@ -0,0 +1,423 @@ +package plugin + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" +) + +func TestGetCatalogAttributes(t *testing.T) { + cases := []struct { + name string + in *structpb.Struct + expected *CatalogAttributes + expectedErrContains string + }{ + { + name: "missing region", + in: mustStruct(map[string]interface{}{}), + expectedErrContains: "missing required value \"region\"", + }, + { + name: "bad value for disable_credential_rotation", + in: mustStruct(map[string]interface{}{ + constRegion: "us-west-2", + constDisableCredentialRotation: "sure", + }), + expectedErrContains: "unexpected type for value \"disable_credential_rotation\": want bool, got string", + }, + { + name: "unknown fields", + in: mustStruct(map[string]interface{}{ + constRegion: "us-west-2", + "foo": true, + "bar": true, + }), + expectedErrContains: "unknown catalog attribute fields provided: bar, foo", + }, + { + name: "default", + in: mustStruct(map[string]interface{}{ + constRegion: "us-west-2", + }), + expected: &CatalogAttributes{ + Region: "us-west-2", + DisableCredentialRotation: false, + }, + }, + { + name: "with disable_credential_rotation", + in: mustStruct(map[string]interface{}{ + constRegion: "us-west-2", + constDisableCredentialRotation: true, + }), + expected: &CatalogAttributes{ + Region: "us-west-2", + DisableCredentialRotation: true, + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + require := require.New(t) + actual, err := getCatalogAttributes(tc.in) + if tc.expectedErrContains != "" { + require.Error(err) + require.Contains(err.Error(), tc.expectedErrContains) + return + } + + require.NoError(err) + require.Equal(tc.expected, actual) + }) + } +} + +func TestGetCatalogSecrets(t *testing.T) { + cases := []struct { + name string + in *structpb.Struct + expected *CatalogSecrets + expectedErrContains string + }{ + { + name: "missing access_key_id", + in: mustStruct(map[string]interface{}{}), + expectedErrContains: "missing required value \"access_key_id\"", + }, + { + name: "missing secret_access_key", + in: mustStruct(map[string]interface{}{ + constAccessKeyId: "foobar", + }), + expectedErrContains: "missing required value \"secret_access_key\"", + }, + { + name: "unknown fields", + in: mustStruct(map[string]interface{}{ + constAccessKeyId: "foobar", + constSecretAccessKey: "bazqux", + "foo": true, + "bar": true, + }), + expectedErrContains: "unknown catalog secret fields provided: bar, foo", + }, + { + name: "good", + in: mustStruct(map[string]interface{}{ + constAccessKeyId: "foobar", + constSecretAccessKey: "bazqux", + }), + expected: &CatalogSecrets{ + AccessKeyId: "foobar", + SecretAccessKey: "bazqux", + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + require := require.New(t) + actual, err := getCatalogSecrets(tc.in) + if tc.expectedErrContains != "" { + require.Error(err) + require.Contains(err.Error(), tc.expectedErrContains) + return + } + + require.NoError(err) + require.Equal(tc.expected, actual) + }) + } +} + +func TestGetSetAttributes(t *testing.T) { + cases := []struct { + name string + in *structpb.Struct + expected *SetAttributes + expectedErrContains string + }{ + { + name: "missing", + in: mustStruct(map[string]interface{}{}), + expected: &SetAttributes{}, + }, + { + name: "non-string-slice value", + in: mustStruct(map[string]interface{}{ + constDescribeInstancesFilters: "zip=foo,bar", + }), + expected: &SetAttributes{ + Filters: []string{"zip=foo,bar"}, + }, + }, + { + name: "bad filter element value", + in: mustStruct(map[string]interface{}{ + constDescribeInstancesFilters: []interface{}{1}, + }), + expectedErrContains: "expected type 'string', got unconvertible type 'float64'", + }, + { + name: "unknown fields", + in: mustStruct(map[string]interface{}{ + "foo": true, + "bar": true, + }), + expectedErrContains: "unknown set attribute fields provided: bar, foo", + }, + { + name: "good", + in: mustStruct(map[string]interface{}{ + constDescribeInstancesFilters: []interface{}{ + "foo=bar", + "zip=zap", + }, + }), + expected: &SetAttributes{ + Filters: []string{"foo=bar", "zip=zap"}, + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + require := require.New(t) + actual, err := getSetAttributes(tc.in) + if tc.expectedErrContains != "" { + require.Error(err) + require.Contains(err.Error(), tc.expectedErrContains) + return + } + + require.NoError(err) + require.Equal(tc.expected, actual) + }) + } +} + +func TestGetStringValue(t *testing.T) { + cases := []struct { + name string + in *structpb.Struct + key string + required bool + expected string + expectedErr string + }{ + { + name: "required missing", + in: mustStruct(map[string]interface{}{}), + key: "foo", + required: true, + expectedErr: "missing required value \"foo\"", + }, + { + name: "optional missing", + in: mustStruct(map[string]interface{}{}), + key: "foo", + expected: "", + }, + { + name: "non-string value", + in: mustStruct(map[string]interface{}{ + "foo": 1, + }), + key: "foo", + expectedErr: "unexpected type for value \"foo\": want string, got float64", + }, + { + name: "required empty", + in: mustStruct(map[string]interface{}{ + "foo": "", + }), + key: "foo", + required: true, + expectedErr: "value \"foo\" cannot be empty", + }, + { + name: "good", + in: mustStruct(map[string]interface{}{ + "foo": "bar", + }), + key: "foo", + expected: "bar", + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + require := require.New(t) + actual, err := getStringValue(tc.in, tc.key, tc.required) + if tc.expectedErr != "" { + require.EqualError(err, tc.expectedErr) + return + } + + require.NoError(err) + require.Equal(tc.expected, actual) + }) + } +} + +func TestGetBoolValue(t *testing.T) { + cases := []struct { + name string + in *structpb.Struct + key string + required bool + expected bool + expectedErr string + }{ + { + name: "required missing", + in: mustStruct(map[string]interface{}{}), + key: "foo", + required: true, + expectedErr: "missing required value \"foo\"", + }, + { + name: "optional missing", + in: mustStruct(map[string]interface{}{}), + key: "foo", + expected: false, + }, + { + name: "non-bool value", + in: mustStruct(map[string]interface{}{ + "foo": "bar", + }), + key: "foo", + expectedErr: "unexpected type for value \"foo\": want bool, got string", + }, + { + name: "good", + in: mustStruct(map[string]interface{}{ + "foo": true, + }), + key: "foo", + expected: true, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + require := require.New(t) + actual, err := getBoolValue(tc.in, tc.key, tc.required) + if tc.expectedErr != "" { + require.EqualError(err, tc.expectedErr) + return + } + + require.NoError(err) + require.Equal(tc.expected, actual) + }) + } +} + +func TestGetTimeValue(t *testing.T) { + staticTime := time.Now() + + cases := []struct { + name string + in *structpb.Struct + key string + expected time.Time + expectedErr string + }{ + { + name: "missing", + in: mustStruct(map[string]interface{}{}), + key: "foo", + expected: time.Time{}, + }, + { + name: "non-time value", + in: mustStruct(map[string]interface{}{ + "foo": 1, + }), + key: "foo", + expectedErr: "unexpected type for value \"foo\": want string, got float64", + }, + { + name: "bad parse", + in: mustStruct(map[string]interface{}{ + "foo": "bar", + }), + key: "foo", + expectedErr: "could not parse time in value \"foo\": parsing time \"bar\" as \"2006-01-02T15:04:05.999999999Z07:00\": cannot parse \"bar\" as \"2006\"", + }, + { + name: "good", + in: mustStruct(map[string]interface{}{ + "foo": staticTime.Format(time.RFC3339Nano), + }), + key: "foo", + expected: func() time.Time { + u, err := time.Parse(time.RFC3339Nano, staticTime.Format(time.RFC3339Nano)) + if err != nil { + panic(err) + } + + return u + }(), + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + require := require.New(t) + actual, err := getTimeValue(tc.in, tc.key) + if tc.expectedErr != "" { + require.EqualError(err, tc.expectedErr) + return + } + + require.NoError(err) + require.Equal(tc.expected, actual) + }) + } +} + +func TestValidateRegion(t *testing.T) { + cases := []struct { + name string + in string + expectedErr string + }{ + { + name: "invalid region", + in: "foobar", + expectedErr: "not a valid region: foobar", + }, + { + name: "good", + in: "us-west-2", + expectedErr: "", + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + require := require.New(t) + err := validateRegion(tc.in) + if tc.expectedErr != "" { + require.EqualError(err, tc.expectedErr) + return + } + + require.NoError(err) + }) + } +} diff --git a/plugin/e2e_test.go b/plugin/e2e_test.go index 1f43e20..e2ccb4c 100644 --- a/plugin/e2e_test.go +++ b/plugin/e2e_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "strings" "testing" "time" @@ -453,16 +454,7 @@ func testPluginOnCreateUpdateSet(ctx context.Context, t *testing.T, p *AwsPlugin }) require.NoError(err) setAttrs, err := structpb.NewStruct(map[string]interface{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": func() []interface{} { - result := make([]interface{}, len(tags)) - for i, tag := range tags { - result[i] = tag - } - - return result - }(), - }, + constDescribeInstancesFilters: []interface{}{fmt.Sprintf("tag-key=%s", strings.Join(tags, ","))}, }) require.NoError(err) reqPersistedSecrets, err := structpb.NewStruct(map[string]interface{}{ @@ -518,11 +510,9 @@ func testPluginListHosts(ctx context.Context, t *testing.T, p *AwsPlugin, region }) require.NoError(err) sets := make([]*hostsets.HostSet, len(tags)) - for i, tag := range tags { + for i := range tags { setAttrs, err := structpb.NewStruct(map[string]interface{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": []interface{}{tag}, - }, + constDescribeInstancesFilters: []interface{}{fmt.Sprintf("tag-key=%s", strings.Join(tags, ","))}, }) require.NoError(err) sets[i] = &hostsets.HostSet{ diff --git a/plugin/plugin.go b/plugin/plugin.go index 2d96e1a..1c897e8 100644 --- a/plugin/plugin.go +++ b/plugin/plugin.go @@ -12,8 +12,6 @@ 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" - "github.com/mitchellh/mapstructure" - "google.golang.org/protobuf/types/known/structpb" ) // rotationWaitTimeout controls the time we wait for credential rotation to @@ -35,12 +33,6 @@ var ( _ pb.HostPluginServiceServer = (*AwsPlugin)(nil) ) -// SetAttributes is a Go-native representation of the Attributes map that can be -// used for decoding the incoming map via mapstructure. -type SetAttributes struct { - Filters []string -} - func (p *AwsPlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalogRequest) (*pb.OnCreateCatalogResponse, error) { catalog := req.GetCatalog() if catalog == nil { @@ -57,36 +49,30 @@ func (p *AwsPlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalog return nil, errors.New("attributes are required") } - if _, err := getValidateRegionValue(attrs); err != nil { + catalogAttributes, err := getCatalogAttributes(attrs) + if err != nil { return nil, err } - - accessKeyId, err := getStringValue(secrets, constAccessKeyId, true) + catalogSecrets, err := getCatalogSecrets(secrets) if err != nil { return nil, err } - secretAccessKey, err := getStringValue(secrets, constSecretAccessKey, true) - if err != nil { - return nil, err - } - - skipRotate, err := getBoolValue(attrs, constDisableCredentialRotation, false) - if err != nil { + if err := validateRegion(catalogAttributes.Region); err != nil { return nil, err } // Set up the creds in our persisted state. state, err := newAwsCatalogPersistedState(append([]awsCatalogPersistedStateOption{ - withAccessKeyId(accessKeyId), - withSecretAccessKey(secretAccessKey), + withAccessKeyId(catalogSecrets.AccessKeyId), + withSecretAccessKey(catalogSecrets.SecretAccessKey), }, p.testStateOpts...)...) if err != nil { return nil, fmt.Errorf("error setting up persisted state: %w", err) } // Try to rotate the credentials if we're not skipping them. - if !skipRotate { + if !catalogAttributes.DisableCredentialRotation { if err := state.RotateCreds(); err != nil { return nil, fmt.Errorf("error during credential rotation: %w", err) } @@ -129,8 +115,12 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog return nil, errors.New("new catalog missing attributes") } - // Validate the region. - if _, err := getValidateRegionValue(attrs); err != nil { + catalogAttributes, err := getCatalogAttributes(attrs) + if err != nil { + return nil, err + } + + if err := validateRegion(catalogAttributes.Region); err != nil { return nil, err } @@ -147,12 +137,7 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog return nil, fmt.Errorf("error loading persisted state: %w", err) } - skipRotate, err := getBoolValue(attrs, constDisableCredentialRotation, false) - if err != nil { - return nil, err - } - - if skipRotate && !updateSecrets { + 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. @@ -162,12 +147,7 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog } if updateSecrets { - accessKeyId, err := getStringValue(secrets, constAccessKeyId, true) - if err != nil { - return nil, err - } - - secretAccessKey, err := getStringValue(secrets, constSecretAccessKey, true) + catalogSecrets, err := getCatalogSecrets(secrets) if err != nil { return nil, err } @@ -175,12 +155,12 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog // 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 := state.ReplaceCreds(accessKeyId, secretAccessKey); err != nil { + if err := state.ReplaceCreds(catalogSecrets.AccessKeyId, catalogSecrets.SecretAccessKey); err != nil { return nil, fmt.Errorf("error attempting to replace credentials: %w", err) } } - if !skipRotate && state.CredsLastRotatedTime.IsZero() { + if !catalogAttributes.DisableCredentialRotation && state.CredsLastRotatedTime.IsZero() { // 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 { @@ -229,22 +209,25 @@ func (p *AwsPlugin) OnCreateSet(ctx context.Context, req *pb.OnCreateSetRequest) return nil, errors.New("catalog is nil") } - catalogAttrs := catalog.GetAttributes() - if catalogAttrs == nil { + catalogAttrsRaw := catalog.GetAttributes() + if catalogAttrsRaw == nil { return nil, errors.New("catalog missing attributes") } + catalogAttributes, err := getCatalogAttributes(catalogAttrsRaw) + if err != nil { + return nil, fmt.Errorf("integrity error in host catalog attributes: %w", err) + } + + if err := validateRegion(catalogAttributes.Region); err != nil { + return nil, fmt.Errorf("integrity error in host catalog attributes: %w", err) + } + state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...) if err != nil { return nil, fmt.Errorf("error loading persisted state: %w", err) } - // Get/validate the region. - region, err := getValidateRegionValue(catalogAttrs) - if err != nil { - return nil, fmt.Errorf("catalog validation error: %s", err) - } - set := req.GetSet() if set == nil { return nil, errors.New("set is nil") @@ -258,7 +241,7 @@ func (p *AwsPlugin) OnCreateSet(ctx context.Context, req *pb.OnCreateSetRequest) return nil, fmt.Errorf("error parsing set attributes: %w", err) } - ec2Client, err := state.EC2Client(region) + ec2Client, err := state.EC2Client(catalogAttributes.Region) if err != nil { return nil, fmt.Errorf("error getting EC2 client: %w", err) } @@ -287,22 +270,25 @@ func (p *AwsPlugin) OnUpdateSet(ctx context.Context, req *pb.OnUpdateSetRequest) return nil, errors.New("catalog is nil") } - catalogAttrs := catalog.GetAttributes() - if catalogAttrs == nil { + catalogAttrsRaw := catalog.GetAttributes() + if catalogAttrsRaw == nil { return nil, errors.New("catalog missing attributes") } + catalogAttributes, err := getCatalogAttributes(catalogAttrsRaw) + if err != nil { + return nil, fmt.Errorf("integrity error in host catalog attributes: %w", err) + } + + if err := validateRegion(catalogAttributes.Region); err != nil { + return nil, fmt.Errorf("integrity error in host catalog attributes: %w", err) + } + state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...) if err != nil { return nil, fmt.Errorf("error loading persisted state: %w", err) } - // Get/validate the region. - region, err := getValidateRegionValue(catalogAttrs) - if err != nil { - return nil, fmt.Errorf("catalog validation error: %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() @@ -318,7 +304,7 @@ func (p *AwsPlugin) OnUpdateSet(ctx context.Context, req *pb.OnUpdateSetRequest) return nil, fmt.Errorf("error parsing set attributes: %w", err) } - ec2Client, err := state.EC2Client(region) + ec2Client, err := state.EC2Client(catalogAttributes.Region) if err != nil { return nil, fmt.Errorf("error getting EC2 client: %w", err) } @@ -353,22 +339,25 @@ func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*p return nil, errors.New("catalog is nil") } - catalogAttrs := catalog.GetAttributes() - if catalogAttrs == nil { + catalogAttrsRaw := catalog.GetAttributes() + if catalogAttrsRaw == nil { return nil, errors.New("catalog missing attributes") } + catalogAttributes, err := getCatalogAttributes(catalogAttrsRaw) + if err != nil { + return nil, fmt.Errorf("integrity error in host catalog attributes: %w", err) + } + + if err := validateRegion(catalogAttributes.Region); err != nil { + return nil, fmt.Errorf("integrity error in host catalog attributes: %w", err) + } + state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...) if err != nil { return nil, fmt.Errorf("error loading persisted state: %w", err) } - // Get/validate the region. - region, err := getValidateRegionValue(catalogAttrs) - if err != nil { - return nil, fmt.Errorf("catalog validation error: %w", err) - } - sets := req.GetSets() if sets == nil { return nil, errors.New("sets is nil") @@ -407,7 +396,7 @@ func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*p } } - ec2Client, err := state.EC2Client(region) + ec2Client, err := state.EC2Client(catalogAttributes.Region) if err != nil { return nil, fmt.Errorf("error getting EC2 client: %w", err) } @@ -469,101 +458,13 @@ func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*p }, nil } -func getStringValue(in *structpb.Struct, k string, required bool) (string, error) { - mv := in.AsMap() - v, ok := mv[k] - if !ok { - if required { - return "", fmt.Errorf("missing required value %q", k) - } - - return "", nil - } - - s, ok := v.(string) - if !ok { - return "", fmt.Errorf("unexpected type for value %q: want string, got %T", k, v) - } - - if s == "" && required { - return "", fmt.Errorf("value %q cannot be empty", k) - } - - return s, nil -} - -func getBoolValue(in *structpb.Struct, k string, required bool) (bool, error) { - mv := in.AsMap() - v, ok := mv[k] - if !ok { - if required { - return false, fmt.Errorf("missing required value %q", k) - } - - return false, nil - } - - b, ok := v.(bool) - if !ok { - return false, fmt.Errorf("unexpected type for value %q: want bool, got %T", k, v) - } - - return b, nil -} - -func getTimeValue(in *structpb.Struct, k string) (time.Time, error) { - mv := in.AsMap() - v, ok := mv[k] - if !ok { - return time.Time{}, nil - } - - tRaw, ok := v.(string) - if !ok { - return time.Time{}, fmt.Errorf("unexpected type for value %q: want string, got %T", k, v) - } - - t, err := time.Parse(time.RFC3339Nano, tRaw) - if err != nil { - return time.Time{}, fmt.Errorf("could not parse time in value %q: %w", k, err) - } - - return t, nil -} - -func getSetAttributes(in *structpb.Struct) (*SetAttributes, error) { - var setAttrs SetAttributes - - // Mapstructure complains if it expects a slice as output and sees a scalar - // value. Rather than use WeakDecode and risk unintended consequences, I'm - // manually making this change if necessary. - inMap := in.AsMap() - if filtersRaw, ok := inMap[constDescribeInstancesFilters]; ok { - switch filterVal := filtersRaw.(type) { - case string: - inMap[constDescribeInstancesFilters] = []string{filterVal} - } - } - - if err := mapstructure.Decode(inMap, &setAttrs); err != nil { - return nil, fmt.Errorf("error decoding set attributes: %w", err) - } - - return &setAttrs, nil -} - -func getValidateRegionValue(in *structpb.Struct) (string, error) { - region, err := getStringValue(in, constRegion, true) - if err != nil { - return "", err - } - +func validateRegion(region string) error { _, found := endpoints.PartitionForRegion(endpoints.DefaultPartitions(), region) if !found { - return "", fmt.Errorf("not a valid region: %s", region) + return fmt.Errorf("not a valid region: %s", region) } - return region, nil + return nil } func buildFilters(attrs *SetAttributes) ([]*ec2.Filter, error) { diff --git a/plugin/plugin_test.go b/plugin/plugin_test.go index 495cbb5..961aa8d 100644 --- a/plugin/plugin_test.go +++ b/plugin/plugin_test.go @@ -6,7 +6,6 @@ import ( "fmt" "sort" "testing" - "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" @@ -47,19 +46,17 @@ func TestPluginOnCreateCatalogErr(t *testing.T) { expectedErr: "attributes are required", }, { - name: "invalid region", + name: "error reading attributes", req: &pb.OnCreateCatalogRequest{ Catalog: &hostcatalogs.HostCatalog{ - Secrets: new(structpb.Struct), - Attributes: mustStruct(map[string]interface{}{ - constRegion: "foobar", - }), + Secrets: new(structpb.Struct), + Attributes: new(structpb.Struct), }, }, - expectedErr: "not a valid region: foobar", + expectedErr: "missing required value \"region\"", }, { - name: "missing access key ID", + name: "error reading secrets", req: &pb.OnCreateCatalogRequest{ Catalog: &hostcatalogs.HostCatalog{ Secrets: new(structpb.Struct), @@ -71,21 +68,7 @@ func TestPluginOnCreateCatalogErr(t *testing.T) { expectedErr: "missing required value \"access_key_id\"", }, { - name: "missing secret access key", - req: &pb.OnCreateCatalogRequest{ - Catalog: &hostcatalogs.HostCatalog{ - Secrets: mustStruct(map[string]interface{}{ - constAccessKeyId: "foobar", - }), - Attributes: mustStruct(map[string]interface{}{ - constRegion: "us-west-2", - }), - }, - }, - expectedErr: "missing required value \"secret_access_key\"", - }, - { - name: "invalid value for skipping rotation", + name: "invalid region", req: &pb.OnCreateCatalogRequest{ Catalog: &hostcatalogs.HostCatalog{ Secrets: mustStruct(map[string]interface{}{ @@ -93,12 +76,11 @@ func TestPluginOnCreateCatalogErr(t *testing.T) { constSecretAccessKey: "bazqux", }), Attributes: mustStruct(map[string]interface{}{ - constRegion: "us-west-2", - constDisableCredentialRotation: "sure", + constRegion: "foobar", }), }, }, - expectedErr: "unexpected type for value \"disable_credential_rotation\": want bool, got string", + expectedErr: "not a valid region: foobar", }, { name: "persisted state setup error", @@ -205,6 +187,16 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { }, expectedErr: "new catalog missing attributes", }, + { + name: "error reading attributes", + req: &pb.OnUpdateCatalogRequest{ + NewCatalog: &hostcatalogs.HostCatalog{ + Secrets: new(structpb.Struct), + Attributes: new(structpb.Struct), + }, + }, + expectedErr: "missing required value \"region\"", + }, { name: "invalid region", req: &pb.OnUpdateCatalogRequest{ @@ -239,25 +231,6 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { }, expectedErr: fmt.Sprintf("error loading persisted state: %s", testOptionErr), }, - { - name: "invalid value for credential rotation", - req: &pb.OnUpdateCatalogRequest{ - NewCatalog: &hostcatalogs.HostCatalog{ - Attributes: mustStruct(map[string]interface{}{ - constRegion: "us-west-2", - constDisableCredentialRotation: "sure", - }), - }, - Persisted: &pb.HostCatalogPersisted{ - Secrets: mustStruct(map[string]interface{}{ - constAccessKeyId: "foobar", - constSecretAccessKey: "bazqux", - constCredsLastRotatedTime: "2006-01-02T15:04:05+07:00", - }), - }, - }, - expectedErr: "unexpected type for value \"disable_credential_rotation\": want bool, got string", - }, { name: "cannot disable rotation for already rotated credentials", req: &pb.OnUpdateCatalogRequest{ @@ -278,7 +251,7 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { expectedErr: "cannot disable rotation for already-rotated credentials", }, { - name: "updating secrets, missing access key id", + name: "error reading secrets", req: &pb.OnUpdateCatalogRequest{ NewCatalog: &hostcatalogs.HostCatalog{ Secrets: new(structpb.Struct), @@ -296,27 +269,6 @@ func TestPluginOnUpdateCatalogErr(t *testing.T) { }, expectedErr: "missing required value \"access_key_id\"", }, - { - name: "updating secrets, missing secret access key", - req: &pb.OnUpdateCatalogRequest{ - NewCatalog: &hostcatalogs.HostCatalog{ - Secrets: mustStruct(map[string]interface{}{ - constAccessKeyId: "foobar", - }), - Attributes: mustStruct(map[string]interface{}{ - constRegion: "us-west-2", - }), - }, - Persisted: &pb.HostCatalogPersisted{ - Secrets: mustStruct(map[string]interface{}{ - constAccessKeyId: "foobar", - constSecretAccessKey: "bazqux", - constCredsLastRotatedTime: "2006-01-02T15:04:05+07:00", - }), - }, - }, - expectedErr: "missing required value \"secret_access_key\"", - }, { name: "updating secrets, replace error", req: &pb.OnUpdateCatalogRequest{ @@ -483,6 +435,27 @@ func TestPluginOnCreateSetErr(t *testing.T) { }, expectedErr: "catalog missing attributes", }, + { + name: "error reading catalog attributes", + req: &pb.OnCreateSetRequest{ + Catalog: &hostcatalogs.HostCatalog{ + Secrets: new(structpb.Struct), + Attributes: new(structpb.Struct), + }, + }, + expectedErr: "integrity error in host catalog attributes: missing required value \"region\"", + }, + { + name: "invalid region", + req: &pb.OnCreateSetRequest{ + Catalog: &hostcatalogs.HostCatalog{ + Attributes: mustStruct(map[string]interface{}{ + constRegion: "foobar", + }), + }, + }, + expectedErr: "integrity error in host catalog attributes: not a valid region: foobar", + }, { name: "persisted state setup error", req: &pb.OnCreateSetRequest{ @@ -506,24 +479,6 @@ func TestPluginOnCreateSetErr(t *testing.T) { }, expectedErr: fmt.Sprintf("error loading persisted state: %s", testOptionErr), }, - { - name: "invalid region", - req: &pb.OnCreateSetRequest{ - Catalog: &hostcatalogs.HostCatalog{ - Attributes: mustStruct(map[string]interface{}{ - constRegion: "foobar", - }), - }, - Persisted: &pb.HostCatalogPersisted{ - Secrets: mustStruct(map[string]interface{}{ - constAccessKeyId: "foobar", - constSecretAccessKey: "bazqux", - constCredsLastRotatedTime: "2006-01-02T15:04:05+07:00", - }), - }, - }, - expectedErr: "catalog validation error: not a valid region: foobar", - }, { name: "nil set", req: &pb.OnCreateSetRequest{ @@ -561,6 +516,30 @@ func TestPluginOnCreateSetErr(t *testing.T) { }, expectedErr: "set missing attributes", }, + { + name: "set attribute load error", + req: &pb.OnCreateSetRequest{ + Catalog: &hostcatalogs.HostCatalog{ + Attributes: mustStruct(map[string]interface{}{ + constRegion: "us-west-2", + }), + }, + Persisted: &pb.HostCatalogPersisted{ + Secrets: mustStruct(map[string]interface{}{ + constAccessKeyId: "foobar", + constSecretAccessKey: "bazqux", + constCredsLastRotatedTime: "2006-01-02T15:04:05+07:00", + }), + }, + Set: &hostsets.HostSet{ + Attributes: mustStruct(map[string]interface{}{ + "foo": true, + "bar": true, + }), + }, + }, + expectedErr: "error parsing set attributes: unknown set attribute fields provided: bar, foo", + }, { name: "client load error", req: &pb.OnCreateSetRequest{ @@ -710,6 +689,28 @@ func TestPluginOnUpdateSetErr(t *testing.T) { }, expectedErr: "catalog missing attributes", }, + { + name: "catalog attribute load error", + req: &pb.OnUpdateSetRequest{ + Catalog: &hostcatalogs.HostCatalog{ + Secrets: new(structpb.Struct), + Attributes: new(structpb.Struct), + }, + }, + expectedErr: "integrity error in host catalog attributes: missing required value \"region\"", + }, + { + name: "invalid region", + req: &pb.OnUpdateSetRequest{ + Catalog: &hostcatalogs.HostCatalog{ + Secrets: new(structpb.Struct), + Attributes: mustStruct(map[string]interface{}{ + constRegion: "foobar", + }), + }, + }, + expectedErr: "integrity error in host catalog attributes: not a valid region: foobar", + }, { name: "persisted state setup error", req: &pb.OnUpdateSetRequest{ @@ -733,24 +734,6 @@ func TestPluginOnUpdateSetErr(t *testing.T) { }, expectedErr: fmt.Sprintf("error loading persisted state: %s", testOptionErr), }, - { - name: "invalid region", - req: &pb.OnUpdateSetRequest{ - Catalog: &hostcatalogs.HostCatalog{ - Attributes: mustStruct(map[string]interface{}{ - constRegion: "foobar", - }), - }, - Persisted: &pb.HostCatalogPersisted{ - Secrets: mustStruct(map[string]interface{}{ - constAccessKeyId: "foobar", - constSecretAccessKey: "bazqux", - constCredsLastRotatedTime: "2006-01-02T15:04:05+07:00", - }), - }, - }, - expectedErr: "catalog validation error: not a valid region: foobar", - }, { name: "nil set", req: &pb.OnUpdateSetRequest{ @@ -788,6 +771,30 @@ func TestPluginOnUpdateSetErr(t *testing.T) { }, expectedErr: "new set missing attributes", }, + { + name: "set attribute load error", + req: &pb.OnUpdateSetRequest{ + Catalog: &hostcatalogs.HostCatalog{ + Attributes: mustStruct(map[string]interface{}{ + constRegion: "us-west-2", + }), + }, + Persisted: &pb.HostCatalogPersisted{ + Secrets: mustStruct(map[string]interface{}{ + constAccessKeyId: "foobar", + constSecretAccessKey: "bazqux", + constCredsLastRotatedTime: "2006-01-02T15:04:05+07:00", + }), + }, + NewSet: &hostsets.HostSet{ + Attributes: mustStruct(map[string]interface{}{ + "foo": true, + "bar": true, + }), + }, + }, + expectedErr: "error parsing set attributes: unknown set attribute fields provided: bar, foo", + }, { name: "client load error", req: &pb.OnUpdateSetRequest{ @@ -909,6 +916,28 @@ func TestPluginListHostsErr(t *testing.T) { }, expectedErr: "catalog missing attributes", }, + { + name: "error reading catalog attributes", + req: &pb.ListHostsRequest{ + Catalog: &hostcatalogs.HostCatalog{ + Secrets: new(structpb.Struct), + Attributes: new(structpb.Struct), + }, + }, + expectedErr: "integrity error in host catalog attributes: missing required value \"region\"", + }, + { + name: "invalid region", + req: &pb.ListHostsRequest{ + Catalog: &hostcatalogs.HostCatalog{ + Secrets: new(structpb.Struct), + Attributes: mustStruct(map[string]interface{}{ + constRegion: "foobar", + }), + }, + }, + expectedErr: "integrity error in host catalog attributes: not a valid region: foobar", + }, { name: "persisted state setup error", req: &pb.ListHostsRequest{ @@ -932,24 +961,6 @@ func TestPluginListHostsErr(t *testing.T) { }, expectedErr: fmt.Sprintf("error loading persisted state: %s", testOptionErr), }, - { - name: "invalid region", - req: &pb.ListHostsRequest{ - Catalog: &hostcatalogs.HostCatalog{ - Attributes: mustStruct(map[string]interface{}{ - constRegion: "foobar", - }), - }, - Persisted: &pb.HostCatalogPersisted{ - Secrets: mustStruct(map[string]interface{}{ - constAccessKeyId: "foobar", - constSecretAccessKey: "bazqux", - constCredsLastRotatedTime: "2006-01-02T15:04:05+07:00", - }), - }, - }, - expectedErr: "catalog validation error: not a valid region: foobar", - }, { name: "nil sets", req: &pb.ListHostsRequest{ @@ -1010,6 +1021,33 @@ func TestPluginListHostsErr(t *testing.T) { }, expectedErr: "set missing attributes", }, + { + name: "set attribute load error", + req: &pb.ListHostsRequest{ + Catalog: &hostcatalogs.HostCatalog{ + Attributes: mustStruct(map[string]interface{}{ + constRegion: "us-west-2", + }), + }, + Persisted: &pb.HostCatalogPersisted{ + Secrets: mustStruct(map[string]interface{}{ + constAccessKeyId: "foobar", + constSecretAccessKey: "bazqux", + constCredsLastRotatedTime: "2006-01-02T15:04:05+07:00", + }), + }, + Sets: []*hostsets.HostSet{ + &hostsets.HostSet{ + Id: "foobar", + Attributes: mustStruct(map[string]interface{}{ + "foo": true, + "bar": true, + }), + }, + }, + }, + expectedErr: "error parsing set attributes: unknown set attribute fields provided: bar, foo", + }, { name: "client load error", req: &pb.ListHostsRequest{ @@ -1132,297 +1170,6 @@ func TestPluginListHostsErr(t *testing.T) { } } -func TestGetStringValue(t *testing.T) { - cases := []struct { - name string - in *structpb.Struct - key string - required bool - expected string - expectedErr string - }{ - { - name: "required missing", - in: mustStruct(map[string]interface{}{}), - key: "foo", - required: true, - expectedErr: "missing required value \"foo\"", - }, - { - name: "optional missing", - in: mustStruct(map[string]interface{}{}), - key: "foo", - expected: "", - }, - { - name: "non-string value", - in: mustStruct(map[string]interface{}{ - "foo": 1, - }), - key: "foo", - expectedErr: "unexpected type for value \"foo\": want string, got float64", - }, - { - name: "required empty", - in: mustStruct(map[string]interface{}{ - "foo": "", - }), - key: "foo", - required: true, - expectedErr: "value \"foo\" cannot be empty", - }, - { - name: "good", - in: mustStruct(map[string]interface{}{ - "foo": "bar", - }), - key: "foo", - expected: "bar", - }, - } - - for _, tc := range cases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - require := require.New(t) - actual, err := getStringValue(tc.in, tc.key, tc.required) - if tc.expectedErr != "" { - require.EqualError(err, tc.expectedErr) - return - } - - require.NoError(err) - require.Equal(tc.expected, actual) - }) - } -} - -func TestGetBoolValue(t *testing.T) { - cases := []struct { - name string - in *structpb.Struct - key string - required bool - expected bool - expectedErr string - }{ - { - name: "required missing", - in: mustStruct(map[string]interface{}{}), - key: "foo", - required: true, - expectedErr: "missing required value \"foo\"", - }, - { - name: "optional missing", - in: mustStruct(map[string]interface{}{}), - key: "foo", - expected: false, - }, - { - name: "non-bool value", - in: mustStruct(map[string]interface{}{ - "foo": "bar", - }), - key: "foo", - expectedErr: "unexpected type for value \"foo\": want bool, got string", - }, - { - name: "good", - in: mustStruct(map[string]interface{}{ - "foo": true, - }), - key: "foo", - expected: true, - }, - } - - for _, tc := range cases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - require := require.New(t) - actual, err := getBoolValue(tc.in, tc.key, tc.required) - if tc.expectedErr != "" { - require.EqualError(err, tc.expectedErr) - return - } - - require.NoError(err) - require.Equal(tc.expected, actual) - }) - } -} - -func TestGetTimeValue(t *testing.T) { - staticTime := time.Now() - - cases := []struct { - name string - in *structpb.Struct - key string - expected time.Time - expectedErr string - }{ - { - name: "missing", - in: mustStruct(map[string]interface{}{}), - key: "foo", - expected: time.Time{}, - }, - { - name: "non-time value", - in: mustStruct(map[string]interface{}{ - "foo": 1, - }), - key: "foo", - expectedErr: "unexpected type for value \"foo\": want string, got float64", - }, - { - name: "bad parse", - in: mustStruct(map[string]interface{}{ - "foo": "bar", - }), - key: "foo", - expectedErr: "could not parse time in value \"foo\": parsing time \"bar\" as \"2006-01-02T15:04:05.999999999Z07:00\": cannot parse \"bar\" as \"2006\"", - }, - { - name: "good", - in: mustStruct(map[string]interface{}{ - "foo": staticTime.Format(time.RFC3339Nano), - }), - key: "foo", - expected: func() time.Time { - u, err := time.Parse(time.RFC3339Nano, staticTime.Format(time.RFC3339Nano)) - if err != nil { - panic(err) - } - - return u - }(), - }, - } - - for _, tc := range cases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - require := require.New(t) - actual, err := getTimeValue(tc.in, tc.key) - if tc.expectedErr != "" { - require.EqualError(err, tc.expectedErr) - return - } - - require.NoError(err) - require.Equal(tc.expected, actual) - }) - } -} - -func TestGetSetAttributes(t *testing.T) { - cases := []struct { - name string - in *structpb.Struct - expected *SetAttributes - expectedErrContains string - }{ - { - name: "missing", - in: mustStruct(map[string]interface{}{}), - expected: &SetAttributes{}, - }, - { - name: "non-string-slice value", - in: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: "zip=foo,bar", - }), - expected: &SetAttributes{ - Filters: []string{"zip=foo,bar"}, - }, - }, - { - name: "bad filter element value", - in: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: []interface{}{1}, - }), - expectedErrContains: "expected type 'string', got unconvertible type 'float64'", - }, - { - name: "good", - in: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: []interface{}{ - "foo=bar", - "zip=zap", - }, - }), - expected: &SetAttributes{ - Filters: []string{"foo=bar", "zip=zap"}, - }, - }, - } - - for _, tc := range cases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - require := require.New(t) - actual, err := getSetAttributes(tc.in) - if tc.expectedErrContains != "" { - require.Error(err) - require.Contains(err.Error(), tc.expectedErrContains) - return - } - - require.NoError(err) - require.Equal(tc.expected, actual) - }) - } -} - -func TestGetValidateRegionValue(t *testing.T) { - cases := []struct { - name string - in *structpb.Struct - key string - expected string - expectedErr string - }{ - { - name: "missing", - in: mustStruct(map[string]interface{}{}), - key: "region", - expectedErr: fmt.Sprintf("missing required value %q", constRegion), - }, - { - name: "invalid region", - in: mustStruct(map[string]interface{}{ - constRegion: "foobar", - }), - expectedErr: "not a valid region: foobar", - }, - { - name: "good", - in: mustStruct(map[string]interface{}{ - constRegion: "us-west-2", - }), - expected: "us-west-2", - }, - } - - for _, tc := range cases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - require := require.New(t) - actual, err := getValidateRegionValue(tc.in) - if tc.expectedErr != "" { - require.EqualError(err, tc.expectedErr) - return - } - - require.NoError(err) - require.Equal(tc.expected, actual) - }) - } -} - func TestBuildFilters(t *testing.T) { cases := []struct { name string diff --git a/plugin/state.go b/plugin/state.go index 982abef..e5a0c7c 100644 --- a/plugin/state.go +++ b/plugin/state.go @@ -100,20 +100,29 @@ func awsCatalogPersistedStateFromProto(in *pb.HostCatalogPersisted, opts ...awsC return nil, errors.New("missing persisted secrets") } + unknownFields := structFields(data) + accessKeyId, err := getStringValue(data, constAccessKeyId, true) if err != nil { return nil, fmt.Errorf("persisted state integrity error: %w", err) } + delete(unknownFields, constAccessKeyId) secretAccessKey, err := getStringValue(data, constSecretAccessKey, true) if err != nil { return nil, fmt.Errorf("persisted state integrity error: %w", err) } + delete(unknownFields, constSecretAccessKey) credsLastRotatedTime, err := getTimeValue(data, constCredsLastRotatedTime) if err != nil { return nil, fmt.Errorf("persisted state integrity error: %w", err) } + delete(unknownFields, constCredsLastRotatedTime) + + if len(unknownFields) != 0 { + return nil, errors.New("one or more unrecognized persisted secret attributes found") + } s, err := newAwsCatalogPersistedState(opts...) if err != nil { diff --git a/plugin/state_test.go b/plugin/state_test.go index fb94500..9a6c3d1 100644 --- a/plugin/state_test.go +++ b/plugin/state_test.go @@ -168,6 +168,20 @@ func TestAwsCatalogPersistedStateFromProto(t *testing.T) { }, expectedErr: testOptionErr, }, + { + name: "unknown fields", + in: &pb.HostCatalogPersisted{ + // Nil should be the same as empty map here + Secrets: mustStruct(map[string]interface{}{ + constAccessKeyId: "foobar", + constSecretAccessKey: "bazqux", + constCredsLastRotatedTime: (time.Time{}).Format(time.RFC3339Nano), + "foo": true, + "bar": true, + }), + }, + expectedErr: "one or more unrecognized persisted secret attributes found", + }, { name: "good with non-zero timestamp", in: &pb.HostCatalogPersisted{