refact(target): Switch subtype attributes to oneof

This updates the target resources to use a oneof for attributes. This
allows requests to the targets service to leverage the new
subtypes.AttributeTransformerInterceptor so that attributes are strongly
typed prior to the service handler.

internal/servers/controller/handlers/targets/options.go

@@ -1,44 +0,0 @@
-package targets
-
-import (
-	"github.com/hashicorp/boundary/internal/target"
-	"google.golang.org/protobuf/types/known/structpb"
-)
-
-func getOpts(opt ...option) options {
-	opts := getDefaultOptions()
-	for _, o := range opt {
-		o(&opts)
-	}
-	return opts
-}
-
-// Option - how Options are passed as arguments
-type option func(*options)
-
-// options = how options are represented
-type options struct {
-	withTarget target.Target
-	withStruct *structpb.Struct
-}
-
-func getDefaultOptions() options {
-	return options{
-		withTarget: nil,
-		withStruct: nil,
-	}
-}
-
-// withTarget provides an option to provide a target.Target.
-func withTarget(t target.Target) option {
-	return func(o *options) {
-		o.withTarget = t
-	}
-}
-
-// withStruct provides an option to provide a structpb.Struct.
-func withStruct(s *structpb.Struct) option {
-	return func(o *options) {
-		o.withStruct = s
-	}
-}

internal/servers/controller/handlers/targets/registry.go

@@ -7,6 +7,7 @@ import (
 	"github.com/hashicorp/boundary/internal/servers/controller/handlers"
 	"github.com/hashicorp/boundary/internal/target"
 	"github.com/hashicorp/boundary/internal/types/subtypes"
+	pb "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/targets"
 	"google.golang.org/protobuf/proto"
 )
 
@@ -21,11 +22,14 @@ type Attributes interface {
 	Vet() map[string]string
 }
 
