Access control: Fix a bug with data source permission resource and allow assigning permissions for basic roles (#692)

IevaVasiljeva · web-flow · commit f1a498b734a3 · 2022-11-15T15:10:56.000Z
* set and update builtin roles

* don't allow updating admin role

* fix tests

* extend docs

* feedback

* fix a typo

* remove restrictions for admin basic role

* doc update

* update Golang API client dependency

* update docs
diff --git a/docs/resources/data_source_permission.md b/docs/resources/data_source_permission.md
@@ -42,6 +42,10 @@ resource "grafana_data_source_permission" "fooPermissions" {
     user_id    = 3 // 3 is the admin user in cloud. It can't be queried
     permission = "Edit"
   }
+  permissions {
+    built_in_role = "Viewer"
+    permission    = "Query"
+  }
 }
 ```
 
@@ -66,6 +70,7 @@ Required:
 
 Optional:
 
+- `built_in_role` (String) Name of the basic role to manage permissions for. Options: `Viewer`, `Editor` or `Admin`. Can only be set from Grafana v9.2.3+. Defaults to ``.
 - `team_id` (Number) ID of the team to manage permissions for. Defaults to `0`.
 - `user_id` (Number) ID of the user to manage permissions for. Defaults to `0`.
 
diff --git a/examples/resources/grafana_data_source_permission/resource.tf b/examples/resources/grafana_data_source_permission/resource.tf
@@ -27,4 +27,8 @@ resource "grafana_data_source_permission" "fooPermissions" {
     user_id    = 3 // 3 is the admin user in cloud. It can't be queried
     permission = "Edit"
   }
+  permissions {
+    built_in_role = "Viewer"
+    permission    = "Query"
+  }
 }
diff --git a/go.mod b/go.mod
@@ -5,7 +5,7 @@ go 1.18
 require (
 	github.com/Masterminds/semver/v3 v3.1.1
 	github.com/grafana/amixr-api-go-client v0.0.5
-	github.com/grafana/grafana-api-golang-client v0.13.0
+	github.com/grafana/grafana-api-golang-client v0.13.1
 	github.com/grafana/machine-learning-go-client v0.2.0
 	github.com/grafana/synthetic-monitoring-agent v0.10.2
 	github.com/grafana/synthetic-monitoring-api-go-client v0.6.3
diff --git a/go.sum b/go.sum
@@ -76,8 +76,8 @@ github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
 github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/grafana/amixr-api-go-client v0.0.5 h1:jqmljnd5FozuOsCNuyhZVpooxmj0BW9MmeLA7PaLK6U=
 github.com/grafana/amixr-api-go-client v0.0.5/go.mod h1:N6x26XUrM5zGtK5zL5vNJnAn2JFMxLFPPLTw/6pDkFE=
-github.com/grafana/grafana-api-golang-client v0.13.0 h1:o/gL3F7EjBSBKgrpjH9/+sYFJ0HBUofvAYlKhrT2opE=
-github.com/grafana/grafana-api-golang-client v0.13.0/go.mod h1:24W29gPe9yl0/3A9X624TPkAOR8DpHno490cPwnkv8E=
+github.com/grafana/grafana-api-golang-client v0.13.1 h1:a5R8bIwL98xd79zFTQnYgpva3ns7Nm5/DnVAWYBdWVk=
+github.com/grafana/grafana-api-golang-client v0.13.1/go.mod h1:24W29gPe9yl0/3A9X624TPkAOR8DpHno490cPwnkv8E=
 github.com/grafana/machine-learning-go-client v0.2.0 h1:5JgfJn5Q72D0jZlXnM0gZ9lV4Q4zzq9X0GVfPu8Vxis=
 github.com/grafana/machine-learning-go-client v0.2.0/go.mod h1:QFfZz8NkqVF8++skjkKQXJEZfpCYd8S0yTWJUpsLLTA=
 github.com/grafana/synthetic-monitoring-agent v0.10.2 h1:lbz1o+ETxOk+2ylY2JJ4PNcbwqT17Rb0ZexyyD0dI4Y=
diff --git a/grafana/resource_datasource_permission.go b/grafana/resource_datasource_permission.go
@@ -51,6 +51,13 @@ func ResourceDatasourcePermission() *schema.Resource {
 							Default:     0,
 							Description: "ID of the user to manage permissions for.",
 						},
+						"built_in_role": {
+							Type:         schema.TypeString,
+							Optional:     true,
+							Default:      "",
+							ValidateFunc: validation.StringInSlice([]string{"Viewer", "Editor", "Admin"}, false),
+							Description:  "Name of the basic role to manage permissions for. Options: `Viewer`, `Editor` or `Admin`. Can only be set from Grafana v9.2.3+.",
+						},
 						"permission": {
 							Type:         schema.TypeString,
 							Required:     true,
@@ -83,6 +90,9 @@ func UpdateDatasourcePermissions(ctx context.Context, d *schema.ResourceData, me
 		if permission["user_id"].(int) != -1 {
 			permissionItem.UserID = int64(permission["user_id"].(int))
 		}
+		if permission["built_in_role"].(string) != "" {
+			permissionItem.BuiltInRole = permission["built_in_role"].(string)
+		}
 		var err error
 		if permissionItem.Permission, err = mapDatasourcePermissionStringToType(permission["permission"].(string)); err != nil {
 			return diag.FromErr(err)
@@ -121,8 +131,10 @@ func ReadDatasourcePermissions(ctx context.Context, d *schema.ResourceData, meta
 	permissionItems := make([]interface{}, len(response.Permissions))
 	for i, permission := range response.Permissions {
 		permissionItem := make(map[string]interface{})
+		permissionItem["built_in_role"] = permission.BuiltInRole
 		permissionItem["team_id"] = permission.TeamID
 		permissionItem["user_id"] = permission.UserID
+
 		if permissionItem["permission"], err = mapDatasourcePermissionTypeToString(permission.Permission); err != nil {
 			return diag.FromErr(err)
 		}
@@ -154,7 +166,7 @@ func DeleteDatasourcePermissions(ctx context.Context, d *schema.ResourceData, me
 
 func updateDatasourcePermissions(client *gapi.Client, id int64, permissions []*gapi.DatasourcePermissionAddPayload, enable, disable bool) error {
 	areEqual := func(a *gapi.DatasourcePermission, b *gapi.DatasourcePermissionAddPayload) bool {
-		return a.Permission == b.Permission && a.TeamID == b.TeamID && a.UserID == b.UserID
+		return a.Permission == b.Permission && a.TeamID == b.TeamID && a.UserID == b.UserID && a.BuiltInRole == b.BuiltInRole
 	}
 
 	response, err := client.DatasourcePermissions(id)
diff --git a/grafana/resource_datasource_permission_test.go b/grafana/resource_datasource_permission_test.go
@@ -10,8 +10,6 @@ import (
 )
 
 func TestAccDatasourcePermission_basic(t *testing.T) {
-	t.Skip("skipping this test as the resource is going be deprecated soon and the test is flaky due to the Grafana server implementation")
-
 	CheckCloudInstanceTestsEnabled(t)
 
 	datasourceID := int64(-1)
@@ -23,7 +21,7 @@ func TestAccDatasourcePermission_basic(t *testing.T) {
 				Config: testAccExample(t, "resources/grafana_data_source_permission/resource.tf"),
 				Check: resource.ComposeAggregateTestCheckFunc(
 					testAccDatasourcePermissionsCheckExists("grafana_data_source_permission.fooPermissions", &datasourceID),
-					resource.TestCheckResourceAttr("grafana_data_source_permission.fooPermissions", "permissions.#", "2"),
+					resource.TestCheckResourceAttr("grafana_data_source_permission.fooPermissions", "permissions.#", "3"),
 				),
 			},
 			{
@@ -72,9 +70,6 @@ func testAccDatasourcePermissionCheckDestroy(datasourceID *int64) resource.TestC
 		if err != nil {
 			return fmt.Errorf("error getting datasource permissions %d: %s", *datasourceID, err)
 		}
-		if response.Enabled {
-			return fmt.Errorf("datasource permissions %d still enabled", *datasourceID)
-		}
 		if len(response.Permissions) > 0 {
 			return fmt.Errorf("permissions were not empty when expected")
 		}

Original file line number	Diff line number	Diff line change
`@@ -42,6 +42,10 @@ resource "grafana_data_source_permission" "fooPermissions" {`
`42`	`42`	`user_id = 3 // 3 is the admin user in cloud. It can't be queried`
`43`	`43`	`permission = "Edit"`
`44`	`44`	`}`
	`45`	`+ permissions {`
	`46`	`+ built_in_role = "Viewer"`
	`47`	`+ permission = "Query"`
	`48`	`+ }`
`45`	`49`	`}`
`46`	`50`	```
`47`	`51`
`@@ -66,6 +70,7 @@ Required:`
`66`	`70`
`67`	`71`	`Optional:`
`68`	`72`
	`73`	+- `built_in_role` (String) Name of the basic role to manage permissions for. Options: `Viewer`, `Editor` or `Admin`. Can only be set from Grafana v9.2.3+. Defaults to ``.
`69`	`74`	- `team_id` (Number) ID of the team to manage permissions for. Defaults to `0`.
`70`	`75`	- `user_id` (Number) ID of the user to manage permissions for. Defaults to `0`.
`71`	`76`
Original file line number	Diff line number	Diff line change
`@@ -27,4 +27,8 @@ resource "grafana_data_source_permission" "fooPermissions" {`
`27`	`27`	`user_id = 3 // 3 is the admin user in cloud. It can't be queried`
`28`	`28`	`permission = "Edit"`
`29`	`29`	`}`
	`30`	`+ permissions {`
	`31`	`+ built_in_role = "Viewer"`
	`32`	`+ permission = "Query"`
	`33`	`+ }`
`30`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -10,8 +10,6 @@ import (`
`10`	`10`	`)`
`11`	`11`
`12`	`12`	`func TestAccDatasourcePermission_basic(t *testing.T) {`
`13`		`- t.Skip("skipping this test as the resource is going be deprecated soon and the test is flaky due to the Grafana server implementation")`
`14`		`-`
`15`	`13`	`CheckCloudInstanceTestsEnabled(t)`
`16`	`14`
`17`	`15`	`datasourceID := int64(-1)`
`@@ -23,7 +21,7 @@ func TestAccDatasourcePermission_basic(t *testing.T) {`
`23`	`21`	`Config: testAccExample(t, "resources/grafana_data_source_permission/resource.tf"),`
`24`	`22`	`Check: resource.ComposeAggregateTestCheckFunc(`
`25`	`23`	`testAccDatasourcePermissionsCheckExists("grafana_data_source_permission.fooPermissions", &datasourceID),`
`26`		`- resource.TestCheckResourceAttr("grafana_data_source_permission.fooPermissions", "permissions.#", "2"),`
	`24`	`+ resource.TestCheckResourceAttr("grafana_data_source_permission.fooPermissions", "permissions.#", "3"),`
`27`	`25`	`),`
`28`	`26`	`},`
`29`	`27`	`{`
`@@ -72,9 +70,6 @@ func testAccDatasourcePermissionCheckDestroy(datasourceID *int64) resource.TestC`
`72`	`70`	`if err != nil {`
`73`	`71`	`return fmt.Errorf("error getting datasource permissions %d: %s", *datasourceID, err)`
`74`	`72`	`}`
`75`		`- if response.Enabled {`
`76`		`- return fmt.Errorf("datasource permissions %d still enabled", *datasourceID)`
`77`		`- }`
`78`	`73`	`if len(response.Permissions) > 0 {`
`79`	`74`	`return fmt.Errorf("permissions were not empty when expected")`
`80`	`75`	`}`