test: refactor error syntax

Update README to have full, correct examples

README.md

@@ -26,6 +26,62 @@ given credentials will be used to create a new credential, and then the given
 credential will be revoked. In this way, after rotation, only Boundary knows the
 client secret in use by this plugin.
 
+## Getting Started
+
+Refer to [Attributes and Secrets](#attributes-and-secrets) for more detail on
+configuration options for host catalogs and sets.
+
+To create a host catalog (using default scope created by `boundary dev`):
+
+```
+boundary host-catalogs create plugin \
+ -scope-id p_1234567890 \
+ -name "Example Plugin-Based Host Catalog" \
+ -description "Description for plugin-based host catalog" \
+ -plugin-name aws \
+ -attr region=REGION \
+ -secret access_key_id='KEY' \
+ -secret secret_access_key='SECRET'
+```
+
+To create a host set, filtering the host set based on tag keys `foo` or `bar`
+(either tag can be present), ensuring that any targets set to this host set only
+connect to external addresses in the `54.0.0.0/8` class A subnet:
+
+```
+boundary host-sets create plugin \
+ -host-catalog-id HOST_CATALOG_ID \
+ -name "Example Plugin-Based Host Set" \
+ -description "Description for plugin-based host set" \
+ -attr filters=tag-key=foo,bar \
+ -preferred-endpoint "cidr:54.0.0.0/8"
+```
+
+As above, but instances must have both tags (both `foo` and `bar` *must* be
+present):
+
+```
+boundary host-sets create plugin \
+ -host-catalog-id HOST_CATALOG_ID \
+ -name "Example Plugin-Based Host Set" \
+ -description "Description for plugin-based host set" \
+ -attr filters=tag-key=foo \
+ -attr filters=tag-key=bar \
+ -preferred-endpoint "cidr:54.0.0.0/8"
+```
+
+As above, but matching on tag key and launch date:
+
+```
+boundary host-sets create plugin \
+ -host-catalog-id HOST_CATALOG_ID \
+ -name "Example Plugin-Based Host Set" \
+ -description "Description for plugin-based host set" \
+ -attr filters=tag-key=foo \
+ -attr filters=launch-time=2022-01-04T* \
+ -preferred-endpoint "cidr:54.0.0.0/8"
+```
+
 ## Required IAM Privileges
 
 The following IAM privileges, at the very least, are required to be attached to
@@ -97,13 +153,3 @@ The following attributes are valid on an AWS host Set resource:
   a comma-separated list. For a list of filter options, check out
   [describe-instances in the AWS CLI
   reference](https://docs.aws.amazon.com/cli/latest/reference/ec2/describe-instances.html).
-
-#### Supplying Filters on the Boundary CLI
-
-As `filters` is part of a host set's attributes and may contain dashes which are
-not identifier-friendly, it's recommended you that supply attributes for AWS
-host sets as a full JSON string. Example:
-
-```
-boundary host-sets create plugin -host-catalog-id hc_1234567890 --attributes '{"filters":{"tag-name":["foo"]}}'
-```

plugin/attributes.go

@@ -0,0 +1,217 @@
+package plugin
+
+import (
+	"fmt"
+	"sort"
+	"strings"
+	"time"
+
+	"github.com/mitchellh/mapstructure"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/types/known/structpb"
+)
+
+func invalidArgumentError(msg string, f map[string]string) error {
+	var fieldMsgs []string
+	for field, val := range f {
+		fieldMsgs = append(fieldMsgs, fmt.Sprintf("%s: %s", field, val))
+	}
+	if len(fieldMsgs) > 0 {
+		sort.Strings(fieldMsgs)
+		msg = fmt.Sprintf("%s: [%s]", msg, strings.Join(fieldMsgs, ", "))
+	}
+	return status.Error(codes.InvalidArgument, msg)
+}
+
+// CatalogAttributes is a Go-native representation of the Attributes
+// map.
+type CatalogAttributes struct {
+	Region                    string
+	DisableCredentialRotation bool
+}
+
+func getCatalogAttributes(in *structpb.Struct) (*CatalogAttributes, error) {
+	unknownFields := structFields(in)
+	result := new(CatalogAttributes)
+	badFields := make(map[string]string)
+
+	var err error
+	result.Region, err = getStringValue(in, constRegion, true)
+	if err != nil {
+		badFields[fmt.Sprintf("attributes.%s", constRegion)] = err.Error()
+	}
+	delete(unknownFields, constRegion)
+
+	result.DisableCredentialRotation, err = getBoolValue(in, constDisableCredentialRotation, false)
+	if err != nil {
+		badFields[fmt.Sprintf("attributes.%s", constDisableCredentialRotation)] = err.Error()
+	}
+	delete(unknownFields, constDisableCredentialRotation)
+
+	for s := range unknownFields {
+		badFields[fmt.Sprintf("attributes.%s", s)] = "unrecognized field"
+	}
+
+	if len(badFields) > 0 {
+		return nil, invalidArgumentError("Invalid arguments in catalog attributes", badFields)
+	}
+
+	return result, nil
+}
+
+// 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)
+	badFields := make(map[string]string)
+
+	var err error
+	result.AccessKeyId, err = getStringValue(in, constAccessKeyId, true)
+	if err != nil {
+		badFields[fmt.Sprintf("secrets.%s", constAccessKeyId)] = err.Error()
+	}
+	delete(unknownFields, constAccessKeyId)
+
+	result.SecretAccessKey, err = getStringValue(in, constSecretAccessKey, true)
+	if err != nil {
+		badFields[fmt.Sprintf("secrets.%s", constSecretAccessKey)] = err.Error()
+	}
+	delete(unknownFields, constSecretAccessKey)
+
+	for s := range unknownFields {
+		badFields[fmt.Sprintf("secrets.%s", s)] = "unrecognized field"
+	}
+
+	if len(badFields) > 0 {
+		return nil, invalidArgumentError("Error in the secrets provided", badFields)
+	}
+
+	return result, nil
+}
+
+// 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)
+	badFields := make(map[string]string)
+	delete(unknownFields, constDescribeInstancesFilters)
+	for a := range unknownFields {
+		badFields[fmt.Sprintf("attributes.%s", a)] = "unrecognized field"
+	}
+	if len(badFields) > 0 {
+		return nil, invalidArgumentError("Error in the attributes provided", badFields)
+	}
+
+	// Mapstructure complains if it expects a slice as output and sees a scalar
+	// 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, status.Errorf(codes.InvalidArgument, "error decoding set attributes: %s", 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,429 @@
+package plugin
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+	"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: "attributes.bar: Unrecognized field, attributes.foo: Unrecognized field",
+		},
+		{
+			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)
+				require.Equal(status.Code(err), codes.InvalidArgument)
+				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: "secrets.bar: Unrecognized field, secrets.foo: Unrecognized field",
+		},
+		{
+			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)
+				require.Equal(status.Code(err), codes.InvalidArgument)
+				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: "attributes.bar: Unrecognized field, attributes.foo: Unrecognized field",
+		},
+		{
+			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)
+				require.Equal(status.Code(err), codes.InvalidArgument)
+				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.Contains(err.Error(), tc.expectedErr)
+				require.Equal(status.Code(err), codes.InvalidArgument)
+				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{}{
+		constDescribeInstancesFilters: []interface{}{fmt.Sprintf("tag-key=%s", strings.Join(tags, ","))},
-			"tag-key": func() []interface{} {
-				result := make([]interface{}, len(tags))
-				for i, tag := range tags {
-					result[i] = tag
-				}
-
-				return result
-			}(),
-		},
 	})
 	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{}{
+			constDescribeInstancesFilters: []interface{}{fmt.Sprintf("tag-key=%s", tag)},
-				"tag-key": []interface{}{tag},
-			},
 		})
 		require.NoError(err)
 		sets[i] = &hostsets.HostSet{

plugin/plugin.go

@@ -12,8 +12,8 @@ import (
 	"github.com/aws/aws-sdk-go/aws/endpoints"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	pb "github.com/hashicorp/boundary/sdk/pbs/plugin"
-	"github.com/mitchellh/mapstructure"
+	"google.golang.org/grpc/codes"
-	"google.golang.org/protobuf/types/known/structpb"
+	"google.golang.org/grpc/status"
 )
 
 // rotationWaitTimeout controls the time we wait for credential rotation to
@@ -35,71 +35,59 @@ 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 {
-		return nil, errors.New("catalog is nil")
+		return nil, status.Error(codes.InvalidArgument, "catalog is nil")
 	}
 
 	secrets := catalog.GetSecrets()
 	if secrets == nil {
-		return nil, errors.New("secrets are required")
+		return nil, status.Error(codes.InvalidArgument, "secrets are required")
 	}
 
 	attrs := catalog.GetAttributes()
 	if attrs == nil {
-		return nil, errors.New("attributes are required")
+		return nil, status.Error(codes.InvalidArgument, "attributes are required")
 	}
 
-	if _, err := getValidateRegionValue(attrs); err != nil {
+	catalogAttributes, err := getCatalogAttributes(attrs)
+	if err != nil {
 		return nil, err
 	}
-
+	catalogSecrets, err := getCatalogSecrets(secrets)
-	accessKeyId, err := getStringValue(secrets, constAccessKeyId, true)
 	if err != nil {
 		return nil, err
 	}
 
-	secretAccessKey, err := getStringValue(secrets, constSecretAccessKey, true)
+	if err := validateRegion(catalogAttributes.Region); err != nil {
-	if err != nil {
-		return nil, err
-	}
-
-	skipRotate, err := getBoolValue(attrs, constDisableCredentialRotation, false)
-	if err != nil {
 		return nil, err
 	}
 
 	// Set up the creds in our persisted state.
 	state, err := newAwsCatalogPersistedState(append([]awsCatalogPersistedStateOption{
-		withAccessKeyId(accessKeyId),
+		withAccessKeyId(catalogSecrets.AccessKeyId),
-		withSecretAccessKey(secretAccessKey),
+		withSecretAccessKey(catalogSecrets.SecretAccessKey),
 	}, p.testStateOpts...)...)
 	if err != nil {
-		return nil, fmt.Errorf("error setting up persisted state: %w", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error setting up persisted state: %s", err)
 	}
 
 	// Try to rotate the credentials if we're not skipping them.
-	if !skipRotate {
+	if !catalogAttributes.DisableCredentialRotation {
 		if err := state.RotateCreds(); err != nil {
-			return nil, fmt.Errorf("error during credential rotation: %w", err)
+			return nil, status.Errorf(codes.InvalidArgument, "error during credential rotation: %s", err)
 		}
 	} else {
 		// Simply validate if we aren't rotating.
 		if err := state.ValidateCreds(); err != nil {
-			return nil, fmt.Errorf("error during credential validation: %w", err)
+			return nil, status.Errorf(codes.InvalidArgument, "error during credential validation: %s", err)
 		}
 	}
 
 	persistedProto, err := state.ToProto()
 	if err != nil {
-		return nil, err
+		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
 	return &pb.OnCreateCatalogResponse{
@@ -113,7 +101,7 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog
 	// fetching the new copy to keep things a little less messy.
 	catalog := req.GetNewCatalog()
 	if catalog == nil {
-		return nil, errors.New("new catalog is nil")
+		return nil, status.Error(codes.InvalidArgument, "new catalog is nil")
 	}
 
 	var updateSecrets bool
@@ -126,11 +114,15 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog
 
 	attrs := catalog.GetAttributes()
 	if attrs == nil {
-		return nil, errors.New("new catalog missing attributes")
+		return nil, status.Error(codes.InvalidArgument, "new catalog missing attributes")
 	}
 
-	// Validate the region.
+	catalogAttributes, err := getCatalogAttributes(attrs)
-	if _, err := getValidateRegionValue(attrs); err != nil {
+	if err != nil {
+		return nil, err
+	}
+
+	if err := validateRegion(catalogAttributes.Region); err != nil {
 		return nil, err
 	}
 
@@ -144,30 +136,20 @@ func (p *AwsPlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalog
 	// is fine and important, but this may change in the future.
 	state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...)
 	if err != nil {
-		return nil, fmt.Errorf("error loading persisted state: %w", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
 	}
 
-	skipRotate, err := getBoolValue(attrs, constDisableCredentialRotation, false)
+	if catalogAttributes.DisableCredentialRotation && !updateSecrets {
-	if err != nil {
-		return nil, err
-	}
-
-	if skipRotate && !updateSecrets {
 		// This is a validate check to make sure that we aren't disabling
 		// rotation for credentials currently being managed by rotation.
 		// This is not allowed.
 		if !state.CredsLastRotatedTime.IsZero() {
-			return nil, errors.New("cannot disable rotation for already-rotated credentials")
+			return nil, status.Error(codes.FailedPrecondition, "cannot disable rotation for already-rotated credentials")
 		}
 	}
 
 	if updateSecrets {
-		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
 		}
@@ -175,23 +157,23 @@ 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)
+			return nil, status.Errorf(codes.InvalidArgument, "error attempting to replace credentials: %s", 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 {
-			return nil, fmt.Errorf("error during credential rotation: %w", err)
+			return nil, status.Errorf(codes.InvalidArgument, "error during credential rotation: %s", err)
 		}
 	}
 
 	// That's it!
 	persistedProto, err := state.ToProto()
 	if err != nil {
-		return nil, err
+		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
 	return &pb.OnUpdateCatalogResponse{
@@ -208,7 +190,7 @@ func (p *AwsPlugin) OnDeleteCatalog(ctx context.Context, req *pb.OnDeleteCatalog
 	// may be corrupt/and or legitimately missing.
 	state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...)
 	if err != nil {
-		return nil, fmt.Errorf("error loading persisted state: %w", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
 	}
 
 	if !state.CredsLastRotatedTime.IsZero() {
@@ -216,7 +198,7 @@ func (p *AwsPlugin) OnDeleteCatalog(ctx context.Context, req *pb.OnDeleteCatalog
 		// credentials to ensure that it has the proper permissions to do
 		// it.
 		if err := state.DeleteCreds(); err != nil {
-			return nil, fmt.Errorf("error removing rotated credentials during catalog deletion: %w", err)
+			return nil, status.Errorf(codes.Aborted, "error removing rotated credentials during catalog deletion: %s", err)
 		}
 	}
 
@@ -226,51 +208,54 @@ func (p *AwsPlugin) OnDeleteCatalog(ctx context.Context, req *pb.OnDeleteCatalog
 func (p *AwsPlugin) OnCreateSet(ctx context.Context, req *pb.OnCreateSetRequest) (*pb.OnCreateSetResponse, error) {
 	catalog := req.GetCatalog()
 	if catalog == nil {
-		return nil, errors.New("catalog is nil")
+		return nil, status.Error(codes.InvalidArgument, "catalog is nil")
 	}
 
-	catalogAttrs := catalog.GetAttributes()
+	catalogAttrsRaw := catalog.GetAttributes()
-	if catalogAttrs == nil {
+	if catalogAttrsRaw == nil {
-		return nil, errors.New("catalog missing attributes")
+		return nil, status.Error(codes.InvalidArgument, "catalog missing attributes")
+	}
+
+	catalogAttributes, err := getCatalogAttributes(catalogAttrsRaw)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := validateRegion(catalogAttributes.Region); err != nil {
+		return nil, err
 	}
 
 	state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...)
 	if err != nil {
-		return nil, fmt.Errorf("error loading persisted state: %w", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
-	}
-
-	// 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")
+		return nil, status.Error(codes.InvalidArgument, "set is nil")
 	}
 
 	if set.GetAttributes() == nil {
-		return nil, errors.New("set missing attributes")
+		return nil, status.Error(codes.InvalidArgument, "set missing attributes")
 	}
 	setAttrs, err := getSetAttributes(set.GetAttributes())
 	if err != nil {
-		return nil, fmt.Errorf("error parsing set attributes: %w", err)
+		return nil, err
 	}
 
-	ec2Client, err := state.EC2Client(region)
+	ec2Client, err := state.EC2Client(catalogAttributes.Region)
 	if err != nil {
-		return nil, fmt.Errorf("error getting EC2 client: %w", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error getting EC2 client: %s", err)
 	}
 
 	input, err := buildDescribeInstancesInput(setAttrs, true)
 	if err != nil {
-		return nil, fmt.Errorf("error building DescribeInstances parameters: %w", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error building DescribeInstances parameters: %s", err)
 	}
 
 	_, err = ec2Client.DescribeInstances(input)
 	if err == nil {
-		return nil, errors.New("query error: DescribeInstances DryRun should have returned error, but none was found")
+		return nil, status.Error(codes.FailedPrecondition, "query error: DescribeInstances DryRun should have returned error, but none was found")
 	}
 
 	if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "DryRunOperation" {
@@ -278,59 +263,62 @@ func (p *AwsPlugin) OnCreateSet(ctx context.Context, req *pb.OnCreateSetRequest)
 		return &pb.OnCreateSetResponse{}, nil
 	}
 
-	return nil, fmt.Errorf("error performing dry run of DescribeInstances: %w", err)
+	return nil, status.Errorf(codes.InvalidArgument, "error performing dry run of DescribeInstances: %s", err)
 }
 
 func (p *AwsPlugin) OnUpdateSet(ctx context.Context, req *pb.OnUpdateSetRequest) (*pb.OnUpdateSetResponse, error) {
 	catalog := req.GetCatalog()
 	if catalog == nil {
-		return nil, errors.New("catalog is nil")
+		return nil, status.Error(codes.InvalidArgument, "catalog is nil")
 	}
 
-	catalogAttrs := catalog.GetAttributes()
+	catalogAttrsRaw := catalog.GetAttributes()
-	if catalogAttrs == nil {
+	if catalogAttrsRaw == nil {
-		return nil, errors.New("catalog missing attributes")
+		return nil, status.Error(codes.InvalidArgument, "catalog missing attributes")
+	}
+
+	catalogAttributes, err := getCatalogAttributes(catalogAttrsRaw)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := validateRegion(catalogAttributes.Region); err != nil {
+		return nil, err
 	}
 
 	state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...)
 	if err != nil {
-		return nil, fmt.Errorf("error loading persisted state: %w", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
-	}
-
-	// 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()
 	if set == nil {
-		return nil, errors.New("new set is nil")
+		return nil, status.Error(codes.InvalidArgument, "new set is nil")
 	}
 
 	if set.GetAttributes() == nil {
-		return nil, errors.New("new set missing attributes")
+		return nil, status.Error(codes.InvalidArgument, "new set missing attributes")
 	}
 	setAttrs, err := getSetAttributes(set.GetAttributes())
 	if err != nil {
-		return nil, fmt.Errorf("error parsing set attributes: %w", err)
+		return nil, err
 	}
 
-	ec2Client, err := state.EC2Client(region)
+	ec2Client, err := state.EC2Client(catalogAttributes.Region)
 	if err != nil {
-		return nil, fmt.Errorf("error getting EC2 client: %w", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error getting EC2 client: %s", err)
 	}
 
 	input, err := buildDescribeInstancesInput(setAttrs, true)
 	if err != nil {
-		return nil, fmt.Errorf("error building DescribeInstances parameters: %w", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error building DescribeInstances parameters: %s", err)
 	}
 
 	_, err = ec2Client.DescribeInstances(input)
 	if err == nil {
-		return nil, errors.New("query error: DescribeInstances DryRun should have returned error, but none was found")
+		return nil, status.Error(codes.FailedPrecondition, "query error: DescribeInstances DryRun should have returned error, but none was found")
 	}
 
 	if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "DryRunOperation" {
@@ -338,7 +326,7 @@ func (p *AwsPlugin) OnUpdateSet(ctx context.Context, req *pb.OnUpdateSetRequest)
 		return &pb.OnUpdateSetResponse{}, nil
 	}
 
-	return nil, fmt.Errorf("error performing dry run of DescribeInstances: %w", err)
+	return nil, status.Errorf(codes.InvalidArgument, "error performing dry run of DescribeInstances: %s", err)
 }
 
 func (p *AwsPlugin) OnDeleteSet(ctx context.Context, req *pb.OnDeleteSetRequest) (*pb.OnDeleteSetResponse, error) {
@@ -350,28 +338,31 @@ func (p *AwsPlugin) OnDeleteSet(ctx context.Context, req *pb.OnDeleteSetRequest)
 func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*pb.ListHostsResponse, error) {
 	catalog := req.GetCatalog()
 	if catalog == nil {
-		return nil, errors.New("catalog is nil")
+		return nil, status.Error(codes.InvalidArgument, "catalog is nil")
 	}
 
-	catalogAttrs := catalog.GetAttributes()
+	catalogAttrsRaw := catalog.GetAttributes()
-	if catalogAttrs == nil {
+	if catalogAttrsRaw == nil {
-		return nil, errors.New("catalog missing attributes")
+		return nil, status.Error(codes.InvalidArgument, "catalog missing attributes")
+	}
+
+	catalogAttributes, err := getCatalogAttributes(catalogAttrsRaw)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := validateRegion(catalogAttributes.Region); err != nil {
+		return nil, err
 	}
 
 	state, err := awsCatalogPersistedStateFromProto(req.GetPersisted(), p.testStateOpts...)
 	if err != nil {
-		return nil, fmt.Errorf("error loading persisted state: %w", err)
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
-	}
-
-	// 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")
+		return nil, status.Error(codes.InvalidArgument, "sets is nil")
 	}
 
 	// Build all the queries in advance.
@@ -386,20 +377,20 @@ func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*p
 	for i, set := range sets {
 		// Validate Id since we use it in output
 		if set.GetId() == "" {
-			return nil, errors.New("set missing id")
+			return nil, status.Error(codes.InvalidArgument, "set missing id")
 		}
 
 		if set.GetAttributes() == nil {
-			return nil, errors.New("set missing attributes")
+			return nil, status.Error(codes.InvalidArgument, "set missing attributes")
 		}
 		setAttrs, err := getSetAttributes(set.GetAttributes())
 		if err != nil {
-			return nil, fmt.Errorf("error parsing set attributes: %w", err)
+			return nil, err
 		}
 
 		input, err := buildDescribeInstancesInput(setAttrs, false)
 		if err != nil {
-			return nil, fmt.Errorf("error building DescribeInstances parameters for host set id %q: %w", set.GetId(), err)
+			return nil, status.Errorf(codes.InvalidArgument, "error building DescribeInstances parameters for host set id %q: %s", set.GetId(), err)
 		}
 		queries[i] = hostSetQuery{
 			Id:    set.GetId(),
@@ -407,9 +398,9 @@ 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)
+		return nil, status.Errorf(codes.InvalidArgument, "error getting EC2 client: %s", err)
 	}
 
 	// Run all queries now and assemble output.
@@ -417,7 +408,7 @@ func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*p
 	for i, query := range queries {
 		output, err := ec2Client.DescribeInstances(query.Input)
 		if err != nil {
-			return nil, fmt.Errorf("error running DescribeInstances for host set id %q: %w", query.Id, err)
+			return nil, status.Errorf(codes.InvalidArgument, "error running DescribeInstances for host set id %q: %s", query.Id, err)
 		}
 
 		queries[i].Output = output
@@ -428,7 +419,7 @@ func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*p
 			for _, instance := range reservation.Instances {
 				host, err := awsInstanceToHost(instance)
 				if err != nil {
-					return nil, fmt.Errorf("error processing host results for host set id %q: %w", query.Id, err)
+					return nil, status.Errorf(codes.InvalidArgument, "error processing host results for host set id %q: %s", query.Id, err)
 				}
 
 				queries[i].OutputHosts = append(queries[i].OutputHosts, host)
@@ -469,101 +460,13 @@ func (p *AwsPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*p
 	}, nil
 }
 
-func getStringValue(in *structpb.Struct, k string, required bool) (string, error) {
+func validateRegion(region 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
-	}
-
 	_, found := endpoints.PartitionForRegion(endpoints.DefaultPartitions(), region)
 	if !found {
-		return "", fmt.Errorf("not a valid region: %s", region)
+		return status.Errorf(codes.InvalidArgument, "not a valid region: %s", region)
 	}
 
-	return region, nil
+	return nil
 }
 
 func buildFilters(attrs *SetAttributes) ([]*ec2.Filter, error) {
@@ -642,16 +545,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
+	// First IP address/dns name are always the private fields if they
-	// populated
+	// 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 +570,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 +586,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
+			// Add private address/dns name if they have not been added yet
-			// reported at the top-level of the instance (let's call it the
+			if aws.StringValue(addr.PrivateIpAddress) != "" && !stringInSlice(result.IpAddresses, aws.StringValue(addr.PrivateIpAddress)) {
-			// "default address"). If it doesn't, add it.
+				result.IpAddresses = append(result.IpAddresses, aws.StringValue(addr.PrivateIpAddress))
-			if aws.StringValue(addr.PrivateIpAddress) != "" {
+			}
-				if aws.StringValue(instance.PrivateIpAddress) != aws.StringValue(addr.PrivateIpAddress) {
+			if aws.StringValue(addr.PrivateDnsName) != "" && !stringInSlice(result.DnsNames, aws.StringValue(addr.PrivateDnsName)) {
-					result.IpAddresses = append(result.IpAddresses, aws.StringValue(addr.PrivateIpAddress))
+				result.DnsNames = append(result.DnsNames, aws.StringValue(addr.PrivateDnsName))
-				}
 			}
 
-			// Do the same for the default public IP address and the
+			// Add public address/dns name if they have not been added yet
-			// public association on the interface.
+			if addr.Association != nil && aws.StringValue(addr.Association.PublicIp) != "" && !stringInSlice(result.IpAddresses, aws.StringValue(addr.Association.PublicIp)) {
-			if addr.Association != nil && addr.Association.PublicIp != nil && aws.StringValue(addr.Association.PublicIp) != "" {
+				result.IpAddresses = append(result.IpAddresses, 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))
+			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 +618,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
+}