-type attributeFunc func(target.Target) Attributes
+type attributeFunc func(interface{}) Attributes
+
+type setAttributeFunc func(target.Target, *pb.Target) error
 
 type registryEntry struct {
 	maskManager handlers.MaskManager
 	attrFunc    attributeFunc
+	setAttrFunc setAttributeFunc
 }
 
 type registry struct {
@@ -56,45 +60,39 @@ func (r registry) maskManager(s subtypes.Subtype) (handlers.MaskManager, error)
 
 // newAttribute creates an Attribute for the given subtype. It delegates the
 // allocation of the Attribute to the registered attrFunc for the given
-// subtype. An error is returned if the provided subtype is not registered. It
-// supports the options of:
-//
-// - withTarget: The Attribute will be initialized based on the given
-// target.Target. This initialization is delegated to the attrFunc.
-//
-// - withStruct: The Attribute will be initialized based on the given
-// structpb.Struct. This is *not* delegated.
-//
-// These two options are mutually exclusive. If both are provided, and error is returned.
-// If no options are provided an empty Attribute will be returned.
-func (r registry) newAttribute(s subtypes.Subtype, opt ...option) (Attributes, error) {
+// subtype. An error is returned if the provided subtype is not registered
+func (r registry) newAttribute(s subtypes.Subtype, m interface{}) (Attributes, error) {
 	re, err := r.get(s)
 	if err != nil {
 		return nil, err
 	}
 
-	opts := getOpts(opt...)
-	if opts.withTarget != nil && opts.withStruct != nil {
-		return nil, fmt.Errorf("cannot use both withTarget and withStruct")
-	}
-
-	attr := re.attrFunc(opts.withTarget)
-	if opts.withStruct != nil {
-		if err := handlers.StructToProto(opts.withStruct, attr); err != nil {
-			return nil, fmt.Errorf("Provided attributes don't match expected format.")
-		}
-	}
+	attr := re.attrFunc(m)
 
 	return attr, nil
 }
 
+// setAttributes is used to set the Attrs field on a pb.Target. It delegates the
+// setting of the specific attribute type to the registered setAttributeFunc for
+// the given subtype. An error is returned if the provided subtype is not
+// registered.
+func (r registry) setAttributes(s subtypes.Subtype, in target.Target, out *pb.Target) error {
+	re, err := r.get(s)
+	if err != nil {
+		return err
+	}
+
+	return re.setAttrFunc(in, out)
+}
+
 var subtypeRegistry = registry{}
 
 // Register registers a subtype for used by the service handler.
-func Register(s subtypes.Subtype, maskManager handlers.MaskManager, af attributeFunc) {
+func Register(s subtypes.Subtype, maskManager handlers.MaskManager, af attributeFunc, sf setAttributeFunc) {
 	if _, existed := subtypeRegistry.LoadOrStore(s, &registryEntry{
 		maskManager: maskManager,
 		attrFunc:    af,
+		setAttrFunc: sf,
 	}); existed {
 		panic(fmt.Sprintf("subtype %s already registered", s))
 	}

internal/servers/controller/handlers/targets/target_service.go

@@ -211,14 +211,18 @@ func (s Service) ListTargets(ctx context.Context, req *pbs.ListTargetsRequest) (
 			return nil, err
 		}
 
-		if filter.Match(item) {
+		filterable, err := subtypes.Filterable(item)
+		if err != nil {
+			return nil, err
+		}
+		if filter.Match(filterable) {
 			finalItems = append(finalItems, item)
 		}
 	}
 	return &pbs.ListTargetsResponse{Items: finalItems}, nil
 }
 
-// GetTargets implements the interface pbs.TargetServiceServer.
+// GetTarget implements the interface pbs.TargetServiceServer.
 func (s Service) GetTarget(ctx context.Context, req *pbs.GetTargetRequest) (*pbs.GetTargetResponse, error) {
 	const op = "targets.(Service).GetTarget"
 
@@ -1243,7 +1247,7 @@ func (s Service) createInRepo(ctx context.Context, item *pb.Target) (target.Targ
 		opts = append(opts, target.WithWorkerFilter(item.GetWorkerFilter().GetValue()))
 	}
 
-	attr, err := subtypeRegistry.newAttribute(target.SubtypeFromType(item.GetType()), withStruct(item.GetAttributes()))
+	attr, err := subtypeRegistry.newAttribute(target.SubtypeFromType(item.GetType()), item.GetAttrs())
 	if err != nil {
 		return nil, nil, nil, handlers.ApiErrorWithCodeAndMessage(codes.InvalidArgument, err.Error())
 	}
@@ -1287,7 +1291,7 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []st
 	}
 	subtype := target.SubtypeFromId(id)
 
-	attr, err := subtypeRegistry.newAttribute(subtype, withStruct(item.GetAttributes()))
+	attr, err := subtypeRegistry.newAttribute(subtype, item.GetAttrs())
 	if err != nil {
 		return nil, nil, nil, handlers.ApiErrorWithCodeAndMessage(codes.InvalidArgument, err.Error())
 	}
@@ -1655,16 +1659,9 @@ func toProto(ctx context.Context, in target.Target, hostSources []target.HostSou
 		}
 	}
 	if outputFields.Has(globals.AttributesField) {
-		attr, err := subtypeRegistry.newAttribute(in.GetType(), withTarget(in))
-		if err != nil {
+		if err := subtypeRegistry.setAttributes(in.GetType(), in, &out); err != nil {
 			return nil, errors.Wrap(ctx, err, op)
 		}
-
-		st, err := handlers.ProtoToStruct(attr)
-		if err != nil {
-			return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "failed building password attribute struct: %v", err)
-		}
-		out.Attributes = st
 	}
 	return &out, nil
 }
@@ -1715,7 +1712,7 @@ func validateCreateRequest(req *pbs.CreateTargetRequest) error {
 		if err != nil {
 			badFields[globals.TypeField] = "Unknown type provided."
 		} else {
-			a, err := subtypeRegistry.newAttribute(subtype, withStruct(req.GetItem().GetAttributes()))
+			a, err := subtypeRegistry.newAttribute(subtype, req.GetItem().GetAttrs())
 			if err != nil {
 				badFields[globals.AttributesField] = "Attribute fields do not match the expected format."
 			} else {
@@ -1760,7 +1757,7 @@ func validateUpdateRequest(req *pbs.UpdateTargetRequest) error {
 				badFields[globals.TypeField] = "Cannot modify the resource type."
 			}
 
-			a, err := subtypeRegistry.newAttribute(subtype, withStruct(req.GetItem().GetAttributes()))
+			a, err := subtypeRegistry.newAttribute(subtype, req.GetItem().GetAttrs())
 			if err != nil {
 				badFields[globals.AttributesField] = "Attribute fields do not match the expected format."
 			} else {

internal/servers/controller/handlers/targets/tcp/target_service_test.go

@@ -133,7 +133,7 @@ func TestGet(t *testing.T) {
 		Type:                   tcp.Subtype.String(),
 		HostSetIds:             []string{hs[0].GetPublicId(), hs[1].GetPublicId()},
 		HostSourceIds:          []string{hs[0].GetPublicId(), hs[1].GetPublicId()},
-		Attributes:             new(structpb.Struct),
+		Attrs:                  &pb.Target_TcpTargetAttributes{},
 		SessionMaxSeconds:      wrapperspb.UInt32(28800),
 		SessionConnectionLimit: wrapperspb.Int32(1),
 		AuthorizedActions:      testAuthorizedActions,
@@ -227,7 +227,7 @@ func TestList(t *testing.T) {
 			UpdatedTime:            tar.GetUpdateTime().GetTimestamp(),
 			Version:                tar.GetVersion(),
 			Type:                   tcp.Subtype.String(),
-			Attributes:             new(structpb.Struct),
+			Attrs:                  &pb.Target_TcpTargetAttributes{},
 			SessionMaxSeconds:      wrapperspb.UInt32(28800),
 			SessionConnectionLimit: wrapperspb.Int32(1),
 			AuthorizedActions:      testAuthorizedActions,
@@ -243,7 +243,7 @@ func TestList(t *testing.T) {
 			UpdatedTime:            tar.GetUpdateTime().GetTimestamp(),
 			Version:                tar.GetVersion(),
 			Type:                   tcp.Subtype.String(),
-			Attributes:             new(structpb.Struct),
+			Attrs:                  &pb.Target_TcpTargetAttributes{},
 			SessionMaxSeconds:      wrapperspb.UInt32(28800),
 			SessionConnectionLimit: wrapperspb.Int32(1),
 			AuthorizedActions:      testAuthorizedActions,
@@ -323,7 +323,7 @@ func TestList(t *testing.T) {
 				require.Nil(item.SessionMaxSeconds)
 				require.Nil(item.SessionConnectionLimit)
 				require.Empty(item.WorkerFilter)
-				require.Nil(item.Attributes)
+				require.Nil(item.Attrs)
 			}
 		})
 	}
@@ -447,9 +447,11 @@ func TestCreate(t *testing.T) {
 				Name:        wrapperspb.String("name"),
 				Description: wrapperspb.String("desc"),
 				Type:        tcp.Subtype.String(),
-				Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
-					"default_port": structpb.NewNumberValue(2),
-				}},
+				Attrs: &pb.Target_TcpTargetAttributes{
+					TcpTargetAttributes: &pb.TcpTargetAttributes{
+						DefaultPort: wrapperspb.UInt32(2),
+					},
+				},
 				WorkerFilter: wrapperspb.String(`type == "bar"`),
 			}},
 			res: &pbs.CreateTargetResponse{
@@ -460,9 +462,11 @@ func TestCreate(t *testing.T) {
 					Name:        wrapperspb.String("name"),
 					Description: wrapperspb.String("desc"),
 					Type:        tcp.Subtype.String(),
-					Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
-						"default_port": structpb.NewNumberValue(2),
-					}},
+					Attrs: &pb.Target_TcpTargetAttributes{
+						TcpTargetAttributes: &pb.TcpTargetAttributes{
+							DefaultPort: wrapperspb.UInt32(2),
+						},
+					},
 					SessionMaxSeconds:      wrapperspb.UInt32(28800),
 					SessionConnectionLimit: wrapperspb.Int32(1),
 					AuthorizedActions:      testAuthorizedActions,
@@ -476,9 +480,11 @@ func TestCreate(t *testing.T) {
 				Name:        wrapperspb.String("name"),
 				Description: wrapperspb.String("desc"),
 				Type:        tcp.Subtype.String(),
-				Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
-					"default_port": structpb.NewNumberValue(0),
-				}},
+				Attrs: &pb.Target_TcpTargetAttributes{
+					TcpTargetAttributes: &pb.TcpTargetAttributes{
+						DefaultPort: wrapperspb.UInt32(2),
+					},
+				},
 			}},
 			err: handlers.ApiErrorWithCode(codes.InvalidArgument),
 		},
