add default address/DNS check/add for interface, not just whole instance

This adds DNS names to the output for host queries.

Hosts are discovered in private, then public order, first at the
top-level, and then through attached network interfaces. This mirrors
the discovery order of IP addresses.

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.GetFields()
+	v, ok := mv[k]
+	if !ok {
+		if required {
+			return "", fmt.Errorf("missing required value %q", k)
+		}
+
+		return "", nil
+	}
+
+	s, ok := v.AsInterface().(string)
+	if !ok {
+		return "", fmt.Errorf("unexpected type for value %q: want string, got %T", k, v.AsInterface())
+	}
+
+	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.GetFields()
+	v, ok := mv[k]
+	if !ok {
+		if required {
+			return false, fmt.Errorf("missing required value %q", k)
+		}
+
+		return false, nil
+	}
+
+	b, ok := v.AsInterface().(bool)
+	if !ok {
+		return false, fmt.Errorf("unexpected type for value %q: want bool, got %T", k, v.AsInterface())
+	}
+
+	return b, nil
+}
+
+func getTimeValue(in *structpb.Struct, k string) (time.Time, error) {
+	mv := in.GetFields()
+	v, ok := mv[k]
+	if !ok {
+		return time.Time{}, nil
+	}
+
+	tRaw, ok := v.AsInterface().(string)
+	if !ok {
+		return time.Time{}, fmt.Errorf("unexpected type for value %q: want string, got %T", k, v.AsInterface())
+	}
+
+	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, ", ")
+}

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)
+		})
+	}
+}

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{}{
@@ -520,9 +512,7 @@ func testPluginListHosts(ctx context.Context, t *testing.T, p *AwsPlugin, region
 	sets := make([]*hostsets.HostSet, len(tags))
 	for i, tag := range tags {
 		setAttrs, err := structpb.NewStruct(map[string]interface{}{
-			constDescribeInstancesFilters: map[string]interface{}{
-				"tag-key": []interface{}{tag},
-			},
+			constDescribeInstancesFilters: []interface{}{fmt.Sprintf("tag-key=%s", tag)},
 		})
 		require.NoError(err)
 		sets[i] = &hostsets.HostSet{

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) {
@@ -642,16 +543,22 @@ func awsInstanceToHost(instance *ec2.Instance) (*pb.ListHostsResponseHost, error
 	// External ID is the instance ID.
 	result.ExternalId = aws.StringValue(instance.InstanceId)
 
-	// First IP address is always the private IP address field if it's
-	// populated
+	// First IP address/dns name are always the private fields if they
+	// are populated
 	if aws.StringValue(instance.PrivateIpAddress) != "" {
 		result.IpAddresses = append(result.IpAddresses, aws.StringValue(instance.PrivateIpAddress))
 	}
+	if aws.StringValue(instance.PrivateDnsName) != "" {
+		result.DnsNames = append(result.DnsNames, aws.StringValue(instance.PrivateDnsName))
+	}
 
-	// Public IP address is next.
+	// Public IP address/dns names are next
 	if aws.StringValue(instance.PublicIpAddress) != "" {
 		result.IpAddresses = append(result.IpAddresses, aws.StringValue(instance.PublicIpAddress))
 	}
+	if aws.StringValue(instance.PublicDnsName) != "" {
+		result.DnsNames = append(result.DnsNames, aws.StringValue(instance.PublicDnsName))
+	}
 
 	// Now go through all of the interfaces and log the IP address of
 	// every interface.
@@ -661,6 +568,15 @@ func awsInstanceToHost(instance *ec2.Instance) (*pb.ListHostsResponseHost, error
 			return nil, errors.New("response integrity error: interface entry is nil")
 		}
 
+		// Populate default IP addresses/DNS name similar to how we do
+		// for the entire instance.
+		if aws.StringValue(iface.PrivateIpAddress) != "" && !stringInSlice(result.IpAddresses, aws.StringValue(iface.PrivateIpAddress)) {
+			result.IpAddresses = append(result.IpAddresses, aws.StringValue(iface.PrivateIpAddress))
+		}
+		if aws.StringValue(iface.PrivateDnsName) != "" && !stringInSlice(result.DnsNames, aws.StringValue(iface.PrivateDnsName)) {
+			result.DnsNames = append(result.DnsNames, aws.StringValue(iface.PrivateDnsName))
+		}
+
 		// Iterate through the private IP addresses and log the
 		// information.
 		for _, addr := range iface.PrivateIpAddresses {
@@ -668,21 +584,20 @@ func awsInstanceToHost(instance *ec2.Instance) (*pb.ListHostsResponseHost, error
 				return nil, errors.New("response integrity error: interface address entry is nil")
 			}
 
-			// Check to see if the PrivateIpAddress matches the one
-			// reported at the top-level of the instance (let's call it the
-			// "default address"). If it doesn't, add it.
-			if aws.StringValue(addr.PrivateIpAddress) != "" {
-				if aws.StringValue(instance.PrivateIpAddress) != aws.StringValue(addr.PrivateIpAddress) {
-					result.IpAddresses = append(result.IpAddresses, aws.StringValue(addr.PrivateIpAddress))
-				}
+			// Add private address/dns name if they have not been added yet
+			if aws.StringValue(addr.PrivateIpAddress) != "" && !stringInSlice(result.IpAddresses, aws.StringValue(addr.PrivateIpAddress)) {
+				result.IpAddresses = append(result.IpAddresses, aws.StringValue(addr.PrivateIpAddress))
+			}
+			if aws.StringValue(addr.PrivateDnsName) != "" && !stringInSlice(result.DnsNames, aws.StringValue(addr.PrivateDnsName)) {
+				result.DnsNames = append(result.DnsNames, aws.StringValue(addr.PrivateDnsName))
 			}
 
-			// Do the same for the default public IP address and the
-			// public association on the interface.
-			if addr.Association != nil && addr.Association.PublicIp != nil && aws.StringValue(addr.Association.PublicIp) != "" {
-				if instance.PublicIpAddress != nil && aws.StringValue(instance.PublicIpAddress) != aws.StringValue(addr.Association.PublicIp) {
-					result.IpAddresses = append(result.IpAddresses, aws.StringValue(addr.Association.PublicIp))
-				}
+			// Add public address/dns name if they have not been added yet
+			if addr.Association != nil && aws.StringValue(addr.Association.PublicIp) != "" && !stringInSlice(result.IpAddresses, aws.StringValue(addr.Association.PublicIp)) {
+				result.IpAddresses = append(result.IpAddresses, aws.StringValue(addr.Association.PublicIp))
+			}
+			if addr.Association != nil && aws.StringValue(addr.Association.PublicDnsName) != "" && !stringInSlice(result.DnsNames, aws.StringValue(addr.Association.PublicDnsName)) {
+				result.DnsNames = append(result.DnsNames, aws.StringValue(addr.Association.PublicDnsName))
 			}
 		}
 
@@ -701,3 +616,13 @@ func awsInstanceToHost(instance *ec2.Instance) (*pb.ListHostsResponseHost, error
 	// Done
 	return result, nil
 }
+
+func stringInSlice(s []string, x string) bool {
+	for _, y := range s {
+		if x == y {
+			return true
+		}
+	}
+
+	return false
+}

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
@@ -1633,15 +1380,21 @@ func TestAwsInstanceToHost(t *testing.T) {
 			instance: &ec2.Instance{
 				InstanceId:       aws.String("foobar"),
 				PrivateIpAddress: aws.String("10.0.0.1"),
+				PrivateDnsName:   aws.String("test.example.internal"),
 				PublicIpAddress:  aws.String("1.1.1.1"),
+				PublicDnsName:    aws.String("test.example.com"),
 				NetworkInterfaces: []*ec2.InstanceNetworkInterface{
 					&ec2.InstanceNetworkInterface{
+						PrivateIpAddress: aws.String("10.0.0.1"),
+						PrivateDnsName:   aws.String("test.example.internal"),
 						PrivateIpAddresses: []*ec2.InstancePrivateIpAddress{
 							&ec2.InstancePrivateIpAddress{
 								Association: &ec2.InstanceNetworkInterfaceAssociation{
-									PublicIp: aws.String("1.1.1.1"),
+									PublicIp:      aws.String("1.1.1.1"),
+									PublicDnsName: aws.String("test.example.com"),
 								},
 								PrivateIpAddress: aws.String("10.0.0.1"),
+								PrivateDnsName:   aws.String("test.example.internal"),
 							},
 						},
 					},
@@ -1650,6 +1403,7 @@ func TestAwsInstanceToHost(t *testing.T) {
 			expected: &pb.ListHostsResponseHost{
 				ExternalId:  "foobar",
 				IpAddresses: []string{"10.0.0.1", "1.1.1.1"},
+				DnsNames:    []string{"test.example.internal", "test.example.com"},
 			},
 		},
 		{
@@ -1657,11 +1411,15 @@ func TestAwsInstanceToHost(t *testing.T) {
 			instance: &ec2.Instance{
 				InstanceId:       aws.String("foobar"),
 				PrivateIpAddress: aws.String("10.0.0.1"),
+				PrivateDnsName:   aws.String("test.example.internal"),
 				NetworkInterfaces: []*ec2.InstanceNetworkInterface{
 					&ec2.InstanceNetworkInterface{
+						PrivateIpAddress: aws.String("10.0.0.1"),
+						PrivateDnsName:   aws.String("test.example.internal"),
 						PrivateIpAddresses: []*ec2.InstancePrivateIpAddress{
 							&ec2.InstancePrivateIpAddress{
 								PrivateIpAddress: aws.String("10.0.0.1"),
+								PrivateDnsName:   aws.String("test.example.internal"),
 							},
 						},
 					},
@@ -1670,6 +1428,7 @@ func TestAwsInstanceToHost(t *testing.T) {
 			expected: &pb.ListHostsResponseHost{
 				ExternalId:  "foobar",
 				IpAddresses: []string{"10.0.0.1"},
+				DnsNames:    []string{"test.example.internal"},
 			},
 		},
 		{
@@ -1677,22 +1436,31 @@ func TestAwsInstanceToHost(t *testing.T) {
 			instance: &ec2.Instance{
 				InstanceId:       aws.String("foobar"),
 				PrivateIpAddress: aws.String("10.0.0.1"),
+				PrivateDnsName:   aws.String("test.example.internal"),
 				PublicIpAddress:  aws.String("1.1.1.1"),
+				PublicDnsName:    aws.String("test.example.com"),
 				NetworkInterfaces: []*ec2.InstanceNetworkInterface{
 					&ec2.InstanceNetworkInterface{
+						PrivateIpAddress: aws.String("10.0.0.2"),
+						PrivateDnsName:   aws.String("test2.example.internal"),
 						PrivateIpAddresses: []*ec2.InstancePrivateIpAddress{
 							&ec2.InstancePrivateIpAddress{
 								PrivateIpAddress: aws.String("10.0.0.2"),
+								PrivateDnsName:   aws.String("test2.example.internal"),
 							},
 						},
 					},
 					&ec2.InstanceNetworkInterface{
+						PrivateIpAddress: aws.String("10.0.0.1"),
+						PrivateDnsName:   aws.String("test.example.internal"),
 						PrivateIpAddresses: []*ec2.InstancePrivateIpAddress{
 							&ec2.InstancePrivateIpAddress{
 								Association: &ec2.InstanceNetworkInterfaceAssociation{
-									PublicIp: aws.String("1.1.1.1"),
+									PublicIp:      aws.String("1.1.1.1"),
+									PublicDnsName: aws.String("test.example.com"),
 								},
 								PrivateIpAddress: aws.String("10.0.0.1"),
+								PrivateDnsName:   aws.String("test.example.internal"),
 							},
 						},
 					},
@@ -1701,6 +1469,7 @@ func TestAwsInstanceToHost(t *testing.T) {
 			expected: &pb.ListHostsResponseHost{
 				ExternalId:  "foobar",
 				IpAddresses: []string{"10.0.0.1", "1.1.1.1", "10.0.0.2"},
+				DnsNames:    []string{"test.example.internal", "test.example.com", "test2.example.internal"},
 			},
 		},
 		{
@@ -1708,25 +1477,35 @@ func TestAwsInstanceToHost(t *testing.T) {
 			instance: &ec2.Instance{
 				InstanceId:       aws.String("foobar"),
 				PrivateIpAddress: aws.String("10.0.0.1"),
+				PrivateDnsName:   aws.String("test.example.internal"),
 				PublicIpAddress:  aws.String("1.1.1.1"),
+				PublicDnsName:    aws.String("test.example.com"),
 				NetworkInterfaces: []*ec2.InstanceNetworkInterface{
 					&ec2.InstanceNetworkInterface{
+						PrivateIpAddress: aws.String("10.0.0.2"),
+						PrivateDnsName:   aws.String("test2.example.internal"),
 						PrivateIpAddresses: []*ec2.InstancePrivateIpAddress{
 							&ec2.InstancePrivateIpAddress{
 								Association: &ec2.InstanceNetworkInterfaceAssociation{
-									PublicIp: aws.String("1.1.1.2"),
+									PublicIp:      aws.String("1.1.1.2"),
+									PublicDnsName: aws.String("test2.example.com"),
 								},
 								PrivateIpAddress: aws.String("10.0.0.2"),
+								PrivateDnsName:   aws.String("test2.example.internal"),
 							},
 						},
 					},
 					&ec2.InstanceNetworkInterface{
+						PrivateIpAddress: aws.String("10.0.0.1"),
+						PrivateDnsName:   aws.String("test.example.internal"),
 						PrivateIpAddresses: []*ec2.InstancePrivateIpAddress{
 							&ec2.InstancePrivateIpAddress{
 								Association: &ec2.InstanceNetworkInterfaceAssociation{
-									PublicIp: aws.String("1.1.1.1"),
+									PublicIp:      aws.String("1.1.1.1"),
+									PublicDnsName: aws.String("test.example.com"),
 								},
 								PrivateIpAddress: aws.String("10.0.0.1"),
+								PrivateDnsName:   aws.String("test.example.internal"),
 							},
 						},
 					},
@@ -1735,6 +1514,7 @@ func TestAwsInstanceToHost(t *testing.T) {
 			expected: &pb.ListHostsResponseHost{
 				ExternalId:  "foobar",
 				IpAddresses: []string{"10.0.0.1", "1.1.1.1", "10.0.0.2", "1.1.1.2"},
+				DnsNames:    []string{"test.example.internal", "test.example.com", "test2.example.internal", "test2.example.com"},
 			},
 		},
 		{
@@ -1742,18 +1522,25 @@ func TestAwsInstanceToHost(t *testing.T) {
 			instance: &ec2.Instance{
 				InstanceId:       aws.String("foobar"),
 				PrivateIpAddress: aws.String("10.0.0.1"),
+				PrivateDnsName:   aws.String("test.example.internal"),
 				PublicIpAddress:  aws.String("1.1.1.1"),
+				PublicDnsName:    aws.String("test.example.com"),
 				NetworkInterfaces: []*ec2.InstanceNetworkInterface{
 					&ec2.InstanceNetworkInterface{
+						PrivateIpAddress: aws.String("10.0.0.1"),
+						PrivateDnsName:   aws.String("test.example.internal"),
 						PrivateIpAddresses: []*ec2.InstancePrivateIpAddress{
 							&ec2.InstancePrivateIpAddress{
 								Association: &ec2.InstanceNetworkInterfaceAssociation{
-									PublicIp: aws.String("1.1.1.1"),
+									PublicIp:      aws.String("1.1.1.1"),
+									PublicDnsName: aws.String("test.example.com"),
 								},
 								PrivateIpAddress: aws.String("10.0.0.1"),
+								PrivateDnsName:   aws.String("test.example.internal"),
 							},
 							&ec2.InstancePrivateIpAddress{
 								PrivateIpAddress: aws.String("10.0.0.2"),
+								PrivateDnsName:   aws.String("test2.example.internal"),
 							},
 						},
 					},
@@ -1762,6 +1549,7 @@ func TestAwsInstanceToHost(t *testing.T) {
 			expected: &pb.ListHostsResponseHost{
 				ExternalId:  "foobar",
 				IpAddresses: []string{"10.0.0.1", "1.1.1.1", "10.0.0.2"},
+				DnsNames:    []string{"test.example.internal", "test.example.com", "test2.example.internal"},
 			},
 		},
 		{
@@ -1769,9 +1557,12 @@ func TestAwsInstanceToHost(t *testing.T) {
 			instance: &ec2.Instance{
 				InstanceId:       aws.String("foobar"),
 				PrivateIpAddress: aws.String("10.0.0.1"),
+				PrivateDnsName:   aws.String("test.example.internal"),
 				PublicIpAddress:  aws.String("1.1.1.1"),
+				PublicDnsName:    aws.String("test.example.com"),
 				NetworkInterfaces: []*ec2.InstanceNetworkInterface{
 					&ec2.InstanceNetworkInterface{
+						PrivateIpAddress: aws.String("10.0.0.1"),
 						PrivateIpAddresses: []*ec2.InstancePrivateIpAddress{
 							&ec2.InstancePrivateIpAddress{
 								Association: &ec2.InstanceNetworkInterfaceAssociation{
@@ -1790,6 +1581,7 @@ func TestAwsInstanceToHost(t *testing.T) {
 			expected: &pb.ListHostsResponseHost{
 				ExternalId:  "foobar",
 				IpAddresses: []string{"10.0.0.1", "1.1.1.1", "some::fake::address"},
+				DnsNames:    []string{"test.example.internal", "test.example.com"},
 			},
 		},
 	}