diff --git a/plugin/plugin.go b/plugin/plugin.go index 4afc552..2d96e1a 100644 --- a/plugin/plugin.go +++ b/plugin/plugin.go @@ -311,7 +311,7 @@ func (p *AwsPlugin) OnUpdateSet(ctx context.Context, req *pb.OnUpdateSetRequest) } if set.GetAttributes() == nil { - return nil, errors.New("set missing attributes") + return nil, errors.New("new set missing attributes") } setAttrs, err := getSetAttributes(set.GetAttributes()) if err != nil { @@ -534,7 +534,18 @@ func getTimeValue(in *structpb.Struct, k string) (time.Time, error) { func getSetAttributes(in *structpb.Struct) (*SetAttributes, error) { var setAttrs SetAttributes - if err := mapstructure.Decode(in.AsMap(), &setAttrs); err != nil { + // 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) } diff --git a/plugin/plugin_test.go b/plugin/plugin_test.go index 73ac7e9..495cbb5 100644 --- a/plugin/plugin_test.go +++ b/plugin/plugin_test.go @@ -578,9 +578,7 @@ func TestPluginOnCreateSetErr(t *testing.T) { }, Set: &hostsets.HostSet{ Attributes: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": []interface{}{"foo"}, - }, + constDescribeInstancesFilters: []interface{}{"tag-key=foo"}, }), }, }, @@ -591,31 +589,6 @@ func TestPluginOnCreateSetErr(t *testing.T) { }, expectedErr: fmt.Sprintf("error getting EC2 client: error getting AWS session: error reading options in NewCredentialsConfig: %s", testOptionErr), }, - { - name: "buildDescribeInstancesInput 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{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": "foo", - }, - }), - }, - }, - expectedErr: "error building DescribeInstances parameters: error building filters: unexpected type for filter values in \"tag-key\": want array, got string", - }, { name: "DescribeInstances error", req: &pb.OnCreateSetRequest{ @@ -633,9 +606,7 @@ func TestPluginOnCreateSetErr(t *testing.T) { }, Set: &hostsets.HostSet{ Attributes: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": []interface{}{"foo"}, - }, + constDescribeInstancesFilters: []interface{}{"tag-key=foo"}, }), }, }, @@ -648,7 +619,7 @@ func TestPluginOnCreateSetErr(t *testing.T) { expectedErr: fmt.Sprintf("error performing dry run of DescribeInstances: %s", testDescribeInstancesError), }, { - name: "DescribeInstances non-error", + name: "DescribeInstances non-error array filter", req: &pb.OnCreateSetRequest{ Catalog: &hostcatalogs.HostCatalog{ Attributes: mustStruct(map[string]interface{}{ @@ -664,9 +635,35 @@ func TestPluginOnCreateSetErr(t *testing.T) { }, Set: &hostsets.HostSet{ Attributes: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": []interface{}{"foo"}, - }, + constDescribeInstancesFilters: []interface{}{"tag-key=foo"}, + }), + }, + }, + opts: []awsCatalogPersistedStateOption{ + withTestEC2APIFunc(newTestMockEC2( + nil, + )), + }, + expectedErr: "query error: DescribeInstances DryRun should have returned error, but none was found", + }, + { + name: "DescribeInstances non-error string filter", + 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{}{ + constDescribeInstancesFilters: "tag-key=foo", }), }, }, @@ -808,9 +805,7 @@ func TestPluginOnUpdateSetErr(t *testing.T) { }, NewSet: &hostsets.HostSet{ Attributes: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": []interface{}{"foo"}, - }, + constDescribeInstancesFilters: []interface{}{"tag-key=foo"}, }), }, }, @@ -821,31 +816,6 @@ func TestPluginOnUpdateSetErr(t *testing.T) { }, expectedErr: fmt.Sprintf("error getting EC2 client: error getting AWS session: error reading options in NewCredentialsConfig: %s", testOptionErr), }, - { - name: "buildDescribeInstancesInput 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{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": "foo", - }, - }), - }, - }, - expectedErr: "error building DescribeInstances parameters: error building filters: unexpected type for filter values in \"tag-key\": want array, got string", - }, { name: "DescribeInstances error", req: &pb.OnUpdateSetRequest{ @@ -863,9 +833,7 @@ func TestPluginOnUpdateSetErr(t *testing.T) { }, NewSet: &hostsets.HostSet{ Attributes: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": []interface{}{"foo"}, - }, + constDescribeInstancesFilters: []interface{}{"tag-key=foo"}, }), }, }, @@ -894,9 +862,7 @@ func TestPluginOnUpdateSetErr(t *testing.T) { }, NewSet: &hostsets.HostSet{ Attributes: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": []interface{}{"foo"}, - }, + constDescribeInstancesFilters: []interface{}{"tag-key=foo"}, }), }, }, @@ -1044,34 +1010,6 @@ func TestPluginListHostsErr(t *testing.T) { }, expectedErr: "set missing attributes", }, - { - name: "buildDescribeInstancesInput 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{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": "foo", - }, - }), - }, - }, - }, - expectedErr: "error building DescribeInstances parameters for host set id \"foobar\": error building filters: unexpected type for filter values in \"tag-key\": want array, got string", - }, { name: "client load error", req: &pb.ListHostsRequest{ @@ -1088,12 +1026,10 @@ func TestPluginListHostsErr(t *testing.T) { }), }, Sets: []*hostsets.HostSet{ - &hostsets.HostSet{ + { Id: "foobar", Attributes: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": []interface{}{"foo"}, - }, + constDescribeInstancesFilters: []interface{}{"tag-key=foo"}, }), }, }, @@ -1121,12 +1057,10 @@ func TestPluginListHostsErr(t *testing.T) { }), }, Sets: []*hostsets.HostSet{ - &hostsets.HostSet{ + { Id: "foobar", Attributes: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": []interface{}{"foo"}, - }, + constDescribeInstancesFilters: []interface{}{"tag-key=foo"}, }), }, }, @@ -1155,12 +1089,10 @@ func TestPluginListHostsErr(t *testing.T) { }), }, Sets: []*hostsets.HostSet{ - &hostsets.HostSet{ + { Id: "foobar", Attributes: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: map[string]interface{}{ - "tag-key": []interface{}{"foo"}, - }, + constDescribeInstancesFilters: []interface{}{"tag-key=foo"}, }), }, }, @@ -1401,9 +1333,11 @@ func TestGetSetAttributes(t *testing.T) { { name: "non-string-slice value", in: mustStruct(map[string]interface{}{ - constDescribeInstancesFilters: "bar", + constDescribeInstancesFilters: "zip=foo,bar", }), - expectedErrContains: "source data must be an array or slice, got string", + expected: &SetAttributes{ + Filters: []string{"zip=foo,bar"}, + }, }, { name: "bad filter element value", @@ -1432,6 +1366,7 @@ func TestGetSetAttributes(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 } @@ -1504,11 +1439,11 @@ func TestBuildFilters(t *testing.T) { }, }), expected: []*ec2.Filter{ - &ec2.Filter{ + { Name: aws.String("foo"), Values: aws.StringSlice([]string{"bar"}), }, - &ec2.Filter{ + { Name: aws.String("instance-state-name"), Values: aws.StringSlice([]string{ec2.InstanceStateNameRunning}), }, @@ -1523,11 +1458,11 @@ func TestBuildFilters(t *testing.T) { }, }), expected: []*ec2.Filter{ - &ec2.Filter{ + { Name: aws.String("foo"), Values: aws.StringSlice([]string{"bar"}), }, - &ec2.Filter{ + { Name: aws.String("instance-state-name"), Values: aws.StringSlice([]string{"static"}), }, @@ -1541,11 +1476,11 @@ func TestBuildFilters(t *testing.T) { }, }), expected: []*ec2.Filter{ - &ec2.Filter{ + { Name: aws.String("foo"), Values: aws.StringSlice([]string{"bar", "baz"}), }, - &ec2.Filter{ + { Name: aws.String("instance-state-name"), Values: aws.StringSlice([]string{ec2.InstanceStateNameRunning}), }, @@ -1557,7 +1492,7 @@ func TestBuildFilters(t *testing.T) { constDescribeInstancesFilters: []interface{}{}, }), expected: []*ec2.Filter{ - &ec2.Filter{ + { Name: aws.String("instance-state-name"), Values: aws.StringSlice([]string{ec2.InstanceStateNameRunning}), }, @@ -1603,11 +1538,11 @@ func TestBuildDescribeInstancesInput(t *testing.T) { expected: &ec2.DescribeInstancesInput{ DryRun: aws.Bool(true), Filters: []*ec2.Filter{ - &ec2.Filter{ + { Name: aws.String("foo"), Values: aws.StringSlice([]string{"bar"}), }, - &ec2.Filter{ + { Name: aws.String("instance-state-name"), Values: aws.StringSlice([]string{ec2.InstanceStateNameRunning}), }, @@ -1625,11 +1560,11 @@ func TestBuildDescribeInstancesInput(t *testing.T) { expected: &ec2.DescribeInstancesInput{ DryRun: aws.Bool(false), Filters: []*ec2.Filter{ - &ec2.Filter{ + { Name: aws.String("foo"), Values: aws.StringSlice([]string{"bar"}), }, - &ec2.Filter{ + { Name: aws.String("instance-state-name"), Values: aws.StringSlice([]string{ec2.InstanceStateNameRunning}), },