@@ -654,9 +660,11 @@ func TestUpdate(t *testing.T) {
 					Name:        wrapperspb.String("name"),
 					Description: wrapperspb.String("desc"),
 					Type:        tcp.Subtype.String(),
-					Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
-						"default_port": structpb.NewNumberValue(2),
-					}},
+					Attrs: &pb.Target_TcpTargetAttributes{
+						TcpTargetAttributes: &pb.TcpTargetAttributes{
+							DefaultPort: wrapperspb.UInt32(2),
+						},
+					},
 					CreatedTime:            tar.GetCreateTime().GetTimestamp(),
 					HostSetIds:             hsIds,
 					HostSets:               hostSets,
@@ -689,9 +697,11 @@ func TestUpdate(t *testing.T) {
 					Description: wrapperspb.String("desc"),
 					CreatedTime: tar.GetCreateTime().GetTimestamp(),
 					Type:        tcp.Subtype.String(),
-					Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
-						"default_port": structpb.NewNumberValue(2),
-					}},
+					Attrs: &pb.Target_TcpTargetAttributes{
+						TcpTargetAttributes: &pb.TcpTargetAttributes{
+							DefaultPort: wrapperspb.UInt32(2),
+						},
+					},
 					HostSetIds:             hsIds,
 					HostSets:               hostSets,
 					HostSourceIds:          hostSourceIds,
