Add more tests and comments for Godoc

internal/credential/attributes.go

@@ -11,11 +11,15 @@ import (
 	"google.golang.org/protobuf/types/known/structpb"
 )
 
+// CredentialAttributes contain attributes used for authenticating to Google Cloud
+// and accessing a list of instances
 type CredentialAttributes struct {
 	Project string
 	Zone    string
 }
 
+// GetCredentialAttributes checks attributes required by Google Cloud to access
+// a list of instances and populate them into Boundary's host catalog
 func GetCredentialAttributes(in *structpb.Struct) (*CredentialAttributes, error) {
 	badFields := make(map[string]string)
 

internal/credential/attributes_test.go

@@ -29,6 +29,17 @@ func TestGetCredentialAttributes(t *testing.T) {
 			in:                  map[string]any{},
 			expectedErrContains: "missing required value \"zone\"",
 		},
+		{
+			name: "valid project and zone",
+			in: map[string]any{
+				ConstProject: "test-project",
+				ConstZone:    "us-central-1",
+			},
+			expected: &CredentialAttributes{
+				Project: "test-project",
+				Zone:    "us-central-1",
+			},
+		},
 	}
 
 	for _, tc := range cases {

internal/credential/const.go

@@ -4,11 +4,9 @@
 package credential
 
 const (
+	// ConstProject defines the attribute name for a Google Cloud project
 	ConstProject = "project"
+
+	// ConstZone defines the attribute name for a Google Cloud zone
 	ConstZone = "zone"
 )
-
-var AllowedCatalogFields = map[string]struct{}{
-	ConstProject: {},
-	ConstZone:    {},
-}

internal/errors/error_test.go

@@ -0,0 +1,37 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package errors
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestInvalidArgumentError(t *testing.T) {
+	// No error message.
+	err := InvalidArgumentError("", map[string]string{
+		"foo_key": "foo_value",
+		"bar_key": "bar_value",
+		"baz_key": "baz_value",
+	})
+	require.EqualError(t, err, "rpc error: code = InvalidArgument desc = : [bar_key: bar_value, baz_key: baz_value, foo_key: foo_value]")
+
+	// Nil fields
+	err = InvalidArgumentError("error message", nil)
+	require.EqualError(t, err, "rpc error: code = InvalidArgument desc = error message")
+
+	// Empty fields.
+	err = InvalidArgumentError("error message", map[string]string{})
+	require.EqualError(t, err, "rpc error: code = InvalidArgument desc = error message")
+
+	// Everything.
+	err = InvalidArgumentError("error message", map[string]string{
+		"foo_key": "foo_value",
+		"bar_key": "bar_value",
+		"baz_key": "baz_value",
+	})
+	require.EqualError(t, err, "rpc error: code = InvalidArgument desc = error message: [bar_key: bar_value, baz_key: baz_value, foo_key: foo_value]")
+
+}

internal/values/proto_test.go

@@ -0,0 +1,210 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package values
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/types/known/structpb"
+)
+
+func TestGetStringValue(t *testing.T) {
+	cases := []struct {
+		name        string
+		in          map[string]any
+		key         string
+		required    bool
+		expected    string
+		expectedErr string
+	}{
+		{
+			name:        "required missing",
+			in:          map[string]any{},
+			key:         "foo",
+			required:    true,
+			expectedErr: "missing required value \"foo\"",
+		},
+		{
+			name:     "optional missing",
+			in:       map[string]any{},
+			key:      "foo",
+			expected: "",
+		},
+		{
+			name: "non-string value",
+			in: map[string]any{
+				"foo": 1,
+			},
+			key:         "foo",
+			expectedErr: "unexpected type for value \"foo\": want string, got float64",
+		},
+		{
+			name: "required empty",
+			in: map[string]any{
+				"foo": "",
+			},
+			key:         "foo",
+			required:    true,
+			expectedErr: "value \"foo\" cannot be empty",
+		},
+		{
+			name: "good",
+			in: map[string]any{
+				"foo": "bar",
+			},
+			key:      "foo",
+			expected: "bar",
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			require := require.New(t)
+
+			input, err := structpb.NewStruct(tc.in)
+			require.NoError(err)
+
+			actual, err := GetStringValue(input, 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          map[string]any
+		key         string
+		required    bool
+		expected    bool
+		expectedErr string
+	}{
+		{
+			name:        "required missing",
+			in:          map[string]any{},
+			key:         "foo",
+			required:    true,
+			expectedErr: "missing required value \"foo\"",
+		},
+		{
+			name:     "optional missing",
+			in:       map[string]any{},
+			key:      "foo",
+			expected: false,
+		},
+		{
+			name: "non-bool value",
+			in: map[string]any{
+				"foo": "bar",
+			},
+			key:         "foo",
+			expectedErr: "unexpected type for value \"foo\": want bool, got string",
+		},
+		{
+			name: "good",
+			in: map[string]any{
+				"foo": true,
+			},
+			key:      "foo",
+			expected: true,
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			require := require.New(t)
+
+			input, err := structpb.NewStruct(tc.in)
+			require.NoError(err)
+
+			actual, err := GetBoolValue(input, 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          map[string]any
+		key         string
+		expected    time.Time
+		expectedErr string
+	}{
+		{
+			name:     "missing",
+			in:       map[string]any{},
+			key:      "foo",
+			expected: time.Time{},
+		},
+		{
+			name: "non-time value",
+			in: map[string]any{
+				"foo": 1,
+			},
+			key:         "foo",
+			expectedErr: "unexpected type for value \"foo\": want string, got float64",
+		},
+		{
+			name: "bad parse",
+			in: map[string]any{
+				"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: map[string]any{
+				"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)
+
+			input, err := structpb.NewStruct(tc.in)
+			require.NoError(err)
+
+			actual, err := GetTimeValue(input, tc.key)
+			if tc.expectedErr != "" {
+				require.EqualError(err, tc.expectedErr)
+				return
+			}
+
+			require.NoError(err)
+			require.Equal(tc.expected, actual)
+		})
+	}
+}

plugin/attributes.go

@@ -16,6 +16,7 @@ import (
 	"google.golang.org/protobuf/types/known/structpb"
 )
 
+// CatalogAttributes defines a set of attributes for the host catalog
 type CatalogAttributes struct {
 	*cred.CredentialAttributes
 }
@@ -51,6 +52,7 @@ func getCatalogAttributes(in *structpb.Struct) (*CatalogAttributes, error) {
 	}, nil
 }
 
+// SetAttributes defines attributes fro the host set
 type SetAttributes struct {
 	Filter        string `mapstructure:"filter"`
 	InstanceGroup string `mapstructure:"instance_group"`
@@ -62,6 +64,12 @@ func getSetAttributes(in *structpb.Struct) (*SetAttributes, error) {
 	badFields := make(map[string]string)
 	unknownFields := values.StructFields(in)
 
+	if _, filter := unknownFields[ConstListInstancesFilter]; filter {
+		if _, ig := unknownFields[ConstInstanceGroup]; ig {
+			return nil, errors.InvalidArgumentError("Error in the attributes provided, cannot define both filter and instance group", nil)
+		}
+	}
+
 	delete(unknownFields, ConstListInstancesFilter)
 	delete(unknownFields, ConstInstanceGroup)
 
@@ -82,6 +90,7 @@ func getSetAttributes(in *structpb.Struct) (*SetAttributes, error) {
 			inMap[ConstListInstancesFilter] = string(filterVal)
 		}
 	}
+
 	if instanceGroupRaw, ok := inMap[ConstInstanceGroup]; ok {
 		switch instanceGroupValue := instanceGroupRaw.(type) {
 		case string:

plugin/attributes_test.go

@@ -6,6 +6,7 @@ package plugin
 import (
 	"testing"
 
+	cred "github.com/hashicorp/boundary-plugin-google/internal/credential"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -38,6 +39,19 @@ func TestGetCatalogAttributes(t *testing.T) {
 			},
 			expectedErrContains: "attributes.bar: unrecognized field, attributes.foo: unrecognized field",
 		},
+		{
+			name: "valid project and zone",
+			in: &structpb.Struct{
+				Fields: map[string]*structpb.Value{
+					"project": structpb.NewStringValue("test-12345"),
+					"zone":    structpb.NewStringValue("us-central-1a"),
+				},
+			},
+			expected: &CatalogAttributes{&cred.CredentialAttributes{
+				Project: "test-12345",
+				Zone:    "us-central-1a",
+			}},
+		},
 	}
 
 	for _, tc := range cases {
@@ -73,7 +87,7 @@ func TestGetSetAttributes(t *testing.T) {
 			expected: &SetAttributes{},
 		},
 		{
-			name: "example filter",
+			name: "valid example filter",
 			in: map[string]any{
 				ConstListInstancesFilter: "tags.items=my-tag AND -tags.items=my-other-tag) OR tags.items=alternative-tag",
 			},
@@ -82,7 +96,7 @@ func TestGetSetAttributes(t *testing.T) {
 			},
 		},
 		{
-			name: "example instance group",
+			name: "valid example instance group",
 			in: map[string]any{
 				ConstInstanceGroup: "test",
 			},
@@ -98,6 +112,14 @@ func TestGetSetAttributes(t *testing.T) {
 			},
 			expectedErrContains: "attributes.bar: unrecognized field, attributes.foo: unrecognized field",
 		},
+		{
+			name: "invalid, cannot have both instance filter and group defined",
+			in: map[string]any{
+				ConstListInstancesFilter: "test",
+				ConstInstanceGroup:       "test",
+			},
+			expectedErrContains: "Error in the attributes provided, cannot define both filter and instance group",
+		},
 	}
 
 	for _, tc := range cases {

plugin/auth.go

@@ -1,4 +0,0 @@
-// Copyright (c) HashiCorp, Inc.
-// SPDX-License-Identifier: MPL-2.0
-
-package plugin

plugin/client.go

@@ -20,6 +20,8 @@ const (
 	NumberMaxResults = uint32(100)
 )
 
+// GoogleClient contains clients and attributes for authenticating to Google Cloud
+// and finding instances
 type GoogleClient struct {
 	InstancesClient     *compute.InstancesClient
 	InstanceGroupClient *compute.InstanceGroupsClient
@@ -76,6 +78,10 @@ func instanceToHost(instance *computepb.Instance) (*pb.ListHostsResponseHost, er
 		return nil, errors.New("response integrity error: missing instance self-link")
 	}
 
+	if instance.GetName() == "" {
+		return nil, errors.New("response integrity error: missing instance name")
+	}
+
 	result := new(pb.ListHostsResponseHost)
 
 	result.ExternalId = instance.GetSelfLink()

plugin/client_test.go

@@ -18,6 +18,7 @@ func TestInstanceToHost(t *testing.T) {
 	examplePublicIp := "1.1.1.1"
 	examplePublicIp2 := "1.1.1.2"
 	exampleIPv6 := "some::fake::address"
+	exampleName := "test-instance"
 
 	cases := []struct {
 		name        string
@@ -27,13 +28,23 @@ func TestInstanceToHost(t *testing.T) {
 	}{
 		{
 			name: "missing instance self-link",
-			instance:    &computepb.Instance{},
+			instance: &computepb.Instance{
+				Name: &exampleName,
+			},
 			expectedErr: "response integrity error: missing instance self-link",
 		},
+		{
+			name: "missing instance name",
+			instance: &computepb.Instance{
+				SelfLink: &exampleId,
+			},
+			expectedErr: "response integrity error: missing instance name",
+		},
 		{
 			name: "good, single private IP with public IP address",
 			instance: &computepb.Instance{
 				SelfLink: &exampleId,
+				Name:     &exampleName,
 				NetworkInterfaces: []*computepb.NetworkInterface{
 					{
 						NetworkIP: &examplePrivateIp,
@@ -47,6 +58,7 @@ func TestInstanceToHost(t *testing.T) {
 			},
 			expected: &pb.ListHostsResponseHost{
 				ExternalId:   exampleId,
+				ExternalName: exampleName,
 				IpAddresses:  []string{examplePrivateIp, examplePublicIp},
 			},
 		},
@@ -54,6 +66,7 @@ func TestInstanceToHost(t *testing.T) {
 			name: "good, single private IP address",
 			instance: &computepb.Instance{
 				SelfLink: &exampleId,
+				Name:     &exampleName,
 				NetworkInterfaces: []*computepb.NetworkInterface{
 					{
 						NetworkIP:     &examplePrivateIp,
@@ -63,6 +76,7 @@ func TestInstanceToHost(t *testing.T) {
 			},
 			expected: &pb.ListHostsResponseHost{
 				ExternalId:   exampleId,
+				ExternalName: exampleName,
 				IpAddresses:  []string{examplePrivateIp},
 			},
 		},
@@ -70,6 +84,7 @@ func TestInstanceToHost(t *testing.T) {
 			name: "good, multiple interfaces",
 			instance: &computepb.Instance{
 				SelfLink: &exampleId,
+				Name:     &exampleName,
 				NetworkInterfaces: []*computepb.NetworkInterface{
 					{
 						NetworkIP: &examplePrivateIp,
@@ -91,6 +106,7 @@ func TestInstanceToHost(t *testing.T) {
 			},
 			expected: &pb.ListHostsResponseHost{
 				ExternalId:   exampleId,
+				ExternalName: exampleName,
 				IpAddresses:  []string{examplePrivateIp, examplePublicIp, examplePrivateIp2, examplePublicIp2},
 			},
 		},
@@ -98,6 +114,7 @@ func TestInstanceToHost(t *testing.T) {
 			name: "good, single private IP address with IPv6",
 			instance: &computepb.Instance{
 				SelfLink: &exampleId,
+				Name:     &exampleName,
 				NetworkInterfaces: []*computepb.NetworkInterface{
 					{
 						NetworkIP:     &examplePrivateIp,
@@ -108,6 +125,7 @@ func TestInstanceToHost(t *testing.T) {
 			},
 			expected: &pb.ListHostsResponseHost{
 				ExternalId:   exampleId,
+				ExternalName: exampleName,
 				IpAddresses:  []string{examplePrivateIp, exampleIPv6},
 			},
 		},

plugin/const.go

@@ -4,7 +4,10 @@
 package plugin
 
 const (
+	// ConstListInstancesFilter refers to a Google Cloud SDK filter to search for instances
 	ConstListInstancesFilter = "filter"
+
+	// ConstInstanceGroup refers to the name of a Google Cloud instance group
 	ConstInstanceGroup = "instance_group"
 )
 

plugin/plugin.go

@@ -17,6 +17,8 @@ import (
 	"google.golang.org/grpc/status"
 )
 
+// GooglePlugin implements the GooglePluginServiceServer interface for the
+// Google host service plugin.
 type GooglePlugin struct {
 	pb.UnimplementedHostPluginServiceServer
 }
@@ -25,6 +27,7 @@ var (
 	_ pb.HostPluginServiceServer = (*GooglePlugin)(nil)
 )
 
+// OnCreateCatalog is called when a dynamic host catalog is created.
 func (p *GooglePlugin) OnCreateCatalog(_ context.Context, req *pb.OnCreateCatalogRequest) (*pb.OnCreateCatalogResponse, error) {
 	catalog := req.GetCatalog()
 	if catalog == nil {
@@ -47,6 +50,7 @@ func (p *GooglePlugin) OnCreateCatalog(_ context.Context, req *pb.OnCreateCatalo
 	}, nil
 }
 
+// OnUpdateCatalog is called when a dynamic host catalog is updated.
 func (p *GooglePlugin) OnUpdateCatalog(_ context.Context, req *pb.OnUpdateCatalogRequest) (*pb.OnUpdateCatalogResponse, error) {
 	currentCatalog := req.GetCurrentCatalog()
 	if currentCatalog == nil {
@@ -67,6 +71,7 @@ func (p *GooglePlugin) OnUpdateCatalog(_ context.Context, req *pb.OnUpdateCatalo
 	}, nil
 }
 
+// OnDeleteCatalog is called when a dynamic host catalog is deleted.
 func (p *GooglePlugin) OnDeleteCatalog(ctx context.Context, req *pb.OnDeleteCatalogRequest) (*pb.OnDeleteCatalogResponse, error) {
 	catalog := req.GetCatalog()
 	if catalog == nil {
@@ -85,6 +90,7 @@ func (p *GooglePlugin) OnDeleteCatalog(ctx context.Context, req *pb.OnDeleteCata
 	return &pb.OnDeleteCatalogResponse{}, nil
 }
 
+// OnCreateSet is called when a dynamic host set is created.
 func (p *GooglePlugin) OnCreateSet(_ context.Context, req *pb.OnCreateSetRequest) (*pb.OnCreateSetResponse, error) {
 	if err := validateSet(req.GetSet()); err != nil {
 		return nil, err
@@ -92,6 +98,7 @@ func (p *GooglePlugin) OnCreateSet(_ context.Context, req *pb.OnCreateSetRequest
 	return &pb.OnCreateSetResponse{}, nil
 }
 
+// OnUpdateSet is called when a dynamic host set is updated.
 func (p *GooglePlugin) OnUpdateSet(_ context.Context, req *pb.OnUpdateSetRequest) (*pb.OnUpdateSetResponse, error) {
 	if err := validateSet(req.GetNewSet()); err != nil {
 		return nil, err
@@ -104,6 +111,7 @@ func (p *GooglePlugin) OnDeleteSet(ctx context.Context, req *pb.OnDeleteSetReque
 	return &pb.OnDeleteSetResponse{}, nil
 }
 
+// ListHosts returns the list of instances based on filter or instance group name.
 func (p *GooglePlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*pb.ListHostsResponse, error) {
 	catalog := req.GetCatalog()
 	if catalog == nil {