Validate host sets when they are created or updated. (#5)

* Update Readme with details around credential rotation needing to be disabled.

* Apply suggestions from code review

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

README.md

@@ -19,24 +19,22 @@ Current limitations of this plugin:
   compute resource
 - A client secret must be generated against a registered application with any
   necessary permissions
+- For now, credential rotation must be disabled using the `disable_credential_rotation`
+  attribute. There is some additional work that the Boundary team identified
+  that is necessary to make this a seamless experience.
 
 ## Credential Rotation
 
-Although credentials are stored encrypted within Boundary, by default this
-plugin will attempt to rotate credentials when they are supplied through the
-`secrets` object on a create or update call to the host catalog resource. The
-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.
+*NOTE*: For now credential rotation must be explicitly disabled by setting
+`disable_credential_rotation` to true on the host catalog.  When we lift this
+restriction Boundary will manage the rotation of credentials for you.
 
-*NOTE*: Azure's credentials are eventually consistent and there may be
-significant delay (up to ten minutes or more) between when a credential is
-created and is available for use, or when a credential is deleted and
-unavailable for use. At the API level, this can be true even if the Azure Portal
-indicates that the credential is (or isn't) there. This delay will manifest as
-authentication failures when performing actions using this plugin. If secrets
-have just been supplied and `disable_credential_rotation` is not specified,
-please be patient and try again later.
+Although credentials are stored encrypted within Boundary, when credential 
+rotation isn't explicitly disabled, this plugin will attempt to rotate
+credentials when they are supplied through the `secrets` object on a create or
+update call to the host catalog resource. The 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.
 
 ## Attributes and Secrets
 

plugin/auth.go

@@ -21,7 +21,7 @@ type AuthorizationInfo struct {
 	AuthParams     AuthParams
 }
 
-type Attributes struct {
+type CatalogAttributes 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 Attributes
+	var attrs CatalogAttributes
 	if err := mapstructure.Decode(hcFields, &attrs); err != nil {
 		return nil, fmt.Errorf("error decoding host catalog attribute fields: %w", err)
 	}

plugin/plugin.go

@@ -10,6 +10,7 @@ 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"
@@ -28,6 +29,10 @@ 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 {
@@ -53,7 +58,7 @@ func rotateCredFromCallback(ctx context.Context, catalog *hostcatalogs.HostCatal
 	}, err
 }
 
-func (p *AzurePlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatalogRequest) (*pb.OnCreateCatalogResponse, error) {
+func (p *AzurePlugin) OnCreateCatalog(_ context.Context, req *pb.OnCreateCatalogRequest) (*pb.OnCreateCatalogResponse, error) {
 	if err := validateCatalog(req.GetCatalog()); err != nil {
 		return nil, err
 	}
@@ -80,7 +85,7 @@ func (p *AzurePlugin) OnCreateCatalog(ctx context.Context, req *pb.OnCreateCatal
 	}, nil
 }
 
-func (p *AzurePlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatalogRequest) (*pb.OnUpdateCatalogResponse, error) {
+func (p *AzurePlugin) OnUpdateCatalog(_ context.Context, req *pb.OnUpdateCatalogRequest) (*pb.OnUpdateCatalogResponse, error) {
 	if err := validateCatalog(req.GetNewCatalog()); err != nil {
 		return nil, err
 	}
@@ -118,6 +123,20 @@ func (p *AzurePlugin) OnUpdateCatalog(ctx context.Context, req *pb.OnUpdateCatal
 	}, 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
@@ -429,19 +448,19 @@ func validateSecrets(s *structpb.Struct) error {
 	return nil
 }
 
-var allowedFields = map[string]struct{}{
+var allowedCatalogFields = map[string]struct{}{
 	constDisableCredentialRotation: {},
 	constSubscriptionId:            {},
 	constTenantId:                  {},
 	constClientId:                  {},
 }
 
-func validateCatalog(catalog *hostcatalogs.HostCatalog) error {
-	if catalog == nil {
+func validateCatalog(c *hostcatalogs.HostCatalog) error {
+	if c == nil {
 		return status.Error(codes.InvalidArgument, "catalog is nil")
 	}
-	var attrs Attributes
-	attrMap := catalog.GetAttributes().AsMap()
+	var attrs CatalogAttributes
+	attrMap := c.GetAttributes().AsMap()
 	if err := mapstructure.Decode(attrMap, &attrs); err != nil {
 		return status.Errorf(codes.InvalidArgument, "error decoding catalog attributes: %s", err)
 	}
@@ -460,7 +479,7 @@ func validateCatalog(catalog *hostcatalogs.HostCatalog) error {
 	}
 
 	for f := range attrMap {
-		if _, ok := allowedFields[f]; !ok {
+		if _, ok := allowedCatalogFields[f]; !ok {
 			badFields[fmt.Sprintf("attributes.%s", f)] = "Unrecognized field."
 		}
 	}
@@ -470,3 +489,33 @@ func validateCatalog(catalog *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,6 +62,16 @@ 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,
@@ -178,6 +188,60 @@ 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{}