Apply suggestions from code review

Co-authored-by: Jeff Mitchell <jeffrey.mitchell@gmail.com>

plugin/auth.go

@@ -21,7 +21,7 @@ type AuthorizationInfo struct {
 	AuthParams     AuthParams
 }
 
-type CatalogAttributes struct {
+type Attributes struct {
 	SubscriptionId            string `mapstructure:"subscription_id"`
 	ClientId                  string `mapstructure:"client_id"`
 	TenantId                  string `mapstructure:"tenant_id"`
@@ -55,7 +55,7 @@ func getAuthorizationInfo(hc *hostcatalogs.HostCatalog) (*AuthorizationInfo, err
 	}
 
 	hcFields := hc.GetAttributes().AsMap()
-	var attrs CatalogAttributes
+	var attrs Attributes
 	if err := mapstructure.Decode(hcFields, &attrs); err != nil {
 		return nil, fmt.Errorf("error decoding host catalog attribute fields: %w", err)
 	}

plugin/plugin.go

@@ -10,7 +10,6 @@ import (
 	"github.com/Azure/azure-sdk-for-go/profiles/latest/network/mgmt/network"
 	"github.com/Azure/azure-sdk-for-go/profiles/latest/resources/mgmt/resources"
 	"github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/hostcatalogs"
-	"github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/hostsets"
 	"github.com/mitchellh/mapstructure"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -29,10 +28,6 @@ var (
 	_ pb.HostPluginServiceServer = (*AzurePlugin)(nil)
 )
 
-type SetAttributes struct {
-	Filter string `mapstructure:"filter"`
-}
-
 func rotateCredFromCallback(ctx context.Context, catalog *hostcatalogs.HostCatalog) (*pb.HostCatalogPersisted, error) {
 	authzInfo, err := getAuthorizationInfo(catalog)
 	if err != nil {
@@ -58,7 +53,7 @@ func rotateCredFromCallback(ctx context.Context, catalog *hostcatalogs.HostCatal
 	}, err
 }
 
-func (p *AzurePlugin) OnCreateCatalog(_ context.Context, req *pb.OnCreateCatalogRequest) (*pb.OnCreateCatalogResponse, error) {
+func (p *AzurePlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalogRequest) (*pb.OnCreateCatalogResponse, error) {
 	if err := validateCatalog(req.GetCatalog()); err != nil {
 		return nil, err
 	}
@@ -85,7 +80,7 @@ func (p *AzurePlugin) OnCreateCatalog(_ context.Context, req *pb.OnCreateCatalog
 	}, nil
 }
 
-func (p *AzurePlugin) OnUpdateCatalog(_ context.Context, req *pb.OnUpdateCatalogRequest) (*pb.OnUpdateCatalogResponse, error) {
+func (p *AzurePlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalogRequest) (*pb.OnUpdateCatalogResponse, error) {
 	if err := validateCatalog(req.GetNewCatalog()); err != nil {
 		return nil, err
 	}
@@ -123,20 +118,6 @@ func (p *AzurePlugin) OnUpdateCatalog(_ context.Context, req *pb.OnUpdateCatalog
 	}, nil
 }
 
-func (p *AzurePlugin) OnCreateSet(_ context.Context, req *pb.OnCreateSetRequest) (*pb.OnCreateSetResponse, error) {
-	if err := validateSet(req.GetSet()); err != nil {
-		return nil, err
-	}
-	return &pb.OnCreateSetResponse{}, nil
-}
-
-func (p *AzurePlugin) OnUpdateSet(_ context.Context, req *pb.OnUpdateSetRequest) (*pb.OnUpdateSetResponse, error) {
-	if err := validateSet(req.GetNewSet()); err != nil {
-		return nil, err
-	}
-	return &pb.OnUpdateSetResponse{}, nil
-}
-
 func (p *AzurePlugin) ListHosts(ctx context.Context, req *pb.ListHostsRequest) (*pb.ListHostsResponse, error) {
 	if len(req.GetSets()) == 0 {
 		// Nothing to fetch
@@ -448,19 +429,19 @@ func validateSecrets(s *structpb.Struct) error {
 	return nil
 }
 
-var allowedCatalogFields = map[string]struct{}{
+var allowedFields = map[string]struct{}{
 	constDisableCredentialRotation: {},
 	constSubscriptionId:            {},
 	constTenantId:                  {},
 	constClientId:                  {},
 }
 
-func validateCatalog(c *hostcatalogs.HostCatalog) error {
-	if c == nil {
+func validateCatalog(catalog *hostcatalogs.HostCatalog) error {
+	if catalog == nil {
 		return status.Error(codes.InvalidArgument, "catalog is nil")
 	}
-	var attrs CatalogAttributes
-	attrMap := c.GetAttributes().AsMap()
+	var attrs Attributes
+	attrMap := catalog.GetAttributes().AsMap()
 	if err := mapstructure.Decode(attrMap, &attrs); err != nil {
 		return status.Errorf(codes.InvalidArgument, "error decoding catalog attributes: %s", err)
 	}
@@ -479,7 +460,7 @@ func validateCatalog(c *hostcatalogs.HostCatalog) error {
 	}
 
 	for f := range attrMap {
-		if _, ok := allowedCatalogFields[f]; !ok {
+		if _, ok := allowedFields[f]; !ok {
 			badFields[fmt.Sprintf("attributes.%s", f)] = "Unrecognized field."
 		}
 	}
@@ -489,33 +470,3 @@ func validateCatalog(c *hostcatalogs.HostCatalog) error {
 	}
 	return nil
 }
-
-var allowedSetFields = map[string]struct{}{
-	constFilter: {},
-}
-
-func validateSet(s *hostsets.HostSet) error {
-	if s == nil {
-		return status.Error(codes.InvalidArgument, "set is nil")
-	}
-	var attrs SetAttributes
-	attrMap := s.GetAttributes().AsMap()
-	if err := mapstructure.Decode(attrMap, &attrs); err != nil {
-		return status.Errorf(codes.InvalidArgument, "error decoding set attributes: %s", err)
-	}
-	badFields := make(map[string]string)
-	if _, ok := attrMap[constFilter]; ok && len(attrs.Filter) == 0 {
-		badFields["attributes.filter"] = "This field must be not empty."
-	}
-
-	for f := range attrMap {
-		if _, ok := allowedSetFields[f]; !ok {
-			badFields[fmt.Sprintf("attributes.%s", f)] = "Unrecognized field."
-		}
-	}
-
-	if len(badFields) > 0 {
-		return invalidArgumentError("Invalid arguments in the new set", badFields)
-	}
-	return nil
-}

plugin/plugin_test.go

@@ -62,16 +62,6 @@ func TestValidateCatalog(t *testing.T) {
 				constSubscriptionId:            "sub_id",
 			}),
 		},
-		{
-			name: "bad type",
-			catalogAttrs: wrapMap(t, map[string]interface{}{
-				constDisableCredentialRotation: "bad type",
-				constClientId:                  "client_id",
-				constTenantId:                  "tenant_id",
-				constSubscriptionId:            "sub_id",
-			}),
-			wantError: true,
-		},
 		{
 			name:         "nil",
 			catalogAttrs: nil,
@@ -188,60 +178,6 @@ func TestValidateSecrets(t *testing.T) {
 	}
 }
 
-func TestValidateSet(t *testing.T) {
-	cases := []struct {
-		name      string
-		setAttrs  *structpb.Struct
-		wantError bool
-	}{
-		{
-			name: "success",
-			setAttrs: wrapMap(t, map[string]interface{}{
-				constFilter: "filter value",
-			}),
-		},
-		{
-			name:     "nil",
-			setAttrs: nil,
-		},
-		{
-			name: "bad type",
-			setAttrs: wrapMap(t, map[string]interface{}{
-				constFilter: true,
-			}),
-			wantError: true,
-		},
-		{
-			name: "empty",
-			setAttrs: wrapMap(t, map[string]interface{}{
-				constFilter: "",
-			}),
-			wantError: true,
-		},
-		{
-			name: "unrecognized field",
-			setAttrs: wrapMap(t, map[string]interface{}{
-				constFilter:    "filter value",
-				"unrecognized": "unrecognized",
-			}),
-			wantError: true,
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			got := validateSet(&hostsets.HostSet{
-				Attributes: tc.setAttrs,
-			})
-			if tc.wantError {
-				assert.Error(t, got)
-			} else {
-				assert.NoError(t, got)
-			}
-		})
-	}
-}
-
 func TestCreateCatalog(t *testing.T) {
 	ctx := context.Background()
 	p := &AzurePlugin{}