Fixes roleset bindings for BigQuery datasets (#130)

CHANGELOG.md

@@ -1,5 +1,19 @@
 ## Next
 
+## v0.11.1
+### Unreleased
+
+BUG FIXES:
+
+* Fixes role bindings for BigQuery dataset resources [[GH-130](https://github.com/hashicorp/vault-plugin-secrets-gcp/pull/130)]
+
+## v0.10.3
+### Unreleased
+
+BUG FIXES:
+
+* Fixes role bindings for BigQuery dataset resources [[GH-130](https://github.com/hashicorp/vault-plugin-secrets-gcp/pull/130)]
+
 ## 0.8.1
 ### December 14, 2020
 

plugin/iamutil/dataset_resource.go

@@ -118,6 +118,18 @@ func datasetAsPolicy(ds *Dataset) *Policy {
 	for _, accessBinding := range ds.Access {
 		var iamMember string
 
+		// Role mapping must be applied for datasets in order to properly
+		// detect when to change bindings (via RemoveBindings()) after a
+		// modification or deletion occurs. This is due to BigQuery
+		// access roles accepting both legacy (e.g., OWNER) and current
+		// (e.g., roles/bigquery.dataOwner) role references. The API will
+		// only return the legacy format, so this mapping allows us to properly
+		// diff the current and desired roles to set the access policy.
+		//
+		// See the access[].role description in the following document for details
+		// https://cloud.google.com/bigquery/docs/reference/rest/v2/datasets#Dataset
+		role := mapLegacyRoles(accessBinding.Role)
+
 		//NOTE: Can either have GroupByEmail or UserByEmail but not both
 		if accessBinding.GroupByEmail != "" {
 			iamMember = fmt.Sprintf("group:%s", accessBinding.GroupByEmail)
@@ -126,11 +138,11 @@ func datasetAsPolicy(ds *Dataset) *Policy {
 		} else {
 			iamMember = fmt.Sprintf("user:%s", accessBinding.UserByEmail)
 		}
-		if binding, ok := bindingMap[accessBinding.Role]; ok {
+		if binding, ok := bindingMap[role]; ok {
 			binding.Members = append(binding.Members, iamMember)
 		} else {
-			bindingMap[accessBinding.Role] = &Binding{
-				Role:    accessBinding.Role,
+			bindingMap[role] = &Binding{
+				Role:    role,
 				Members: []string{iamMember},
 			}
 		}
@@ -140,3 +152,27 @@ func datasetAsPolicy(ds *Dataset) *Policy {
 	}
 	return policy
 }
+
+// mapLegacyRoles returns a current role name given a legacy role name.
+//
+// The following role mappings will be applied:
+// - OWNER -> roles/bigquery.dataOwner
+// - WRITER -> roles/bigquery.dataEditor
+// - READER -> roles/bigquery.dataViewer
+//
+// See the access[].role description in the following document for details
+// https://cloud.google.com/bigquery/docs/reference/rest/v2/datasets#Dataset
+//
+// Returns the given role if no mapping applies.
+func mapLegacyRoles(role string) string {
+	switch role {
+	case "OWNER":
+		return "roles/bigquery.dataOwner"
+	case "WRITER":
+		return "roles/bigquery.dataEditor"
+	case "READER":
+		return "roles/bigquery.dataViewer"
+	default:
+		return role
+	}
+}

plugin/iamutil/dataset_resource_test.go

@@ -210,8 +210,8 @@ func getTestFixtures() (*Policy, *Dataset) {
 func testResource() *DatasetResource {
 	return &DatasetResource{
 		relativeId: &gcputil.RelativeResourceName{
-			Name:    "dataset",
-			TypeKey: "projects/dataset",
+			Name:    "datasets",
+			TypeKey: "projects/datasets",
 			IdTuples: map[string]string{
 				"projects": "project",
 				"datasets": "dataset",

plugin/iamutil/resource_parser.go

@@ -125,7 +125,7 @@ func (apis GeneratedResources) Parse(rawName string) (Resource, error) {
 		return nil, err
 	}
 	switch cfg.TypeKey {
-	case "projects/dataset":
+	case "projects/datasets":
 		return &DatasetResource{relativeId: relName, config: cfg}, nil
 	default:
 		return &IamResource{relativeId: relName, config: cfg}, nil

plugin/iamutil/resource_parser_test.go

@@ -13,7 +13,7 @@ import (
 
 var letters = "ABCDEFGHIJKLMNOP"
 
-func TestEnabledIamResources_RelativeName(t *testing.T) {
+func TestEnabledResources_RelativeName(t *testing.T) {
 	enabledApis := GetEnabledResources()
 
 	for resourceType, services := range generatedResources {
@@ -39,7 +39,7 @@ func TestEnabledIamResources_RelativeName(t *testing.T) {
 			}
 
 			if resource != nil {
-				if err = verifyResource(resourceType, resource.(*IamResource)); err != nil {
+				if err = verifyResource(resourceType, resource); err != nil {
 					t.Errorf("could not verify resource for relative resource name %q: %sv", testRelName, err)
 				}
 			}
@@ -50,7 +50,7 @@ func TestEnabledIamResources_RelativeName(t *testing.T) {
 	}
 }
 
-func TestEnabledIamResources_FullName(t *testing.T) {
+func TestEnabledResources_FullName(t *testing.T) {
 	enabledApis := GetEnabledResources()
 
 	for resourceType, services := range generatedResources {
@@ -67,7 +67,7 @@ func TestEnabledIamResources_FullName(t *testing.T) {
 					t.Errorf("failed to get resource for full resource name %s (type: %s): %v", testFullName, resourceType, err)
 					continue
 				}
-				if err = verifyResource(resourceType, resource.(*IamResource)); err != nil {
+				if err = verifyResource(resourceType, resource); err != nil {
 					t.Errorf("could not verify resource for relative resource name %s: %v", testFullName, err)
 					continue
 				}
@@ -226,21 +226,21 @@ func getFakeId(resourceType string) string {
 	return strings.Trim(fakeId, "/")
 }
 
-func verifyResource(rType string, resource *IamResource) (err error) {
+func verifyResource(rType string, resource Resource) (err error) {
 	var req *http.Request
-	if resource.relativeId.TypeKey != rType {
-		return fmt.Errorf("expected resource type %s, actual resource has different type %s", rType, resource.relativeId.TypeKey)
+	if resource.GetRelativeId().TypeKey != rType {
+		return fmt.Errorf("expected resource type %s, actual resource has different type %s", rType, resource.GetRelativeId().TypeKey)
 	}
 
-	req, err = constructRequest(resource, &resource.config.GetMethod, nil)
+	req, err = constructRequest(resource, &resource.GetConfig().GetMethod, nil)
 	if err != nil {
 		return errwrap.Wrapf("unable to construct GetIamPolicyRequest: {{err}}", err)
 	}
 	if err := verifyConstructRequest(req, rType); err != nil {
 		return err
 	}
 
-	req, err = constructRequest(resource, &resource.config.SetMethod, strings.NewReader("{}"))
+	req, err = constructRequest(resource, &resource.GetConfig().SetMethod, strings.NewReader("{}"))
 	if err != nil {
 		return errwrap.Wrapf("unable to construct SetIamPolicyRequest: {{err}}", err)
 	}