feat: OnUpdateSet (#13) & ListHosts (#14)

* feat: OnUpdateSet

Add support for OnUpdateSet with validation. OnUpdateSet will validate he attributes passed to it and run a dryRunValidation with the new set

* feat: ListHosts (#14)

- Update list hosts to only support listing instances
- Update host client to support Instances API
- Add reading IPV6 Address by `Ipv6AccessConfigs`
- Add internal DNS which is the hostname

plugin/service/host/attributes.go

@@ -62,7 +62,6 @@ func getCatalogAttributes(in *structpb.Struct) (*CatalogAttributes, error) {
 // SetAttributes defines attributes fro the host set
 type SetAttributes struct {
 	Filters []string `mapstructure:"filters"`
-	InstanceGroup string   `mapstructure:"instance_group"`
 }
 
 func getSetAttributes(in *structpb.Struct) (*SetAttributes, error) {
@@ -78,7 +77,6 @@ func getSetAttributes(in *structpb.Struct) (*SetAttributes, error) {
 	}
 
 	delete(unknownFields, ConstListInstancesFilter)
-	delete(unknownFields, ConstInstanceGroup)
 
 	for a := range unknownFields {
 		badFields[fmt.Sprintf("attributes.%s", a)] = "unrecognized field"
@@ -98,13 +96,6 @@ func getSetAttributes(in *structpb.Struct) (*SetAttributes, error) {
 		}
 	}
 
-	if instanceGroupRaw, ok := inMap[ConstInstanceGroup]; ok {
-		switch instanceGroupValue := instanceGroupRaw.(type) {
-		case string:
-			inMap[ConstInstanceGroup] = string(instanceGroupValue)
-		}
-	}
-
 	if err := mapstructure.Decode(inMap, &setAttrs); err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "error decoding set attributes: %s", err)
 	}
@@ -112,31 +103,21 @@ func getSetAttributes(in *structpb.Struct) (*SetAttributes, error) {
 	return &setAttrs, nil
 }
 
-func buildListInstancesRequest(attributes *SetAttributes, catalog *CatalogAttributes) *computepb.ListInstancesRequest {
+func buildListInstancesRequest(attributes *SetAttributes, catalog *CatalogAttributes) (*computepb.ListInstancesRequest, error) {
 	request := &computepb.ListInstancesRequest{
 		Project: catalog.ProjectId,
 		Zone:    catalog.Zone,
 	}
 
-	if len(attributes.Filters) > 1 {
-		filters := strings.Join(attributes.Filters, " ")
+	filters, err := buildFilters(attributes)
+	if err != nil {
+		return nil, fmt.Errorf("error building filters: %w", err)
+	}
+
+	if len(filters) > 1 {
+		filters := strings.Join(filters, " AND ")
 		request.Filter = &filters
 	}
 
-	return request
-}
-
-func buildListInstanceGroupsRequest(attributes *SetAttributes, catalog *CatalogAttributes) *computepb.ListInstancesInstanceGroupsRequest {
-	request := &computepb.ListInstancesInstanceGroupsRequest{
-		InstanceGroup: attributes.InstanceGroup,
-		Project:       catalog.ProjectId,
-		Zone:          catalog.Zone,
-	}
-
-	if len(attributes.Filters) > 1 {
-		filters := strings.Join(attributes.Filters, " ")
-		request.Filter = &filters
-	}
-
-	return request
+	return request, nil
 }

plugin/service/host/attributes_test.go

@@ -95,15 +95,6 @@ func TestGetSetAttributes(t *testing.T) {
 				Filters: []string{"tags.items=my-tag AND -tags.items=my-other-tag) OR tags.items=alternative-tag"},
 			},
 		},
-		{
-			name: "valid example instance group",
-			in: map[string]any{
-				ConstInstanceGroup: "test",
-			},
-			expected: &SetAttributes{
-				InstanceGroup: "test",
-			},
-		},
 		{
 			name: "unknown fields",
 			in: map[string]any{

plugin/service/host/client.go

@@ -6,9 +6,8 @@ package host
 import (
 	"context"
 	"errors"
-	"path"
+	"strconv"
 
-	compute "cloud.google.com/go/compute/apiv1"
 	computepb "cloud.google.com/go/compute/apiv1/computepb"
 	pb "github.com/hashicorp/boundary/sdk/pbs/plugin"
 	"google.golang.org/api/iterator"
@@ -16,66 +15,25 @@ import (
 	"google.golang.org/grpc/status"
 )
 
-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
-	Context             context.Context
-	Project             string
-	Zone                string
-}
-
-func (c *GoogleClient) getInstances(request *computepb.ListInstancesRequest) ([]*computepb.Instance, error) {
+func getInstances(ctx context.Context, instancesClient InstancesAPI, request *computepb.ListInstancesRequest) ([]*computepb.Instance, error) {
 	hosts := []*computepb.Instance{}
-	it := c.InstancesClient.List(c.Context, request)
+	it := instancesClient.List(ctx, request)
 	for {
 		resp, err := it.Next()
 		if err == iterator.Done {
 			break
 		}
 		if err != nil {
-			return nil, status.Errorf(codes.InvalidArgument, "error listing instances: %s", err)
+			return nil, status.Errorf(codes.Unknown, "error listing instances: %s", err)
 		}
 		hosts = append(hosts, resp)
 	}
 	return hosts, nil
 }
 
-func (c *GoogleClient) getInstancesForInstanceGroup(request *computepb.ListInstancesInstanceGroupsRequest) ([]*computepb.Instance, error) {
-	hosts := []*computepb.Instance{}
-	instances := c.InstanceGroupClient.ListInstances(c.Context, request)
-
-	for {
-		resp, err := instances.Next()
-		if err == iterator.Done {
-			break
-		}
-		if err != nil {
-			return nil, status.Errorf(codes.InvalidArgument, "error listing instances for instance group %s: %s", request.InstanceGroup, err)
-		}
-
-		instance, err := c.InstancesClient.Get(c.Context, &computepb.GetInstanceRequest{
-			Instance: path.Base(resp.GetInstance()),
-			Project:  request.Project,
-			Zone:     request.Zone,
-		})
-		if err != nil {
-			return nil, status.Errorf(codes.InvalidArgument, "error getting instance %s for instance group %s: %s", resp.GetInstance(), request.InstanceGroup, err)
-		}
-
-		hosts = append(hosts, instance)
-	}
-	return hosts, nil
-}
-
 func instanceToHost(instance *computepb.Instance) (*pb.ListHostsResponseHost, error) {
-	if instance.GetSelfLink() == "" {
-		return nil, errors.New("response integrity error: missing instance self-link")
+	if instance.GetId() == 0 {
+		return nil, errors.New("response integrity error: missing instance id")
 	}
 
 	if instance.GetName() == "" {
@@ -84,22 +42,33 @@ func instanceToHost(instance *computepb.Instance) (*pb.ListHostsResponseHost, er
 
 	result := new(pb.ListHostsResponseHost)
 
-	result.ExternalId = instance.GetSelfLink()
+	result.ExternalId = strconv.FormatUint(instance.GetId(), 10)
 	result.ExternalName = instance.GetName()
 
+	// Internal DNS name is the hostname of the instance.
+	// https://cloud.google.com/compute/docs/networking/using-internal-dns
+	dnsName := instance.GetHostname()
+	result.DnsNames = appendDistinct(result.DnsNames, &dnsName)
+
 	// Now go through all of the interfaces and log the IP address of
 	// every interface.
 	for _, iface := range instance.GetNetworkInterfaces() {
-		// Populate default IP addresses/DNS name similar to how we do
+		// Populate default IP addresses name similar to how we do
 		// for the entire instance.
 		result.IpAddresses = appendDistinct(result.IpAddresses, iface.NetworkIP)
 
 		for _, external := range iface.AccessConfigs {
 			result.IpAddresses = appendDistinct(result.IpAddresses, external.NatIP)
+			result.IpAddresses = appendDistinct(result.IpAddresses, external.ExternalIpv6)
 		}
 
 		// Add the IPv6 addresses.
 		result.IpAddresses = appendDistinct(result.IpAddresses, iface.Ipv6Address)
+		if iface.Ipv6AccessConfigs != nil {
+			for _, external := range iface.Ipv6AccessConfigs {
+				result.IpAddresses = appendDistinct(result.IpAddresses, external.ExternalIpv6)
+			}
+		}
 	}
 
 	// Done

plugin/service/host/client_test.go

@@ -4,15 +4,19 @@
 package host
 
 import (
+	"context"
+	"errors"
+	"strconv"
 	"testing"
 
 	"cloud.google.com/go/compute/apiv1/computepb"
+	"github.com/hashicorp/boundary-plugin-gcp/internal/credential"
 	pb "github.com/hashicorp/boundary/sdk/pbs/plugin"
 	"github.com/stretchr/testify/require"
 )
 
 func TestInstanceToHost(t *testing.T) {
-	exampleId := "https://www.googleapis.com/compute/v1/projects/test-project/zones/us-central1-a/instances/test-instance"
+	exampleId := uint64(123456789)
 	examplePrivateIp := "10.0.0.1"
 	examplePrivateIp2 := "10.0.0.2"
 	examplePublicIp := "1.1.1.1"
@@ -27,23 +31,23 @@ func TestInstanceToHost(t *testing.T) {
 		expectedErr string
 	}{
 		{
-			name: "missing instance self-link",
+			name: "missing instance id",
 			instance: &computepb.Instance{
 				Name: &exampleName,
 			},
-			expectedErr: "response integrity error: missing instance self-link",
+			expectedErr: "response integrity error: missing instance id",
 		},
 		{
 			name: "missing instance name",
 			instance: &computepb.Instance{
-				SelfLink: &exampleId,
+				Id: &exampleId,
 			},
 			expectedErr: "response integrity error: missing instance name",
 		},
 		{
 			name: "good, single private IP with public IP address",
 			instance: &computepb.Instance{
-				SelfLink: &exampleId,
+				Id:   &exampleId,
 				Name: &exampleName,
 				NetworkInterfaces: []*computepb.NetworkInterface{
 					{
@@ -57,7 +61,7 @@ func TestInstanceToHost(t *testing.T) {
 				},
 			},
 			expected: &pb.ListHostsResponseHost{
-				ExternalId:   exampleId,
+				ExternalId:   strconv.FormatUint(exampleId, 10),
 				ExternalName: exampleName,
 				IpAddresses:  []string{examplePrivateIp, examplePublicIp},
 			},
@@ -65,7 +69,7 @@ func TestInstanceToHost(t *testing.T) {
 		{
 			name: "good, single private IP address",
 			instance: &computepb.Instance{
-				SelfLink: &exampleId,
+				Id:   &exampleId,
 				Name: &exampleName,
 				NetworkInterfaces: []*computepb.NetworkInterface{
 					{
@@ -75,7 +79,7 @@ func TestInstanceToHost(t *testing.T) {
 				},
 			},
 			expected: &pb.ListHostsResponseHost{
-				ExternalId:   exampleId,
+				ExternalId:   strconv.FormatUint(exampleId, 10),
 				ExternalName: exampleName,
 				IpAddresses:  []string{examplePrivateIp},
 			},
@@ -83,7 +87,7 @@ func TestInstanceToHost(t *testing.T) {
 		{
 			name: "good, multiple interfaces",
 			instance: &computepb.Instance{
-				SelfLink: &exampleId,
+				Id:   &exampleId,
 				Name: &exampleName,
 				NetworkInterfaces: []*computepb.NetworkInterface{
 					{
@@ -105,7 +109,7 @@ func TestInstanceToHost(t *testing.T) {
 				},
 			},
 			expected: &pb.ListHostsResponseHost{
-				ExternalId:   exampleId,
+				ExternalId:   strconv.FormatUint(exampleId, 10),
 				ExternalName: exampleName,
 				IpAddresses:  []string{examplePrivateIp, examplePublicIp, examplePrivateIp2, examplePublicIp2},
 			},
@@ -113,7 +117,7 @@ func TestInstanceToHost(t *testing.T) {
 		{
 			name: "good, single private IP address with IPv6",
 			instance: &computepb.Instance{
-				SelfLink: &exampleId,
+				Id:   &exampleId,
 				Name: &exampleName,
 				NetworkInterfaces: []*computepb.NetworkInterface{
 					{
@@ -124,7 +128,7 @@ func TestInstanceToHost(t *testing.T) {
 				},
 			},
 			expected: &pb.ListHostsResponseHost{
-				ExternalId:   exampleId,
+				ExternalId:   strconv.FormatUint(exampleId, 10),
 				ExternalName: exampleName,
 				IpAddresses:  []string{examplePrivateIp, exampleIPv6},
 			},
@@ -146,3 +150,88 @@ func TestInstanceToHost(t *testing.T) {
 		})
 	}
 }
+
+func TestGetInstances(t *testing.T) {
+	ctx := context.Background()
+
+	exampleId := uint64(123456789)
+	exampleName := "test-instance"
+	exampleInstance := &computepb.Instance{
+		Id:   &exampleId,
+		Name: &exampleName,
+	}
+
+	cases := []struct {
+		name            string
+		instancesClient InstancesAPI
+		request         *computepb.ListInstancesRequest
+		expected        []*computepb.Instance
+		expectedErr     string
+	}{
+		{
+			name: "successful retrieval",
+			instancesClient: func() InstancesAPI {
+				fn := newTestMockInstances(ctx,
+					nil,
+					testMockInstancesWithListInstancesOutput(&computepb.InstanceList{
+						Items: []*computepb.Instance{exampleInstance},
+					}),
+					testMockInstancesWithListInstancesError(nil),
+				)
+				client, err := fn(&credential.Config{})
+				require.NoError(t, err)
+				return client
+			}(),
+			request:  &computepb.ListInstancesRequest{},
+			expected: []*computepb.Instance{exampleInstance},
+		},
+		{
+			name: "error during listing",
+			instancesClient: func() InstancesAPI {
+				fn := newTestMockInstances(ctx,
+					nil,
+					testMockInstancesWithListInstancesOutput(&computepb.InstanceList{}),
+					testMockInstancesWithListInstancesError(errors.New("internal error")),
+				)
+				client, err := fn(&credential.Config{})
+				require.NoError(t, err)
+				return client
+			}(),
+			request:     &computepb.ListInstancesRequest{},
+			expectedErr: "error listing instances",
+		},
+		{
+			name: "no instances",
+			instancesClient: func() InstancesAPI {
+				fn := newTestMockInstances(ctx,
+					nil,
+					testMockInstancesWithListInstancesOutput(&computepb.InstanceList{}),
+					testMockInstancesWithListInstancesError(nil),
+				)
+				client, err := fn(&credential.Config{})
+				require.NoError(t, err)
+				return client
+			}(),
+			request:  &computepb.ListInstancesRequest{},
+			expected: []*computepb.Instance{},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			require := require.New(t)
+			actual, err := getInstances(ctx, tc.instancesClient, tc.request)
+			if tc.expectedErr != "" {
+				require.ErrorContains(err, tc.expectedErr)
+				return
+			}
+
+			require.NoError(err)
+			require.Len(actual, len(tc.expected))
+			if len(actual) > 0 {
+				require.Equal(tc.expected[0].Name, actual[0].Name)
+				require.Equal(tc.expected[0].Id, actual[0].Id)
+			}
+		})
+	}
+}

plugin/service/host/plugin.go

@@ -9,7 +9,6 @@ import (
 	"regexp"
 	"strings"
 
-	compute "cloud.google.com/go/compute/apiv1"
 	computepb "cloud.google.com/go/compute/apiv1/computepb"
 	"github.com/hashicorp/boundary-plugin-gcp/internal/credential"
 	errors "github.com/hashicorp/boundary-plugin-gcp/internal/errors"
@@ -38,6 +37,13 @@ var (
 	_ pb.HostPluginServiceServer = (*HostPlugin)(nil)
 )
 
+type hostSetQuery struct {
+	Id             string
+	InputInstances *computepb.ListInstancesRequest
+	Output         []*computepb.Instance
+	OutputHosts    []*pb.ListHostsResponseHost
+}
+
 const (
 	// defaultStatusFilter is the default filter to apply to instances
 	// if no status filter is provided.
@@ -380,10 +386,64 @@ func (p *HostPlugin) OnCreateSet(ctx context.Context, req *pb.OnCreateSetRequest
 }
 
 // OnUpdateSet is called when a dynamic host set is updated.
-func (p *HostPlugin) OnUpdateSet(_ context.Context, req *pb.OnUpdateSetRequest) (*pb.OnUpdateSetResponse, error) {
-	if err := validateSet(req.GetNewSet()); err != nil {
+func (p *HostPlugin) OnUpdateSet(ctx context.Context, req *pb.OnUpdateSetRequest) (*pb.OnUpdateSetResponse, error) {
+	catalog := req.GetCatalog()
+	if catalog == nil {
+		return nil, status.Error(codes.InvalidArgument, "catalog is required")
+	}
+
+	attrs := catalog.GetAttributes()
+	if attrs == nil {
+		return nil, status.Error(codes.InvalidArgument, "catalog attributes are required")
+	}
+
+	catalogAttributes, err := getCatalogAttributes(attrs)
+	if err != nil {
 		return nil, err
 	}
+
+	credState, err := credential.PersistedStateFromProto(
+		req.GetPersisted().GetSecrets(),
+		catalogAttributes.CredentialAttributes)
+	if err != nil {
+		return nil, status.Errorf(codes.InvalidArgument, "error getting persisted state from proto: %s", err)
+	}
+
+	catalogState, err := newGCPCatalogPersistedState(
+		append([]gcpCatalogPersistedStateOption{
+			withCredentials(credState),
+		}, p.testCatalogStateOpts...)...,
+	)
+	if err != nil {
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
+	}
+
+	set := req.GetNewSet()
+	if set == nil {
+		return nil, status.Error(codes.InvalidArgument, "set is required")
+	}
+	if err := validateSet(set); err != nil {
+		return nil, err
+	}
+
+	if set.GetAttributes() == nil {
+		return nil, status.Error(codes.InvalidArgument, "set attributes are required")
+	}
+	setAttrs, err := getSetAttributes(set.GetAttributes())
+	if err != nil {
+		return nil, err
+	}
+
+	listInstanceFilters, err := buildFilters(setAttrs)
+	if err != nil {
+		return nil, status.Errorf(codes.InvalidArgument, "error building filters: %s", err)
+	}
+
+	// perform dry run to ensure we can interact with GCP as expected.
+	if st := dryRunValidation(ctx, catalogState, listInstanceFilters, p.testGCPClientOpts...); st != nil {
+		return nil, st.Err()
+	}
+
 	return &pb.OnUpdateSetResponse{}, nil
 }
 
@@ -409,21 +469,27 @@ func (p *HostPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*
 		return nil, err
 	}
 
+	credState, err := credential.PersistedStateFromProto(
+		req.GetPersisted().GetSecrets(),
+		catalogAttributes.CredentialAttributes)
+	if err != nil {
+		return nil, status.Errorf(codes.InvalidArgument, "error getting persisted state from proto: %s", err)
+	}
+
+	catalogState, err := newGCPCatalogPersistedState(
+		append([]gcpCatalogPersistedStateOption{
+			withCredentials(credState),
+		}, p.testCatalogStateOpts...)...,
+	)
+	if err != nil {
+		return nil, status.Errorf(codes.InvalidArgument, "error loading persisted state: %s", err)
+	}
+
 	sets := req.GetSets()
 	if sets == nil {
 		return nil, status.Error(codes.InvalidArgument, "sets is nil")
 	}
 
-	type hostSetQuery struct {
-		Id             string
-		InputInstances *computepb.ListInstancesRequest
-		InputGroups    *computepb.ListInstancesInstanceGroupsRequest
-		Project        string
-		Zone           string
-		Output         []*computepb.Instance
-		OutputHosts    []*pb.ListHostsResponseHost
-	}
-
 	queries := make([]hostSetQuery, len(sets))
 	for i, set := range sets {
 		// Validate Id since we use it in output
@@ -432,56 +498,35 @@ func (p *HostPlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*
 		}
 
 		if set.GetAttributes() == nil {
-			return nil, status.Error(codes.InvalidArgument, "set missing attributes")
+			return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("set %s missing attributes", set.GetId()))
 		}
 		setAttrs, err := getSetAttributes(set.GetAttributes())
 		if err != nil {
 			return nil, err
 		}
 
-		if setAttrs.InstanceGroup != "" {
+		input, err := buildListInstancesRequest(setAttrs, catalogAttributes)
+		if err != nil {
+			return nil, status.Errorf(codes.InvalidArgument, "error building list instances request: %s", err)
+		}
+
 		queries[i] = hostSetQuery{
 			Id:             set.GetId(),
-				InputGroups: buildListInstanceGroupsRequest(setAttrs, catalogAttributes),
-			}
-		} else {
-			queries[i] = hostSetQuery{
-				Id:             set.GetId(),
-				InputInstances: buildListInstancesRequest(setAttrs, catalogAttributes),
-			}
+			InputInstances: input,
 		}
 	}
 
-	instancesClient, err := compute.NewInstancesRESTClient(ctx)
+	instancesClient, err := catalogState.InstancesClient(ctx, p.testGCPClientOpts...)
 	if err != nil {
-		return nil, status.Errorf(codes.InvalidArgument, "error creating NewInstancesRESTClient: %s", err)
-	}
-
-	instanceGroupsClient, err := compute.NewInstanceGroupsRESTClient(ctx)
-	if err != nil {
-		return nil, status.Errorf(codes.InvalidArgument, "error creating NewInstanceGroupsRESTClient: %s", err)
-	}
-
-	gclient := GoogleClient{
-		InstancesClient:     instancesClient,
-		InstanceGroupClient: instanceGroupsClient,
-		Context:             ctx,
+		return nil, status.Errorf(codes.InvalidArgument, "error getting instances client: %s", err)
 	}
 
 	// Run all queries now and assemble output.
 	var maxLen int
 	for i, query := range queries {
-		var output []*computepb.Instance
-		if query.InputGroups != nil {
-			output, err = gclient.getInstancesForInstanceGroup(query.InputGroups)
+		output, err := getInstances(ctx, instancesClient, query.InputInstances)
 		if err != nil {
-				return nil, status.Errorf(codes.InvalidArgument, "error running getInstancesForInstanceGroup for host set id %q: %s", query.Id, err)
-			}
-		} else {
-			output, err = gclient.getInstances(query.InputInstances)
-			if err != nil {
-				return nil, status.Errorf(codes.InvalidArgument, "error running getInstances for host set id %q: %s", query.Id, err)
-			}
+			return nil, status.Errorf(codes.InvalidArgument, "error running list instances for host set id %q: %s", query.Id, err)
 		}
 
 		queries[i].Output = output
@@ -547,8 +592,6 @@ func validateSet(s *hostsets.HostSet) error {
 
 	if instanceGroupSet && filterSet {
 		badFields["attributes"] = "must set instance group or filter, cannot set both"
-	} else if instanceGroupSet && len(attrs.InstanceGroup) == 0 {
-		badFields[fmt.Sprintf("attributes.%s", ConstInstanceGroup)] = "must not be empty"
 	} else if filterSet && len(attrs.Filters) == 0 {
 		badFields[fmt.Sprintf("attributes.%s", ConstListInstancesFilter)] = "must not be empty"
 	}

plugin/service/host/state.go

@@ -17,10 +17,6 @@ import (
 	"google.golang.org/protobuf/types/known/structpb"
 )
 
-type Iterator interface {
-	Next() (*computepb.Instance, error)
-}
-
 type InstancesAPI interface {
 	List(ctx context.Context, req *computepb.ListInstancesRequest, opts ...gax.CallOption) *compute.InstanceIterator
 }