@@ -728,9 +738,11 @@ func TestUpdate(t *testing.T) {
 			req: &pbs.UpdateTargetRequest{
 				UpdateMask: &field_mask.FieldMask{Paths: []string{"default_port"}},
 				Item: &pb.Target{
-					Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
-						"default_port": structpb.NewNumberValue(0),
-					}},
+					Attrs: &pb.Target_TcpTargetAttributes{
+						TcpTargetAttributes: &pb.TcpTargetAttributes{
+							DefaultPort: wrapperspb.UInt32(0),
+						},
+					},
 				},
 			},
 			err: handlers.ApiErrorWithCode(codes.InvalidArgument),
@@ -776,9 +788,11 @@ func TestUpdate(t *testing.T) {
 					Name:        wrapperspb.String("default"),
 					CreatedTime: tar.GetCreateTime().GetTimestamp(),
 					Type:        tcp.Subtype.String(),
-					Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
-						"default_port": structpb.NewNumberValue(2),
-					}},
+					Attrs: &pb.Target_TcpTargetAttributes{
+						TcpTargetAttributes: &pb.TcpTargetAttributes{
+							DefaultPort: wrapperspb.UInt32(2),
+						},
+					},
 					HostSetIds:             hsIds,
 					HostSets:               hostSets,
 					HostSourceIds:          hostSourceIds,
@@ -809,9 +823,11 @@ func TestUpdate(t *testing.T) {
 					Description: wrapperspb.String("default"),
 					CreatedTime: tar.GetCreateTime().GetTimestamp(),
 					Type:        tcp.Subtype.String(),
-					Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
-						"default_port": structpb.NewNumberValue(2),
-					}},
+					Attrs: &pb.Target_TcpTargetAttributes{
+						TcpTargetAttributes: &pb.TcpTargetAttributes{
+							DefaultPort: wrapperspb.UInt32(2),
+						},
+					},
 					HostSetIds:             hsIds,
 					HostSets:               hostSets,
 					HostSourceIds:          hostSourceIds,
@@ -841,9 +857,11 @@ func TestUpdate(t *testing.T) {
 					Name:        wrapperspb.String("default"),
 					Description: wrapperspb.String("notignored"),
 					CreatedTime: tar.GetCreateTime().GetTimestamp(),
-					Attributes: &structpb.Struct{Fields: map[string]*structpb.Value{
-						"default_port": structpb.NewNumberValue(2),
-					}},
+					Attrs: &pb.Target_TcpTargetAttributes{
+						TcpTargetAttributes: &pb.TcpTargetAttributes{
+							DefaultPort: wrapperspb.UInt32(2),
+						},
+					},
 					Type:                   tcp.Subtype.String(),
 					HostSetIds:             hsIds,
 					HostSets:               hostSets,

internal/servers/controller/handlers/targets/tcp/tcp.go

@@ -30,18 +30,32 @@ func (a *attribute) Vet() map[string]string {
 	return badFields
 }
 
-func newAttribute(t target.Target) targets.Attributes {
+func newAttribute(m interface{}) targets.Attributes {
 	a := &attribute{
 		&pb.TcpTargetAttributes{},
 	}
-	if t != nil {
-		if t.GetDefaultPort() > 0 {
-			a.DefaultPort = &wrappers.UInt32Value{Value: t.GetDefaultPort()}
-		}
+	if tcpAttr, ok := m.(*pb.Target_TcpTargetAttributes); ok {
+		a.TcpTargetAttributes = tcpAttr.TcpTargetAttributes
 	}
 	return a
 }
 
+func setAttributes(t target.Target, out *pb.Target) error {
+	if t == nil {
+		return nil
+	}
+
+	attrs := &pb.Target_TcpTargetAttributes{
+		TcpTargetAttributes: &pb.TcpTargetAttributes{},
+	}
+	if t.GetDefaultPort() > 0 {
+		attrs.TcpTargetAttributes.DefaultPort = &wrappers.UInt32Value{Value: t.GetDefaultPort()}
+	}
+
+	out.Attrs = attrs
+	return nil
+}
+
 func init() {
 	var maskManager handlers.MaskManager
 	var err error
@@ -53,5 +67,5 @@ func init() {
 		panic(err)
 	}
 
-	targets.Register(tcp.Subtype, maskManager, newAttribute)
+	targets.Register(tcp.Subtype, maskManager, newAttribute, setAttributes)
 }