From e31e1b9910f1448fbf263e3977663bfd6066a55d Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Fri, 19 Jun 2020 18:34:47 +0200 Subject: [PATCH 01/42] Added databricks_permissions resource --- client/model/permissions.go | 75 +++++++ client/service/permissions.go | 50 +++++ client/service/users.go | 12 +- databricks/provider.go | 1 + databricks/resource_databricks_permissions.go | 204 ++++++++++++++++++ .../resource_databricks_permissions_test.go | 142 ++++++++++++ 6 files changed, 482 insertions(+), 2 deletions(-) create mode 100644 client/model/permissions.go create mode 100644 client/service/permissions.go create mode 100644 databricks/resource_databricks_permissions.go create mode 100644 databricks/resource_databricks_permissions_test.go diff --git a/client/model/permissions.go b/client/model/permissions.go new file mode 100644 index 000000000..b81df1d58 --- /dev/null +++ b/client/model/permissions.go @@ -0,0 +1,75 @@ +package model + +// ObjectACL ... +type ObjectACL struct { + ObjectID string `json:"object_id,omitempty"` + ObjectType string `json:"object_type,omitempty"` + AccessControlList []*AccessControl `json:"access_control_list"` +} + +// AccessControlChangeList is wrapper around ACL changes +type AccessControlChangeList struct { + AccessControlList []*AccessControlChange `json:"access_control_list"` +} + +// AccessControlChange is API wrapper for changing permissions +type AccessControlChange struct { + UserName *string `json:"user_name,omitempty"` + GroupName *string `json:"group_name,omitempty"` + ServicePrincipalName *string `json:"service_principal_name,omitempty"` + PermissionLevel string `json:"permission_level"` +} + +// AccessControl ... +type AccessControl struct { + UserName *string `json:"user_name,omitempty"` + GroupName *string `json:"group_name,omitempty"` + AllPermissions []*Permission `json:"all_permissions,omitempty"` +} + +// Permission ... +type Permission struct { + PermissionLevel string `json:"permission_level"` + Inherited bool `json:"inherited,omitempty"` + InheritedFromObject []string `json:"inherited_from_object,omitempty"` +} + +// ToAccessControlChangeList converts data formats +func (oa *ObjectACL) ToAccessControlChangeList() *AccessControlChangeList { + acl := new(AccessControlChangeList) + for _, accessControl := range oa.AccessControlList { + for _, permission := range accessControl.AllPermissions { + if permission.Inherited { + continue + } + item := new(AccessControlChange) + acl.AccessControlList = append(acl.AccessControlList, item) + item.PermissionLevel = permission.PermissionLevel + if accessControl.UserName != nil { + item.UserName = accessControl.UserName + } else if accessControl.GroupName != nil { + item.GroupName = accessControl.GroupName + } + } + } + return acl +} + +// AccessControl exports data for TF +func (acl *AccessControlChangeList) AccessControl(me string) []map[string]string { + result := []map[string]string{} + for _, control := range acl.AccessControlList { + item := map[string]string{} + if control.UserName != nil && *control.UserName != "" { + if me == *control.UserName { + continue + } + item["user_name"] = *control.UserName + } else if control.GroupName != nil && *control.GroupName != "" { + item["group_name"] = *control.GroupName + } + item["permission_level"] = control.PermissionLevel + result = append(result, item) + } + return result +} diff --git a/client/service/permissions.go b/client/service/permissions.go new file mode 100644 index 000000000..e7dc9ac8c --- /dev/null +++ b/client/service/permissions.go @@ -0,0 +1,50 @@ +package service + +import ( + "encoding/json" + "net/http" + + "github.com/databrickslabs/databricks-terraform/client/model" +) + +// PermissionsAPI ... +type PermissionsAPI struct { + Client *DBApiClient +} + +// AddOrModify ... +func (a PermissionsAPI) AddOrModify(objectID string, objectACL *model.AccessControlChangeList) error { + _, err := a.Client.performQuery(http.MethodPatch, + "/preview/permissions"+objectID, + "2.0", nil, objectACL, nil) + if err != nil { + return err + } + + return err +} + +// SetOrDelete ... +func (a PermissionsAPI) SetOrDelete(objectID string, objectACL *model.AccessControlChangeList) error { + _, err := a.Client.performQuery(http.MethodPut, + "/preview/permissions"+objectID, + "2.0", nil, objectACL, nil) + if err != nil { + return err + } + + return err +} + +// Read ... +func (a PermissionsAPI) Read(objectID string) (*model.ObjectACL, error) { + resp, err := a.Client.performQuery(http.MethodGet, + "/preview/permissions"+objectID, + "2.0", nil, nil, nil) + if err != nil { + return nil, err + } + var objectACL = new(model.ObjectACL) + err = json.Unmarshal(resp, &objectACL) + return objectACL, err +} diff --git a/client/service/users.go b/client/service/users.go index a97a20d57..35a741ff4 100644 --- a/client/service/users.go +++ b/client/service/users.go @@ -72,18 +72,26 @@ func (a UsersAPI) Read(userID string) (model.User, error) { } func (a UsersAPI) read(userID string) (model.User, error) { - var user model.User userPath := fmt.Sprintf("/preview/scim/v2/Users/%v", userID) + return a.readByPath(userPath) +} + +// Me gets user information about caller +func (a UsersAPI) Me() (model.User, error) { + return a.readByPath("/preview/scim/v2/Me") +} +func (a UsersAPI) readByPath(userPath string) (model.User, error) { + var user model.User resp, err := a.Client.performQuery(http.MethodGet, userPath, "2.0", scimHeaders, nil, nil) if err != nil { return user, err } - err = json.Unmarshal(resp, &user) return user, err } + // Update will update the user given the user id, username, display name, entitlements and roles func (a UsersAPI) Update(userID string, userName string, displayName string, entitlements []string, roles []string) error { userPath := fmt.Sprintf("/preview/scim/v2/Users/%v", userID) diff --git a/databricks/provider.go b/databricks/provider.go index 23b319871..e8a65033e 100644 --- a/databricks/provider.go +++ b/databricks/provider.go @@ -30,6 +30,7 @@ func Provider(version string) terraform.ResourceProvider { "databricks_secret_scope": resourceSecretScope(), "databricks_secret": resourceSecret(), "databricks_secret_acl": resourceSecretACL(), + "databricks_permissions": resourcePermissions(), "databricks_instance_pool": resourceInstancePool(), "databricks_scim_user": resourceScimUser(), "databricks_scim_group": resourceScimGroup(), diff --git a/databricks/resource_databricks_permissions.go b/databricks/resource_databricks_permissions.go new file mode 100644 index 000000000..f31e23a79 --- /dev/null +++ b/databricks/resource_databricks_permissions.go @@ -0,0 +1,204 @@ +package databricks + +import ( + "fmt" + "path" + "strconv" + + "github.com/databrickslabs/databricks-terraform/client/model" + "github.com/databrickslabs/databricks-terraform/client/service" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/pkg/errors" +) + +func parsePermissionsFromData(d *schema.ResourceData, + client *service.DBApiClient) (*model.AccessControlChangeList, string, error) { + var objectID string + acl := new(model.AccessControlChangeList) + for _, mapping := range permissionsResourceIDFields() { + v, ok := d.GetOk(mapping.field) + if !ok { + continue + } + id, err := mapping.idRetriever(client, v.(string)) + if err != nil { + return nil, "", err + } + objectID = fmt.Sprintf("/%s/%s", mapping.resourceType, id) + err = d.Set("object_type", mapping.objectType) + if err != nil { + return nil, "", err + } + } + if objectID == "" { + return nil, "", fmt.Errorf("At least one type of resource identifiers must be set") + } + if data, ok := d.GetOk("access_control"); ok { + for _, element := range data.([]interface{}) { + rawAccessControl := element.(map[string]interface{}) + change := new(model.AccessControlChange) + acl.AccessControlList = append(acl.AccessControlList, change) + if v, ok := rawAccessControl["group_name"].(string); ok && v != "" { + change.GroupName = &v + } + if v, ok := rawAccessControl["user_name"].(string); ok && v != "" { + change.UserName = &v + } + if v, ok := rawAccessControl["permission_level"].(string); ok { + change.PermissionLevel = v + } + } + } + return acl, objectID, nil +} + +func resourcePermissionsCreate(d *schema.ResourceData, m interface{}) error { + client := m.(*service.DBApiClient) + acl, objectID, err := parsePermissionsFromData(d, client) + if err != nil { + return err + } + err = client.Permissions().AddOrModify(objectID, acl) + if err != nil { + return err + } + d.SetId(objectID) + return resourcePermissionsRead(d, m) +} + +func resourcePermissionsRead(d *schema.ResourceData, m interface{}) error { + id := d.Id() + client := m.(*service.DBApiClient) + objectACL, err := client.Permissions().Read(id) + if err != nil { + return err + } + for _, mapping := range permissionsResourceIDFields() { + if mapping.objectType != d.Get("object_type").(string) { + continue + } + identifier := path.Base(id) + err := d.Set(mapping.field, identifier) + if err != nil { + return errors.Wrapf(err, + "Cannot set mapping field %s to %s", + mapping.field, id) + } + break + } + acl := objectACL.ToAccessControlChangeList() + me, err := client.Users().Me() + if err != nil { + return errors.Wrapf(err, "Cannot self-identify") + } + accessControl := acl.AccessControl(me.UserName) + err = d.Set("access_control", accessControl) + if err != nil { + return err + } + return nil +} + +func resourcePermissionsDelete(d *schema.ResourceData, m interface{}) error { + id := d.Id() + client := m.(*service.DBApiClient) + err := client.Permissions().SetOrDelete(id, new(model.AccessControlChangeList)) + if err != nil { + return err + } + return nil +} + +// permissionsIDFieldMapping holds mapping +type permissionsIDFieldMapping struct { + field string + objectType string + resourceType string + idRetriever func(client *service.DBApiClient, id string) (string, error) +} + +// PermissionsResourceIDFields shows mapping of id columns to resource types +func permissionsResourceIDFields() []permissionsIDFieldMapping { + SIMPLE := func(client *service.DBApiClient, id string) (string, error) { + return id, nil + } + PATH := func(client *service.DBApiClient, path string) (string, error) { + info, err := client.Notebooks().Read(path) + if err != nil { + return "", errors.Wrapf(err, "Cannot load path %s", path) + } + return strconv.FormatInt(info.ObjectID, 10), nil + } + return []permissionsIDFieldMapping{ + {"cluster_policy_id", "cluster-policy", "cluster-policies", SIMPLE}, + {"instance_pool_id", "instance-pool", "instance-pools", SIMPLE}, + {"cluster_id", "cluster", "clusters", SIMPLE}, + {"job_id", "job", "jobs", SIMPLE}, + {"notebook_id", "notebook", "notebooks", SIMPLE}, + {"notebook_path", "notebook", "notebooks", PATH}, + {"directory_id", "directory", "directories", SIMPLE}, + {"directory_path", "directory", "directories", PATH}, + } +} + +func conflictingFields(field string) []string { + conflicting := []string{} + for _, mapping := range permissionsResourceIDFields() { + if mapping.field == field { + continue + } + conflicting = append(conflicting, mapping.field) + } + return conflicting +} + +func resourcePermissions() *schema.Resource { + fields := map[string]*schema.Schema{ + "object_type": { + Type: schema.TypeString, + Computed: true, + }, + "access_control": { + ForceNew: true, + Type: schema.TypeList, + MinItems: 1, + Required: true, + ConfigMode: schema.SchemaConfigModeAttr, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "user_name": { + ForceNew: true, + Type: schema.TypeString, + Optional: true, + //ConflictsWith: []string{"group_name"}, + }, + "group_name": { + ForceNew: true, + Type: schema.TypeString, + Optional: true, + //ConflictsWith: []string{"user_name"}, + }, + "permission_level": { + ForceNew: true, + Type: schema.TypeString, + Required: true, + }, + }, + }, + }, + } + for _, mapping := range permissionsResourceIDFields() { + fields[mapping.field] = &schema.Schema{ + ForceNew: true, + Type: schema.TypeString, + Optional: true, + ConflictsWith: conflictingFields(mapping.field), + } + } + return &schema.Resource{ + Create: resourcePermissionsCreate, + Read: resourcePermissionsRead, + Delete: resourcePermissionsDelete, + Schema: fields, + } +} diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go new file mode 100644 index 000000000..b7b3be005 --- /dev/null +++ b/databricks/resource_databricks_permissions_test.go @@ -0,0 +1,142 @@ +package databricks + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/stretchr/testify/assert" + + "github.com/databrickslabs/databricks-terraform/client/model" + "github.com/databrickslabs/databricks-terraform/client/service" +) + +func TestAccDatabricksPermissionsResourceFullLifecycle(t *testing.T) { + var permissions model.ObjectACL + randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + resource.Test(t, resource.TestCase{ + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + // create a resource + Config: testClusterPolicyPermissions(randomName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("databricks_permissions.dummy_can_use", + "object_type", "cluster-policy"), + testAccIDCallback(t, "databricks_permissions.dummy_can_use", + func(client *service.DBApiClient, id string) error { + resp, err := client.Permissions().Read(id) + if err != nil { + return err + } + permissions = *resp + assert.Len(t, permissions.AccessControlList, 3) + return nil + }), + ), + }, + { + Config: testClusterPolicyPermissionsSecondGroupAdded(randomName), + Check: testAccIDCallback(t, "databricks_permissions.dummy_can_use", + func(client *service.DBApiClient, id string) error { + resp, err := client.Permissions().Read(id) + if err != nil { + return err + } + permissions = *resp + assert.Len(t, permissions.AccessControlList, 3) + return nil + }), + }, + }, + }) +} + +func testClusterPolicyPermissions(name string) string { + return fmt.Sprintf(` + resource "databricks_cluster_policy" "something_simple" { + name = "Terraform Policy %[1]s" + definition = jsonencode({ + "spark_conf.spark.hadoop.javax.jdo.option.ConnectionURL": { + "type": "forbidden" + } + }) + } + resource "databricks_scim_group" "dummy_group" { + display_name = "Terraform Group %[1]s" + } + resource "databricks_permissions" "dummy_can_use" { + cluster_policy_id = databricks_cluster_policy.something_simple.id + access_control { + group_name = databricks_scim_group.dummy_group.display_name + permission_level = "CAN_USE" + } + } + `, name) +} + +func testClusterPolicyPermissionsSecondGroupAdded(name string) string { + return fmt.Sprintf(` + resource "databricks_cluster_policy" "something_simple" { + name = "Terraform Policy %[1]s" + definition = jsonencode({ + "spark_conf.spark.hadoop.javax.jdo.option.ConnectionURL": { + "type": "forbidden" + }, + "spark_conf.spark.secondkey": { + "type": "forbidden" + } + }) + } + resource "databricks_scim_group" "dummy_group" { + display_name = "Terraform Group %[1]s" + } + resource "databricks_scim_group" "second_group" { + display_name = "Terraform Second Group %[1]s" + } + resource "databricks_permissions" "dummy_can_use" { + cluster_policy_id = databricks_cluster_policy.something_simple.id + access_control { + group_name = databricks_scim_group.dummy_group.display_name + permission_level = "CAN_USE" + } + access_control { + group_name = databricks_scim_group.second_group.display_name + permission_level = "CAN_USE" + } + } + `, name) +} + +func TestAccNotebookPermissions(t *testing.T) { + randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + resource.Test(t, resource.TestCase{ + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + // create a resource + Config: fmt.Sprintf(` + resource "databricks_notebook" "dummy" { + content = base64encode("# Databricks notebook source\nprint(1)") + path = "/Beginning/Init" + overwrite = true + mkdirs = true + language = "PYTHON" + format = "SOURCE" + } + resource "databricks_scim_group" "dummy_group" { + display_name = "Terraform Group %[1]s" + } + resource "databricks_permissions" "dummy_can_use" { + directory_path = "/Beginning" + access_control { + group_name = databricks_scim_group.dummy_group.display_name + permission_level = "CAN_MANAGE" + } + } + `, randomName), + }, + }, + }) +} \ No newline at end of file From e38a53fbba98ada4002cf3db05eca00d05a0a5a1 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Mon, 22 Jun 2020 21:32:45 +0200 Subject: [PATCH 02/42] Minor commenting tweaks --- client/model/permissions.go | 32 +++++++++---------- client/service/permissions.go | 8 ++--- client/service/users.go | 1 - .../resource_databricks_permissions_test.go | 2 +- go.mod | 1 + 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/client/model/permissions.go b/client/model/permissions.go index b81df1d58..d9c7007ee 100644 --- a/client/model/permissions.go +++ b/client/model/permissions.go @@ -1,39 +1,39 @@ package model -// ObjectACL ... +// ObjectACL is a structure to generically describe access control type ObjectACL struct { ObjectID string `json:"object_id,omitempty"` ObjectType string `json:"object_type,omitempty"` AccessControlList []*AccessControl `json:"access_control_list"` } -// AccessControlChangeList is wrapper around ACL changes -type AccessControlChangeList struct { - AccessControlList []*AccessControlChange `json:"access_control_list"` -} - -// AccessControlChange is API wrapper for changing permissions -type AccessControlChange struct { - UserName *string `json:"user_name,omitempty"` - GroupName *string `json:"group_name,omitempty"` - ServicePrincipalName *string `json:"service_principal_name,omitempty"` - PermissionLevel string `json:"permission_level"` -} - -// AccessControl ... +// AccessControl is a structure to describe user/group permissions type AccessControl struct { UserName *string `json:"user_name,omitempty"` GroupName *string `json:"group_name,omitempty"` AllPermissions []*Permission `json:"all_permissions,omitempty"` } -// Permission ... +// Permission is a structure to describe permission level type Permission struct { PermissionLevel string `json:"permission_level"` Inherited bool `json:"inherited,omitempty"` InheritedFromObject []string `json:"inherited_from_object,omitempty"` } +// AccessControlChangeList is wrapper around ACL changes for REST API +type AccessControlChangeList struct { + AccessControlList []*AccessControlChange `json:"access_control_list"` +} + +// AccessControlChange is API wrapper for changing permissions +type AccessControlChange struct { + UserName *string `json:"user_name,omitempty"` + GroupName *string `json:"group_name,omitempty"` + ServicePrincipalName *string `json:"service_principal_name,omitempty"` + PermissionLevel string `json:"permission_level"` +} + // ToAccessControlChangeList converts data formats func (oa *ObjectACL) ToAccessControlChangeList() *AccessControlChangeList { acl := new(AccessControlChangeList) diff --git a/client/service/permissions.go b/client/service/permissions.go index e7dc9ac8c..e2e0bd5a9 100644 --- a/client/service/permissions.go +++ b/client/service/permissions.go @@ -7,12 +7,12 @@ import ( "github.com/databrickslabs/databricks-terraform/client/model" ) -// PermissionsAPI ... +// PermissionsAPI exposes general permission related methods type PermissionsAPI struct { Client *DBApiClient } -// AddOrModify ... +// AddOrModify works with permissions change list func (a PermissionsAPI) AddOrModify(objectID string, objectACL *model.AccessControlChangeList) error { _, err := a.Client.performQuery(http.MethodPatch, "/preview/permissions"+objectID, @@ -24,7 +24,7 @@ func (a PermissionsAPI) AddOrModify(objectID string, objectACL *model.AccessCont return err } -// SetOrDelete ... +// SetOrDelete updates object permissions func (a PermissionsAPI) SetOrDelete(objectID string, objectACL *model.AccessControlChangeList) error { _, err := a.Client.performQuery(http.MethodPut, "/preview/permissions"+objectID, @@ -36,7 +36,7 @@ func (a PermissionsAPI) SetOrDelete(objectID string, objectACL *model.AccessCont return err } -// Read ... +// Read gets all relevant permissions for the object, including inherited ones func (a PermissionsAPI) Read(objectID string) (*model.ObjectACL, error) { resp, err := a.Client.performQuery(http.MethodGet, "/preview/permissions"+objectID, diff --git a/client/service/users.go b/client/service/users.go index 35a741ff4..6df7050e7 100644 --- a/client/service/users.go +++ b/client/service/users.go @@ -91,7 +91,6 @@ func (a UsersAPI) readByPath(userPath string) (model.User, error) { return user, err } - // Update will update the user given the user id, username, display name, entitlements and roles func (a UsersAPI) Update(userID string, userName string, displayName string, entitlements []string, roles []string) error { userPath := fmt.Sprintf("/preview/scim/v2/Users/%v", userID) diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index b7b3be005..5591cb19b 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -139,4 +139,4 @@ func TestAccNotebookPermissions(t *testing.T) { }, }, }) -} \ No newline at end of file +} diff --git a/go.mod b/go.mod index 4e6be37bb..ea6f508b0 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/joho/godotenv v1.3.0 github.com/mattn/go-colorable v0.1.6 // indirect github.com/mitchellh/go-homedir v1.1.0 + github.com/pkg/errors v0.9.1 github.com/r3labs/diff v0.0.0-20191120142937-b4ed99a31f5a github.com/sergi/go-diff v1.1.0 // indirect github.com/smartystreets/goconvey v1.6.4 // indirect From 732aeda54d388b9bb315d1749be6ea55531d2080 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Mon, 22 Jun 2020 21:43:02 +0200 Subject: [PATCH 03/42] Register permissions api in the client --- client/service/apis.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/client/service/apis.go b/client/service/apis.go index b1704cd9e..8d450685b 100644 --- a/client/service/apis.go +++ b/client/service/apis.go @@ -116,6 +116,11 @@ func (c *DBApiClient) MWSCustomerManagedKeys() MWSCustomerManagedKeysAPI { return MWSCustomerManagedKeysAPI{Client: c} } +// Permissions returns an instance of CommandsAPI +func (c *DBApiClient) Permissions() PermissionsAPI { + return PermissionsAPI{Client: c} +} + func (c *DBApiClient) performQuery(method, path string, apiVersion string, headers map[string]string, data interface{}, secretsMask *SecretsMask) ([]byte, error) { err := c.Config.getOrCreateToken() if err != nil { From cc6727a379916da3564ce89bad0552651ab1f3f6 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Mon, 22 Jun 2020 21:55:30 +0200 Subject: [PATCH 04/42] Add pkg/errors --- vendor/github.com/pkg/errors/.gitignore | 24 ++ vendor/github.com/pkg/errors/.travis.yml | 10 + vendor/github.com/pkg/errors/LICENSE | 23 ++ vendor/github.com/pkg/errors/Makefile | 44 ++++ vendor/github.com/pkg/errors/README.md | 59 +++++ vendor/github.com/pkg/errors/appveyor.yml | 32 +++ vendor/github.com/pkg/errors/errors.go | 288 ++++++++++++++++++++++ vendor/github.com/pkg/errors/go113.go | 38 +++ vendor/github.com/pkg/errors/stack.go | 177 +++++++++++++ vendor/modules.txt | 2 + 10 files changed, 697 insertions(+) create mode 100644 vendor/github.com/pkg/errors/.gitignore create mode 100644 vendor/github.com/pkg/errors/.travis.yml create mode 100644 vendor/github.com/pkg/errors/LICENSE create mode 100644 vendor/github.com/pkg/errors/Makefile create mode 100644 vendor/github.com/pkg/errors/README.md create mode 100644 vendor/github.com/pkg/errors/appveyor.yml create mode 100644 vendor/github.com/pkg/errors/errors.go create mode 100644 vendor/github.com/pkg/errors/go113.go create mode 100644 vendor/github.com/pkg/errors/stack.go diff --git a/vendor/github.com/pkg/errors/.gitignore b/vendor/github.com/pkg/errors/.gitignore new file mode 100644 index 000000000..daf913b1b --- /dev/null +++ b/vendor/github.com/pkg/errors/.gitignore @@ -0,0 +1,24 @@ +# Compiled Object files, Static and Dynamic libs (Shared Objects) +*.o +*.a +*.so + +# Folders +_obj +_test + +# Architecture specific extensions/prefixes +*.[568vq] +[568vq].out + +*.cgo1.go +*.cgo2.c +_cgo_defun.c +_cgo_gotypes.go +_cgo_export.* + +_testmain.go + +*.exe +*.test +*.prof diff --git a/vendor/github.com/pkg/errors/.travis.yml b/vendor/github.com/pkg/errors/.travis.yml new file mode 100644 index 000000000..9159de03e --- /dev/null +++ b/vendor/github.com/pkg/errors/.travis.yml @@ -0,0 +1,10 @@ +language: go +go_import_path: github.com/pkg/errors +go: + - 1.11.x + - 1.12.x + - 1.13.x + - tip + +script: + - make check diff --git a/vendor/github.com/pkg/errors/LICENSE b/vendor/github.com/pkg/errors/LICENSE new file mode 100644 index 000000000..835ba3e75 --- /dev/null +++ b/vendor/github.com/pkg/errors/LICENSE @@ -0,0 +1,23 @@ +Copyright (c) 2015, Dave Cheney +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + +* Redistributions of source code must retain the above copyright notice, this + list of conditions and the following disclaimer. + +* Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/vendor/github.com/pkg/errors/Makefile b/vendor/github.com/pkg/errors/Makefile new file mode 100644 index 000000000..ce9d7cded --- /dev/null +++ b/vendor/github.com/pkg/errors/Makefile @@ -0,0 +1,44 @@ +PKGS := github.com/pkg/errors +SRCDIRS := $(shell go list -f '{{.Dir}}' $(PKGS)) +GO := go + +check: test vet gofmt misspell unconvert staticcheck ineffassign unparam + +test: + $(GO) test $(PKGS) + +vet: | test + $(GO) vet $(PKGS) + +staticcheck: + $(GO) get honnef.co/go/tools/cmd/staticcheck + staticcheck -checks all $(PKGS) + +misspell: + $(GO) get github.com/client9/misspell/cmd/misspell + misspell \ + -locale GB \ + -error \ + *.md *.go + +unconvert: + $(GO) get github.com/mdempsky/unconvert + unconvert -v $(PKGS) + +ineffassign: + $(GO) get github.com/gordonklaus/ineffassign + find $(SRCDIRS) -name '*.go' | xargs ineffassign + +pedantic: check errcheck + +unparam: + $(GO) get mvdan.cc/unparam + unparam ./... + +errcheck: + $(GO) get github.com/kisielk/errcheck + errcheck $(PKGS) + +gofmt: + @echo Checking code is gofmted + @test -z "$(shell gofmt -s -l -d -e $(SRCDIRS) | tee /dev/stderr)" diff --git a/vendor/github.com/pkg/errors/README.md b/vendor/github.com/pkg/errors/README.md new file mode 100644 index 000000000..54dfdcb12 --- /dev/null +++ b/vendor/github.com/pkg/errors/README.md @@ -0,0 +1,59 @@ +# errors [![Travis-CI](https://travis-ci.org/pkg/errors.svg)](https://travis-ci.org/pkg/errors) [![AppVeyor](https://ci.appveyor.com/api/projects/status/b98mptawhudj53ep/branch/master?svg=true)](https://ci.appveyor.com/project/davecheney/errors/branch/master) [![GoDoc](https://godoc.org/github.com/pkg/errors?status.svg)](http://godoc.org/github.com/pkg/errors) [![Report card](https://goreportcard.com/badge/github.com/pkg/errors)](https://goreportcard.com/report/github.com/pkg/errors) [![Sourcegraph](https://sourcegraph.com/github.com/pkg/errors/-/badge.svg)](https://sourcegraph.com/github.com/pkg/errors?badge) + +Package errors provides simple error handling primitives. + +`go get github.com/pkg/errors` + +The traditional error handling idiom in Go is roughly akin to +```go +if err != nil { + return err +} +``` +which applied recursively up the call stack results in error reports without context or debugging information. The errors package allows programmers to add context to the failure path in their code in a way that does not destroy the original value of the error. + +## Adding context to an error + +The errors.Wrap function returns a new error that adds context to the original error. For example +```go +_, err := ioutil.ReadAll(r) +if err != nil { + return errors.Wrap(err, "read failed") +} +``` +## Retrieving the cause of an error + +Using `errors.Wrap` constructs a stack of errors, adding context to the preceding error. Depending on the nature of the error it may be necessary to reverse the operation of errors.Wrap to retrieve the original error for inspection. Any error value which implements this interface can be inspected by `errors.Cause`. +```go +type causer interface { + Cause() error +} +``` +`errors.Cause` will recursively retrieve the topmost error which does not implement `causer`, which is assumed to be the original cause. For example: +```go +switch err := errors.Cause(err).(type) { +case *MyError: + // handle specifically +default: + // unknown error +} +``` + +[Read the package documentation for more information](https://godoc.org/github.com/pkg/errors). + +## Roadmap + +With the upcoming [Go2 error proposals](https://go.googlesource.com/proposal/+/master/design/go2draft.md) this package is moving into maintenance mode. The roadmap for a 1.0 release is as follows: + +- 0.9. Remove pre Go 1.9 and Go 1.10 support, address outstanding pull requests (if possible) +- 1.0. Final release. + +## Contributing + +Because of the Go2 errors changes, this package is not accepting proposals for new functionality. With that said, we welcome pull requests, bug fixes and issue reports. + +Before sending a PR, please discuss your change by raising an issue. + +## License + +BSD-2-Clause diff --git a/vendor/github.com/pkg/errors/appveyor.yml b/vendor/github.com/pkg/errors/appveyor.yml new file mode 100644 index 000000000..a932eade0 --- /dev/null +++ b/vendor/github.com/pkg/errors/appveyor.yml @@ -0,0 +1,32 @@ +version: build-{build}.{branch} + +clone_folder: C:\gopath\src\github.com\pkg\errors +shallow_clone: true # for startup speed + +environment: + GOPATH: C:\gopath + +platform: + - x64 + +# http://www.appveyor.com/docs/installed-software +install: + # some helpful output for debugging builds + - go version + - go env + # pre-installed MinGW at C:\MinGW is 32bit only + # but MSYS2 at C:\msys64 has mingw64 + - set PATH=C:\msys64\mingw64\bin;%PATH% + - gcc --version + - g++ --version + +build_script: + - go install -v ./... + +test_script: + - set PATH=C:\gopath\bin;%PATH% + - go test -v ./... + +#artifacts: +# - path: '%GOPATH%\bin\*.exe' +deploy: off diff --git a/vendor/github.com/pkg/errors/errors.go b/vendor/github.com/pkg/errors/errors.go new file mode 100644 index 000000000..161aea258 --- /dev/null +++ b/vendor/github.com/pkg/errors/errors.go @@ -0,0 +1,288 @@ +// Package errors provides simple error handling primitives. +// +// The traditional error handling idiom in Go is roughly akin to +// +// if err != nil { +// return err +// } +// +// which when applied recursively up the call stack results in error reports +// without context or debugging information. The errors package allows +// programmers to add context to the failure path in their code in a way +// that does not destroy the original value of the error. +// +// Adding context to an error +// +// The errors.Wrap function returns a new error that adds context to the +// original error by recording a stack trace at the point Wrap is called, +// together with the supplied message. For example +// +// _, err := ioutil.ReadAll(r) +// if err != nil { +// return errors.Wrap(err, "read failed") +// } +// +// If additional control is required, the errors.WithStack and +// errors.WithMessage functions destructure errors.Wrap into its component +// operations: annotating an error with a stack trace and with a message, +// respectively. +// +// Retrieving the cause of an error +// +// Using errors.Wrap constructs a stack of errors, adding context to the +// preceding error. Depending on the nature of the error it may be necessary +// to reverse the operation of errors.Wrap to retrieve the original error +// for inspection. Any error value which implements this interface +// +// type causer interface { +// Cause() error +// } +// +// can be inspected by errors.Cause. errors.Cause will recursively retrieve +// the topmost error that does not implement causer, which is assumed to be +// the original cause. For example: +// +// switch err := errors.Cause(err).(type) { +// case *MyError: +// // handle specifically +// default: +// // unknown error +// } +// +// Although the causer interface is not exported by this package, it is +// considered a part of its stable public interface. +// +// Formatted printing of errors +// +// All error values returned from this package implement fmt.Formatter and can +// be formatted by the fmt package. The following verbs are supported: +// +// %s print the error. If the error has a Cause it will be +// printed recursively. +// %v see %s +// %+v extended format. Each Frame of the error's StackTrace will +// be printed in detail. +// +// Retrieving the stack trace of an error or wrapper +// +// New, Errorf, Wrap, and Wrapf record a stack trace at the point they are +// invoked. This information can be retrieved with the following interface: +// +// type stackTracer interface { +// StackTrace() errors.StackTrace +// } +// +// The returned errors.StackTrace type is defined as +// +// type StackTrace []Frame +// +// The Frame type represents a call site in the stack trace. Frame supports +// the fmt.Formatter interface that can be used for printing information about +// the stack trace of this error. For example: +// +// if err, ok := err.(stackTracer); ok { +// for _, f := range err.StackTrace() { +// fmt.Printf("%+s:%d\n", f, f) +// } +// } +// +// Although the stackTracer interface is not exported by this package, it is +// considered a part of its stable public interface. +// +// See the documentation for Frame.Format for more details. +package errors + +import ( + "fmt" + "io" +) + +// New returns an error with the supplied message. +// New also records the stack trace at the point it was called. +func New(message string) error { + return &fundamental{ + msg: message, + stack: callers(), + } +} + +// Errorf formats according to a format specifier and returns the string +// as a value that satisfies error. +// Errorf also records the stack trace at the point it was called. +func Errorf(format string, args ...interface{}) error { + return &fundamental{ + msg: fmt.Sprintf(format, args...), + stack: callers(), + } +} + +// fundamental is an error that has a message and a stack, but no caller. +type fundamental struct { + msg string + *stack +} + +func (f *fundamental) Error() string { return f.msg } + +func (f *fundamental) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + if s.Flag('+') { + io.WriteString(s, f.msg) + f.stack.Format(s, verb) + return + } + fallthrough + case 's': + io.WriteString(s, f.msg) + case 'q': + fmt.Fprintf(s, "%q", f.msg) + } +} + +// WithStack annotates err with a stack trace at the point WithStack was called. +// If err is nil, WithStack returns nil. +func WithStack(err error) error { + if err == nil { + return nil + } + return &withStack{ + err, + callers(), + } +} + +type withStack struct { + error + *stack +} + +func (w *withStack) Cause() error { return w.error } + +// Unwrap provides compatibility for Go 1.13 error chains. +func (w *withStack) Unwrap() error { return w.error } + +func (w *withStack) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + if s.Flag('+') { + fmt.Fprintf(s, "%+v", w.Cause()) + w.stack.Format(s, verb) + return + } + fallthrough + case 's': + io.WriteString(s, w.Error()) + case 'q': + fmt.Fprintf(s, "%q", w.Error()) + } +} + +// Wrap returns an error annotating err with a stack trace +// at the point Wrap is called, and the supplied message. +// If err is nil, Wrap returns nil. +func Wrap(err error, message string) error { + if err == nil { + return nil + } + err = &withMessage{ + cause: err, + msg: message, + } + return &withStack{ + err, + callers(), + } +} + +// Wrapf returns an error annotating err with a stack trace +// at the point Wrapf is called, and the format specifier. +// If err is nil, Wrapf returns nil. +func Wrapf(err error, format string, args ...interface{}) error { + if err == nil { + return nil + } + err = &withMessage{ + cause: err, + msg: fmt.Sprintf(format, args...), + } + return &withStack{ + err, + callers(), + } +} + +// WithMessage annotates err with a new message. +// If err is nil, WithMessage returns nil. +func WithMessage(err error, message string) error { + if err == nil { + return nil + } + return &withMessage{ + cause: err, + msg: message, + } +} + +// WithMessagef annotates err with the format specifier. +// If err is nil, WithMessagef returns nil. +func WithMessagef(err error, format string, args ...interface{}) error { + if err == nil { + return nil + } + return &withMessage{ + cause: err, + msg: fmt.Sprintf(format, args...), + } +} + +type withMessage struct { + cause error + msg string +} + +func (w *withMessage) Error() string { return w.msg + ": " + w.cause.Error() } +func (w *withMessage) Cause() error { return w.cause } + +// Unwrap provides compatibility for Go 1.13 error chains. +func (w *withMessage) Unwrap() error { return w.cause } + +func (w *withMessage) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + if s.Flag('+') { + fmt.Fprintf(s, "%+v\n", w.Cause()) + io.WriteString(s, w.msg) + return + } + fallthrough + case 's', 'q': + io.WriteString(s, w.Error()) + } +} + +// Cause returns the underlying cause of the error, if possible. +// An error value has a cause if it implements the following +// interface: +// +// type causer interface { +// Cause() error +// } +// +// If the error does not implement Cause, the original error will +// be returned. If the error is nil, nil will be returned without further +// investigation. +func Cause(err error) error { + type causer interface { + Cause() error + } + + for err != nil { + cause, ok := err.(causer) + if !ok { + break + } + err = cause.Cause() + } + return err +} diff --git a/vendor/github.com/pkg/errors/go113.go b/vendor/github.com/pkg/errors/go113.go new file mode 100644 index 000000000..be0d10d0c --- /dev/null +++ b/vendor/github.com/pkg/errors/go113.go @@ -0,0 +1,38 @@ +// +build go1.13 + +package errors + +import ( + stderrors "errors" +) + +// Is reports whether any error in err's chain matches target. +// +// The chain consists of err itself followed by the sequence of errors obtained by +// repeatedly calling Unwrap. +// +// An error is considered to match a target if it is equal to that target or if +// it implements a method Is(error) bool such that Is(target) returns true. +func Is(err, target error) bool { return stderrors.Is(err, target) } + +// As finds the first error in err's chain that matches target, and if so, sets +// target to that error value and returns true. +// +// The chain consists of err itself followed by the sequence of errors obtained by +// repeatedly calling Unwrap. +// +// An error matches target if the error's concrete value is assignable to the value +// pointed to by target, or if the error has a method As(interface{}) bool such that +// As(target) returns true. In the latter case, the As method is responsible for +// setting target. +// +// As will panic if target is not a non-nil pointer to either a type that implements +// error, or to any interface type. As returns false if err is nil. +func As(err error, target interface{}) bool { return stderrors.As(err, target) } + +// Unwrap returns the result of calling the Unwrap method on err, if err's +// type contains an Unwrap method returning error. +// Otherwise, Unwrap returns nil. +func Unwrap(err error) error { + return stderrors.Unwrap(err) +} diff --git a/vendor/github.com/pkg/errors/stack.go b/vendor/github.com/pkg/errors/stack.go new file mode 100644 index 000000000..779a8348f --- /dev/null +++ b/vendor/github.com/pkg/errors/stack.go @@ -0,0 +1,177 @@ +package errors + +import ( + "fmt" + "io" + "path" + "runtime" + "strconv" + "strings" +) + +// Frame represents a program counter inside a stack frame. +// For historical reasons if Frame is interpreted as a uintptr +// its value represents the program counter + 1. +type Frame uintptr + +// pc returns the program counter for this frame; +// multiple frames may have the same PC value. +func (f Frame) pc() uintptr { return uintptr(f) - 1 } + +// file returns the full path to the file that contains the +// function for this Frame's pc. +func (f Frame) file() string { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return "unknown" + } + file, _ := fn.FileLine(f.pc()) + return file +} + +// line returns the line number of source code of the +// function for this Frame's pc. +func (f Frame) line() int { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return 0 + } + _, line := fn.FileLine(f.pc()) + return line +} + +// name returns the name of this function, if known. +func (f Frame) name() string { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return "unknown" + } + return fn.Name() +} + +// Format formats the frame according to the fmt.Formatter interface. +// +// %s source file +// %d source line +// %n function name +// %v equivalent to %s:%d +// +// Format accepts flags that alter the printing of some verbs, as follows: +// +// %+s function name and path of source file relative to the compile time +// GOPATH separated by \n\t (\n\t) +// %+v equivalent to %+s:%d +func (f Frame) Format(s fmt.State, verb rune) { + switch verb { + case 's': + switch { + case s.Flag('+'): + io.WriteString(s, f.name()) + io.WriteString(s, "\n\t") + io.WriteString(s, f.file()) + default: + io.WriteString(s, path.Base(f.file())) + } + case 'd': + io.WriteString(s, strconv.Itoa(f.line())) + case 'n': + io.WriteString(s, funcname(f.name())) + case 'v': + f.Format(s, 's') + io.WriteString(s, ":") + f.Format(s, 'd') + } +} + +// MarshalText formats a stacktrace Frame as a text string. The output is the +// same as that of fmt.Sprintf("%+v", f), but without newlines or tabs. +func (f Frame) MarshalText() ([]byte, error) { + name := f.name() + if name == "unknown" { + return []byte(name), nil + } + return []byte(fmt.Sprintf("%s %s:%d", name, f.file(), f.line())), nil +} + +// StackTrace is stack of Frames from innermost (newest) to outermost (oldest). +type StackTrace []Frame + +// Format formats the stack of Frames according to the fmt.Formatter interface. +// +// %s lists source files for each Frame in the stack +// %v lists the source file and line number for each Frame in the stack +// +// Format accepts flags that alter the printing of some verbs, as follows: +// +// %+v Prints filename, function, and line number for each Frame in the stack. +func (st StackTrace) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + switch { + case s.Flag('+'): + for _, f := range st { + io.WriteString(s, "\n") + f.Format(s, verb) + } + case s.Flag('#'): + fmt.Fprintf(s, "%#v", []Frame(st)) + default: + st.formatSlice(s, verb) + } + case 's': + st.formatSlice(s, verb) + } +} + +// formatSlice will format this StackTrace into the given buffer as a slice of +// Frame, only valid when called with '%s' or '%v'. +func (st StackTrace) formatSlice(s fmt.State, verb rune) { + io.WriteString(s, "[") + for i, f := range st { + if i > 0 { + io.WriteString(s, " ") + } + f.Format(s, verb) + } + io.WriteString(s, "]") +} + +// stack represents a stack of program counters. +type stack []uintptr + +func (s *stack) Format(st fmt.State, verb rune) { + switch verb { + case 'v': + switch { + case st.Flag('+'): + for _, pc := range *s { + f := Frame(pc) + fmt.Fprintf(st, "\n%+v", f) + } + } + } +} + +func (s *stack) StackTrace() StackTrace { + f := make([]Frame, len(*s)) + for i := 0; i < len(f); i++ { + f[i] = Frame((*s)[i]) + } + return f +} + +func callers() *stack { + const depth = 32 + var pcs [depth]uintptr + n := runtime.Callers(3, pcs[:]) + var st stack = pcs[0:n] + return &st +} + +// funcname removes the path prefix component of a function's name reported by func.Name(). +func funcname(name string) string { + i := strings.LastIndex(name, "/") + name = name[i+1:] + i = strings.Index(name, ".") + return name[i+1:] +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 42e5460cb..8f6047483 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -229,6 +229,8 @@ github.com/mitchellh/mapstructure github.com/mitchellh/reflectwalk # github.com/oklog/run v1.0.0 github.com/oklog/run +# github.com/pkg/errors v0.9.1 +github.com/pkg/errors # github.com/pmezard/go-difflib v1.0.0 github.com/pmezard/go-difflib/difflib # github.com/posener/complete v1.2.1 From 78f8b72ed837ca753f86131c43185d9e97af5c54 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Wed, 24 Jun 2020 00:07:01 +0200 Subject: [PATCH 05/42] Added real unit tests for resource --- CONTRIBUTING.md | 61 ++++++- databricks/resource_databricks_permissions.go | 5 + .../resource_databricks_permissions_test.go | 150 ++++++++++++++++++ databricks/utils_test.go | 71 +++++++++ 4 files changed, 286 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7426f2eaf..b843bdce1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1 +1,60 @@ -We happily welcome contributions to databricks-terraform. We use GitHub Issues to track community reported issues and GitHub Pull Requests for accepting changes. \ No newline at end of file +We happily welcome contributions to databricks-terraform. We use GitHub Issues to track community reported issues and GitHub Pull Requests for accepting changes. + +## Unit testing resources + +In order to unit test a resource, which runs fast and could be included in code coverage, one should use `ResourceTester`, that launches embedded HTTP server with `HTTPFixture`'s containing all calls that should have been made in given scenario: + +```go +func TestPermissionsCreate(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPatch, + // requires full URI + Resource: "/api/2.0/preview/permissions/clusters/abc", + // works with entities, not JSON. Diff is displayed in case of missmatch + ExpectedRequest: model.AccessControlChangeList{ + AccessControlList: []*model.AccessControlChange{ + { + UserName: &TestingUser, + PermissionLevel: "CAN_USE", + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: model.AccessControlChangeList{ + AccessControlList: []*model.AccessControlChange{ + { + UserName: &TestingUser, + PermissionLevel: "CAN_MANAGE", + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: model.User{ + UserName: "chuck.norris", + }, + }, + }, + // next argument is function, that creates resource (to make schema for ResourceData) + resourcePermissions, + // state represented as native structure (though a bit clunky) + map[string]interface{}{ + "cluster_id": "abc", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_USE", + }, + }, + }, + // the last argument is a function, that performs a stage on resource (Create/update/delete/read) + resourcePermissionsCreate) + assert.NoError(t, err, err) +} +``` \ No newline at end of file diff --git a/databricks/resource_databricks_permissions.go b/databricks/resource_databricks_permissions.go index f31e23a79..914629d96 100644 --- a/databricks/resource_databricks_permissions.go +++ b/databricks/resource_databricks_permissions.go @@ -33,6 +33,7 @@ func parsePermissionsFromData(d *schema.ResourceData, if objectID == "" { return nil, "", fmt.Errorf("At least one type of resource identifiers must be set") } + changes := 0 if data, ok := d.GetOk("access_control"); ok { for _, element := range data.([]interface{}) { rawAccessControl := element.(map[string]interface{}) @@ -47,8 +48,12 @@ func parsePermissionsFromData(d *schema.ResourceData, if v, ok := rawAccessControl["permission_level"].(string); ok { change.PermissionLevel = v } + changes++ } } + if changes < 1 { + return nil, "", fmt.Errorf("At least one access_control is required") + } return acl, objectID, nil } diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index 5591cb19b..c50f06246 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -2,16 +2,166 @@ package databricks import ( "fmt" + "net/http" "testing" "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/stretchr/testify/assert" "github.com/databrickslabs/databricks-terraform/client/model" "github.com/databrickslabs/databricks-terraform/client/service" ) +var ( + TestingUser = "ben" + TestingAdminUser = "admin" +) + +func TestPermissionsRead(t *testing.T) { + d, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: model.ObjectACL{ + ObjectID: "/clusters/abc", + ObjectType: "clusters", + AccessControlList: []*model.AccessControl{ + { + UserName: &TestingUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_READ", + Inherited: false, + }, + }, + }, + { + UserName: &TestingAdminUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_MANAGE", + Inherited: false, + }, + }, + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: model.User{ + UserName: TestingAdminUser, + }, + }, + }, resourcePermissions, map[string]interface{}{}, + func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsRead(d, c) + }) + assert.NoError(t, err, err) + assert.Equal(t, "/clusters/abc", d.Id()) + assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) + assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) + assert.Equal(t, 1, d.Get("access_control.#")) +} + +func TestPermissionsDelete(t *testing.T) { + d, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPut, + Resource: "/api/2.0/preview/permissions/clusters/abc", + ExpectedRequest: model.ObjectACL{}, + }, + }, resourcePermissions, map[string]interface{}{}, + func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsDelete(d, c) + }) + assert.NoError(t, err, err) + assert.Equal(t, "/clusters/abc", d.Id()) +} + +func TestPermissionsCreate_invalid(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{}, resourcePermissions, map[string]interface{}{}, + resourcePermissionsCreate) + assert.EqualError(t, err, "At least one type of resource identifiers must be set") +} + +func TestPermissionsCreate_no_access_control(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{}, resourcePermissions, + map[string]interface{}{ + "cluster_id": "abc", + }, resourcePermissionsCreate) + assert.EqualError(t, err, "At least one access_control is required") +} + +func TestPermissionsCreate(t *testing.T) { + d, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPatch, + Resource: "/api/2.0/preview/permissions/clusters/abc", + ExpectedRequest: model.AccessControlChangeList{ + AccessControlList: []*model.AccessControlChange{ + { + UserName: &TestingUser, + PermissionLevel: "CAN_USE", + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: model.ObjectACL{ + ObjectID: "/clusters/abc", + ObjectType: "clusters", + AccessControlList: []*model.AccessControl{ + { + UserName: &TestingUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_READ", + Inherited: false, + }, + }, + }, + { + UserName: &TestingAdminUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_MANAGE", + Inherited: false, + }, + }, + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: model.User{ + UserName: TestingAdminUser, + }, + }, + }, resourcePermissions, map[string]interface{}{ + "cluster_id": "abc", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_USE", + }, + }, + }, resourcePermissionsCreate) + assert.NoError(t, err, err) + assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) + assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) + assert.Equal(t, 1, d.Get("access_control.#")) +} + func TestAccDatabricksPermissionsResourceFullLifecycle(t *testing.T) { var permissions model.ObjectACL randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) diff --git a/databricks/utils_test.go b/databricks/utils_test.go index 5ef49ca30..1a09f951b 100644 --- a/databricks/utils_test.go +++ b/databricks/utils_test.go @@ -1,11 +1,82 @@ package databricks import ( + "bytes" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" "testing" + "github.com/databrickslabs/databricks-terraform/client/service" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/stretchr/testify/assert" ) +// HTTPFixture defines request structure for test +type HTTPFixture struct { + Method string + Resource string + Response interface{} + Status int + ExpectedRequest interface{} +} + +// ResourceTester helps testing HTTP resources with fixtures +func ResourceTester(t *testing.T, + fixtures []HTTPFixture, + resouceFunc func() *schema.Resource, + state map[string]interface{}, + whatever func(d *schema.ResourceData, c interface{}) error) (*schema.ResourceData, error) { + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + found := false + for _, fixture := range fixtures { + if req.Method == fixture.Method && req.RequestURI == fixture.Resource { + if fixture.Status == 0 { + rw.WriteHeader(200) + } else { + rw.WriteHeader(fixture.Status) + } + if fixture.ExpectedRequest != nil { + buf := new(bytes.Buffer) + _, err := buf.ReadFrom(req.Body) + assert.NoError(t, err, err) + jsonStr, err := json.Marshal(fixture.ExpectedRequest) + assert.NoError(t, err, err) + assert.JSONEq(t, string(jsonStr), buf.String()) + } + if fixture.Response != nil { + responseBytes, err := json.Marshal(fixture.Response) + if err != nil { + assert.NoError(t, err, err) + t.FailNow() + } + _, err = rw.Write(responseBytes) + assert.NoError(t, err, err) + } + found = true + break + } + } + if !found { + assert.Fail(t, fmt.Sprintf("Received unexpected call: %s %s", req.Method, req.RequestURI)) + t.FailNow() + } + })) + + defer server.Close() + var config service.DBApiClientConfig + config.Host = server.URL + config.Setup() + + var client service.DBApiClient + client.SetConfig(&config) + + resource := resouceFunc() + resourceData := schema.TestResourceDataRaw(t, resource.Schema, state) + return resourceData, whatever(resourceData, &client) +} + func TestIsClusterMissingTrueWhenClusterIdSpecifiedPresent(t *testing.T) { errorMessage := "{\"error_code\":\"INVALID_PARAMETER_VALUE\",\"message\":\"Cluster 123 does not exist\"}" From 8865b03526b1ae3457c33751c7435aa96564ebe9 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Wed, 24 Jun 2020 00:14:04 +0200 Subject: [PATCH 06/42] trigger build --- databricks/resource_databricks_permissions_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index c50f06246..ef02d8772 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -156,6 +156,7 @@ func TestPermissionsCreate(t *testing.T) { }, }, }, resourcePermissionsCreate) + assert.NoError(t, err, err) assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) From 391e80ff93b2658ac5597f93946f5aa83afaad05 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Wed, 24 Jun 2020 00:17:07 +0200 Subject: [PATCH 07/42] trigger build --- databricks/resource_databricks_permissions_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index ef02d8772..547146e0a 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -156,7 +156,7 @@ func TestPermissionsCreate(t *testing.T) { }, }, }, resourcePermissionsCreate) - + assert.NoError(t, err, err) assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) From c7f2b3d599be4ea99276ab13991db181deb8d092 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Fri, 19 Jun 2020 18:34:47 +0200 Subject: [PATCH 08/42] Added databricks_permissions resource --- client/model/permissions.go | 75 +++++++ client/service/permissions.go | 50 +++++ client/service/users.go | 12 +- databricks/provider.go | 1 + databricks/resource_databricks_permissions.go | 204 ++++++++++++++++++ .../resource_databricks_permissions_test.go | 142 ++++++++++++ 6 files changed, 482 insertions(+), 2 deletions(-) create mode 100644 client/model/permissions.go create mode 100644 client/service/permissions.go create mode 100644 databricks/resource_databricks_permissions.go create mode 100644 databricks/resource_databricks_permissions_test.go diff --git a/client/model/permissions.go b/client/model/permissions.go new file mode 100644 index 000000000..b81df1d58 --- /dev/null +++ b/client/model/permissions.go @@ -0,0 +1,75 @@ +package model + +// ObjectACL ... +type ObjectACL struct { + ObjectID string `json:"object_id,omitempty"` + ObjectType string `json:"object_type,omitempty"` + AccessControlList []*AccessControl `json:"access_control_list"` +} + +// AccessControlChangeList is wrapper around ACL changes +type AccessControlChangeList struct { + AccessControlList []*AccessControlChange `json:"access_control_list"` +} + +// AccessControlChange is API wrapper for changing permissions +type AccessControlChange struct { + UserName *string `json:"user_name,omitempty"` + GroupName *string `json:"group_name,omitempty"` + ServicePrincipalName *string `json:"service_principal_name,omitempty"` + PermissionLevel string `json:"permission_level"` +} + +// AccessControl ... +type AccessControl struct { + UserName *string `json:"user_name,omitempty"` + GroupName *string `json:"group_name,omitempty"` + AllPermissions []*Permission `json:"all_permissions,omitempty"` +} + +// Permission ... +type Permission struct { + PermissionLevel string `json:"permission_level"` + Inherited bool `json:"inherited,omitempty"` + InheritedFromObject []string `json:"inherited_from_object,omitempty"` +} + +// ToAccessControlChangeList converts data formats +func (oa *ObjectACL) ToAccessControlChangeList() *AccessControlChangeList { + acl := new(AccessControlChangeList) + for _, accessControl := range oa.AccessControlList { + for _, permission := range accessControl.AllPermissions { + if permission.Inherited { + continue + } + item := new(AccessControlChange) + acl.AccessControlList = append(acl.AccessControlList, item) + item.PermissionLevel = permission.PermissionLevel + if accessControl.UserName != nil { + item.UserName = accessControl.UserName + } else if accessControl.GroupName != nil { + item.GroupName = accessControl.GroupName + } + } + } + return acl +} + +// AccessControl exports data for TF +func (acl *AccessControlChangeList) AccessControl(me string) []map[string]string { + result := []map[string]string{} + for _, control := range acl.AccessControlList { + item := map[string]string{} + if control.UserName != nil && *control.UserName != "" { + if me == *control.UserName { + continue + } + item["user_name"] = *control.UserName + } else if control.GroupName != nil && *control.GroupName != "" { + item["group_name"] = *control.GroupName + } + item["permission_level"] = control.PermissionLevel + result = append(result, item) + } + return result +} diff --git a/client/service/permissions.go b/client/service/permissions.go new file mode 100644 index 000000000..e7dc9ac8c --- /dev/null +++ b/client/service/permissions.go @@ -0,0 +1,50 @@ +package service + +import ( + "encoding/json" + "net/http" + + "github.com/databrickslabs/databricks-terraform/client/model" +) + +// PermissionsAPI ... +type PermissionsAPI struct { + Client *DBApiClient +} + +// AddOrModify ... +func (a PermissionsAPI) AddOrModify(objectID string, objectACL *model.AccessControlChangeList) error { + _, err := a.Client.performQuery(http.MethodPatch, + "/preview/permissions"+objectID, + "2.0", nil, objectACL, nil) + if err != nil { + return err + } + + return err +} + +// SetOrDelete ... +func (a PermissionsAPI) SetOrDelete(objectID string, objectACL *model.AccessControlChangeList) error { + _, err := a.Client.performQuery(http.MethodPut, + "/preview/permissions"+objectID, + "2.0", nil, objectACL, nil) + if err != nil { + return err + } + + return err +} + +// Read ... +func (a PermissionsAPI) Read(objectID string) (*model.ObjectACL, error) { + resp, err := a.Client.performQuery(http.MethodGet, + "/preview/permissions"+objectID, + "2.0", nil, nil, nil) + if err != nil { + return nil, err + } + var objectACL = new(model.ObjectACL) + err = json.Unmarshal(resp, &objectACL) + return objectACL, err +} diff --git a/client/service/users.go b/client/service/users.go index a97a20d57..35a741ff4 100644 --- a/client/service/users.go +++ b/client/service/users.go @@ -72,18 +72,26 @@ func (a UsersAPI) Read(userID string) (model.User, error) { } func (a UsersAPI) read(userID string) (model.User, error) { - var user model.User userPath := fmt.Sprintf("/preview/scim/v2/Users/%v", userID) + return a.readByPath(userPath) +} + +// Me gets user information about caller +func (a UsersAPI) Me() (model.User, error) { + return a.readByPath("/preview/scim/v2/Me") +} +func (a UsersAPI) readByPath(userPath string) (model.User, error) { + var user model.User resp, err := a.Client.performQuery(http.MethodGet, userPath, "2.0", scimHeaders, nil, nil) if err != nil { return user, err } - err = json.Unmarshal(resp, &user) return user, err } + // Update will update the user given the user id, username, display name, entitlements and roles func (a UsersAPI) Update(userID string, userName string, displayName string, entitlements []string, roles []string) error { userPath := fmt.Sprintf("/preview/scim/v2/Users/%v", userID) diff --git a/databricks/provider.go b/databricks/provider.go index eeef5a3c3..4aaedb15c 100644 --- a/databricks/provider.go +++ b/databricks/provider.go @@ -30,6 +30,7 @@ func Provider(version string) terraform.ResourceProvider { "databricks_secret_scope": resourceSecretScope(), "databricks_secret": resourceSecret(), "databricks_secret_acl": resourceSecretACL(), + "databricks_permissions": resourcePermissions(), "databricks_instance_pool": resourceInstancePool(), "databricks_scim_user": resourceScimUser(), "databricks_scim_group": resourceScimGroup(), diff --git a/databricks/resource_databricks_permissions.go b/databricks/resource_databricks_permissions.go new file mode 100644 index 000000000..f31e23a79 --- /dev/null +++ b/databricks/resource_databricks_permissions.go @@ -0,0 +1,204 @@ +package databricks + +import ( + "fmt" + "path" + "strconv" + + "github.com/databrickslabs/databricks-terraform/client/model" + "github.com/databrickslabs/databricks-terraform/client/service" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/pkg/errors" +) + +func parsePermissionsFromData(d *schema.ResourceData, + client *service.DBApiClient) (*model.AccessControlChangeList, string, error) { + var objectID string + acl := new(model.AccessControlChangeList) + for _, mapping := range permissionsResourceIDFields() { + v, ok := d.GetOk(mapping.field) + if !ok { + continue + } + id, err := mapping.idRetriever(client, v.(string)) + if err != nil { + return nil, "", err + } + objectID = fmt.Sprintf("/%s/%s", mapping.resourceType, id) + err = d.Set("object_type", mapping.objectType) + if err != nil { + return nil, "", err + } + } + if objectID == "" { + return nil, "", fmt.Errorf("At least one type of resource identifiers must be set") + } + if data, ok := d.GetOk("access_control"); ok { + for _, element := range data.([]interface{}) { + rawAccessControl := element.(map[string]interface{}) + change := new(model.AccessControlChange) + acl.AccessControlList = append(acl.AccessControlList, change) + if v, ok := rawAccessControl["group_name"].(string); ok && v != "" { + change.GroupName = &v + } + if v, ok := rawAccessControl["user_name"].(string); ok && v != "" { + change.UserName = &v + } + if v, ok := rawAccessControl["permission_level"].(string); ok { + change.PermissionLevel = v + } + } + } + return acl, objectID, nil +} + +func resourcePermissionsCreate(d *schema.ResourceData, m interface{}) error { + client := m.(*service.DBApiClient) + acl, objectID, err := parsePermissionsFromData(d, client) + if err != nil { + return err + } + err = client.Permissions().AddOrModify(objectID, acl) + if err != nil { + return err + } + d.SetId(objectID) + return resourcePermissionsRead(d, m) +} + +func resourcePermissionsRead(d *schema.ResourceData, m interface{}) error { + id := d.Id() + client := m.(*service.DBApiClient) + objectACL, err := client.Permissions().Read(id) + if err != nil { + return err + } + for _, mapping := range permissionsResourceIDFields() { + if mapping.objectType != d.Get("object_type").(string) { + continue + } + identifier := path.Base(id) + err := d.Set(mapping.field, identifier) + if err != nil { + return errors.Wrapf(err, + "Cannot set mapping field %s to %s", + mapping.field, id) + } + break + } + acl := objectACL.ToAccessControlChangeList() + me, err := client.Users().Me() + if err != nil { + return errors.Wrapf(err, "Cannot self-identify") + } + accessControl := acl.AccessControl(me.UserName) + err = d.Set("access_control", accessControl) + if err != nil { + return err + } + return nil +} + +func resourcePermissionsDelete(d *schema.ResourceData, m interface{}) error { + id := d.Id() + client := m.(*service.DBApiClient) + err := client.Permissions().SetOrDelete(id, new(model.AccessControlChangeList)) + if err != nil { + return err + } + return nil +} + +// permissionsIDFieldMapping holds mapping +type permissionsIDFieldMapping struct { + field string + objectType string + resourceType string + idRetriever func(client *service.DBApiClient, id string) (string, error) +} + +// PermissionsResourceIDFields shows mapping of id columns to resource types +func permissionsResourceIDFields() []permissionsIDFieldMapping { + SIMPLE := func(client *service.DBApiClient, id string) (string, error) { + return id, nil + } + PATH := func(client *service.DBApiClient, path string) (string, error) { + info, err := client.Notebooks().Read(path) + if err != nil { + return "", errors.Wrapf(err, "Cannot load path %s", path) + } + return strconv.FormatInt(info.ObjectID, 10), nil + } + return []permissionsIDFieldMapping{ + {"cluster_policy_id", "cluster-policy", "cluster-policies", SIMPLE}, + {"instance_pool_id", "instance-pool", "instance-pools", SIMPLE}, + {"cluster_id", "cluster", "clusters", SIMPLE}, + {"job_id", "job", "jobs", SIMPLE}, + {"notebook_id", "notebook", "notebooks", SIMPLE}, + {"notebook_path", "notebook", "notebooks", PATH}, + {"directory_id", "directory", "directories", SIMPLE}, + {"directory_path", "directory", "directories", PATH}, + } +} + +func conflictingFields(field string) []string { + conflicting := []string{} + for _, mapping := range permissionsResourceIDFields() { + if mapping.field == field { + continue + } + conflicting = append(conflicting, mapping.field) + } + return conflicting +} + +func resourcePermissions() *schema.Resource { + fields := map[string]*schema.Schema{ + "object_type": { + Type: schema.TypeString, + Computed: true, + }, + "access_control": { + ForceNew: true, + Type: schema.TypeList, + MinItems: 1, + Required: true, + ConfigMode: schema.SchemaConfigModeAttr, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "user_name": { + ForceNew: true, + Type: schema.TypeString, + Optional: true, + //ConflictsWith: []string{"group_name"}, + }, + "group_name": { + ForceNew: true, + Type: schema.TypeString, + Optional: true, + //ConflictsWith: []string{"user_name"}, + }, + "permission_level": { + ForceNew: true, + Type: schema.TypeString, + Required: true, + }, + }, + }, + }, + } + for _, mapping := range permissionsResourceIDFields() { + fields[mapping.field] = &schema.Schema{ + ForceNew: true, + Type: schema.TypeString, + Optional: true, + ConflictsWith: conflictingFields(mapping.field), + } + } + return &schema.Resource{ + Create: resourcePermissionsCreate, + Read: resourcePermissionsRead, + Delete: resourcePermissionsDelete, + Schema: fields, + } +} diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go new file mode 100644 index 000000000..b7b3be005 --- /dev/null +++ b/databricks/resource_databricks_permissions_test.go @@ -0,0 +1,142 @@ +package databricks + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/stretchr/testify/assert" + + "github.com/databrickslabs/databricks-terraform/client/model" + "github.com/databrickslabs/databricks-terraform/client/service" +) + +func TestAccDatabricksPermissionsResourceFullLifecycle(t *testing.T) { + var permissions model.ObjectACL + randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + resource.Test(t, resource.TestCase{ + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + // create a resource + Config: testClusterPolicyPermissions(randomName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("databricks_permissions.dummy_can_use", + "object_type", "cluster-policy"), + testAccIDCallback(t, "databricks_permissions.dummy_can_use", + func(client *service.DBApiClient, id string) error { + resp, err := client.Permissions().Read(id) + if err != nil { + return err + } + permissions = *resp + assert.Len(t, permissions.AccessControlList, 3) + return nil + }), + ), + }, + { + Config: testClusterPolicyPermissionsSecondGroupAdded(randomName), + Check: testAccIDCallback(t, "databricks_permissions.dummy_can_use", + func(client *service.DBApiClient, id string) error { + resp, err := client.Permissions().Read(id) + if err != nil { + return err + } + permissions = *resp + assert.Len(t, permissions.AccessControlList, 3) + return nil + }), + }, + }, + }) +} + +func testClusterPolicyPermissions(name string) string { + return fmt.Sprintf(` + resource "databricks_cluster_policy" "something_simple" { + name = "Terraform Policy %[1]s" + definition = jsonencode({ + "spark_conf.spark.hadoop.javax.jdo.option.ConnectionURL": { + "type": "forbidden" + } + }) + } + resource "databricks_scim_group" "dummy_group" { + display_name = "Terraform Group %[1]s" + } + resource "databricks_permissions" "dummy_can_use" { + cluster_policy_id = databricks_cluster_policy.something_simple.id + access_control { + group_name = databricks_scim_group.dummy_group.display_name + permission_level = "CAN_USE" + } + } + `, name) +} + +func testClusterPolicyPermissionsSecondGroupAdded(name string) string { + return fmt.Sprintf(` + resource "databricks_cluster_policy" "something_simple" { + name = "Terraform Policy %[1]s" + definition = jsonencode({ + "spark_conf.spark.hadoop.javax.jdo.option.ConnectionURL": { + "type": "forbidden" + }, + "spark_conf.spark.secondkey": { + "type": "forbidden" + } + }) + } + resource "databricks_scim_group" "dummy_group" { + display_name = "Terraform Group %[1]s" + } + resource "databricks_scim_group" "second_group" { + display_name = "Terraform Second Group %[1]s" + } + resource "databricks_permissions" "dummy_can_use" { + cluster_policy_id = databricks_cluster_policy.something_simple.id + access_control { + group_name = databricks_scim_group.dummy_group.display_name + permission_level = "CAN_USE" + } + access_control { + group_name = databricks_scim_group.second_group.display_name + permission_level = "CAN_USE" + } + } + `, name) +} + +func TestAccNotebookPermissions(t *testing.T) { + randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + resource.Test(t, resource.TestCase{ + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + // create a resource + Config: fmt.Sprintf(` + resource "databricks_notebook" "dummy" { + content = base64encode("# Databricks notebook source\nprint(1)") + path = "/Beginning/Init" + overwrite = true + mkdirs = true + language = "PYTHON" + format = "SOURCE" + } + resource "databricks_scim_group" "dummy_group" { + display_name = "Terraform Group %[1]s" + } + resource "databricks_permissions" "dummy_can_use" { + directory_path = "/Beginning" + access_control { + group_name = databricks_scim_group.dummy_group.display_name + permission_level = "CAN_MANAGE" + } + } + `, randomName), + }, + }, + }) +} \ No newline at end of file From 686b435653e570379d1087eab80de66316fe491b Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Mon, 22 Jun 2020 21:32:45 +0200 Subject: [PATCH 09/42] Minor commenting tweaks --- client/model/permissions.go | 32 +++++++++---------- client/service/permissions.go | 8 ++--- client/service/users.go | 1 - .../resource_databricks_permissions_test.go | 2 +- go.mod | 1 + 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/client/model/permissions.go b/client/model/permissions.go index b81df1d58..d9c7007ee 100644 --- a/client/model/permissions.go +++ b/client/model/permissions.go @@ -1,39 +1,39 @@ package model -// ObjectACL ... +// ObjectACL is a structure to generically describe access control type ObjectACL struct { ObjectID string `json:"object_id,omitempty"` ObjectType string `json:"object_type,omitempty"` AccessControlList []*AccessControl `json:"access_control_list"` } -// AccessControlChangeList is wrapper around ACL changes -type AccessControlChangeList struct { - AccessControlList []*AccessControlChange `json:"access_control_list"` -} - -// AccessControlChange is API wrapper for changing permissions -type AccessControlChange struct { - UserName *string `json:"user_name,omitempty"` - GroupName *string `json:"group_name,omitempty"` - ServicePrincipalName *string `json:"service_principal_name,omitempty"` - PermissionLevel string `json:"permission_level"` -} - -// AccessControl ... +// AccessControl is a structure to describe user/group permissions type AccessControl struct { UserName *string `json:"user_name,omitempty"` GroupName *string `json:"group_name,omitempty"` AllPermissions []*Permission `json:"all_permissions,omitempty"` } -// Permission ... +// Permission is a structure to describe permission level type Permission struct { PermissionLevel string `json:"permission_level"` Inherited bool `json:"inherited,omitempty"` InheritedFromObject []string `json:"inherited_from_object,omitempty"` } +// AccessControlChangeList is wrapper around ACL changes for REST API +type AccessControlChangeList struct { + AccessControlList []*AccessControlChange `json:"access_control_list"` +} + +// AccessControlChange is API wrapper for changing permissions +type AccessControlChange struct { + UserName *string `json:"user_name,omitempty"` + GroupName *string `json:"group_name,omitempty"` + ServicePrincipalName *string `json:"service_principal_name,omitempty"` + PermissionLevel string `json:"permission_level"` +} + // ToAccessControlChangeList converts data formats func (oa *ObjectACL) ToAccessControlChangeList() *AccessControlChangeList { acl := new(AccessControlChangeList) diff --git a/client/service/permissions.go b/client/service/permissions.go index e7dc9ac8c..e2e0bd5a9 100644 --- a/client/service/permissions.go +++ b/client/service/permissions.go @@ -7,12 +7,12 @@ import ( "github.com/databrickslabs/databricks-terraform/client/model" ) -// PermissionsAPI ... +// PermissionsAPI exposes general permission related methods type PermissionsAPI struct { Client *DBApiClient } -// AddOrModify ... +// AddOrModify works with permissions change list func (a PermissionsAPI) AddOrModify(objectID string, objectACL *model.AccessControlChangeList) error { _, err := a.Client.performQuery(http.MethodPatch, "/preview/permissions"+objectID, @@ -24,7 +24,7 @@ func (a PermissionsAPI) AddOrModify(objectID string, objectACL *model.AccessCont return err } -// SetOrDelete ... +// SetOrDelete updates object permissions func (a PermissionsAPI) SetOrDelete(objectID string, objectACL *model.AccessControlChangeList) error { _, err := a.Client.performQuery(http.MethodPut, "/preview/permissions"+objectID, @@ -36,7 +36,7 @@ func (a PermissionsAPI) SetOrDelete(objectID string, objectACL *model.AccessCont return err } -// Read ... +// Read gets all relevant permissions for the object, including inherited ones func (a PermissionsAPI) Read(objectID string) (*model.ObjectACL, error) { resp, err := a.Client.performQuery(http.MethodGet, "/preview/permissions"+objectID, diff --git a/client/service/users.go b/client/service/users.go index 35a741ff4..6df7050e7 100644 --- a/client/service/users.go +++ b/client/service/users.go @@ -91,7 +91,6 @@ func (a UsersAPI) readByPath(userPath string) (model.User, error) { return user, err } - // Update will update the user given the user id, username, display name, entitlements and roles func (a UsersAPI) Update(userID string, userName string, displayName string, entitlements []string, roles []string) error { userPath := fmt.Sprintf("/preview/scim/v2/Users/%v", userID) diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index b7b3be005..5591cb19b 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -139,4 +139,4 @@ func TestAccNotebookPermissions(t *testing.T) { }, }, }) -} \ No newline at end of file +} diff --git a/go.mod b/go.mod index e9ee7c27b..c5dd07c50 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/joho/godotenv v1.3.0 github.com/mattn/go-colorable v0.1.6 // indirect github.com/mitchellh/go-homedir v1.1.0 + github.com/pkg/errors v0.9.1 github.com/r3labs/diff v0.0.0-20191120142937-b4ed99a31f5a github.com/sergi/go-diff v1.1.0 // indirect github.com/smartystreets/goconvey v1.6.4 // indirect From 4b154482bfb22ebf966e610b47048739608e2de1 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Mon, 22 Jun 2020 21:43:02 +0200 Subject: [PATCH 10/42] Register permissions api in the client --- client/service/apis.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/client/service/apis.go b/client/service/apis.go index b1704cd9e..8d450685b 100644 --- a/client/service/apis.go +++ b/client/service/apis.go @@ -116,6 +116,11 @@ func (c *DBApiClient) MWSCustomerManagedKeys() MWSCustomerManagedKeysAPI { return MWSCustomerManagedKeysAPI{Client: c} } +// Permissions returns an instance of CommandsAPI +func (c *DBApiClient) Permissions() PermissionsAPI { + return PermissionsAPI{Client: c} +} + func (c *DBApiClient) performQuery(method, path string, apiVersion string, headers map[string]string, data interface{}, secretsMask *SecretsMask) ([]byte, error) { err := c.Config.getOrCreateToken() if err != nil { From 1ed8448ed814bc49cedeed84a468d2c261a610c8 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Mon, 22 Jun 2020 21:55:30 +0200 Subject: [PATCH 11/42] Add pkg/errors --- vendor/github.com/pkg/errors/.gitignore | 24 ++ vendor/github.com/pkg/errors/.travis.yml | 10 + vendor/github.com/pkg/errors/LICENSE | 23 ++ vendor/github.com/pkg/errors/Makefile | 44 ++++ vendor/github.com/pkg/errors/README.md | 59 +++++ vendor/github.com/pkg/errors/appveyor.yml | 32 +++ vendor/github.com/pkg/errors/errors.go | 288 ++++++++++++++++++++++ vendor/github.com/pkg/errors/go113.go | 38 +++ vendor/github.com/pkg/errors/stack.go | 177 +++++++++++++ vendor/modules.txt | 2 + 10 files changed, 697 insertions(+) create mode 100644 vendor/github.com/pkg/errors/.gitignore create mode 100644 vendor/github.com/pkg/errors/.travis.yml create mode 100644 vendor/github.com/pkg/errors/LICENSE create mode 100644 vendor/github.com/pkg/errors/Makefile create mode 100644 vendor/github.com/pkg/errors/README.md create mode 100644 vendor/github.com/pkg/errors/appveyor.yml create mode 100644 vendor/github.com/pkg/errors/errors.go create mode 100644 vendor/github.com/pkg/errors/go113.go create mode 100644 vendor/github.com/pkg/errors/stack.go diff --git a/vendor/github.com/pkg/errors/.gitignore b/vendor/github.com/pkg/errors/.gitignore new file mode 100644 index 000000000..daf913b1b --- /dev/null +++ b/vendor/github.com/pkg/errors/.gitignore @@ -0,0 +1,24 @@ +# Compiled Object files, Static and Dynamic libs (Shared Objects) +*.o +*.a +*.so + +# Folders +_obj +_test + +# Architecture specific extensions/prefixes +*.[568vq] +[568vq].out + +*.cgo1.go +*.cgo2.c +_cgo_defun.c +_cgo_gotypes.go +_cgo_export.* + +_testmain.go + +*.exe +*.test +*.prof diff --git a/vendor/github.com/pkg/errors/.travis.yml b/vendor/github.com/pkg/errors/.travis.yml new file mode 100644 index 000000000..9159de03e --- /dev/null +++ b/vendor/github.com/pkg/errors/.travis.yml @@ -0,0 +1,10 @@ +language: go +go_import_path: github.com/pkg/errors +go: + - 1.11.x + - 1.12.x + - 1.13.x + - tip + +script: + - make check diff --git a/vendor/github.com/pkg/errors/LICENSE b/vendor/github.com/pkg/errors/LICENSE new file mode 100644 index 000000000..835ba3e75 --- /dev/null +++ b/vendor/github.com/pkg/errors/LICENSE @@ -0,0 +1,23 @@ +Copyright (c) 2015, Dave Cheney +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + +* Redistributions of source code must retain the above copyright notice, this + list of conditions and the following disclaimer. + +* Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/vendor/github.com/pkg/errors/Makefile b/vendor/github.com/pkg/errors/Makefile new file mode 100644 index 000000000..ce9d7cded --- /dev/null +++ b/vendor/github.com/pkg/errors/Makefile @@ -0,0 +1,44 @@ +PKGS := github.com/pkg/errors +SRCDIRS := $(shell go list -f '{{.Dir}}' $(PKGS)) +GO := go + +check: test vet gofmt misspell unconvert staticcheck ineffassign unparam + +test: + $(GO) test $(PKGS) + +vet: | test + $(GO) vet $(PKGS) + +staticcheck: + $(GO) get honnef.co/go/tools/cmd/staticcheck + staticcheck -checks all $(PKGS) + +misspell: + $(GO) get github.com/client9/misspell/cmd/misspell + misspell \ + -locale GB \ + -error \ + *.md *.go + +unconvert: + $(GO) get github.com/mdempsky/unconvert + unconvert -v $(PKGS) + +ineffassign: + $(GO) get github.com/gordonklaus/ineffassign + find $(SRCDIRS) -name '*.go' | xargs ineffassign + +pedantic: check errcheck + +unparam: + $(GO) get mvdan.cc/unparam + unparam ./... + +errcheck: + $(GO) get github.com/kisielk/errcheck + errcheck $(PKGS) + +gofmt: + @echo Checking code is gofmted + @test -z "$(shell gofmt -s -l -d -e $(SRCDIRS) | tee /dev/stderr)" diff --git a/vendor/github.com/pkg/errors/README.md b/vendor/github.com/pkg/errors/README.md new file mode 100644 index 000000000..54dfdcb12 --- /dev/null +++ b/vendor/github.com/pkg/errors/README.md @@ -0,0 +1,59 @@ +# errors [![Travis-CI](https://travis-ci.org/pkg/errors.svg)](https://travis-ci.org/pkg/errors) [![AppVeyor](https://ci.appveyor.com/api/projects/status/b98mptawhudj53ep/branch/master?svg=true)](https://ci.appveyor.com/project/davecheney/errors/branch/master) [![GoDoc](https://godoc.org/github.com/pkg/errors?status.svg)](http://godoc.org/github.com/pkg/errors) [![Report card](https://goreportcard.com/badge/github.com/pkg/errors)](https://goreportcard.com/report/github.com/pkg/errors) [![Sourcegraph](https://sourcegraph.com/github.com/pkg/errors/-/badge.svg)](https://sourcegraph.com/github.com/pkg/errors?badge) + +Package errors provides simple error handling primitives. + +`go get github.com/pkg/errors` + +The traditional error handling idiom in Go is roughly akin to +```go +if err != nil { + return err +} +``` +which applied recursively up the call stack results in error reports without context or debugging information. The errors package allows programmers to add context to the failure path in their code in a way that does not destroy the original value of the error. + +## Adding context to an error + +The errors.Wrap function returns a new error that adds context to the original error. For example +```go +_, err := ioutil.ReadAll(r) +if err != nil { + return errors.Wrap(err, "read failed") +} +``` +## Retrieving the cause of an error + +Using `errors.Wrap` constructs a stack of errors, adding context to the preceding error. Depending on the nature of the error it may be necessary to reverse the operation of errors.Wrap to retrieve the original error for inspection. Any error value which implements this interface can be inspected by `errors.Cause`. +```go +type causer interface { + Cause() error +} +``` +`errors.Cause` will recursively retrieve the topmost error which does not implement `causer`, which is assumed to be the original cause. For example: +```go +switch err := errors.Cause(err).(type) { +case *MyError: + // handle specifically +default: + // unknown error +} +``` + +[Read the package documentation for more information](https://godoc.org/github.com/pkg/errors). + +## Roadmap + +With the upcoming [Go2 error proposals](https://go.googlesource.com/proposal/+/master/design/go2draft.md) this package is moving into maintenance mode. The roadmap for a 1.0 release is as follows: + +- 0.9. Remove pre Go 1.9 and Go 1.10 support, address outstanding pull requests (if possible) +- 1.0. Final release. + +## Contributing + +Because of the Go2 errors changes, this package is not accepting proposals for new functionality. With that said, we welcome pull requests, bug fixes and issue reports. + +Before sending a PR, please discuss your change by raising an issue. + +## License + +BSD-2-Clause diff --git a/vendor/github.com/pkg/errors/appveyor.yml b/vendor/github.com/pkg/errors/appveyor.yml new file mode 100644 index 000000000..a932eade0 --- /dev/null +++ b/vendor/github.com/pkg/errors/appveyor.yml @@ -0,0 +1,32 @@ +version: build-{build}.{branch} + +clone_folder: C:\gopath\src\github.com\pkg\errors +shallow_clone: true # for startup speed + +environment: + GOPATH: C:\gopath + +platform: + - x64 + +# http://www.appveyor.com/docs/installed-software +install: + # some helpful output for debugging builds + - go version + - go env + # pre-installed MinGW at C:\MinGW is 32bit only + # but MSYS2 at C:\msys64 has mingw64 + - set PATH=C:\msys64\mingw64\bin;%PATH% + - gcc --version + - g++ --version + +build_script: + - go install -v ./... + +test_script: + - set PATH=C:\gopath\bin;%PATH% + - go test -v ./... + +#artifacts: +# - path: '%GOPATH%\bin\*.exe' +deploy: off diff --git a/vendor/github.com/pkg/errors/errors.go b/vendor/github.com/pkg/errors/errors.go new file mode 100644 index 000000000..161aea258 --- /dev/null +++ b/vendor/github.com/pkg/errors/errors.go @@ -0,0 +1,288 @@ +// Package errors provides simple error handling primitives. +// +// The traditional error handling idiom in Go is roughly akin to +// +// if err != nil { +// return err +// } +// +// which when applied recursively up the call stack results in error reports +// without context or debugging information. The errors package allows +// programmers to add context to the failure path in their code in a way +// that does not destroy the original value of the error. +// +// Adding context to an error +// +// The errors.Wrap function returns a new error that adds context to the +// original error by recording a stack trace at the point Wrap is called, +// together with the supplied message. For example +// +// _, err := ioutil.ReadAll(r) +// if err != nil { +// return errors.Wrap(err, "read failed") +// } +// +// If additional control is required, the errors.WithStack and +// errors.WithMessage functions destructure errors.Wrap into its component +// operations: annotating an error with a stack trace and with a message, +// respectively. +// +// Retrieving the cause of an error +// +// Using errors.Wrap constructs a stack of errors, adding context to the +// preceding error. Depending on the nature of the error it may be necessary +// to reverse the operation of errors.Wrap to retrieve the original error +// for inspection. Any error value which implements this interface +// +// type causer interface { +// Cause() error +// } +// +// can be inspected by errors.Cause. errors.Cause will recursively retrieve +// the topmost error that does not implement causer, which is assumed to be +// the original cause. For example: +// +// switch err := errors.Cause(err).(type) { +// case *MyError: +// // handle specifically +// default: +// // unknown error +// } +// +// Although the causer interface is not exported by this package, it is +// considered a part of its stable public interface. +// +// Formatted printing of errors +// +// All error values returned from this package implement fmt.Formatter and can +// be formatted by the fmt package. The following verbs are supported: +// +// %s print the error. If the error has a Cause it will be +// printed recursively. +// %v see %s +// %+v extended format. Each Frame of the error's StackTrace will +// be printed in detail. +// +// Retrieving the stack trace of an error or wrapper +// +// New, Errorf, Wrap, and Wrapf record a stack trace at the point they are +// invoked. This information can be retrieved with the following interface: +// +// type stackTracer interface { +// StackTrace() errors.StackTrace +// } +// +// The returned errors.StackTrace type is defined as +// +// type StackTrace []Frame +// +// The Frame type represents a call site in the stack trace. Frame supports +// the fmt.Formatter interface that can be used for printing information about +// the stack trace of this error. For example: +// +// if err, ok := err.(stackTracer); ok { +// for _, f := range err.StackTrace() { +// fmt.Printf("%+s:%d\n", f, f) +// } +// } +// +// Although the stackTracer interface is not exported by this package, it is +// considered a part of its stable public interface. +// +// See the documentation for Frame.Format for more details. +package errors + +import ( + "fmt" + "io" +) + +// New returns an error with the supplied message. +// New also records the stack trace at the point it was called. +func New(message string) error { + return &fundamental{ + msg: message, + stack: callers(), + } +} + +// Errorf formats according to a format specifier and returns the string +// as a value that satisfies error. +// Errorf also records the stack trace at the point it was called. +func Errorf(format string, args ...interface{}) error { + return &fundamental{ + msg: fmt.Sprintf(format, args...), + stack: callers(), + } +} + +// fundamental is an error that has a message and a stack, but no caller. +type fundamental struct { + msg string + *stack +} + +func (f *fundamental) Error() string { return f.msg } + +func (f *fundamental) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + if s.Flag('+') { + io.WriteString(s, f.msg) + f.stack.Format(s, verb) + return + } + fallthrough + case 's': + io.WriteString(s, f.msg) + case 'q': + fmt.Fprintf(s, "%q", f.msg) + } +} + +// WithStack annotates err with a stack trace at the point WithStack was called. +// If err is nil, WithStack returns nil. +func WithStack(err error) error { + if err == nil { + return nil + } + return &withStack{ + err, + callers(), + } +} + +type withStack struct { + error + *stack +} + +func (w *withStack) Cause() error { return w.error } + +// Unwrap provides compatibility for Go 1.13 error chains. +func (w *withStack) Unwrap() error { return w.error } + +func (w *withStack) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + if s.Flag('+') { + fmt.Fprintf(s, "%+v", w.Cause()) + w.stack.Format(s, verb) + return + } + fallthrough + case 's': + io.WriteString(s, w.Error()) + case 'q': + fmt.Fprintf(s, "%q", w.Error()) + } +} + +// Wrap returns an error annotating err with a stack trace +// at the point Wrap is called, and the supplied message. +// If err is nil, Wrap returns nil. +func Wrap(err error, message string) error { + if err == nil { + return nil + } + err = &withMessage{ + cause: err, + msg: message, + } + return &withStack{ + err, + callers(), + } +} + +// Wrapf returns an error annotating err with a stack trace +// at the point Wrapf is called, and the format specifier. +// If err is nil, Wrapf returns nil. +func Wrapf(err error, format string, args ...interface{}) error { + if err == nil { + return nil + } + err = &withMessage{ + cause: err, + msg: fmt.Sprintf(format, args...), + } + return &withStack{ + err, + callers(), + } +} + +// WithMessage annotates err with a new message. +// If err is nil, WithMessage returns nil. +func WithMessage(err error, message string) error { + if err == nil { + return nil + } + return &withMessage{ + cause: err, + msg: message, + } +} + +// WithMessagef annotates err with the format specifier. +// If err is nil, WithMessagef returns nil. +func WithMessagef(err error, format string, args ...interface{}) error { + if err == nil { + return nil + } + return &withMessage{ + cause: err, + msg: fmt.Sprintf(format, args...), + } +} + +type withMessage struct { + cause error + msg string +} + +func (w *withMessage) Error() string { return w.msg + ": " + w.cause.Error() } +func (w *withMessage) Cause() error { return w.cause } + +// Unwrap provides compatibility for Go 1.13 error chains. +func (w *withMessage) Unwrap() error { return w.cause } + +func (w *withMessage) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + if s.Flag('+') { + fmt.Fprintf(s, "%+v\n", w.Cause()) + io.WriteString(s, w.msg) + return + } + fallthrough + case 's', 'q': + io.WriteString(s, w.Error()) + } +} + +// Cause returns the underlying cause of the error, if possible. +// An error value has a cause if it implements the following +// interface: +// +// type causer interface { +// Cause() error +// } +// +// If the error does not implement Cause, the original error will +// be returned. If the error is nil, nil will be returned without further +// investigation. +func Cause(err error) error { + type causer interface { + Cause() error + } + + for err != nil { + cause, ok := err.(causer) + if !ok { + break + } + err = cause.Cause() + } + return err +} diff --git a/vendor/github.com/pkg/errors/go113.go b/vendor/github.com/pkg/errors/go113.go new file mode 100644 index 000000000..be0d10d0c --- /dev/null +++ b/vendor/github.com/pkg/errors/go113.go @@ -0,0 +1,38 @@ +// +build go1.13 + +package errors + +import ( + stderrors "errors" +) + +// Is reports whether any error in err's chain matches target. +// +// The chain consists of err itself followed by the sequence of errors obtained by +// repeatedly calling Unwrap. +// +// An error is considered to match a target if it is equal to that target or if +// it implements a method Is(error) bool such that Is(target) returns true. +func Is(err, target error) bool { return stderrors.Is(err, target) } + +// As finds the first error in err's chain that matches target, and if so, sets +// target to that error value and returns true. +// +// The chain consists of err itself followed by the sequence of errors obtained by +// repeatedly calling Unwrap. +// +// An error matches target if the error's concrete value is assignable to the value +// pointed to by target, or if the error has a method As(interface{}) bool such that +// As(target) returns true. In the latter case, the As method is responsible for +// setting target. +// +// As will panic if target is not a non-nil pointer to either a type that implements +// error, or to any interface type. As returns false if err is nil. +func As(err error, target interface{}) bool { return stderrors.As(err, target) } + +// Unwrap returns the result of calling the Unwrap method on err, if err's +// type contains an Unwrap method returning error. +// Otherwise, Unwrap returns nil. +func Unwrap(err error) error { + return stderrors.Unwrap(err) +} diff --git a/vendor/github.com/pkg/errors/stack.go b/vendor/github.com/pkg/errors/stack.go new file mode 100644 index 000000000..779a8348f --- /dev/null +++ b/vendor/github.com/pkg/errors/stack.go @@ -0,0 +1,177 @@ +package errors + +import ( + "fmt" + "io" + "path" + "runtime" + "strconv" + "strings" +) + +// Frame represents a program counter inside a stack frame. +// For historical reasons if Frame is interpreted as a uintptr +// its value represents the program counter + 1. +type Frame uintptr + +// pc returns the program counter for this frame; +// multiple frames may have the same PC value. +func (f Frame) pc() uintptr { return uintptr(f) - 1 } + +// file returns the full path to the file that contains the +// function for this Frame's pc. +func (f Frame) file() string { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return "unknown" + } + file, _ := fn.FileLine(f.pc()) + return file +} + +// line returns the line number of source code of the +// function for this Frame's pc. +func (f Frame) line() int { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return 0 + } + _, line := fn.FileLine(f.pc()) + return line +} + +// name returns the name of this function, if known. +func (f Frame) name() string { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return "unknown" + } + return fn.Name() +} + +// Format formats the frame according to the fmt.Formatter interface. +// +// %s source file +// %d source line +// %n function name +// %v equivalent to %s:%d +// +// Format accepts flags that alter the printing of some verbs, as follows: +// +// %+s function name and path of source file relative to the compile time +// GOPATH separated by \n\t (\n\t) +// %+v equivalent to %+s:%d +func (f Frame) Format(s fmt.State, verb rune) { + switch verb { + case 's': + switch { + case s.Flag('+'): + io.WriteString(s, f.name()) + io.WriteString(s, "\n\t") + io.WriteString(s, f.file()) + default: + io.WriteString(s, path.Base(f.file())) + } + case 'd': + io.WriteString(s, strconv.Itoa(f.line())) + case 'n': + io.WriteString(s, funcname(f.name())) + case 'v': + f.Format(s, 's') + io.WriteString(s, ":") + f.Format(s, 'd') + } +} + +// MarshalText formats a stacktrace Frame as a text string. The output is the +// same as that of fmt.Sprintf("%+v", f), but without newlines or tabs. +func (f Frame) MarshalText() ([]byte, error) { + name := f.name() + if name == "unknown" { + return []byte(name), nil + } + return []byte(fmt.Sprintf("%s %s:%d", name, f.file(), f.line())), nil +} + +// StackTrace is stack of Frames from innermost (newest) to outermost (oldest). +type StackTrace []Frame + +// Format formats the stack of Frames according to the fmt.Formatter interface. +// +// %s lists source files for each Frame in the stack +// %v lists the source file and line number for each Frame in the stack +// +// Format accepts flags that alter the printing of some verbs, as follows: +// +// %+v Prints filename, function, and line number for each Frame in the stack. +func (st StackTrace) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + switch { + case s.Flag('+'): + for _, f := range st { + io.WriteString(s, "\n") + f.Format(s, verb) + } + case s.Flag('#'): + fmt.Fprintf(s, "%#v", []Frame(st)) + default: + st.formatSlice(s, verb) + } + case 's': + st.formatSlice(s, verb) + } +} + +// formatSlice will format this StackTrace into the given buffer as a slice of +// Frame, only valid when called with '%s' or '%v'. +func (st StackTrace) formatSlice(s fmt.State, verb rune) { + io.WriteString(s, "[") + for i, f := range st { + if i > 0 { + io.WriteString(s, " ") + } + f.Format(s, verb) + } + io.WriteString(s, "]") +} + +// stack represents a stack of program counters. +type stack []uintptr + +func (s *stack) Format(st fmt.State, verb rune) { + switch verb { + case 'v': + switch { + case st.Flag('+'): + for _, pc := range *s { + f := Frame(pc) + fmt.Fprintf(st, "\n%+v", f) + } + } + } +} + +func (s *stack) StackTrace() StackTrace { + f := make([]Frame, len(*s)) + for i := 0; i < len(f); i++ { + f[i] = Frame((*s)[i]) + } + return f +} + +func callers() *stack { + const depth = 32 + var pcs [depth]uintptr + n := runtime.Callers(3, pcs[:]) + var st stack = pcs[0:n] + return &st +} + +// funcname removes the path prefix component of a function's name reported by func.Name(). +func funcname(name string) string { + i := strings.LastIndex(name, "/") + name = name[i+1:] + i = strings.Index(name, ".") + return name[i+1:] +} diff --git a/vendor/modules.txt b/vendor/modules.txt index c1ba8fa71..f72453a1a 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -230,6 +230,8 @@ github.com/mitchellh/mapstructure github.com/mitchellh/reflectwalk # github.com/oklog/run v1.0.0 github.com/oklog/run +# github.com/pkg/errors v0.9.1 +github.com/pkg/errors # github.com/pmezard/go-difflib v1.0.0 github.com/pmezard/go-difflib/difflib # github.com/posener/complete v1.2.1 From 57b2267d7a24b068fb4acd0d8d17baf434707998 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Wed, 24 Jun 2020 00:07:01 +0200 Subject: [PATCH 12/42] Added real unit tests for resource --- CONTRIBUTING.md | 67 +++++++- databricks/resource_databricks_permissions.go | 5 + .../resource_databricks_permissions_test.go | 150 ++++++++++++++++++ databricks/utils_test.go | 74 +++++++++ go.sum | 1 + 5 files changed, 294 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3cf74cfcc..7c6476742 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,10 +8,11 @@ Contributing to Databricks Terraform Provider - [Building and Installing with Docker](#building-and-installing-with-docker) - [Testing](#testing) - [Linting](#linting) +- [Unit testing resources](#unit-testing-resources) - [Integration Testing](#integration-testing) - [Project Components](#project-components) - - [Databricks Terraform Provider Resources State](#databricks-terraform-provider-resources-state) - - [Databricks Terraform Data Sources State](#databricks-terraform-data-sources-state) + - [Databricks Terraform Provider Resources State](#databricks-terraform-provider-resources-state) + - [Databricks Terraform Data Sources State](#databricks-terraform-data-sources-state) We happily welcome contributions to databricks-terraform. We use GitHub Issues to track community reported issues and GitHub Pull Requests for accepting changes. @@ -128,6 +129,67 @@ $ docker run -it -v $(pwd):/workpace -w /workpace databricks-terraform apply Please use makefile for linting. If you run `golangci-lint` by itself it will fail due to different tags containing same functions. So please run `make lint` instead. +## Unit testing resources + +In order to unit test a resource, which runs fast and could be included in code coverage, one should use `ResourceTester`, that launches embedded HTTP server with `HTTPFixture`'s containing all calls that should have been made in given scenario. Some may argue that this is not a pure unit test, because it creates a side effect in form of embedded server, though it's always on different random port, making it possible to execute these tests in parallel. Therefore comments about non-pure unit tests will be ignored, if they use `ResourceTester` helper. + +```go +func TestPermissionsCreate(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPatch, + // requires full URI + Resource: "/api/2.0/preview/permissions/clusters/abc", + // works with entities, not JSON. Diff is displayed in case of missmatch + ExpectedRequest: model.AccessControlChangeList{ + AccessControlList: []*model.AccessControlChange{ + { + UserName: &TestingUser, + PermissionLevel: "CAN_USE", + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: model.AccessControlChangeList{ + AccessControlList: []*model.AccessControlChange{ + { + UserName: &TestingUser, + PermissionLevel: "CAN_MANAGE", + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: model.User{ + UserName: "chuck.norris", + }, + }, + }, + // next argument is function, that creates resource (to make schema for ResourceData) + resourcePermissions, + // state represented as native structure (though a bit clunky) + map[string]interface{}{ + "cluster_id": "abc", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_USE", + }, + }, + }, + // the last argument is a function, that performs a stage on resource (Create/update/delete/read) + resourcePermissionsCreate) + assert.NoError(t, err, err) +} +``` + +Each resource should have both unit and integration tests. + ## Integration Testing Currently Databricks supports two cloud providers `azure` and `aws` thus integration testing with the correct cloud service provider is @@ -162,7 +224,6 @@ DATABRICKS_AZURE_WORKSPACE_NAME= Note that azure integration tests will use service principal based auth. Even though it is using a service principal, it will still be generating a personal access token to perform creation of resources. - ## Project Components ### Databricks Terraform Provider Resources State diff --git a/databricks/resource_databricks_permissions.go b/databricks/resource_databricks_permissions.go index f31e23a79..914629d96 100644 --- a/databricks/resource_databricks_permissions.go +++ b/databricks/resource_databricks_permissions.go @@ -33,6 +33,7 @@ func parsePermissionsFromData(d *schema.ResourceData, if objectID == "" { return nil, "", fmt.Errorf("At least one type of resource identifiers must be set") } + changes := 0 if data, ok := d.GetOk("access_control"); ok { for _, element := range data.([]interface{}) { rawAccessControl := element.(map[string]interface{}) @@ -47,8 +48,12 @@ func parsePermissionsFromData(d *schema.ResourceData, if v, ok := rawAccessControl["permission_level"].(string); ok { change.PermissionLevel = v } + changes++ } } + if changes < 1 { + return nil, "", fmt.Errorf("At least one access_control is required") + } return acl, objectID, nil } diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index 5591cb19b..c50f06246 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -2,16 +2,166 @@ package databricks import ( "fmt" + "net/http" "testing" "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/stretchr/testify/assert" "github.com/databrickslabs/databricks-terraform/client/model" "github.com/databrickslabs/databricks-terraform/client/service" ) +var ( + TestingUser = "ben" + TestingAdminUser = "admin" +) + +func TestPermissionsRead(t *testing.T) { + d, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: model.ObjectACL{ + ObjectID: "/clusters/abc", + ObjectType: "clusters", + AccessControlList: []*model.AccessControl{ + { + UserName: &TestingUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_READ", + Inherited: false, + }, + }, + }, + { + UserName: &TestingAdminUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_MANAGE", + Inherited: false, + }, + }, + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: model.User{ + UserName: TestingAdminUser, + }, + }, + }, resourcePermissions, map[string]interface{}{}, + func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsRead(d, c) + }) + assert.NoError(t, err, err) + assert.Equal(t, "/clusters/abc", d.Id()) + assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) + assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) + assert.Equal(t, 1, d.Get("access_control.#")) +} + +func TestPermissionsDelete(t *testing.T) { + d, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPut, + Resource: "/api/2.0/preview/permissions/clusters/abc", + ExpectedRequest: model.ObjectACL{}, + }, + }, resourcePermissions, map[string]interface{}{}, + func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsDelete(d, c) + }) + assert.NoError(t, err, err) + assert.Equal(t, "/clusters/abc", d.Id()) +} + +func TestPermissionsCreate_invalid(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{}, resourcePermissions, map[string]interface{}{}, + resourcePermissionsCreate) + assert.EqualError(t, err, "At least one type of resource identifiers must be set") +} + +func TestPermissionsCreate_no_access_control(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{}, resourcePermissions, + map[string]interface{}{ + "cluster_id": "abc", + }, resourcePermissionsCreate) + assert.EqualError(t, err, "At least one access_control is required") +} + +func TestPermissionsCreate(t *testing.T) { + d, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPatch, + Resource: "/api/2.0/preview/permissions/clusters/abc", + ExpectedRequest: model.AccessControlChangeList{ + AccessControlList: []*model.AccessControlChange{ + { + UserName: &TestingUser, + PermissionLevel: "CAN_USE", + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: model.ObjectACL{ + ObjectID: "/clusters/abc", + ObjectType: "clusters", + AccessControlList: []*model.AccessControl{ + { + UserName: &TestingUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_READ", + Inherited: false, + }, + }, + }, + { + UserName: &TestingAdminUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_MANAGE", + Inherited: false, + }, + }, + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: model.User{ + UserName: TestingAdminUser, + }, + }, + }, resourcePermissions, map[string]interface{}{ + "cluster_id": "abc", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_USE", + }, + }, + }, resourcePermissionsCreate) + assert.NoError(t, err, err) + assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) + assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) + assert.Equal(t, 1, d.Get("access_control.#")) +} + func TestAccDatabricksPermissionsResourceFullLifecycle(t *testing.T) { var permissions model.ObjectACL randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) diff --git a/databricks/utils_test.go b/databricks/utils_test.go index eba2ffea2..e2384909e 100644 --- a/databricks/utils_test.go +++ b/databricks/utils_test.go @@ -260,6 +260,80 @@ func testVerifyResourceIsMissing(t *testing.T, readFunc func() error) { "\nerror code: %s", apiError, apiError.StatusCode, apiError.ErrorCode)) } + "bytes" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/databrickslabs/databricks-terraform/client/service" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/stretchr/testify/assert" +) + +// HTTPFixture defines request structure for test +type HTTPFixture struct { + Method string + Resource string + Response interface{} + Status int + ExpectedRequest interface{} +} + +// ResourceTester helps testing HTTP resources with fixtures +func ResourceTester(t *testing.T, + fixtures []HTTPFixture, + resouceFunc func() *schema.Resource, + state map[string]interface{}, + whatever func(d *schema.ResourceData, c interface{}) error) (*schema.ResourceData, error) { + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + found := false + for _, fixture := range fixtures { + if req.Method == fixture.Method && req.RequestURI == fixture.Resource { + if fixture.Status == 0 { + rw.WriteHeader(200) + } else { + rw.WriteHeader(fixture.Status) + } + if fixture.ExpectedRequest != nil { + buf := new(bytes.Buffer) + _, err := buf.ReadFrom(req.Body) + assert.NoError(t, err, err) + jsonStr, err := json.Marshal(fixture.ExpectedRequest) + assert.NoError(t, err, err) + assert.JSONEq(t, string(jsonStr), buf.String()) + } + if fixture.Response != nil { + responseBytes, err := json.Marshal(fixture.Response) + if err != nil { + assert.NoError(t, err, err) + t.FailNow() + } + _, err = rw.Write(responseBytes) + assert.NoError(t, err, err) + } + found = true + break + } + } + if !found { + assert.Fail(t, fmt.Sprintf("Received unexpected call: %s %s", req.Method, req.RequestURI)) + t.FailNow() + } + })) + + defer server.Close() + var config service.DBApiClientConfig + config.Host = server.URL + config.Setup() + + var client service.DBApiClient + client.SetConfig(&config) + + resource := resouceFunc() + resourceData := schema.TestResourceDataRaw(t, resource.Schema, state) + return resourceData, whatever(resourceData, &client) } func TestIsClusterMissingTrueWhenClusterIdSpecifiedPresent(t *testing.T) { diff --git a/go.sum b/go.sum index 1a406cd3b..bb67768b2 100644 --- a/go.sum +++ b/go.sum @@ -147,6 +147,7 @@ github.com/hashicorp/terraform-svchost v0.0.0-20191011084731-65d371908596/go.mod github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d h1:kJCB4vdITiW1eC1vq2e6IsrXKrZit1bv/TDYFGMp4BQ= github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= +github.com/jhump/protoreflect v1.6.0 h1:h5jfMVslIg6l29nsMs0D8Wj17RDVdNYti0vDN/PZZoE= github.com/jhump/protoreflect v1.6.0/go.mod h1:eaTn3RZAmMBcV0fifFvlm6VHNz3wSkYyXYWUh7ymB74= github.com/jmespath/go-jmespath v0.0.0-20160202185014-0b12d6b521d8/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af h1:pmfjZENx5imkbgOkpRUYLnmbU7UEFbjtDA2hxJ1ichM= From 515d61e34dbe8b4b433abb2c7a625bb69994b32e Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Wed, 24 Jun 2020 00:14:04 +0200 Subject: [PATCH 13/42] trigger build --- databricks/resource_databricks_permissions_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index c50f06246..ef02d8772 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -156,6 +156,7 @@ func TestPermissionsCreate(t *testing.T) { }, }, }, resourcePermissionsCreate) + assert.NoError(t, err, err) assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) From f4a35e0422decda1cf02b9edbed690185eb597d0 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Wed, 24 Jun 2020 00:17:07 +0200 Subject: [PATCH 14/42] trigger build --- databricks/resource_databricks_permissions_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index ef02d8772..547146e0a 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -156,7 +156,7 @@ func TestPermissionsCreate(t *testing.T) { }, }, }, resourcePermissionsCreate) - + assert.NoError(t, err, err) assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) From 9b4cdf93db095b911e891bbcb7ca00e358b9e5b9 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 11:25:31 +0200 Subject: [PATCH 15/42] Fix code style violations --- CONTRIBUTING.md | 5 ++ databricks/azure_auth.go | 16 ++-- databricks/mounts_test.go | 6 +- databricks/provider.go | 44 +++++------ ...ource_databricks_group_instance_profile.go | 14 ++-- .../resource_databricks_group_member.go | 14 ++-- databricks/utils_test.go | 79 +++++++++---------- 7 files changed, 87 insertions(+), 91 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7c6476742..31ce6e2f9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,6 +7,7 @@ Contributing to Databricks Terraform Provider - [Developing with Visual Studio Code Devcontainers](#developing-with-visual-studio-code-devcontainers) - [Building and Installing with Docker](#building-and-installing-with-docker) - [Testing](#testing) +- [Code conventions](#code-conventions) - [Linting](#linting) - [Unit testing resources](#unit-testing-resources) - [Integration Testing](#integration-testing) @@ -124,6 +125,10 @@ $ docker run -it -v $(pwd):/workpace -w /workpace databricks-terraform apply * [ ] Integration tests should be run at a client level against both azure and aws to maintain sdk parity against both apis **(currently only on one cloud)** * [x] Terraform acceptance tests should be run against both aws and azure to maintain parity of provider between both cloud services **(currently only on one cloud)** +## Code conventions + +* Import statements should all be first ordered by "GoLang internal", "Vendor packages" and then "current provider packages". Within those sections imports must follow alphabetical order. + ## Linting Please use makefile for linting. If you run `golangci-lint` by itself it will fail due to different tags containing same functions. diff --git a/databricks/azure_auth.go b/databricks/azure_auth.go index ba2bae571..66a1fa815 100644 --- a/databricks/azure_auth.go +++ b/databricks/azure_auth.go @@ -31,7 +31,7 @@ type TokenPayload struct { TenantID string } -type Workspace struct { +type azureDatabricksWorkspace struct { Name string `json:"name"` ID string `json:"id"` Type string `json:"type"` @@ -100,7 +100,7 @@ func (t *TokenPayload) getManagementToken() (string, error) { return mgmtToken.OAuthToken(), nil } -func (t *TokenPayload) getWorkspace(config *service.DBApiClientConfig, managementToken string) (*Workspace, error) { +func (t *TokenPayload) getWorkspace(config *service.DBApiClientConfig, managementToken string) (*azureDatabricksWorkspace, error) { log.Println("[DEBUG] Getting Workspace ID via management token.") // Escape all the ids url := fmt.Sprintf("https://management.azure.com/subscriptions/%s/resourceGroups/%s"+ @@ -124,7 +124,7 @@ func (t *TokenPayload) getWorkspace(config *service.DBApiClientConfig, managemen return nil, err } - var workspace Workspace + var workspace azureDatabricksWorkspace err = json.Unmarshal(resp, &workspace) if err != nil { return nil, err @@ -157,9 +157,9 @@ func (t *TokenPayload) getADBPlatformToken() (string, error) { return platformToken.OAuthToken(), nil } -func getWorkspaceAccessToken(config *service.DBApiClientConfig, managementToken, adbWorkspaceUrl, adbWorkspaceResourceID, adbPlatformToken string) (*model.TokenResponse, error) { +func getWorkspaceAccessToken(config *service.DBApiClientConfig, managementToken, adbWorkspaceURL, adbWorkspaceResourceID, adbPlatformToken string) (*model.TokenResponse, error) { log.Println("[DEBUG] Creating workspace token") - url := adbWorkspaceUrl + "/api/2.0/token/create" + url := adbWorkspaceURL + "/api/2.0/token/create" headers := map[string]string{ "Content-Type": "application/x-www-form-urlencoded", "X-Databricks-Azure-Workspace-Resource-Id": adbWorkspaceResourceID, @@ -201,7 +201,7 @@ func (t *TokenPayload) initWorkspaceAndGetClient(config *service.DBApiClientConf if err != nil { return err } - adbWorkspaceUrl := "https://" + adbWorkspace.Properties.WorkspaceURL + adbWorkspaceURL := "https://" + adbWorkspace.Properties.WorkspaceURL // Get platform token adbPlatformToken, err := t.getADBPlatformToken() @@ -210,12 +210,12 @@ func (t *TokenPayload) initWorkspaceAndGetClient(config *service.DBApiClientConf } // Get workspace personal access token - workspaceAccessTokenResp, err := getWorkspaceAccessToken(config, managementToken, adbWorkspaceUrl, adbWorkspace.ID, adbPlatformToken) + workspaceAccessTokenResp, err := getWorkspaceAccessToken(config, managementToken, adbWorkspaceURL, adbWorkspace.ID, adbPlatformToken) if err != nil { return err } - config.Host = adbWorkspaceUrl + config.Host = adbWorkspaceURL config.Token = workspaceAccessTokenResp.TokenValue if workspaceAccessTokenResp.TokenInfo != nil { config.TokenExpiryTime = workspaceAccessTokenResp.TokenInfo.ExpiryTime diff --git a/databricks/mounts_test.go b/databricks/mounts_test.go index 173e7f578..86ac8678c 100644 --- a/databricks/mounts_test.go +++ b/databricks/mounts_test.go @@ -46,7 +46,7 @@ func TestAzureBlobMountReadRetrievesMountInformation(t *testing.T) { }, { Name: "Mount found - but does not match configuration", - ExpectedError: fmt.Errorf("does not match uri with storage account and container values %s@%s != abfss://x@y.dfs.core.windows.net/z!", cn, sacc), + ExpectedError: fmt.Errorf("does not match uri with storage account and container values %s@%s != abfss://x@y.dfs.core.windows.net/z", cn, sacc), CommandResult: &model.CommandResults{ ResultType: "text", Data: "abfss://x@y.dfs.core.windows.net/z", @@ -109,7 +109,7 @@ func TestAzureADLSGen1MountReadRetrievesMountInformation(t *testing.T) { }, { Name: "Mount found - but does not match configuration", - ExpectedError: fmt.Errorf("does not match uri with storage account and container values %s@%s != adl://x.azuredatalakestore.net/z!", sacc, dir), + ExpectedError: fmt.Errorf("does not match uri with storage account and container values %s@%s != adl://x.azuredatalakestore.net/z", sacc, dir), CommandResult: &model.CommandResults{ ResultType: "text", Data: "adl://x.azuredatalakestore.net/z", @@ -173,7 +173,7 @@ func TestAzureADLSGen2MountReadRetrievesMountInformation(t *testing.T) { }, { Name: "Mount found - but does not match configuration", - ExpectedError: fmt.Errorf("does not match uri with storage account and container values %s@%s != abfss://x@y.dfs.core.windows.net/z!", cn, sacc), + ExpectedError: fmt.Errorf("does not match uri with storage account and container values %s@%s != abfss://x@y.dfs.core.windows.net/z", cn, sacc), CommandResult: &model.CommandResults{ ResultType: "text", Data: "abfss://x@y.dfs.core.windows.net/z", diff --git a/databricks/provider.go b/databricks/provider.go index 4aaedb15c..1683c1baf 100644 --- a/databricks/provider.go +++ b/databricks/provider.go @@ -308,33 +308,29 @@ func providerConfigure(d *schema.ResourceData, providerVersion string) (interfac // Abstracted logic to another function that returns a interface{}, error to inject directly // for the providers during cloud integration testing return providerConfigureAzureClient(d, &config) - } else { - if host, ok := d.GetOk("host"); ok { - config.Host = host.(string) - } - if token, ok := d.GetOk("token"); ok { - config.Token = token.(string) - } - - // Basic authentication setup via username and password - if _, ok := d.GetOk("basic_auth"); ok { - username, userOk := d.GetOk("basic_auth.0.username") - password, passOk := d.GetOk("basic_auth.0.password") - if userOk && passOk { - tokenUnB64 := fmt.Sprintf("%s:%s", username.(string), password.(string)) - config.Token = base64.StdEncoding.EncodeToString([]byte(tokenUnB64)) - config.AuthType = service.BasicAuth - } + } + if host, ok := d.GetOk("host"); ok { + config.Host = host.(string) + } + if token, ok := d.GetOk("token"); ok { + config.Token = token.(string) + } + // Basic authentication setup via username and password + if _, ok := d.GetOk("basic_auth"); ok { + username, userOk := d.GetOk("basic_auth.0.username") + password, passOk := d.GetOk("basic_auth.0.password") + if userOk && passOk { + tokenUnB64 := fmt.Sprintf("%s:%s", username.(string), password.(string)) + config.Token = base64.StdEncoding.EncodeToString([]byte(tokenUnB64)) + config.AuthType = service.BasicAuth } - - // Final catch all in case basic_auth/token + host is not setup - if config.Host == "" || config.Token == "" { - if err := tryDatabricksCliConfigFile(d, &config); err != nil { - return nil, fmt.Errorf("failed to get credentials from config file; error msg: %w", err) - } + } + // Final catch all in case basic_auth/token + host is not setup + if config.Host == "" || config.Token == "" { + if err := tryDatabricksCliConfigFile(d, &config); err != nil { + return nil, fmt.Errorf("failed to get credentials from config file; error msg: %w", err) } } - var dbClient service.DBApiClient dbClient.SetConfig(&config) return &dbClient, nil diff --git a/databricks/resource_databricks_group_instance_profile.go b/databricks/resource_databricks_group_instance_profile.go index 424804cde..aa104ed14 100644 --- a/databricks/resource_databricks_group_instance_profile.go +++ b/databricks/resource_databricks_group_instance_profile.go @@ -36,7 +36,7 @@ func resourceGroupInstanceProfileCreate(d *schema.ResourceData, m interface{}) e client := m.(*service.DBApiClient) groupID := d.Get("group_id").(string) instanceProfileID := d.Get("instance_profile_id").(string) - groupInstanceProfileID := &GroupInstanceProfileID{ + groupInstanceProfileID := &groupInstanceProfileID{ GroupID: groupID, InstanceProfileID: instanceProfileID, } @@ -54,7 +54,7 @@ func resourceGroupInstanceProfileCreate(d *schema.ResourceData, m interface{}) e func resourceGroupInstanceProfileRead(d *schema.ResourceData, m interface{}) error { id := d.Id() client := m.(*service.DBApiClient) - groupInstanceProfileID := parseGroupInstanceProfileID(id) + groupInstanceProfileID := parsegroupInstanceProfileID(id) group, err := client.Groups().Read(groupInstanceProfileID.GroupID) // First verify if the group exists @@ -86,7 +86,7 @@ func resourceGroupInstanceProfileRead(d *schema.ResourceData, m interface{}) err func resourceGroupInstanceProfileDelete(d *schema.ResourceData, m interface{}) error { id := d.Id() client := m.(*service.DBApiClient) - groupInstanceProfileID := parseGroupInstanceProfileID(id) + groupInstanceProfileID := parsegroupInstanceProfileID(id) roleRemoveList := []string{groupInstanceProfileID.InstanceProfileID} // Patch op to remove role from group @@ -94,18 +94,18 @@ func resourceGroupInstanceProfileDelete(d *schema.ResourceData, m interface{}) e return err } -type GroupInstanceProfileID struct { +type groupInstanceProfileID struct { GroupID string InstanceProfileID string } -func (g GroupInstanceProfileID) String() string { +func (g groupInstanceProfileID) String() string { return fmt.Sprintf("%s|%s", g.GroupID, g.InstanceProfileID) } -func parseGroupInstanceProfileID(id string) *GroupInstanceProfileID { +func parsegroupInstanceProfileID(id string) *groupInstanceProfileID { parts := strings.Split(id, "|") - return &GroupInstanceProfileID{ + return &groupInstanceProfileID{ GroupID: parts[0], InstanceProfileID: parts[1], } diff --git a/databricks/resource_databricks_group_member.go b/databricks/resource_databricks_group_member.go index 61d965af3..62afdd668 100644 --- a/databricks/resource_databricks_group_member.go +++ b/databricks/resource_databricks_group_member.go @@ -37,7 +37,7 @@ func resourceGroupMemberCreate(d *schema.ResourceData, m interface{}) error { groupID := d.Get("group_id").(string) memberID := d.Get("member_id").(string) - groupMemberID := &GroupMemberID{ + groupMemberID := &groupMemberID{ GroupID: groupID, MemberID: memberID, } @@ -55,7 +55,7 @@ func resourceGroupMemberCreate(d *schema.ResourceData, m interface{}) error { func resourceGroupMemberRead(d *schema.ResourceData, m interface{}) error { id := d.Id() client := m.(*service.DBApiClient) - groupMemberID := parseGroupMemberID(id) + groupMemberID := parsegroupMemberID(id) group, err := client.Groups().Read(groupMemberID.GroupID) // First verify if the group exists @@ -87,7 +87,7 @@ func resourceGroupMemberRead(d *schema.ResourceData, m interface{}) error { func resourceGroupMemberDelete(d *schema.ResourceData, m interface{}) error { id := d.Id() client := m.(*service.DBApiClient) - groupMemberID := parseGroupMemberID(id) + groupMemberID := parsegroupMemberID(id) memberRemoveList := []string{groupMemberID.MemberID} // Patch op to remove member from group @@ -95,18 +95,18 @@ func resourceGroupMemberDelete(d *schema.ResourceData, m interface{}) error { return err } -type GroupMemberID struct { +type groupMemberID struct { GroupID string MemberID string } -func (g GroupMemberID) String() string { +func (g groupMemberID) String() string { return fmt.Sprintf("%s|%s", g.GroupID, g.MemberID) } -func parseGroupMemberID(id string) *GroupMemberID { +func parsegroupMemberID(id string) *groupMemberID { parts := strings.Split(id, "|") - return &GroupMemberID{ + return &groupMemberID{ GroupID: parts[0], MemberID: parts[1], } diff --git a/databricks/utils_test.go b/databricks/utils_test.go index e2384909e..5a571592e 100644 --- a/databricks/utils_test.go +++ b/databricks/utils_test.go @@ -1,16 +1,21 @@ package databricks import ( + "bytes" + "encoding/json" "errors" "fmt" + "net/http" + "net/http/httptest" "os" "strconv" "testing" - "github.com/databrickslabs/databricks-terraform/client/service" "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" - + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/stretchr/testify/assert" + + "github.com/databrickslabs/databricks-terraform/client/service" ) func TestMissingMWSResources(t *testing.T) { @@ -18,9 +23,9 @@ func TestMissingMWSResources(t *testing.T) { t.Skip("skipping integration test in short mode.") } - mwsAcctId := os.Getenv("DATABRICKS_MWS_ACCT_ID") - randStringId := acctest.RandString(10) - randIntId := 2000000 + acctest.RandIntRange(100000, 20000000) + mwsAcctID := os.Getenv("DATABRICKS_MWS_ACCT_ID") + randStringID := acctest.RandString(10) + randIntID := 2000000 + acctest.RandIntRange(100000, 20000000) client := getMWSClient() tests := []struct { @@ -33,28 +38,28 @@ func TestMissingMWSResources(t *testing.T) { { name: "CheckIfMWSCredentialsAreMissing", readFunc: func() error { - _, err := client.MWSCredentials().Read(mwsAcctId, randStringId) + _, err := client.MWSCredentials().Read(mwsAcctID, randStringID) return err }, }, { name: "CheckIfMWSNetworksAreMissing", readFunc: func() error { - _, err := client.MWSNetworks().Read(mwsAcctId, randStringId) + _, err := client.MWSNetworks().Read(mwsAcctID, randStringID) return err }, }, { name: "CheckIfMWSStorageConfigurationsAreMissing", readFunc: func() error { - _, err := client.MWSStorageConfigurations().Read(mwsAcctId, randStringId) + _, err := client.MWSStorageConfigurations().Read(mwsAcctID, randStringID) return err }, }, { name: "CheckIfMWSWorkspacesAreMissing", readFunc: func() error { - _, err := client.MWSWorkspaces().Read(mwsAcctId, int64(randIntId)) + _, err := client.MWSWorkspaces().Read(mwsAcctID, int64(randIntID)) return err }, }, @@ -86,14 +91,14 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi t.Skip("Acceptance tests skipped unless env 'TF_ACC' set") } - randIntId := 2000000 + acctest.RandIntRange(100000, 20000000) - randStringId := acctest.RandString(10) + randIntID := 2000000 + acctest.RandIntRange(100000, 20000000) + randStringID := acctest.RandString(10) // example 405E7E8E4A000024 - randomClusterPolicyId := fmt.Sprintf("400E9E9E9A%d", + randomClusterPolicyID := fmt.Sprintf("400E9E9E9A%d", acctest.RandIntRange(100000, 999999), ) // example 0101-120000-brick1-pool-ABCD1234 - randomInstancePoolId := fmt.Sprintf( + randomInstancePoolID := fmt.Sprintf( "%v-%v-%s-pool-%s", acctest.RandIntRange(1000, 9999), acctest.RandIntRange(100000, 999999), @@ -113,35 +118,35 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi { name: "CheckIfTokensAreMissing", readFunc: func() error { - _, err := client.Tokens().Read(randStringId) + _, err := client.Tokens().Read(randStringID) return err }, }, { name: "CheckIfSecretScopesAreMissing", readFunc: func() error { - _, err := client.SecretScopes().Read(randStringId) + _, err := client.SecretScopes().Read(randStringID) return err }, }, { name: "CheckIfSecretsAreMissing", readFunc: func() error { - _, err := client.Secrets().Read(randStringId, randStringId) + _, err := client.Secrets().Read(randStringID, randStringID) return err }, }, { name: "CheckIfSecretsACLsAreMissing", readFunc: func() error { - _, err := client.SecretAcls().Read(randStringId, randStringId) + _, err := client.SecretAcls().Read(randStringID, randStringID) return err }, }, { name: "CheckIfSecretsACLsAreMissing", readFunc: func() error { - _, err := client.SecretAcls().Read(randStringId, randStringId) + _, err := client.SecretAcls().Read(randStringID, randStringID) return err }, }, @@ -149,38 +154,38 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi name: "CheckIfNotebooksAreMissing", readFunc: func() error { // ID must start with a / - _, err := client.Notebooks().Read("/" + randStringId) + _, err := client.Notebooks().Read("/" + randStringID) return err }, }, { name: "CheckIfInstancePoolsAreMissing", readFunc: func() error { - _, err := client.InstancePools().Read(randomInstancePoolId) + _, err := client.InstancePools().Read(randomInstancePoolID) return err }, }, { name: "CheckIfClustersAreMissing", readFunc: func() error { - _, err := client.Clusters().Get(randStringId) + _, err := client.Clusters().Get(randStringID) return err }, isCustomCheck: true, customCheckFunc: isClusterMissing, - resourceID: randStringId, + resourceID: randStringID, }, { name: "CheckIfDBFSFilesAreMissing", readFunc: func() error { - _, err := client.DBFS().Read("/" + randStringId) + _, err := client.DBFS().Read("/" + randStringID) return err }, }, { name: "CheckIfGroupsAreMissing", readFunc: func() error { - _, err := client.Groups().Read(randStringId) + _, err := client.Groups().Read(randStringID) t.Log(err) return err }, @@ -188,7 +193,7 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi { name: "CheckIfUsersAreMissing", readFunc: func() error { - _, err := client.Users().Read(randStringId) + _, err := client.Users().Read(randStringID) t.Log(err) return err }, @@ -196,7 +201,7 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi { name: "CheckIfClusterPoliciesAreMissing", readFunc: func() error { - _, err := client.ClusterPolicies().Get(randomClusterPolicyId) + _, err := client.ClusterPolicies().Get(randomClusterPolicyID) t.Log(err) return err }, @@ -204,12 +209,12 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi { name: "CheckIfJobsAreMissing", readFunc: func() error { - _, err := client.Jobs().Read(int64(randIntId)) + _, err := client.Jobs().Read(int64(randIntID)) return err }, isCustomCheck: true, customCheckFunc: isJobMissing, - resourceID: strconv.Itoa(randIntId), + resourceID: strconv.Itoa(randIntID), }, } // Handle aws only tests where instance profiles only exist on aws @@ -218,7 +223,7 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi { name: "CheckIfInstanceProfilesAreMissing", readFunc: func() error { - _, err := client.InstanceProfiles().Read(randStringId) + _, err := client.InstanceProfiles().Read(randStringID) return err }, }, @@ -237,13 +242,13 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi } } -func testVerifyResourceIsMissingCustomVerification(t *testing.T, resourceId string, readFunc func() error, +func testVerifyResourceIsMissingCustomVerification(t *testing.T, resourceID string, readFunc func() error, customCheck func(err error, rId string) bool) { err := readFunc() assert.NotNil(t, err, "err should not be nil") assert.IsType(t, err, service.APIError{}, fmt.Sprintf("error: %s is not type api error", err.Error())) if apiError, ok := err.(service.APIError); ok { - assert.True(t, customCheck(err, resourceId), fmt.Sprintf("error: %v is not missing;"+ + assert.True(t, customCheck(err, resourceID), fmt.Sprintf("error: %v is not missing;"+ "\nstatus code: %v;"+ "\nerror code: %s", apiError, apiError.StatusCode, apiError.ErrorCode)) @@ -260,17 +265,7 @@ func testVerifyResourceIsMissing(t *testing.T, readFunc func() error) { "\nerror code: %s", apiError, apiError.StatusCode, apiError.ErrorCode)) } - "bytes" - "encoding/json" - "fmt" - "net/http" - "net/http/httptest" - "testing" - - "github.com/databrickslabs/databricks-terraform/client/service" - "github.com/hashicorp/terraform-plugin-sdk/helper/schema" - "github.com/stretchr/testify/assert" -) +} // HTTPFixture defines request structure for test type HTTPFixture struct { From ef4ed409b7b4bdf2105f72fdbbcab922c22d1091 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 11:36:09 +0200 Subject: [PATCH 16/42] Added make install --- Makefile | 20 ++++++++++++-------- databricks/mounts.go | 6 +++--- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index f603c8315..8cea675c4 100644 --- a/Makefile +++ b/Makefile @@ -26,14 +26,6 @@ coverage-int: int int-build: int build -build: lint test - @echo "==> Building source code with go build..." - @go build -mod vendor -v -o terraform-provider-databricks - -lint: - @echo "==> Linting source code with golangci-lint make sure you run make fmt ..." - @golangci-lint run --skip-dirs-use-default --timeout 5m - fmt: @echo "==> Formatting source code with gofmt..." @goimports -w client @@ -44,6 +36,18 @@ fmt: @gofmt -s -w main.go @go fmt ./... +lint: fmt + @echo "==> Linting source code with golangci-lint make sure you run make fmt ..." + @golangci-lint run --skip-dirs-use-default --timeout 5m + +build: lint test + @echo "==> Building source code with go build..." + @go build -mod vendor -v -o terraform-provider-databricks + +install: build + @rm ~/.terraform.d/plugins/terraform-provider-databricks* + @mv terraform-provider-databricks ~/.terraform.d/plugins + vendor: @echo "==> Filling vendor folder with library code..." @go mod vendor diff --git a/databricks/mounts.go b/databricks/mounts.go index a5a3cedbd..410d8963c 100644 --- a/databricks/mounts.go +++ b/databricks/mounts.go @@ -189,7 +189,7 @@ for mount in dbutils.fs.mounts(): storageAccount != m.StorageAccountName && m.Directory != directory { return "", fmt.Errorf("does not match uri with storage account and container values"+ - " %s@%s != %s!", m.ContainerName, m.StorageAccountName, resp.Results.Data.(string)) + " %s@%s != %s", m.ContainerName, m.StorageAccountName, resp.Results.Data.(string)) } return resp.Results.Data.(string), nil } @@ -289,7 +289,7 @@ for mount in dbutils.fs.mounts(): if resp.Results.ResultType == "text" && storageResource != m.StorageResource && m.Directory != directory { return "", fmt.Errorf("does not match uri with storage account and container values"+ - " %s@%s != %s!", m.StorageResource, m.Directory, resp.Results.Data.(string)) + " %s@%s != %s", m.StorageResource, m.Directory, resp.Results.Data.(string)) } return resp.Results.Data.(string), nil } @@ -395,7 +395,7 @@ for mount in dbutils.fs.mounts(): if resp.Results.ResultType == "text" && containerName != m.ContainerName && m.StorageAccountName != storageAccountName && m.Directory != directory { return "", fmt.Errorf("does not match uri with storage account and container values"+ - " %s@%s != %s!", m.ContainerName, m.StorageAccountName, resp.Results.Data.(string)) + " %s@%s != %s", m.ContainerName, m.StorageAccountName, resp.Results.Data.(string)) } return resp.Results.Data.(string), nil } From 6656e48aa5f97e792582f15495d4cea5585cbcad Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 11:46:35 +0200 Subject: [PATCH 17/42] make build will also test now (provider + client) --- .travis.yml | 5 +---- Makefile | 25 +++++++++++++------------ 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/.travis.yml b/.travis.yml index 83caf834a..d2a0bbbf4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,8 +9,6 @@ jobs: script: - curl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsum - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0 - - time make provider-test - - time make client-test - time make build - go: 1.14.x gemfile: gemfiles/Gemfile.rails-3.0.x @@ -21,8 +19,6 @@ jobs: script: - curl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsum - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0 - - time make provider-test - - time make client-test - time make build @@ -30,6 +26,7 @@ after_success: - echo "travis go version='$TRAVIS_GO_VERSION'" - bash <(curl -s https://codecov.io/bash) -f client-coverage.txt -F client - bash <(curl -s https://codecov.io/bash) -f provider-coverage.txt -F provider + - bash <(curl -s https://codecov.io/bash) -f coverage.txt -F repository notifications: webhooks: https://www.travisbuddy.com/ diff --git a/Makefile b/Makefile index 8cea675c4..54e87d66d 100644 --- a/Makefile +++ b/Makefile @@ -1,17 +1,5 @@ default: build -test: lint - @echo "==> Running tests..." - @gotestsum --format short-verbose --raw-command go test -v -json -short -coverprofile=coverage.txt ./... - -client-test: - @echo "==> Running tests..." - @gotestsum --format short-verbose --raw-command go test -v -json -short -coverprofile=client-coverage.txt ./client/... - -provider-test: - @echo "==> Running tests..." - @gotestsum --format short-verbose --raw-command go test -v -json -short -coverprofile=provider-coverage.txt ./databricks/... - int: @echo "==> Running tests..." @gotestsum --raw-command go test -v -json -coverprofile=coverage.txt ./... @@ -40,11 +28,24 @@ lint: fmt @echo "==> Linting source code with golangci-lint make sure you run make fmt ..." @golangci-lint run --skip-dirs-use-default --timeout 5m +client-test: + @echo "==> Running tests..." + @gotestsum --format short-verbose --raw-command go test -v -json -short -coverprofile=client-coverage.txt ./client/... + +provider-test: + @echo "==> Running tests..." + @gotestsum --format short-verbose --raw-command go test -v -json -short -coverprofile=provider-coverage.txt ./databricks/... + +test: lint client-test provider-test + @echo "==> Running tests..." + @gotestsum --format short-verbose --raw-command go test -v -json -short -coverprofile=coverage.txt ./... + build: lint test @echo "==> Building source code with go build..." @go build -mod vendor -v -o terraform-provider-databricks install: build + @echo "==> Installing provider into ~/.terraform.d/plugins ..." @rm ~/.terraform.d/plugins/terraform-provider-databricks* @mv terraform-provider-databricks ~/.terraform.d/plugins From 7d09b9d1bf3477564d238324579abd34fba10652 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 13:26:39 +0200 Subject: [PATCH 18/42] Added more coverage for edge cases on permissions --- client/model/notebook.go | 4 +- client/service/client.go | 5 +- client/service/clusters_test.go | 4 +- client/service/notebooks.go | 14 +- client/service/notebooks_test.go | 14 +- databricks/resource_databricks_permissions.go | 4 + .../resource_databricks_permissions_test.go | 252 ++++++++++++++++-- databricks/utils_test.go | 41 ++- 8 files changed, 301 insertions(+), 37 deletions(-) diff --git a/client/model/notebook.go b/client/model/notebook.go index fa078836d..e6a28b33a 100644 --- a/client/model/notebook.go +++ b/client/model/notebook.go @@ -32,8 +32,8 @@ const ( LibraryObject ObjectType = "LIBRARY" ) -// NotebookInfo contains information when doing a get request or list request on the workspace api -type NotebookInfo struct { +// WorkspaceObjectStatus contains information when doing a get request or list request on the workspace api +type WorkspaceObjectStatus struct { ObjectID int64 `json:"object_id,omitempty"` ObjectType ObjectType `json:"object_type,omitempty"` Path string `json:"path,omitempty"` diff --git a/client/service/client.go b/client/service/client.go index f80c32bb0..c2538c243 100644 --- a/client/service/client.go +++ b/client/service/client.go @@ -29,7 +29,8 @@ const ( Azure CloudServiceProvider = "Azure" ) -type apiErrorBody struct { +// APIErrorBody maps "proper" databricks rest api errors to a struct +type APIErrorBody struct { ErrorCode string `json:"error_code,omitempty"` Message string `json:"message,omitempty"` // The following two are for scim api only for RFC 7644 Section 3.7.3 https://tools.ietf.org/html/rfc7644#section-3.7.3 @@ -154,7 +155,7 @@ func checkHTTPRetry(ctx context.Context, resp *http.Response, err error) (bool, if err != nil { return false, err } - var errorBody apiErrorBody + var errorBody APIErrorBody err = json.Unmarshal(body, &errorBody) // this is most likely HTML... since un-marshalling JSON failed if err != nil { diff --git a/client/service/clusters_test.go b/client/service/clusters_test.go index 4be6dedb8..b7a46d723 100644 --- a/client/service/clusters_test.go +++ b/client/service/clusters_test.go @@ -459,7 +459,7 @@ func TestClustersAPI_WaitForClusterRunning(t *testing.T) { requestMethod []string args []interface{} wantURI []string - want []model.NotebookInfo + want []model.WorkspaceObjectStatus wantErr bool }{ { @@ -605,7 +605,7 @@ func TestClustersAPI_WaitForClusterTerminated(t *testing.T) { requestMethod []string args []interface{} wantURI []string - want []model.NotebookInfo + want []model.WorkspaceObjectStatus wantErr bool }{ { diff --git a/client/service/notebooks.go b/client/service/notebooks.go index 239124462..507ba86ae 100644 --- a/client/service/notebooks.go +++ b/client/service/notebooks.go @@ -32,8 +32,8 @@ func (a NotebooksAPI) Create(path string, content string, language model.Languag } // Read returns the notebook metadata and not the contents -func (a NotebooksAPI) Read(path string) (model.NotebookInfo, error) { - var notebookInfo model.NotebookInfo +func (a NotebooksAPI) Read(path string) (model.WorkspaceObjectStatus, error) { + var notebookInfo model.WorkspaceObjectStatus notebookGetStatusRequest := struct { Path string `json:"path,omitempty" url:"path,omitempty"` }{} @@ -79,9 +79,9 @@ func (a NotebooksAPI) Mkdirs(path string) error { // List will list all objects in a path on the workspace and with the recursive flag it will recursively list // all the objects -func (a NotebooksAPI) List(path string, recursive bool) ([]model.NotebookInfo, error) { +func (a NotebooksAPI) List(path string, recursive bool) ([]model.WorkspaceObjectStatus, error) { if recursive { - var paths []model.NotebookInfo + var paths []model.WorkspaceObjectStatus err := a.recursiveAddPaths(path, &paths) if err != nil { return nil, err @@ -91,7 +91,7 @@ func (a NotebooksAPI) List(path string, recursive bool) ([]model.NotebookInfo, e return a.list(path) } -func (a NotebooksAPI) recursiveAddPaths(path string, pathList *[]model.NotebookInfo) error { +func (a NotebooksAPI) recursiveAddPaths(path string, pathList *[]model.WorkspaceObjectStatus) error { notebookInfoList, err := a.list(path) if err != nil { return err @@ -109,9 +109,9 @@ func (a NotebooksAPI) recursiveAddPaths(path string, pathList *[]model.NotebookI return err } -func (a NotebooksAPI) list(path string) ([]model.NotebookInfo, error) { +func (a NotebooksAPI) list(path string) ([]model.WorkspaceObjectStatus, error) { var notebookList struct { - Objects []model.NotebookInfo `json:"objects,omitempty" url:"objects,omitempty"` + Objects []model.WorkspaceObjectStatus `json:"objects,omitempty" url:"objects,omitempty"` } listRequest := struct { Path string `json:"path,omitempty" url:"path,omitempty"` diff --git a/client/service/notebooks_test.go b/client/service/notebooks_test.go index d1a6a66d8..3bcc13aab 100644 --- a/client/service/notebooks_test.go +++ b/client/service/notebooks_test.go @@ -119,7 +119,7 @@ func TestNotebooksAPI_ListNonRecursive(t *testing.T) { responseStatus int args args wantURI string - want []model.NotebookInfo + want []model.WorkspaceObjectStatus wantErr bool }{ { @@ -146,7 +146,7 @@ func TestNotebooksAPI_ListNonRecursive(t *testing.T) { Recursive: false, }, wantURI: "/api/2.0/workspace/list?path=%2Ftest%2Fpath", - want: []model.NotebookInfo{ + want: []model.WorkspaceObjectStatus{ { ObjectID: 123, ObjectType: model.Directory, @@ -183,7 +183,7 @@ func TestNotebooksAPI_ListRecursive(t *testing.T) { responseStatus []int args []interface{} wantURI []string - want []model.NotebookInfo + want []model.WorkspaceObjectStatus wantErr bool }{ { @@ -222,7 +222,7 @@ func TestNotebooksAPI_ListRecursive(t *testing.T) { }, }, wantURI: []string{"/api/2.0/workspace/list?path=%2Ftest%2Fpath", "/api/2.0/workspace/list?path=%2FUsers%2Fuser%40example.com%2Fproject"}, - want: []model.NotebookInfo{ + want: []model.WorkspaceObjectStatus{ { ObjectID: 457, ObjectType: model.Notebook, @@ -288,7 +288,7 @@ func TestNotebooksAPI_Read(t *testing.T) { args args responseStatus int wantURI string - want model.NotebookInfo + want model.WorkspaceObjectStatus wantErr bool }{ { @@ -303,7 +303,7 @@ func TestNotebooksAPI_Read(t *testing.T) { Path: "/test/path", }, responseStatus: http.StatusOK, - want: model.NotebookInfo{ + want: model.WorkspaceObjectStatus{ ObjectID: 789, ObjectType: model.Notebook, Path: "/Users/user@example.com/project/ScalaExampleNotebook", @@ -320,7 +320,7 @@ func TestNotebooksAPI_Read(t *testing.T) { Path: "/test/path", }, responseStatus: http.StatusBadRequest, - want: model.NotebookInfo{}, + want: model.WorkspaceObjectStatus{}, wantURI: "/api/2.0/workspace/get-status?path=%2Ftest%2Fpath", wantErr: true, }, diff --git a/databricks/resource_databricks_permissions.go b/databricks/resource_databricks_permissions.go index 914629d96..4b8db33fa 100644 --- a/databricks/resource_databricks_permissions.go +++ b/databricks/resource_databricks_permissions.go @@ -75,6 +75,10 @@ func resourcePermissionsRead(d *schema.ResourceData, m interface{}) error { id := d.Id() client := m.(*service.DBApiClient) objectACL, err := client.Permissions().Read(id) + if e, ok := err.(service.APIError); ok && e.IsMissing() { + d.SetId("") + return nil + } if err != nil { return err } diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index 547146e0a..0b9349e24 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -56,11 +56,10 @@ func TestPermissionsRead(t *testing.T) { UserName: TestingAdminUser, }, }, - }, resourcePermissions, map[string]interface{}{}, - func(d *schema.ResourceData, c interface{}) error { - d.SetId("/clusters/abc") - return resourcePermissionsRead(d, c) - }) + }, resourcePermissions, nil, func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsRead(d, c) + }) assert.NoError(t, err, err) assert.Equal(t, "/clusters/abc", d.Id()) assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) @@ -68,6 +67,72 @@ func TestPermissionsRead(t *testing.T) { assert.Equal(t, 1, d.Get("access_control.#")) } +func TestPermissionsRead_some_error(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: service.APIErrorBody{ + ErrorCode: "INVALID_REQUEST", + Message: "Internal error happened", + }, + Status: 400, + }, + }, resourcePermissions, map[string]interface{}{}, + func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsRead(d, c) + }) + assert.Error(t, err) +} + +func TestPermissionsRead_ErrorOnScimMe(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: model.ObjectACL{ + ObjectID: "/clusters/abc", + ObjectType: "clusters", + AccessControlList: []*model.AccessControl{ + { + UserName: &TestingUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_READ", + Inherited: false, + }, + }, + }, + { + UserName: &TestingAdminUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_MANAGE", + Inherited: false, + }, + }, + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: service.APIErrorBody{ + ErrorCode: "INVALID_REQUEST", + Message: "Internal error happened", + }, + Status: 400, + }, + }, resourcePermissions, map[string]interface{}{}, + func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsRead(d, c) + }) + assert.Error(t, err) +} + func TestPermissionsDelete(t *testing.T) { d, err := ResourceTester(t, []HTTPFixture{ { @@ -75,18 +140,36 @@ func TestPermissionsDelete(t *testing.T) { Resource: "/api/2.0/preview/permissions/clusters/abc", ExpectedRequest: model.ObjectACL{}, }, - }, resourcePermissions, map[string]interface{}{}, - func(d *schema.ResourceData, c interface{}) error { - d.SetId("/clusters/abc") - return resourcePermissionsDelete(d, c) - }) + }, resourcePermissions, nil, func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsDelete(d, c) + }) assert.NoError(t, err, err) assert.Equal(t, "/clusters/abc", d.Id()) } +func TestPermissionsDelete_error(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPut, + Resource: "/api/2.0/preview/permissions/clusters/abc", + ExpectedRequest: model.ObjectACL{}, + Response: service.APIErrorBody{ + ErrorCode: "INVALID_REQUEST", + Message: "Internal error happened", + }, + Status: 400, + }, + }, resourcePermissions, nil, func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsDelete(d, c) + }) + assert.Error(t, err) +} + func TestPermissionsCreate_invalid(t *testing.T) { - _, err := ResourceTester(t, []HTTPFixture{}, resourcePermissions, map[string]interface{}{}, - resourcePermissionsCreate) + _, err := ResourceTester(t, []HTTPFixture{}, resourcePermissions, + nil, resourcePermissionsCreate) assert.EqualError(t, err, "At least one type of resource identifiers must be set") } @@ -95,7 +178,22 @@ func TestPermissionsCreate_no_access_control(t *testing.T) { map[string]interface{}{ "cluster_id": "abc", }, resourcePermissionsCreate) - assert.EqualError(t, err, "At least one access_control is required") + assert.EqualError(t, err, "Invalid config supplied. access_control: required field is not set") +} + +func TestPermissionsCreate_conflicting_fields(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{}, resourcePermissions, + map[string]interface{}{ + "cluster_id": "abc", + "notebook_path": "/Init", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_READ", + }, + }, + }, resourcePermissionsCreate) + assert.EqualError(t, err, "Invalid config supplied. cluster_id: conflicts with notebook_path. notebook_path: conflicts with cluster_id") } func TestPermissionsCreate(t *testing.T) { @@ -107,7 +205,7 @@ func TestPermissionsCreate(t *testing.T) { AccessControlList: []*model.AccessControlChange{ { UserName: &TestingUser, - PermissionLevel: "CAN_USE", + PermissionLevel: "CAN_READ", }, }, }, @@ -152,7 +250,7 @@ func TestPermissionsCreate(t *testing.T) { "access_control": []interface{}{ map[string]interface{}{ "user_name": TestingUser, - "permission_level": "CAN_USE", + "permission_level": "CAN_READ", }, }, }, resourcePermissionsCreate) @@ -163,6 +261,130 @@ func TestPermissionsCreate(t *testing.T) { assert.Equal(t, 1, d.Get("access_control.#")) } +func TestPermissionsCreate_NotebookPath_NotExists(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodGet, + Resource: "/api/2.0/workspace/get-status?path=%2FDevelopment%2FInit", + Response: service.APIErrorBody{ + ErrorCode: "INVALID_REQUEST", + Message: "Internal error happened", + }, + Status: 400, + }, + }, resourcePermissions, map[string]interface{}{ + "notebook_path": "/Development/Init", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_USE", + }, + }, + }, resourcePermissionsCreate) + + assert.Error(t, err) +} + +func TestPermissionsCreate_NotebookPath(t *testing.T) { + d, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodGet, + Resource: "/api/2.0/workspace/get-status?path=%2FDevelopment%2FInit", + Response: model.WorkspaceObjectStatus{ + ObjectID: 988765, + ObjectType: "NOTEBOOK", + }, + }, + { + Method: http.MethodPatch, + Resource: "/api/2.0/preview/permissions/notebooks/988765", + ExpectedRequest: model.AccessControlChangeList{ + AccessControlList: []*model.AccessControlChange{ + { + UserName: &TestingUser, + PermissionLevel: "CAN_USE", + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/notebooks/988765?", + Response: model.ObjectACL{ + ObjectID: "/notebooks/988765", + ObjectType: "notebooks", + AccessControlList: []*model.AccessControl{ + { + UserName: &TestingUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_USE", + Inherited: false, + }, + }, + }, + { + UserName: &TestingAdminUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_MANAGE", + Inherited: false, + }, + }, + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: model.User{ + UserName: TestingAdminUser, + }, + }, + }, resourcePermissions, map[string]interface{}{ + "notebook_path": "/Development/Init", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_USE", + }, + }, + }, resourcePermissionsCreate) + + assert.NoError(t, err, err) + assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) + assert.Equal(t, "CAN_USE", d.Get("access_control.0.permission_level")) + assert.Equal(t, 1, d.Get("access_control.#")) +} + +func TestPermissionsCreate_error(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPatch, + Resource: "/api/2.0/preview/permissions/clusters/abc", + Response: service.APIErrorBody{ + ErrorCode: "INVALID_REQUEST", + Message: "Internal error happened", + }, + Status: 400, + }, + }, resourcePermissions, map[string]interface{}{ + "cluster_id": "abc", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_USE", + }, + }, + }, resourcePermissionsCreate) + if assert.Error(t, err) { + if e, ok := err.(service.APIError); ok { + assert.Equal(t, "INVALID_REQUEST", e.ErrorCode) + } + } +} + func TestAccDatabricksPermissionsResourceFullLifecycle(t *testing.T) { var permissions model.ObjectACL randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) diff --git a/databricks/utils_test.go b/databricks/utils_test.go index 5a571592e..808a5b8e1 100644 --- a/databricks/utils_test.go +++ b/databricks/utils_test.go @@ -8,11 +8,14 @@ import ( "net/http" "net/http/httptest" "os" + "sort" "strconv" + "strings" "testing" "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/terraform" "github.com/stretchr/testify/assert" "github.com/databrickslabs/databricks-terraform/client/service" @@ -267,6 +270,12 @@ func testVerifyResourceIsMissing(t *testing.T, readFunc func() error) { } } +type errorSlice []error + +func (a errorSlice) Len() int { return len(a) } +func (a errorSlice) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a errorSlice) Less(i, j int) bool { return a[i].Error() < a[j].Error() } + // HTTPFixture defines request structure for test type HTTPFixture struct { Method string @@ -326,8 +335,36 @@ func ResourceTester(t *testing.T, var client service.DBApiClient client.SetConfig(&config) - resource := resouceFunc() - resourceData := schema.TestResourceDataRaw(t, resource.Schema, state) + res := resouceFunc() + + if state != nil { + resourceConfig := terraform.NewResourceConfigRaw(state) + warns, errs := res.Validate(resourceConfig) + if len(warns) > 0 || len(errs) > 0 { + var issues string + if len(warns) > 0 { + sort.Strings(warns) + issues += ". " + strings.Join(warns, ". ") + } + if len(errs) > 0 { + sort.Sort(errorSlice(errs)) + for _, err := range errs { + issues += ". " + err.Error() + } + } + // remove characters that need escaping, it's only tests... + issues = strings.ReplaceAll(issues, "\"", "") + return nil, fmt.Errorf("Invalid config supplied%s", issues) + } + } + + resourceData := schema.TestResourceDataRaw(t, res.Schema, state) + err := res.InternalValidate(res.Schema, true) + if err != nil { + return nil, err + } + + // warns, errs := schemaMap(r.Schema).Validate(c) return resourceData, whatever(resourceData, &client) } From e8e85e1b4a93898e53befcdc32e911c7562e11f7 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 13:49:19 +0200 Subject: [PATCH 19/42] try fixing build --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index d2a0bbbf4..ae9749ded 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,6 +7,7 @@ jobs: - $HOME/.cache/go-build - $HOME/gopath/pkg/mod script: + - go get golang.org/x/tools/cmd/goimports - curl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsum - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0 - time make build @@ -17,6 +18,7 @@ jobs: - $HOME/.cache/go-build - $HOME/gopath/pkg/mod script: + - go get golang.org/x/tools/cmd/goimports - curl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsum - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0 - time make build From eeeb779a830170c5d248b411c023be6030c49ee3 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 13:55:09 +0200 Subject: [PATCH 20/42] test linter failing build --- databricks/resource_databricks_permissions.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/databricks/resource_databricks_permissions.go b/databricks/resource_databricks_permissions.go index 4b8db33fa..93652f527 100644 --- a/databricks/resource_databricks_permissions.go +++ b/databricks/resource_databricks_permissions.go @@ -13,7 +13,7 @@ import ( func parsePermissionsFromData(d *schema.ResourceData, client *service.DBApiClient) (*model.AccessControlChangeList, string, error) { - var objectID string + var objectId string acl := new(model.AccessControlChangeList) for _, mapping := range permissionsResourceIDFields() { v, ok := d.GetOk(mapping.field) @@ -24,13 +24,15 @@ func parsePermissionsFromData(d *schema.ResourceData, if err != nil { return nil, "", err } - objectID = fmt.Sprintf("/%s/%s", mapping.resourceType, id) + objectId = fmt.Sprintf( + "/%s/%s", + mapping.resourceType, id) err = d.Set("object_type", mapping.objectType) if err != nil { return nil, "", err } } - if objectID == "" { + if objectId == "" { return nil, "", fmt.Errorf("At least one type of resource identifiers must be set") } changes := 0 @@ -52,9 +54,9 @@ func parsePermissionsFromData(d *schema.ResourceData, } } if changes < 1 { - return nil, "", fmt.Errorf("At least one access_control is required") + return nil, "", fmt.Errorf("At least one access_control is required!") } - return acl, objectID, nil + return acl, objectId, nil } func resourcePermissionsCreate(d *schema.ResourceData, m interface{}) error { From 08a8b5439bc993bf8677e257f7417c9f94ec6f26 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 13:58:54 +0200 Subject: [PATCH 21/42] remove fmt from lint --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 54e87d66d..1cf770e82 100644 --- a/Makefile +++ b/Makefile @@ -24,7 +24,7 @@ fmt: @gofmt -s -w main.go @go fmt ./... -lint: fmt +lint: @echo "==> Linting source code with golangci-lint make sure you run make fmt ..." @golangci-lint run --skip-dirs-use-default --timeout 5m From ecd11ec9604c6b1b1d5188ffb6dd18ff69753035 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 14:04:14 +0200 Subject: [PATCH 22/42] format it back --- databricks/resource_databricks_permissions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/databricks/resource_databricks_permissions.go b/databricks/resource_databricks_permissions.go index 93652f527..3dbf87e9a 100644 --- a/databricks/resource_databricks_permissions.go +++ b/databricks/resource_databricks_permissions.go @@ -26,7 +26,7 @@ func parsePermissionsFromData(d *schema.ResourceData, } objectId = fmt.Sprintf( "/%s/%s", - mapping.resourceType, id) + mapping.resourceType, id) err = d.Set("object_type", mapping.objectType) if err != nil { return nil, "", err From fe7d72eb7effe32789dcdc8386c33750da74c6e3 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Fri, 19 Jun 2020 18:34:47 +0200 Subject: [PATCH 23/42] Added databricks_permissions resource --- client/model/permissions.go | 75 +++++++ client/service/permissions.go | 50 +++++ client/service/users.go | 12 +- databricks/provider.go | 1 + databricks/resource_databricks_permissions.go | 204 ++++++++++++++++++ .../resource_databricks_permissions_test.go | 142 ++++++++++++ 6 files changed, 482 insertions(+), 2 deletions(-) create mode 100644 client/model/permissions.go create mode 100644 client/service/permissions.go create mode 100644 databricks/resource_databricks_permissions.go create mode 100644 databricks/resource_databricks_permissions_test.go diff --git a/client/model/permissions.go b/client/model/permissions.go new file mode 100644 index 000000000..b81df1d58 --- /dev/null +++ b/client/model/permissions.go @@ -0,0 +1,75 @@ +package model + +// ObjectACL ... +type ObjectACL struct { + ObjectID string `json:"object_id,omitempty"` + ObjectType string `json:"object_type,omitempty"` + AccessControlList []*AccessControl `json:"access_control_list"` +} + +// AccessControlChangeList is wrapper around ACL changes +type AccessControlChangeList struct { + AccessControlList []*AccessControlChange `json:"access_control_list"` +} + +// AccessControlChange is API wrapper for changing permissions +type AccessControlChange struct { + UserName *string `json:"user_name,omitempty"` + GroupName *string `json:"group_name,omitempty"` + ServicePrincipalName *string `json:"service_principal_name,omitempty"` + PermissionLevel string `json:"permission_level"` +} + +// AccessControl ... +type AccessControl struct { + UserName *string `json:"user_name,omitempty"` + GroupName *string `json:"group_name,omitempty"` + AllPermissions []*Permission `json:"all_permissions,omitempty"` +} + +// Permission ... +type Permission struct { + PermissionLevel string `json:"permission_level"` + Inherited bool `json:"inherited,omitempty"` + InheritedFromObject []string `json:"inherited_from_object,omitempty"` +} + +// ToAccessControlChangeList converts data formats +func (oa *ObjectACL) ToAccessControlChangeList() *AccessControlChangeList { + acl := new(AccessControlChangeList) + for _, accessControl := range oa.AccessControlList { + for _, permission := range accessControl.AllPermissions { + if permission.Inherited { + continue + } + item := new(AccessControlChange) + acl.AccessControlList = append(acl.AccessControlList, item) + item.PermissionLevel = permission.PermissionLevel + if accessControl.UserName != nil { + item.UserName = accessControl.UserName + } else if accessControl.GroupName != nil { + item.GroupName = accessControl.GroupName + } + } + } + return acl +} + +// AccessControl exports data for TF +func (acl *AccessControlChangeList) AccessControl(me string) []map[string]string { + result := []map[string]string{} + for _, control := range acl.AccessControlList { + item := map[string]string{} + if control.UserName != nil && *control.UserName != "" { + if me == *control.UserName { + continue + } + item["user_name"] = *control.UserName + } else if control.GroupName != nil && *control.GroupName != "" { + item["group_name"] = *control.GroupName + } + item["permission_level"] = control.PermissionLevel + result = append(result, item) + } + return result +} diff --git a/client/service/permissions.go b/client/service/permissions.go new file mode 100644 index 000000000..e7dc9ac8c --- /dev/null +++ b/client/service/permissions.go @@ -0,0 +1,50 @@ +package service + +import ( + "encoding/json" + "net/http" + + "github.com/databrickslabs/databricks-terraform/client/model" +) + +// PermissionsAPI ... +type PermissionsAPI struct { + Client *DBApiClient +} + +// AddOrModify ... +func (a PermissionsAPI) AddOrModify(objectID string, objectACL *model.AccessControlChangeList) error { + _, err := a.Client.performQuery(http.MethodPatch, + "/preview/permissions"+objectID, + "2.0", nil, objectACL, nil) + if err != nil { + return err + } + + return err +} + +// SetOrDelete ... +func (a PermissionsAPI) SetOrDelete(objectID string, objectACL *model.AccessControlChangeList) error { + _, err := a.Client.performQuery(http.MethodPut, + "/preview/permissions"+objectID, + "2.0", nil, objectACL, nil) + if err != nil { + return err + } + + return err +} + +// Read ... +func (a PermissionsAPI) Read(objectID string) (*model.ObjectACL, error) { + resp, err := a.Client.performQuery(http.MethodGet, + "/preview/permissions"+objectID, + "2.0", nil, nil, nil) + if err != nil { + return nil, err + } + var objectACL = new(model.ObjectACL) + err = json.Unmarshal(resp, &objectACL) + return objectACL, err +} diff --git a/client/service/users.go b/client/service/users.go index a97a20d57..35a741ff4 100644 --- a/client/service/users.go +++ b/client/service/users.go @@ -72,18 +72,26 @@ func (a UsersAPI) Read(userID string) (model.User, error) { } func (a UsersAPI) read(userID string) (model.User, error) { - var user model.User userPath := fmt.Sprintf("/preview/scim/v2/Users/%v", userID) + return a.readByPath(userPath) +} + +// Me gets user information about caller +func (a UsersAPI) Me() (model.User, error) { + return a.readByPath("/preview/scim/v2/Me") +} +func (a UsersAPI) readByPath(userPath string) (model.User, error) { + var user model.User resp, err := a.Client.performQuery(http.MethodGet, userPath, "2.0", scimHeaders, nil, nil) if err != nil { return user, err } - err = json.Unmarshal(resp, &user) return user, err } + // Update will update the user given the user id, username, display name, entitlements and roles func (a UsersAPI) Update(userID string, userName string, displayName string, entitlements []string, roles []string) error { userPath := fmt.Sprintf("/preview/scim/v2/Users/%v", userID) diff --git a/databricks/provider.go b/databricks/provider.go index eeef5a3c3..4aaedb15c 100644 --- a/databricks/provider.go +++ b/databricks/provider.go @@ -30,6 +30,7 @@ func Provider(version string) terraform.ResourceProvider { "databricks_secret_scope": resourceSecretScope(), "databricks_secret": resourceSecret(), "databricks_secret_acl": resourceSecretACL(), + "databricks_permissions": resourcePermissions(), "databricks_instance_pool": resourceInstancePool(), "databricks_scim_user": resourceScimUser(), "databricks_scim_group": resourceScimGroup(), diff --git a/databricks/resource_databricks_permissions.go b/databricks/resource_databricks_permissions.go new file mode 100644 index 000000000..f31e23a79 --- /dev/null +++ b/databricks/resource_databricks_permissions.go @@ -0,0 +1,204 @@ +package databricks + +import ( + "fmt" + "path" + "strconv" + + "github.com/databrickslabs/databricks-terraform/client/model" + "github.com/databrickslabs/databricks-terraform/client/service" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/pkg/errors" +) + +func parsePermissionsFromData(d *schema.ResourceData, + client *service.DBApiClient) (*model.AccessControlChangeList, string, error) { + var objectID string + acl := new(model.AccessControlChangeList) + for _, mapping := range permissionsResourceIDFields() { + v, ok := d.GetOk(mapping.field) + if !ok { + continue + } + id, err := mapping.idRetriever(client, v.(string)) + if err != nil { + return nil, "", err + } + objectID = fmt.Sprintf("/%s/%s", mapping.resourceType, id) + err = d.Set("object_type", mapping.objectType) + if err != nil { + return nil, "", err + } + } + if objectID == "" { + return nil, "", fmt.Errorf("At least one type of resource identifiers must be set") + } + if data, ok := d.GetOk("access_control"); ok { + for _, element := range data.([]interface{}) { + rawAccessControl := element.(map[string]interface{}) + change := new(model.AccessControlChange) + acl.AccessControlList = append(acl.AccessControlList, change) + if v, ok := rawAccessControl["group_name"].(string); ok && v != "" { + change.GroupName = &v + } + if v, ok := rawAccessControl["user_name"].(string); ok && v != "" { + change.UserName = &v + } + if v, ok := rawAccessControl["permission_level"].(string); ok { + change.PermissionLevel = v + } + } + } + return acl, objectID, nil +} + +func resourcePermissionsCreate(d *schema.ResourceData, m interface{}) error { + client := m.(*service.DBApiClient) + acl, objectID, err := parsePermissionsFromData(d, client) + if err != nil { + return err + } + err = client.Permissions().AddOrModify(objectID, acl) + if err != nil { + return err + } + d.SetId(objectID) + return resourcePermissionsRead(d, m) +} + +func resourcePermissionsRead(d *schema.ResourceData, m interface{}) error { + id := d.Id() + client := m.(*service.DBApiClient) + objectACL, err := client.Permissions().Read(id) + if err != nil { + return err + } + for _, mapping := range permissionsResourceIDFields() { + if mapping.objectType != d.Get("object_type").(string) { + continue + } + identifier := path.Base(id) + err := d.Set(mapping.field, identifier) + if err != nil { + return errors.Wrapf(err, + "Cannot set mapping field %s to %s", + mapping.field, id) + } + break + } + acl := objectACL.ToAccessControlChangeList() + me, err := client.Users().Me() + if err != nil { + return errors.Wrapf(err, "Cannot self-identify") + } + accessControl := acl.AccessControl(me.UserName) + err = d.Set("access_control", accessControl) + if err != nil { + return err + } + return nil +} + +func resourcePermissionsDelete(d *schema.ResourceData, m interface{}) error { + id := d.Id() + client := m.(*service.DBApiClient) + err := client.Permissions().SetOrDelete(id, new(model.AccessControlChangeList)) + if err != nil { + return err + } + return nil +} + +// permissionsIDFieldMapping holds mapping +type permissionsIDFieldMapping struct { + field string + objectType string + resourceType string + idRetriever func(client *service.DBApiClient, id string) (string, error) +} + +// PermissionsResourceIDFields shows mapping of id columns to resource types +func permissionsResourceIDFields() []permissionsIDFieldMapping { + SIMPLE := func(client *service.DBApiClient, id string) (string, error) { + return id, nil + } + PATH := func(client *service.DBApiClient, path string) (string, error) { + info, err := client.Notebooks().Read(path) + if err != nil { + return "", errors.Wrapf(err, "Cannot load path %s", path) + } + return strconv.FormatInt(info.ObjectID, 10), nil + } + return []permissionsIDFieldMapping{ + {"cluster_policy_id", "cluster-policy", "cluster-policies", SIMPLE}, + {"instance_pool_id", "instance-pool", "instance-pools", SIMPLE}, + {"cluster_id", "cluster", "clusters", SIMPLE}, + {"job_id", "job", "jobs", SIMPLE}, + {"notebook_id", "notebook", "notebooks", SIMPLE}, + {"notebook_path", "notebook", "notebooks", PATH}, + {"directory_id", "directory", "directories", SIMPLE}, + {"directory_path", "directory", "directories", PATH}, + } +} + +func conflictingFields(field string) []string { + conflicting := []string{} + for _, mapping := range permissionsResourceIDFields() { + if mapping.field == field { + continue + } + conflicting = append(conflicting, mapping.field) + } + return conflicting +} + +func resourcePermissions() *schema.Resource { + fields := map[string]*schema.Schema{ + "object_type": { + Type: schema.TypeString, + Computed: true, + }, + "access_control": { + ForceNew: true, + Type: schema.TypeList, + MinItems: 1, + Required: true, + ConfigMode: schema.SchemaConfigModeAttr, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "user_name": { + ForceNew: true, + Type: schema.TypeString, + Optional: true, + //ConflictsWith: []string{"group_name"}, + }, + "group_name": { + ForceNew: true, + Type: schema.TypeString, + Optional: true, + //ConflictsWith: []string{"user_name"}, + }, + "permission_level": { + ForceNew: true, + Type: schema.TypeString, + Required: true, + }, + }, + }, + }, + } + for _, mapping := range permissionsResourceIDFields() { + fields[mapping.field] = &schema.Schema{ + ForceNew: true, + Type: schema.TypeString, + Optional: true, + ConflictsWith: conflictingFields(mapping.field), + } + } + return &schema.Resource{ + Create: resourcePermissionsCreate, + Read: resourcePermissionsRead, + Delete: resourcePermissionsDelete, + Schema: fields, + } +} diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go new file mode 100644 index 000000000..b7b3be005 --- /dev/null +++ b/databricks/resource_databricks_permissions_test.go @@ -0,0 +1,142 @@ +package databricks + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/stretchr/testify/assert" + + "github.com/databrickslabs/databricks-terraform/client/model" + "github.com/databrickslabs/databricks-terraform/client/service" +) + +func TestAccDatabricksPermissionsResourceFullLifecycle(t *testing.T) { + var permissions model.ObjectACL + randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + resource.Test(t, resource.TestCase{ + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + // create a resource + Config: testClusterPolicyPermissions(randomName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("databricks_permissions.dummy_can_use", + "object_type", "cluster-policy"), + testAccIDCallback(t, "databricks_permissions.dummy_can_use", + func(client *service.DBApiClient, id string) error { + resp, err := client.Permissions().Read(id) + if err != nil { + return err + } + permissions = *resp + assert.Len(t, permissions.AccessControlList, 3) + return nil + }), + ), + }, + { + Config: testClusterPolicyPermissionsSecondGroupAdded(randomName), + Check: testAccIDCallback(t, "databricks_permissions.dummy_can_use", + func(client *service.DBApiClient, id string) error { + resp, err := client.Permissions().Read(id) + if err != nil { + return err + } + permissions = *resp + assert.Len(t, permissions.AccessControlList, 3) + return nil + }), + }, + }, + }) +} + +func testClusterPolicyPermissions(name string) string { + return fmt.Sprintf(` + resource "databricks_cluster_policy" "something_simple" { + name = "Terraform Policy %[1]s" + definition = jsonencode({ + "spark_conf.spark.hadoop.javax.jdo.option.ConnectionURL": { + "type": "forbidden" + } + }) + } + resource "databricks_scim_group" "dummy_group" { + display_name = "Terraform Group %[1]s" + } + resource "databricks_permissions" "dummy_can_use" { + cluster_policy_id = databricks_cluster_policy.something_simple.id + access_control { + group_name = databricks_scim_group.dummy_group.display_name + permission_level = "CAN_USE" + } + } + `, name) +} + +func testClusterPolicyPermissionsSecondGroupAdded(name string) string { + return fmt.Sprintf(` + resource "databricks_cluster_policy" "something_simple" { + name = "Terraform Policy %[1]s" + definition = jsonencode({ + "spark_conf.spark.hadoop.javax.jdo.option.ConnectionURL": { + "type": "forbidden" + }, + "spark_conf.spark.secondkey": { + "type": "forbidden" + } + }) + } + resource "databricks_scim_group" "dummy_group" { + display_name = "Terraform Group %[1]s" + } + resource "databricks_scim_group" "second_group" { + display_name = "Terraform Second Group %[1]s" + } + resource "databricks_permissions" "dummy_can_use" { + cluster_policy_id = databricks_cluster_policy.something_simple.id + access_control { + group_name = databricks_scim_group.dummy_group.display_name + permission_level = "CAN_USE" + } + access_control { + group_name = databricks_scim_group.second_group.display_name + permission_level = "CAN_USE" + } + } + `, name) +} + +func TestAccNotebookPermissions(t *testing.T) { + randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) + resource.Test(t, resource.TestCase{ + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + // create a resource + Config: fmt.Sprintf(` + resource "databricks_notebook" "dummy" { + content = base64encode("# Databricks notebook source\nprint(1)") + path = "/Beginning/Init" + overwrite = true + mkdirs = true + language = "PYTHON" + format = "SOURCE" + } + resource "databricks_scim_group" "dummy_group" { + display_name = "Terraform Group %[1]s" + } + resource "databricks_permissions" "dummy_can_use" { + directory_path = "/Beginning" + access_control { + group_name = databricks_scim_group.dummy_group.display_name + permission_level = "CAN_MANAGE" + } + } + `, randomName), + }, + }, + }) +} \ No newline at end of file From dd79a2dcd3cefcd71cf06be51645f5d286675105 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Mon, 22 Jun 2020 21:32:45 +0200 Subject: [PATCH 24/42] Minor commenting tweaks --- client/model/permissions.go | 32 +++++++++---------- client/service/permissions.go | 8 ++--- client/service/users.go | 1 - .../resource_databricks_permissions_test.go | 2 +- go.mod | 1 + 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/client/model/permissions.go b/client/model/permissions.go index b81df1d58..d9c7007ee 100644 --- a/client/model/permissions.go +++ b/client/model/permissions.go @@ -1,39 +1,39 @@ package model -// ObjectACL ... +// ObjectACL is a structure to generically describe access control type ObjectACL struct { ObjectID string `json:"object_id,omitempty"` ObjectType string `json:"object_type,omitempty"` AccessControlList []*AccessControl `json:"access_control_list"` } -// AccessControlChangeList is wrapper around ACL changes -type AccessControlChangeList struct { - AccessControlList []*AccessControlChange `json:"access_control_list"` -} - -// AccessControlChange is API wrapper for changing permissions -type AccessControlChange struct { - UserName *string `json:"user_name,omitempty"` - GroupName *string `json:"group_name,omitempty"` - ServicePrincipalName *string `json:"service_principal_name,omitempty"` - PermissionLevel string `json:"permission_level"` -} - -// AccessControl ... +// AccessControl is a structure to describe user/group permissions type AccessControl struct { UserName *string `json:"user_name,omitempty"` GroupName *string `json:"group_name,omitempty"` AllPermissions []*Permission `json:"all_permissions,omitempty"` } -// Permission ... +// Permission is a structure to describe permission level type Permission struct { PermissionLevel string `json:"permission_level"` Inherited bool `json:"inherited,omitempty"` InheritedFromObject []string `json:"inherited_from_object,omitempty"` } +// AccessControlChangeList is wrapper around ACL changes for REST API +type AccessControlChangeList struct { + AccessControlList []*AccessControlChange `json:"access_control_list"` +} + +// AccessControlChange is API wrapper for changing permissions +type AccessControlChange struct { + UserName *string `json:"user_name,omitempty"` + GroupName *string `json:"group_name,omitempty"` + ServicePrincipalName *string `json:"service_principal_name,omitempty"` + PermissionLevel string `json:"permission_level"` +} + // ToAccessControlChangeList converts data formats func (oa *ObjectACL) ToAccessControlChangeList() *AccessControlChangeList { acl := new(AccessControlChangeList) diff --git a/client/service/permissions.go b/client/service/permissions.go index e7dc9ac8c..e2e0bd5a9 100644 --- a/client/service/permissions.go +++ b/client/service/permissions.go @@ -7,12 +7,12 @@ import ( "github.com/databrickslabs/databricks-terraform/client/model" ) -// PermissionsAPI ... +// PermissionsAPI exposes general permission related methods type PermissionsAPI struct { Client *DBApiClient } -// AddOrModify ... +// AddOrModify works with permissions change list func (a PermissionsAPI) AddOrModify(objectID string, objectACL *model.AccessControlChangeList) error { _, err := a.Client.performQuery(http.MethodPatch, "/preview/permissions"+objectID, @@ -24,7 +24,7 @@ func (a PermissionsAPI) AddOrModify(objectID string, objectACL *model.AccessCont return err } -// SetOrDelete ... +// SetOrDelete updates object permissions func (a PermissionsAPI) SetOrDelete(objectID string, objectACL *model.AccessControlChangeList) error { _, err := a.Client.performQuery(http.MethodPut, "/preview/permissions"+objectID, @@ -36,7 +36,7 @@ func (a PermissionsAPI) SetOrDelete(objectID string, objectACL *model.AccessCont return err } -// Read ... +// Read gets all relevant permissions for the object, including inherited ones func (a PermissionsAPI) Read(objectID string) (*model.ObjectACL, error) { resp, err := a.Client.performQuery(http.MethodGet, "/preview/permissions"+objectID, diff --git a/client/service/users.go b/client/service/users.go index 35a741ff4..6df7050e7 100644 --- a/client/service/users.go +++ b/client/service/users.go @@ -91,7 +91,6 @@ func (a UsersAPI) readByPath(userPath string) (model.User, error) { return user, err } - // Update will update the user given the user id, username, display name, entitlements and roles func (a UsersAPI) Update(userID string, userName string, displayName string, entitlements []string, roles []string) error { userPath := fmt.Sprintf("/preview/scim/v2/Users/%v", userID) diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index b7b3be005..5591cb19b 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -139,4 +139,4 @@ func TestAccNotebookPermissions(t *testing.T) { }, }, }) -} \ No newline at end of file +} diff --git a/go.mod b/go.mod index e9ee7c27b..c5dd07c50 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/joho/godotenv v1.3.0 github.com/mattn/go-colorable v0.1.6 // indirect github.com/mitchellh/go-homedir v1.1.0 + github.com/pkg/errors v0.9.1 github.com/r3labs/diff v0.0.0-20191120142937-b4ed99a31f5a github.com/sergi/go-diff v1.1.0 // indirect github.com/smartystreets/goconvey v1.6.4 // indirect From 0088df457aaeda55076c7b6834adc81cd85641e8 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Mon, 22 Jun 2020 21:43:02 +0200 Subject: [PATCH 25/42] Register permissions api in the client --- client/service/apis.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/client/service/apis.go b/client/service/apis.go index b1704cd9e..8d450685b 100644 --- a/client/service/apis.go +++ b/client/service/apis.go @@ -116,6 +116,11 @@ func (c *DBApiClient) MWSCustomerManagedKeys() MWSCustomerManagedKeysAPI { return MWSCustomerManagedKeysAPI{Client: c} } +// Permissions returns an instance of CommandsAPI +func (c *DBApiClient) Permissions() PermissionsAPI { + return PermissionsAPI{Client: c} +} + func (c *DBApiClient) performQuery(method, path string, apiVersion string, headers map[string]string, data interface{}, secretsMask *SecretsMask) ([]byte, error) { err := c.Config.getOrCreateToken() if err != nil { From 259e467f058650b5a980dd7845b88a9b92693169 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Mon, 22 Jun 2020 21:55:30 +0200 Subject: [PATCH 26/42] Add pkg/errors --- vendor/github.com/pkg/errors/.gitignore | 24 ++ vendor/github.com/pkg/errors/.travis.yml | 10 + vendor/github.com/pkg/errors/LICENSE | 23 ++ vendor/github.com/pkg/errors/Makefile | 44 ++++ vendor/github.com/pkg/errors/README.md | 59 +++++ vendor/github.com/pkg/errors/appveyor.yml | 32 +++ vendor/github.com/pkg/errors/errors.go | 288 ++++++++++++++++++++++ vendor/github.com/pkg/errors/go113.go | 38 +++ vendor/github.com/pkg/errors/stack.go | 177 +++++++++++++ vendor/modules.txt | 2 + 10 files changed, 697 insertions(+) create mode 100644 vendor/github.com/pkg/errors/.gitignore create mode 100644 vendor/github.com/pkg/errors/.travis.yml create mode 100644 vendor/github.com/pkg/errors/LICENSE create mode 100644 vendor/github.com/pkg/errors/Makefile create mode 100644 vendor/github.com/pkg/errors/README.md create mode 100644 vendor/github.com/pkg/errors/appveyor.yml create mode 100644 vendor/github.com/pkg/errors/errors.go create mode 100644 vendor/github.com/pkg/errors/go113.go create mode 100644 vendor/github.com/pkg/errors/stack.go diff --git a/vendor/github.com/pkg/errors/.gitignore b/vendor/github.com/pkg/errors/.gitignore new file mode 100644 index 000000000..daf913b1b --- /dev/null +++ b/vendor/github.com/pkg/errors/.gitignore @@ -0,0 +1,24 @@ +# Compiled Object files, Static and Dynamic libs (Shared Objects) +*.o +*.a +*.so + +# Folders +_obj +_test + +# Architecture specific extensions/prefixes +*.[568vq] +[568vq].out + +*.cgo1.go +*.cgo2.c +_cgo_defun.c +_cgo_gotypes.go +_cgo_export.* + +_testmain.go + +*.exe +*.test +*.prof diff --git a/vendor/github.com/pkg/errors/.travis.yml b/vendor/github.com/pkg/errors/.travis.yml new file mode 100644 index 000000000..9159de03e --- /dev/null +++ b/vendor/github.com/pkg/errors/.travis.yml @@ -0,0 +1,10 @@ +language: go +go_import_path: github.com/pkg/errors +go: + - 1.11.x + - 1.12.x + - 1.13.x + - tip + +script: + - make check diff --git a/vendor/github.com/pkg/errors/LICENSE b/vendor/github.com/pkg/errors/LICENSE new file mode 100644 index 000000000..835ba3e75 --- /dev/null +++ b/vendor/github.com/pkg/errors/LICENSE @@ -0,0 +1,23 @@ +Copyright (c) 2015, Dave Cheney +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + +* Redistributions of source code must retain the above copyright notice, this + list of conditions and the following disclaimer. + +* Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/vendor/github.com/pkg/errors/Makefile b/vendor/github.com/pkg/errors/Makefile new file mode 100644 index 000000000..ce9d7cded --- /dev/null +++ b/vendor/github.com/pkg/errors/Makefile @@ -0,0 +1,44 @@ +PKGS := github.com/pkg/errors +SRCDIRS := $(shell go list -f '{{.Dir}}' $(PKGS)) +GO := go + +check: test vet gofmt misspell unconvert staticcheck ineffassign unparam + +test: + $(GO) test $(PKGS) + +vet: | test + $(GO) vet $(PKGS) + +staticcheck: + $(GO) get honnef.co/go/tools/cmd/staticcheck + staticcheck -checks all $(PKGS) + +misspell: + $(GO) get github.com/client9/misspell/cmd/misspell + misspell \ + -locale GB \ + -error \ + *.md *.go + +unconvert: + $(GO) get github.com/mdempsky/unconvert + unconvert -v $(PKGS) + +ineffassign: + $(GO) get github.com/gordonklaus/ineffassign + find $(SRCDIRS) -name '*.go' | xargs ineffassign + +pedantic: check errcheck + +unparam: + $(GO) get mvdan.cc/unparam + unparam ./... + +errcheck: + $(GO) get github.com/kisielk/errcheck + errcheck $(PKGS) + +gofmt: + @echo Checking code is gofmted + @test -z "$(shell gofmt -s -l -d -e $(SRCDIRS) | tee /dev/stderr)" diff --git a/vendor/github.com/pkg/errors/README.md b/vendor/github.com/pkg/errors/README.md new file mode 100644 index 000000000..54dfdcb12 --- /dev/null +++ b/vendor/github.com/pkg/errors/README.md @@ -0,0 +1,59 @@ +# errors [![Travis-CI](https://travis-ci.org/pkg/errors.svg)](https://travis-ci.org/pkg/errors) [![AppVeyor](https://ci.appveyor.com/api/projects/status/b98mptawhudj53ep/branch/master?svg=true)](https://ci.appveyor.com/project/davecheney/errors/branch/master) [![GoDoc](https://godoc.org/github.com/pkg/errors?status.svg)](http://godoc.org/github.com/pkg/errors) [![Report card](https://goreportcard.com/badge/github.com/pkg/errors)](https://goreportcard.com/report/github.com/pkg/errors) [![Sourcegraph](https://sourcegraph.com/github.com/pkg/errors/-/badge.svg)](https://sourcegraph.com/github.com/pkg/errors?badge) + +Package errors provides simple error handling primitives. + +`go get github.com/pkg/errors` + +The traditional error handling idiom in Go is roughly akin to +```go +if err != nil { + return err +} +``` +which applied recursively up the call stack results in error reports without context or debugging information. The errors package allows programmers to add context to the failure path in their code in a way that does not destroy the original value of the error. + +## Adding context to an error + +The errors.Wrap function returns a new error that adds context to the original error. For example +```go +_, err := ioutil.ReadAll(r) +if err != nil { + return errors.Wrap(err, "read failed") +} +``` +## Retrieving the cause of an error + +Using `errors.Wrap` constructs a stack of errors, adding context to the preceding error. Depending on the nature of the error it may be necessary to reverse the operation of errors.Wrap to retrieve the original error for inspection. Any error value which implements this interface can be inspected by `errors.Cause`. +```go +type causer interface { + Cause() error +} +``` +`errors.Cause` will recursively retrieve the topmost error which does not implement `causer`, which is assumed to be the original cause. For example: +```go +switch err := errors.Cause(err).(type) { +case *MyError: + // handle specifically +default: + // unknown error +} +``` + +[Read the package documentation for more information](https://godoc.org/github.com/pkg/errors). + +## Roadmap + +With the upcoming [Go2 error proposals](https://go.googlesource.com/proposal/+/master/design/go2draft.md) this package is moving into maintenance mode. The roadmap for a 1.0 release is as follows: + +- 0.9. Remove pre Go 1.9 and Go 1.10 support, address outstanding pull requests (if possible) +- 1.0. Final release. + +## Contributing + +Because of the Go2 errors changes, this package is not accepting proposals for new functionality. With that said, we welcome pull requests, bug fixes and issue reports. + +Before sending a PR, please discuss your change by raising an issue. + +## License + +BSD-2-Clause diff --git a/vendor/github.com/pkg/errors/appveyor.yml b/vendor/github.com/pkg/errors/appveyor.yml new file mode 100644 index 000000000..a932eade0 --- /dev/null +++ b/vendor/github.com/pkg/errors/appveyor.yml @@ -0,0 +1,32 @@ +version: build-{build}.{branch} + +clone_folder: C:\gopath\src\github.com\pkg\errors +shallow_clone: true # for startup speed + +environment: + GOPATH: C:\gopath + +platform: + - x64 + +# http://www.appveyor.com/docs/installed-software +install: + # some helpful output for debugging builds + - go version + - go env + # pre-installed MinGW at C:\MinGW is 32bit only + # but MSYS2 at C:\msys64 has mingw64 + - set PATH=C:\msys64\mingw64\bin;%PATH% + - gcc --version + - g++ --version + +build_script: + - go install -v ./... + +test_script: + - set PATH=C:\gopath\bin;%PATH% + - go test -v ./... + +#artifacts: +# - path: '%GOPATH%\bin\*.exe' +deploy: off diff --git a/vendor/github.com/pkg/errors/errors.go b/vendor/github.com/pkg/errors/errors.go new file mode 100644 index 000000000..161aea258 --- /dev/null +++ b/vendor/github.com/pkg/errors/errors.go @@ -0,0 +1,288 @@ +// Package errors provides simple error handling primitives. +// +// The traditional error handling idiom in Go is roughly akin to +// +// if err != nil { +// return err +// } +// +// which when applied recursively up the call stack results in error reports +// without context or debugging information. The errors package allows +// programmers to add context to the failure path in their code in a way +// that does not destroy the original value of the error. +// +// Adding context to an error +// +// The errors.Wrap function returns a new error that adds context to the +// original error by recording a stack trace at the point Wrap is called, +// together with the supplied message. For example +// +// _, err := ioutil.ReadAll(r) +// if err != nil { +// return errors.Wrap(err, "read failed") +// } +// +// If additional control is required, the errors.WithStack and +// errors.WithMessage functions destructure errors.Wrap into its component +// operations: annotating an error with a stack trace and with a message, +// respectively. +// +// Retrieving the cause of an error +// +// Using errors.Wrap constructs a stack of errors, adding context to the +// preceding error. Depending on the nature of the error it may be necessary +// to reverse the operation of errors.Wrap to retrieve the original error +// for inspection. Any error value which implements this interface +// +// type causer interface { +// Cause() error +// } +// +// can be inspected by errors.Cause. errors.Cause will recursively retrieve +// the topmost error that does not implement causer, which is assumed to be +// the original cause. For example: +// +// switch err := errors.Cause(err).(type) { +// case *MyError: +// // handle specifically +// default: +// // unknown error +// } +// +// Although the causer interface is not exported by this package, it is +// considered a part of its stable public interface. +// +// Formatted printing of errors +// +// All error values returned from this package implement fmt.Formatter and can +// be formatted by the fmt package. The following verbs are supported: +// +// %s print the error. If the error has a Cause it will be +// printed recursively. +// %v see %s +// %+v extended format. Each Frame of the error's StackTrace will +// be printed in detail. +// +// Retrieving the stack trace of an error or wrapper +// +// New, Errorf, Wrap, and Wrapf record a stack trace at the point they are +// invoked. This information can be retrieved with the following interface: +// +// type stackTracer interface { +// StackTrace() errors.StackTrace +// } +// +// The returned errors.StackTrace type is defined as +// +// type StackTrace []Frame +// +// The Frame type represents a call site in the stack trace. Frame supports +// the fmt.Formatter interface that can be used for printing information about +// the stack trace of this error. For example: +// +// if err, ok := err.(stackTracer); ok { +// for _, f := range err.StackTrace() { +// fmt.Printf("%+s:%d\n", f, f) +// } +// } +// +// Although the stackTracer interface is not exported by this package, it is +// considered a part of its stable public interface. +// +// See the documentation for Frame.Format for more details. +package errors + +import ( + "fmt" + "io" +) + +// New returns an error with the supplied message. +// New also records the stack trace at the point it was called. +func New(message string) error { + return &fundamental{ + msg: message, + stack: callers(), + } +} + +// Errorf formats according to a format specifier and returns the string +// as a value that satisfies error. +// Errorf also records the stack trace at the point it was called. +func Errorf(format string, args ...interface{}) error { + return &fundamental{ + msg: fmt.Sprintf(format, args...), + stack: callers(), + } +} + +// fundamental is an error that has a message and a stack, but no caller. +type fundamental struct { + msg string + *stack +} + +func (f *fundamental) Error() string { return f.msg } + +func (f *fundamental) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + if s.Flag('+') { + io.WriteString(s, f.msg) + f.stack.Format(s, verb) + return + } + fallthrough + case 's': + io.WriteString(s, f.msg) + case 'q': + fmt.Fprintf(s, "%q", f.msg) + } +} + +// WithStack annotates err with a stack trace at the point WithStack was called. +// If err is nil, WithStack returns nil. +func WithStack(err error) error { + if err == nil { + return nil + } + return &withStack{ + err, + callers(), + } +} + +type withStack struct { + error + *stack +} + +func (w *withStack) Cause() error { return w.error } + +// Unwrap provides compatibility for Go 1.13 error chains. +func (w *withStack) Unwrap() error { return w.error } + +func (w *withStack) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + if s.Flag('+') { + fmt.Fprintf(s, "%+v", w.Cause()) + w.stack.Format(s, verb) + return + } + fallthrough + case 's': + io.WriteString(s, w.Error()) + case 'q': + fmt.Fprintf(s, "%q", w.Error()) + } +} + +// Wrap returns an error annotating err with a stack trace +// at the point Wrap is called, and the supplied message. +// If err is nil, Wrap returns nil. +func Wrap(err error, message string) error { + if err == nil { + return nil + } + err = &withMessage{ + cause: err, + msg: message, + } + return &withStack{ + err, + callers(), + } +} + +// Wrapf returns an error annotating err with a stack trace +// at the point Wrapf is called, and the format specifier. +// If err is nil, Wrapf returns nil. +func Wrapf(err error, format string, args ...interface{}) error { + if err == nil { + return nil + } + err = &withMessage{ + cause: err, + msg: fmt.Sprintf(format, args...), + } + return &withStack{ + err, + callers(), + } +} + +// WithMessage annotates err with a new message. +// If err is nil, WithMessage returns nil. +func WithMessage(err error, message string) error { + if err == nil { + return nil + } + return &withMessage{ + cause: err, + msg: message, + } +} + +// WithMessagef annotates err with the format specifier. +// If err is nil, WithMessagef returns nil. +func WithMessagef(err error, format string, args ...interface{}) error { + if err == nil { + return nil + } + return &withMessage{ + cause: err, + msg: fmt.Sprintf(format, args...), + } +} + +type withMessage struct { + cause error + msg string +} + +func (w *withMessage) Error() string { return w.msg + ": " + w.cause.Error() } +func (w *withMessage) Cause() error { return w.cause } + +// Unwrap provides compatibility for Go 1.13 error chains. +func (w *withMessage) Unwrap() error { return w.cause } + +func (w *withMessage) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + if s.Flag('+') { + fmt.Fprintf(s, "%+v\n", w.Cause()) + io.WriteString(s, w.msg) + return + } + fallthrough + case 's', 'q': + io.WriteString(s, w.Error()) + } +} + +// Cause returns the underlying cause of the error, if possible. +// An error value has a cause if it implements the following +// interface: +// +// type causer interface { +// Cause() error +// } +// +// If the error does not implement Cause, the original error will +// be returned. If the error is nil, nil will be returned without further +// investigation. +func Cause(err error) error { + type causer interface { + Cause() error + } + + for err != nil { + cause, ok := err.(causer) + if !ok { + break + } + err = cause.Cause() + } + return err +} diff --git a/vendor/github.com/pkg/errors/go113.go b/vendor/github.com/pkg/errors/go113.go new file mode 100644 index 000000000..be0d10d0c --- /dev/null +++ b/vendor/github.com/pkg/errors/go113.go @@ -0,0 +1,38 @@ +// +build go1.13 + +package errors + +import ( + stderrors "errors" +) + +// Is reports whether any error in err's chain matches target. +// +// The chain consists of err itself followed by the sequence of errors obtained by +// repeatedly calling Unwrap. +// +// An error is considered to match a target if it is equal to that target or if +// it implements a method Is(error) bool such that Is(target) returns true. +func Is(err, target error) bool { return stderrors.Is(err, target) } + +// As finds the first error in err's chain that matches target, and if so, sets +// target to that error value and returns true. +// +// The chain consists of err itself followed by the sequence of errors obtained by +// repeatedly calling Unwrap. +// +// An error matches target if the error's concrete value is assignable to the value +// pointed to by target, or if the error has a method As(interface{}) bool such that +// As(target) returns true. In the latter case, the As method is responsible for +// setting target. +// +// As will panic if target is not a non-nil pointer to either a type that implements +// error, or to any interface type. As returns false if err is nil. +func As(err error, target interface{}) bool { return stderrors.As(err, target) } + +// Unwrap returns the result of calling the Unwrap method on err, if err's +// type contains an Unwrap method returning error. +// Otherwise, Unwrap returns nil. +func Unwrap(err error) error { + return stderrors.Unwrap(err) +} diff --git a/vendor/github.com/pkg/errors/stack.go b/vendor/github.com/pkg/errors/stack.go new file mode 100644 index 000000000..779a8348f --- /dev/null +++ b/vendor/github.com/pkg/errors/stack.go @@ -0,0 +1,177 @@ +package errors + +import ( + "fmt" + "io" + "path" + "runtime" + "strconv" + "strings" +) + +// Frame represents a program counter inside a stack frame. +// For historical reasons if Frame is interpreted as a uintptr +// its value represents the program counter + 1. +type Frame uintptr + +// pc returns the program counter for this frame; +// multiple frames may have the same PC value. +func (f Frame) pc() uintptr { return uintptr(f) - 1 } + +// file returns the full path to the file that contains the +// function for this Frame's pc. +func (f Frame) file() string { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return "unknown" + } + file, _ := fn.FileLine(f.pc()) + return file +} + +// line returns the line number of source code of the +// function for this Frame's pc. +func (f Frame) line() int { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return 0 + } + _, line := fn.FileLine(f.pc()) + return line +} + +// name returns the name of this function, if known. +func (f Frame) name() string { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return "unknown" + } + return fn.Name() +} + +// Format formats the frame according to the fmt.Formatter interface. +// +// %s source file +// %d source line +// %n function name +// %v equivalent to %s:%d +// +// Format accepts flags that alter the printing of some verbs, as follows: +// +// %+s function name and path of source file relative to the compile time +// GOPATH separated by \n\t (\n\t) +// %+v equivalent to %+s:%d +func (f Frame) Format(s fmt.State, verb rune) { + switch verb { + case 's': + switch { + case s.Flag('+'): + io.WriteString(s, f.name()) + io.WriteString(s, "\n\t") + io.WriteString(s, f.file()) + default: + io.WriteString(s, path.Base(f.file())) + } + case 'd': + io.WriteString(s, strconv.Itoa(f.line())) + case 'n': + io.WriteString(s, funcname(f.name())) + case 'v': + f.Format(s, 's') + io.WriteString(s, ":") + f.Format(s, 'd') + } +} + +// MarshalText formats a stacktrace Frame as a text string. The output is the +// same as that of fmt.Sprintf("%+v", f), but without newlines or tabs. +func (f Frame) MarshalText() ([]byte, error) { + name := f.name() + if name == "unknown" { + return []byte(name), nil + } + return []byte(fmt.Sprintf("%s %s:%d", name, f.file(), f.line())), nil +} + +// StackTrace is stack of Frames from innermost (newest) to outermost (oldest). +type StackTrace []Frame + +// Format formats the stack of Frames according to the fmt.Formatter interface. +// +// %s lists source files for each Frame in the stack +// %v lists the source file and line number for each Frame in the stack +// +// Format accepts flags that alter the printing of some verbs, as follows: +// +// %+v Prints filename, function, and line number for each Frame in the stack. +func (st StackTrace) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + switch { + case s.Flag('+'): + for _, f := range st { + io.WriteString(s, "\n") + f.Format(s, verb) + } + case s.Flag('#'): + fmt.Fprintf(s, "%#v", []Frame(st)) + default: + st.formatSlice(s, verb) + } + case 's': + st.formatSlice(s, verb) + } +} + +// formatSlice will format this StackTrace into the given buffer as a slice of +// Frame, only valid when called with '%s' or '%v'. +func (st StackTrace) formatSlice(s fmt.State, verb rune) { + io.WriteString(s, "[") + for i, f := range st { + if i > 0 { + io.WriteString(s, " ") + } + f.Format(s, verb) + } + io.WriteString(s, "]") +} + +// stack represents a stack of program counters. +type stack []uintptr + +func (s *stack) Format(st fmt.State, verb rune) { + switch verb { + case 'v': + switch { + case st.Flag('+'): + for _, pc := range *s { + f := Frame(pc) + fmt.Fprintf(st, "\n%+v", f) + } + } + } +} + +func (s *stack) StackTrace() StackTrace { + f := make([]Frame, len(*s)) + for i := 0; i < len(f); i++ { + f[i] = Frame((*s)[i]) + } + return f +} + +func callers() *stack { + const depth = 32 + var pcs [depth]uintptr + n := runtime.Callers(3, pcs[:]) + var st stack = pcs[0:n] + return &st +} + +// funcname removes the path prefix component of a function's name reported by func.Name(). +func funcname(name string) string { + i := strings.LastIndex(name, "/") + name = name[i+1:] + i = strings.Index(name, ".") + return name[i+1:] +} diff --git a/vendor/modules.txt b/vendor/modules.txt index c1ba8fa71..f72453a1a 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -230,6 +230,8 @@ github.com/mitchellh/mapstructure github.com/mitchellh/reflectwalk # github.com/oklog/run v1.0.0 github.com/oklog/run +# github.com/pkg/errors v0.9.1 +github.com/pkg/errors # github.com/pmezard/go-difflib v1.0.0 github.com/pmezard/go-difflib/difflib # github.com/posener/complete v1.2.1 From e7cec50449da1e16bd204a898e31cfdcc2560eef Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Wed, 24 Jun 2020 00:07:01 +0200 Subject: [PATCH 27/42] Added real unit tests for resource --- CONTRIBUTING.md | 67 +++++++- databricks/resource_databricks_permissions.go | 5 + .../resource_databricks_permissions_test.go | 150 ++++++++++++++++++ databricks/utils_test.go | 74 +++++++++ go.sum | 1 + 5 files changed, 294 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3cf74cfcc..7c6476742 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,10 +8,11 @@ Contributing to Databricks Terraform Provider - [Building and Installing with Docker](#building-and-installing-with-docker) - [Testing](#testing) - [Linting](#linting) +- [Unit testing resources](#unit-testing-resources) - [Integration Testing](#integration-testing) - [Project Components](#project-components) - - [Databricks Terraform Provider Resources State](#databricks-terraform-provider-resources-state) - - [Databricks Terraform Data Sources State](#databricks-terraform-data-sources-state) + - [Databricks Terraform Provider Resources State](#databricks-terraform-provider-resources-state) + - [Databricks Terraform Data Sources State](#databricks-terraform-data-sources-state) We happily welcome contributions to databricks-terraform. We use GitHub Issues to track community reported issues and GitHub Pull Requests for accepting changes. @@ -128,6 +129,67 @@ $ docker run -it -v $(pwd):/workpace -w /workpace databricks-terraform apply Please use makefile for linting. If you run `golangci-lint` by itself it will fail due to different tags containing same functions. So please run `make lint` instead. +## Unit testing resources + +In order to unit test a resource, which runs fast and could be included in code coverage, one should use `ResourceTester`, that launches embedded HTTP server with `HTTPFixture`'s containing all calls that should have been made in given scenario. Some may argue that this is not a pure unit test, because it creates a side effect in form of embedded server, though it's always on different random port, making it possible to execute these tests in parallel. Therefore comments about non-pure unit tests will be ignored, if they use `ResourceTester` helper. + +```go +func TestPermissionsCreate(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPatch, + // requires full URI + Resource: "/api/2.0/preview/permissions/clusters/abc", + // works with entities, not JSON. Diff is displayed in case of missmatch + ExpectedRequest: model.AccessControlChangeList{ + AccessControlList: []*model.AccessControlChange{ + { + UserName: &TestingUser, + PermissionLevel: "CAN_USE", + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: model.AccessControlChangeList{ + AccessControlList: []*model.AccessControlChange{ + { + UserName: &TestingUser, + PermissionLevel: "CAN_MANAGE", + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: model.User{ + UserName: "chuck.norris", + }, + }, + }, + // next argument is function, that creates resource (to make schema for ResourceData) + resourcePermissions, + // state represented as native structure (though a bit clunky) + map[string]interface{}{ + "cluster_id": "abc", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_USE", + }, + }, + }, + // the last argument is a function, that performs a stage on resource (Create/update/delete/read) + resourcePermissionsCreate) + assert.NoError(t, err, err) +} +``` + +Each resource should have both unit and integration tests. + ## Integration Testing Currently Databricks supports two cloud providers `azure` and `aws` thus integration testing with the correct cloud service provider is @@ -162,7 +224,6 @@ DATABRICKS_AZURE_WORKSPACE_NAME= Note that azure integration tests will use service principal based auth. Even though it is using a service principal, it will still be generating a personal access token to perform creation of resources. - ## Project Components ### Databricks Terraform Provider Resources State diff --git a/databricks/resource_databricks_permissions.go b/databricks/resource_databricks_permissions.go index f31e23a79..914629d96 100644 --- a/databricks/resource_databricks_permissions.go +++ b/databricks/resource_databricks_permissions.go @@ -33,6 +33,7 @@ func parsePermissionsFromData(d *schema.ResourceData, if objectID == "" { return nil, "", fmt.Errorf("At least one type of resource identifiers must be set") } + changes := 0 if data, ok := d.GetOk("access_control"); ok { for _, element := range data.([]interface{}) { rawAccessControl := element.(map[string]interface{}) @@ -47,8 +48,12 @@ func parsePermissionsFromData(d *schema.ResourceData, if v, ok := rawAccessControl["permission_level"].(string); ok { change.PermissionLevel = v } + changes++ } } + if changes < 1 { + return nil, "", fmt.Errorf("At least one access_control is required") + } return acl, objectID, nil } diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index 5591cb19b..c50f06246 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -2,16 +2,166 @@ package databricks import ( "fmt" + "net/http" "testing" "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/stretchr/testify/assert" "github.com/databrickslabs/databricks-terraform/client/model" "github.com/databrickslabs/databricks-terraform/client/service" ) +var ( + TestingUser = "ben" + TestingAdminUser = "admin" +) + +func TestPermissionsRead(t *testing.T) { + d, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: model.ObjectACL{ + ObjectID: "/clusters/abc", + ObjectType: "clusters", + AccessControlList: []*model.AccessControl{ + { + UserName: &TestingUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_READ", + Inherited: false, + }, + }, + }, + { + UserName: &TestingAdminUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_MANAGE", + Inherited: false, + }, + }, + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: model.User{ + UserName: TestingAdminUser, + }, + }, + }, resourcePermissions, map[string]interface{}{}, + func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsRead(d, c) + }) + assert.NoError(t, err, err) + assert.Equal(t, "/clusters/abc", d.Id()) + assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) + assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) + assert.Equal(t, 1, d.Get("access_control.#")) +} + +func TestPermissionsDelete(t *testing.T) { + d, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPut, + Resource: "/api/2.0/preview/permissions/clusters/abc", + ExpectedRequest: model.ObjectACL{}, + }, + }, resourcePermissions, map[string]interface{}{}, + func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsDelete(d, c) + }) + assert.NoError(t, err, err) + assert.Equal(t, "/clusters/abc", d.Id()) +} + +func TestPermissionsCreate_invalid(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{}, resourcePermissions, map[string]interface{}{}, + resourcePermissionsCreate) + assert.EqualError(t, err, "At least one type of resource identifiers must be set") +} + +func TestPermissionsCreate_no_access_control(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{}, resourcePermissions, + map[string]interface{}{ + "cluster_id": "abc", + }, resourcePermissionsCreate) + assert.EqualError(t, err, "At least one access_control is required") +} + +func TestPermissionsCreate(t *testing.T) { + d, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPatch, + Resource: "/api/2.0/preview/permissions/clusters/abc", + ExpectedRequest: model.AccessControlChangeList{ + AccessControlList: []*model.AccessControlChange{ + { + UserName: &TestingUser, + PermissionLevel: "CAN_USE", + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: model.ObjectACL{ + ObjectID: "/clusters/abc", + ObjectType: "clusters", + AccessControlList: []*model.AccessControl{ + { + UserName: &TestingUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_READ", + Inherited: false, + }, + }, + }, + { + UserName: &TestingAdminUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_MANAGE", + Inherited: false, + }, + }, + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: model.User{ + UserName: TestingAdminUser, + }, + }, + }, resourcePermissions, map[string]interface{}{ + "cluster_id": "abc", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_USE", + }, + }, + }, resourcePermissionsCreate) + assert.NoError(t, err, err) + assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) + assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) + assert.Equal(t, 1, d.Get("access_control.#")) +} + func TestAccDatabricksPermissionsResourceFullLifecycle(t *testing.T) { var permissions model.ObjectACL randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) diff --git a/databricks/utils_test.go b/databricks/utils_test.go index eba2ffea2..e2384909e 100644 --- a/databricks/utils_test.go +++ b/databricks/utils_test.go @@ -260,6 +260,80 @@ func testVerifyResourceIsMissing(t *testing.T, readFunc func() error) { "\nerror code: %s", apiError, apiError.StatusCode, apiError.ErrorCode)) } + "bytes" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/databrickslabs/databricks-terraform/client/service" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/stretchr/testify/assert" +) + +// HTTPFixture defines request structure for test +type HTTPFixture struct { + Method string + Resource string + Response interface{} + Status int + ExpectedRequest interface{} +} + +// ResourceTester helps testing HTTP resources with fixtures +func ResourceTester(t *testing.T, + fixtures []HTTPFixture, + resouceFunc func() *schema.Resource, + state map[string]interface{}, + whatever func(d *schema.ResourceData, c interface{}) error) (*schema.ResourceData, error) { + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + found := false + for _, fixture := range fixtures { + if req.Method == fixture.Method && req.RequestURI == fixture.Resource { + if fixture.Status == 0 { + rw.WriteHeader(200) + } else { + rw.WriteHeader(fixture.Status) + } + if fixture.ExpectedRequest != nil { + buf := new(bytes.Buffer) + _, err := buf.ReadFrom(req.Body) + assert.NoError(t, err, err) + jsonStr, err := json.Marshal(fixture.ExpectedRequest) + assert.NoError(t, err, err) + assert.JSONEq(t, string(jsonStr), buf.String()) + } + if fixture.Response != nil { + responseBytes, err := json.Marshal(fixture.Response) + if err != nil { + assert.NoError(t, err, err) + t.FailNow() + } + _, err = rw.Write(responseBytes) + assert.NoError(t, err, err) + } + found = true + break + } + } + if !found { + assert.Fail(t, fmt.Sprintf("Received unexpected call: %s %s", req.Method, req.RequestURI)) + t.FailNow() + } + })) + + defer server.Close() + var config service.DBApiClientConfig + config.Host = server.URL + config.Setup() + + var client service.DBApiClient + client.SetConfig(&config) + + resource := resouceFunc() + resourceData := schema.TestResourceDataRaw(t, resource.Schema, state) + return resourceData, whatever(resourceData, &client) } func TestIsClusterMissingTrueWhenClusterIdSpecifiedPresent(t *testing.T) { diff --git a/go.sum b/go.sum index 1a406cd3b..bb67768b2 100644 --- a/go.sum +++ b/go.sum @@ -147,6 +147,7 @@ github.com/hashicorp/terraform-svchost v0.0.0-20191011084731-65d371908596/go.mod github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d h1:kJCB4vdITiW1eC1vq2e6IsrXKrZit1bv/TDYFGMp4BQ= github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= +github.com/jhump/protoreflect v1.6.0 h1:h5jfMVslIg6l29nsMs0D8Wj17RDVdNYti0vDN/PZZoE= github.com/jhump/protoreflect v1.6.0/go.mod h1:eaTn3RZAmMBcV0fifFvlm6VHNz3wSkYyXYWUh7ymB74= github.com/jmespath/go-jmespath v0.0.0-20160202185014-0b12d6b521d8/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k= github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af h1:pmfjZENx5imkbgOkpRUYLnmbU7UEFbjtDA2hxJ1ichM= From 9c9b58951910864813bb2c06466757b8474f4852 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Wed, 24 Jun 2020 00:14:04 +0200 Subject: [PATCH 28/42] trigger build --- databricks/resource_databricks_permissions_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index c50f06246..ef02d8772 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -156,6 +156,7 @@ func TestPermissionsCreate(t *testing.T) { }, }, }, resourcePermissionsCreate) + assert.NoError(t, err, err) assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) From 88da24fc3589750d9cc01f94fd45ee180d1785a5 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Wed, 24 Jun 2020 00:17:07 +0200 Subject: [PATCH 29/42] trigger build --- databricks/resource_databricks_permissions_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index ef02d8772..547146e0a 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -156,7 +156,7 @@ func TestPermissionsCreate(t *testing.T) { }, }, }, resourcePermissionsCreate) - + assert.NoError(t, err, err) assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) From 9fd9e380d2dc13f4bc24fb841fae13146b5d6a79 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 11:25:31 +0200 Subject: [PATCH 30/42] Fix code style violations --- CONTRIBUTING.md | 5 ++ databricks/azure_auth.go | 16 ++-- databricks/mounts_test.go | 6 +- databricks/provider.go | 44 +++++------ ...ource_databricks_group_instance_profile.go | 14 ++-- .../resource_databricks_group_member.go | 14 ++-- databricks/utils_test.go | 79 +++++++++---------- 7 files changed, 87 insertions(+), 91 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7c6476742..31ce6e2f9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,6 +7,7 @@ Contributing to Databricks Terraform Provider - [Developing with Visual Studio Code Devcontainers](#developing-with-visual-studio-code-devcontainers) - [Building and Installing with Docker](#building-and-installing-with-docker) - [Testing](#testing) +- [Code conventions](#code-conventions) - [Linting](#linting) - [Unit testing resources](#unit-testing-resources) - [Integration Testing](#integration-testing) @@ -124,6 +125,10 @@ $ docker run -it -v $(pwd):/workpace -w /workpace databricks-terraform apply * [ ] Integration tests should be run at a client level against both azure and aws to maintain sdk parity against both apis **(currently only on one cloud)** * [x] Terraform acceptance tests should be run against both aws and azure to maintain parity of provider between both cloud services **(currently only on one cloud)** +## Code conventions + +* Import statements should all be first ordered by "GoLang internal", "Vendor packages" and then "current provider packages". Within those sections imports must follow alphabetical order. + ## Linting Please use makefile for linting. If you run `golangci-lint` by itself it will fail due to different tags containing same functions. diff --git a/databricks/azure_auth.go b/databricks/azure_auth.go index ba2bae571..66a1fa815 100644 --- a/databricks/azure_auth.go +++ b/databricks/azure_auth.go @@ -31,7 +31,7 @@ type TokenPayload struct { TenantID string } -type Workspace struct { +type azureDatabricksWorkspace struct { Name string `json:"name"` ID string `json:"id"` Type string `json:"type"` @@ -100,7 +100,7 @@ func (t *TokenPayload) getManagementToken() (string, error) { return mgmtToken.OAuthToken(), nil } -func (t *TokenPayload) getWorkspace(config *service.DBApiClientConfig, managementToken string) (*Workspace, error) { +func (t *TokenPayload) getWorkspace(config *service.DBApiClientConfig, managementToken string) (*azureDatabricksWorkspace, error) { log.Println("[DEBUG] Getting Workspace ID via management token.") // Escape all the ids url := fmt.Sprintf("https://management.azure.com/subscriptions/%s/resourceGroups/%s"+ @@ -124,7 +124,7 @@ func (t *TokenPayload) getWorkspace(config *service.DBApiClientConfig, managemen return nil, err } - var workspace Workspace + var workspace azureDatabricksWorkspace err = json.Unmarshal(resp, &workspace) if err != nil { return nil, err @@ -157,9 +157,9 @@ func (t *TokenPayload) getADBPlatformToken() (string, error) { return platformToken.OAuthToken(), nil } -func getWorkspaceAccessToken(config *service.DBApiClientConfig, managementToken, adbWorkspaceUrl, adbWorkspaceResourceID, adbPlatformToken string) (*model.TokenResponse, error) { +func getWorkspaceAccessToken(config *service.DBApiClientConfig, managementToken, adbWorkspaceURL, adbWorkspaceResourceID, adbPlatformToken string) (*model.TokenResponse, error) { log.Println("[DEBUG] Creating workspace token") - url := adbWorkspaceUrl + "/api/2.0/token/create" + url := adbWorkspaceURL + "/api/2.0/token/create" headers := map[string]string{ "Content-Type": "application/x-www-form-urlencoded", "X-Databricks-Azure-Workspace-Resource-Id": adbWorkspaceResourceID, @@ -201,7 +201,7 @@ func (t *TokenPayload) initWorkspaceAndGetClient(config *service.DBApiClientConf if err != nil { return err } - adbWorkspaceUrl := "https://" + adbWorkspace.Properties.WorkspaceURL + adbWorkspaceURL := "https://" + adbWorkspace.Properties.WorkspaceURL // Get platform token adbPlatformToken, err := t.getADBPlatformToken() @@ -210,12 +210,12 @@ func (t *TokenPayload) initWorkspaceAndGetClient(config *service.DBApiClientConf } // Get workspace personal access token - workspaceAccessTokenResp, err := getWorkspaceAccessToken(config, managementToken, adbWorkspaceUrl, adbWorkspace.ID, adbPlatformToken) + workspaceAccessTokenResp, err := getWorkspaceAccessToken(config, managementToken, adbWorkspaceURL, adbWorkspace.ID, adbPlatformToken) if err != nil { return err } - config.Host = adbWorkspaceUrl + config.Host = adbWorkspaceURL config.Token = workspaceAccessTokenResp.TokenValue if workspaceAccessTokenResp.TokenInfo != nil { config.TokenExpiryTime = workspaceAccessTokenResp.TokenInfo.ExpiryTime diff --git a/databricks/mounts_test.go b/databricks/mounts_test.go index 173e7f578..86ac8678c 100644 --- a/databricks/mounts_test.go +++ b/databricks/mounts_test.go @@ -46,7 +46,7 @@ func TestAzureBlobMountReadRetrievesMountInformation(t *testing.T) { }, { Name: "Mount found - but does not match configuration", - ExpectedError: fmt.Errorf("does not match uri with storage account and container values %s@%s != abfss://x@y.dfs.core.windows.net/z!", cn, sacc), + ExpectedError: fmt.Errorf("does not match uri with storage account and container values %s@%s != abfss://x@y.dfs.core.windows.net/z", cn, sacc), CommandResult: &model.CommandResults{ ResultType: "text", Data: "abfss://x@y.dfs.core.windows.net/z", @@ -109,7 +109,7 @@ func TestAzureADLSGen1MountReadRetrievesMountInformation(t *testing.T) { }, { Name: "Mount found - but does not match configuration", - ExpectedError: fmt.Errorf("does not match uri with storage account and container values %s@%s != adl://x.azuredatalakestore.net/z!", sacc, dir), + ExpectedError: fmt.Errorf("does not match uri with storage account and container values %s@%s != adl://x.azuredatalakestore.net/z", sacc, dir), CommandResult: &model.CommandResults{ ResultType: "text", Data: "adl://x.azuredatalakestore.net/z", @@ -173,7 +173,7 @@ func TestAzureADLSGen2MountReadRetrievesMountInformation(t *testing.T) { }, { Name: "Mount found - but does not match configuration", - ExpectedError: fmt.Errorf("does not match uri with storage account and container values %s@%s != abfss://x@y.dfs.core.windows.net/z!", cn, sacc), + ExpectedError: fmt.Errorf("does not match uri with storage account and container values %s@%s != abfss://x@y.dfs.core.windows.net/z", cn, sacc), CommandResult: &model.CommandResults{ ResultType: "text", Data: "abfss://x@y.dfs.core.windows.net/z", diff --git a/databricks/provider.go b/databricks/provider.go index 4aaedb15c..1683c1baf 100644 --- a/databricks/provider.go +++ b/databricks/provider.go @@ -308,33 +308,29 @@ func providerConfigure(d *schema.ResourceData, providerVersion string) (interfac // Abstracted logic to another function that returns a interface{}, error to inject directly // for the providers during cloud integration testing return providerConfigureAzureClient(d, &config) - } else { - if host, ok := d.GetOk("host"); ok { - config.Host = host.(string) - } - if token, ok := d.GetOk("token"); ok { - config.Token = token.(string) - } - - // Basic authentication setup via username and password - if _, ok := d.GetOk("basic_auth"); ok { - username, userOk := d.GetOk("basic_auth.0.username") - password, passOk := d.GetOk("basic_auth.0.password") - if userOk && passOk { - tokenUnB64 := fmt.Sprintf("%s:%s", username.(string), password.(string)) - config.Token = base64.StdEncoding.EncodeToString([]byte(tokenUnB64)) - config.AuthType = service.BasicAuth - } + } + if host, ok := d.GetOk("host"); ok { + config.Host = host.(string) + } + if token, ok := d.GetOk("token"); ok { + config.Token = token.(string) + } + // Basic authentication setup via username and password + if _, ok := d.GetOk("basic_auth"); ok { + username, userOk := d.GetOk("basic_auth.0.username") + password, passOk := d.GetOk("basic_auth.0.password") + if userOk && passOk { + tokenUnB64 := fmt.Sprintf("%s:%s", username.(string), password.(string)) + config.Token = base64.StdEncoding.EncodeToString([]byte(tokenUnB64)) + config.AuthType = service.BasicAuth } - - // Final catch all in case basic_auth/token + host is not setup - if config.Host == "" || config.Token == "" { - if err := tryDatabricksCliConfigFile(d, &config); err != nil { - return nil, fmt.Errorf("failed to get credentials from config file; error msg: %w", err) - } + } + // Final catch all in case basic_auth/token + host is not setup + if config.Host == "" || config.Token == "" { + if err := tryDatabricksCliConfigFile(d, &config); err != nil { + return nil, fmt.Errorf("failed to get credentials from config file; error msg: %w", err) } } - var dbClient service.DBApiClient dbClient.SetConfig(&config) return &dbClient, nil diff --git a/databricks/resource_databricks_group_instance_profile.go b/databricks/resource_databricks_group_instance_profile.go index 424804cde..aa104ed14 100644 --- a/databricks/resource_databricks_group_instance_profile.go +++ b/databricks/resource_databricks_group_instance_profile.go @@ -36,7 +36,7 @@ func resourceGroupInstanceProfileCreate(d *schema.ResourceData, m interface{}) e client := m.(*service.DBApiClient) groupID := d.Get("group_id").(string) instanceProfileID := d.Get("instance_profile_id").(string) - groupInstanceProfileID := &GroupInstanceProfileID{ + groupInstanceProfileID := &groupInstanceProfileID{ GroupID: groupID, InstanceProfileID: instanceProfileID, } @@ -54,7 +54,7 @@ func resourceGroupInstanceProfileCreate(d *schema.ResourceData, m interface{}) e func resourceGroupInstanceProfileRead(d *schema.ResourceData, m interface{}) error { id := d.Id() client := m.(*service.DBApiClient) - groupInstanceProfileID := parseGroupInstanceProfileID(id) + groupInstanceProfileID := parsegroupInstanceProfileID(id) group, err := client.Groups().Read(groupInstanceProfileID.GroupID) // First verify if the group exists @@ -86,7 +86,7 @@ func resourceGroupInstanceProfileRead(d *schema.ResourceData, m interface{}) err func resourceGroupInstanceProfileDelete(d *schema.ResourceData, m interface{}) error { id := d.Id() client := m.(*service.DBApiClient) - groupInstanceProfileID := parseGroupInstanceProfileID(id) + groupInstanceProfileID := parsegroupInstanceProfileID(id) roleRemoveList := []string{groupInstanceProfileID.InstanceProfileID} // Patch op to remove role from group @@ -94,18 +94,18 @@ func resourceGroupInstanceProfileDelete(d *schema.ResourceData, m interface{}) e return err } -type GroupInstanceProfileID struct { +type groupInstanceProfileID struct { GroupID string InstanceProfileID string } -func (g GroupInstanceProfileID) String() string { +func (g groupInstanceProfileID) String() string { return fmt.Sprintf("%s|%s", g.GroupID, g.InstanceProfileID) } -func parseGroupInstanceProfileID(id string) *GroupInstanceProfileID { +func parsegroupInstanceProfileID(id string) *groupInstanceProfileID { parts := strings.Split(id, "|") - return &GroupInstanceProfileID{ + return &groupInstanceProfileID{ GroupID: parts[0], InstanceProfileID: parts[1], } diff --git a/databricks/resource_databricks_group_member.go b/databricks/resource_databricks_group_member.go index 61d965af3..62afdd668 100644 --- a/databricks/resource_databricks_group_member.go +++ b/databricks/resource_databricks_group_member.go @@ -37,7 +37,7 @@ func resourceGroupMemberCreate(d *schema.ResourceData, m interface{}) error { groupID := d.Get("group_id").(string) memberID := d.Get("member_id").(string) - groupMemberID := &GroupMemberID{ + groupMemberID := &groupMemberID{ GroupID: groupID, MemberID: memberID, } @@ -55,7 +55,7 @@ func resourceGroupMemberCreate(d *schema.ResourceData, m interface{}) error { func resourceGroupMemberRead(d *schema.ResourceData, m interface{}) error { id := d.Id() client := m.(*service.DBApiClient) - groupMemberID := parseGroupMemberID(id) + groupMemberID := parsegroupMemberID(id) group, err := client.Groups().Read(groupMemberID.GroupID) // First verify if the group exists @@ -87,7 +87,7 @@ func resourceGroupMemberRead(d *schema.ResourceData, m interface{}) error { func resourceGroupMemberDelete(d *schema.ResourceData, m interface{}) error { id := d.Id() client := m.(*service.DBApiClient) - groupMemberID := parseGroupMemberID(id) + groupMemberID := parsegroupMemberID(id) memberRemoveList := []string{groupMemberID.MemberID} // Patch op to remove member from group @@ -95,18 +95,18 @@ func resourceGroupMemberDelete(d *schema.ResourceData, m interface{}) error { return err } -type GroupMemberID struct { +type groupMemberID struct { GroupID string MemberID string } -func (g GroupMemberID) String() string { +func (g groupMemberID) String() string { return fmt.Sprintf("%s|%s", g.GroupID, g.MemberID) } -func parseGroupMemberID(id string) *GroupMemberID { +func parsegroupMemberID(id string) *groupMemberID { parts := strings.Split(id, "|") - return &GroupMemberID{ + return &groupMemberID{ GroupID: parts[0], MemberID: parts[1], } diff --git a/databricks/utils_test.go b/databricks/utils_test.go index e2384909e..5a571592e 100644 --- a/databricks/utils_test.go +++ b/databricks/utils_test.go @@ -1,16 +1,21 @@ package databricks import ( + "bytes" + "encoding/json" "errors" "fmt" + "net/http" + "net/http/httptest" "os" "strconv" "testing" - "github.com/databrickslabs/databricks-terraform/client/service" "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" - + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/stretchr/testify/assert" + + "github.com/databrickslabs/databricks-terraform/client/service" ) func TestMissingMWSResources(t *testing.T) { @@ -18,9 +23,9 @@ func TestMissingMWSResources(t *testing.T) { t.Skip("skipping integration test in short mode.") } - mwsAcctId := os.Getenv("DATABRICKS_MWS_ACCT_ID") - randStringId := acctest.RandString(10) - randIntId := 2000000 + acctest.RandIntRange(100000, 20000000) + mwsAcctID := os.Getenv("DATABRICKS_MWS_ACCT_ID") + randStringID := acctest.RandString(10) + randIntID := 2000000 + acctest.RandIntRange(100000, 20000000) client := getMWSClient() tests := []struct { @@ -33,28 +38,28 @@ func TestMissingMWSResources(t *testing.T) { { name: "CheckIfMWSCredentialsAreMissing", readFunc: func() error { - _, err := client.MWSCredentials().Read(mwsAcctId, randStringId) + _, err := client.MWSCredentials().Read(mwsAcctID, randStringID) return err }, }, { name: "CheckIfMWSNetworksAreMissing", readFunc: func() error { - _, err := client.MWSNetworks().Read(mwsAcctId, randStringId) + _, err := client.MWSNetworks().Read(mwsAcctID, randStringID) return err }, }, { name: "CheckIfMWSStorageConfigurationsAreMissing", readFunc: func() error { - _, err := client.MWSStorageConfigurations().Read(mwsAcctId, randStringId) + _, err := client.MWSStorageConfigurations().Read(mwsAcctID, randStringID) return err }, }, { name: "CheckIfMWSWorkspacesAreMissing", readFunc: func() error { - _, err := client.MWSWorkspaces().Read(mwsAcctId, int64(randIntId)) + _, err := client.MWSWorkspaces().Read(mwsAcctID, int64(randIntID)) return err }, }, @@ -86,14 +91,14 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi t.Skip("Acceptance tests skipped unless env 'TF_ACC' set") } - randIntId := 2000000 + acctest.RandIntRange(100000, 20000000) - randStringId := acctest.RandString(10) + randIntID := 2000000 + acctest.RandIntRange(100000, 20000000) + randStringID := acctest.RandString(10) // example 405E7E8E4A000024 - randomClusterPolicyId := fmt.Sprintf("400E9E9E9A%d", + randomClusterPolicyID := fmt.Sprintf("400E9E9E9A%d", acctest.RandIntRange(100000, 999999), ) // example 0101-120000-brick1-pool-ABCD1234 - randomInstancePoolId := fmt.Sprintf( + randomInstancePoolID := fmt.Sprintf( "%v-%v-%s-pool-%s", acctest.RandIntRange(1000, 9999), acctest.RandIntRange(100000, 999999), @@ -113,35 +118,35 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi { name: "CheckIfTokensAreMissing", readFunc: func() error { - _, err := client.Tokens().Read(randStringId) + _, err := client.Tokens().Read(randStringID) return err }, }, { name: "CheckIfSecretScopesAreMissing", readFunc: func() error { - _, err := client.SecretScopes().Read(randStringId) + _, err := client.SecretScopes().Read(randStringID) return err }, }, { name: "CheckIfSecretsAreMissing", readFunc: func() error { - _, err := client.Secrets().Read(randStringId, randStringId) + _, err := client.Secrets().Read(randStringID, randStringID) return err }, }, { name: "CheckIfSecretsACLsAreMissing", readFunc: func() error { - _, err := client.SecretAcls().Read(randStringId, randStringId) + _, err := client.SecretAcls().Read(randStringID, randStringID) return err }, }, { name: "CheckIfSecretsACLsAreMissing", readFunc: func() error { - _, err := client.SecretAcls().Read(randStringId, randStringId) + _, err := client.SecretAcls().Read(randStringID, randStringID) return err }, }, @@ -149,38 +154,38 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi name: "CheckIfNotebooksAreMissing", readFunc: func() error { // ID must start with a / - _, err := client.Notebooks().Read("/" + randStringId) + _, err := client.Notebooks().Read("/" + randStringID) return err }, }, { name: "CheckIfInstancePoolsAreMissing", readFunc: func() error { - _, err := client.InstancePools().Read(randomInstancePoolId) + _, err := client.InstancePools().Read(randomInstancePoolID) return err }, }, { name: "CheckIfClustersAreMissing", readFunc: func() error { - _, err := client.Clusters().Get(randStringId) + _, err := client.Clusters().Get(randStringID) return err }, isCustomCheck: true, customCheckFunc: isClusterMissing, - resourceID: randStringId, + resourceID: randStringID, }, { name: "CheckIfDBFSFilesAreMissing", readFunc: func() error { - _, err := client.DBFS().Read("/" + randStringId) + _, err := client.DBFS().Read("/" + randStringID) return err }, }, { name: "CheckIfGroupsAreMissing", readFunc: func() error { - _, err := client.Groups().Read(randStringId) + _, err := client.Groups().Read(randStringID) t.Log(err) return err }, @@ -188,7 +193,7 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi { name: "CheckIfUsersAreMissing", readFunc: func() error { - _, err := client.Users().Read(randStringId) + _, err := client.Users().Read(randStringID) t.Log(err) return err }, @@ -196,7 +201,7 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi { name: "CheckIfClusterPoliciesAreMissing", readFunc: func() error { - _, err := client.ClusterPolicies().Get(randomClusterPolicyId) + _, err := client.ClusterPolicies().Get(randomClusterPolicyID) t.Log(err) return err }, @@ -204,12 +209,12 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi { name: "CheckIfJobsAreMissing", readFunc: func() error { - _, err := client.Jobs().Read(int64(randIntId)) + _, err := client.Jobs().Read(int64(randIntID)) return err }, isCustomCheck: true, customCheckFunc: isJobMissing, - resourceID: strconv.Itoa(randIntId), + resourceID: strconv.Itoa(randIntID), }, } // Handle aws only tests where instance profiles only exist on aws @@ -218,7 +223,7 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi { name: "CheckIfInstanceProfilesAreMissing", readFunc: func() error { - _, err := client.InstanceProfiles().Read(randStringId) + _, err := client.InstanceProfiles().Read(randStringID) return err }, }, @@ -237,13 +242,13 @@ func testMissingWorkspaceResources(t *testing.T, cloud service.CloudServiceProvi } } -func testVerifyResourceIsMissingCustomVerification(t *testing.T, resourceId string, readFunc func() error, +func testVerifyResourceIsMissingCustomVerification(t *testing.T, resourceID string, readFunc func() error, customCheck func(err error, rId string) bool) { err := readFunc() assert.NotNil(t, err, "err should not be nil") assert.IsType(t, err, service.APIError{}, fmt.Sprintf("error: %s is not type api error", err.Error())) if apiError, ok := err.(service.APIError); ok { - assert.True(t, customCheck(err, resourceId), fmt.Sprintf("error: %v is not missing;"+ + assert.True(t, customCheck(err, resourceID), fmt.Sprintf("error: %v is not missing;"+ "\nstatus code: %v;"+ "\nerror code: %s", apiError, apiError.StatusCode, apiError.ErrorCode)) @@ -260,17 +265,7 @@ func testVerifyResourceIsMissing(t *testing.T, readFunc func() error) { "\nerror code: %s", apiError, apiError.StatusCode, apiError.ErrorCode)) } - "bytes" - "encoding/json" - "fmt" - "net/http" - "net/http/httptest" - "testing" - - "github.com/databrickslabs/databricks-terraform/client/service" - "github.com/hashicorp/terraform-plugin-sdk/helper/schema" - "github.com/stretchr/testify/assert" -) +} // HTTPFixture defines request structure for test type HTTPFixture struct { From 91d824348767421c850554232d16c9c6faa355c6 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 11:36:09 +0200 Subject: [PATCH 31/42] Added make install --- Makefile | 20 ++++++++++++-------- databricks/mounts.go | 6 +++--- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index f603c8315..8cea675c4 100644 --- a/Makefile +++ b/Makefile @@ -26,14 +26,6 @@ coverage-int: int int-build: int build -build: lint test - @echo "==> Building source code with go build..." - @go build -mod vendor -v -o terraform-provider-databricks - -lint: - @echo "==> Linting source code with golangci-lint make sure you run make fmt ..." - @golangci-lint run --skip-dirs-use-default --timeout 5m - fmt: @echo "==> Formatting source code with gofmt..." @goimports -w client @@ -44,6 +36,18 @@ fmt: @gofmt -s -w main.go @go fmt ./... +lint: fmt + @echo "==> Linting source code with golangci-lint make sure you run make fmt ..." + @golangci-lint run --skip-dirs-use-default --timeout 5m + +build: lint test + @echo "==> Building source code with go build..." + @go build -mod vendor -v -o terraform-provider-databricks + +install: build + @rm ~/.terraform.d/plugins/terraform-provider-databricks* + @mv terraform-provider-databricks ~/.terraform.d/plugins + vendor: @echo "==> Filling vendor folder with library code..." @go mod vendor diff --git a/databricks/mounts.go b/databricks/mounts.go index a5a3cedbd..410d8963c 100644 --- a/databricks/mounts.go +++ b/databricks/mounts.go @@ -189,7 +189,7 @@ for mount in dbutils.fs.mounts(): storageAccount != m.StorageAccountName && m.Directory != directory { return "", fmt.Errorf("does not match uri with storage account and container values"+ - " %s@%s != %s!", m.ContainerName, m.StorageAccountName, resp.Results.Data.(string)) + " %s@%s != %s", m.ContainerName, m.StorageAccountName, resp.Results.Data.(string)) } return resp.Results.Data.(string), nil } @@ -289,7 +289,7 @@ for mount in dbutils.fs.mounts(): if resp.Results.ResultType == "text" && storageResource != m.StorageResource && m.Directory != directory { return "", fmt.Errorf("does not match uri with storage account and container values"+ - " %s@%s != %s!", m.StorageResource, m.Directory, resp.Results.Data.(string)) + " %s@%s != %s", m.StorageResource, m.Directory, resp.Results.Data.(string)) } return resp.Results.Data.(string), nil } @@ -395,7 +395,7 @@ for mount in dbutils.fs.mounts(): if resp.Results.ResultType == "text" && containerName != m.ContainerName && m.StorageAccountName != storageAccountName && m.Directory != directory { return "", fmt.Errorf("does not match uri with storage account and container values"+ - " %s@%s != %s!", m.ContainerName, m.StorageAccountName, resp.Results.Data.(string)) + " %s@%s != %s", m.ContainerName, m.StorageAccountName, resp.Results.Data.(string)) } return resp.Results.Data.(string), nil } From 47267fdfeef9bfe0c236ae988db42dbc6469d4d0 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 11:46:35 +0200 Subject: [PATCH 32/42] make build will also test now (provider + client) --- .travis.yml | 5 +---- Makefile | 25 +++++++++++++------------ 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/.travis.yml b/.travis.yml index 83caf834a..d2a0bbbf4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,8 +9,6 @@ jobs: script: - curl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsum - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0 - - time make provider-test - - time make client-test - time make build - go: 1.14.x gemfile: gemfiles/Gemfile.rails-3.0.x @@ -21,8 +19,6 @@ jobs: script: - curl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsum - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0 - - time make provider-test - - time make client-test - time make build @@ -30,6 +26,7 @@ after_success: - echo "travis go version='$TRAVIS_GO_VERSION'" - bash <(curl -s https://codecov.io/bash) -f client-coverage.txt -F client - bash <(curl -s https://codecov.io/bash) -f provider-coverage.txt -F provider + - bash <(curl -s https://codecov.io/bash) -f coverage.txt -F repository notifications: webhooks: https://www.travisbuddy.com/ diff --git a/Makefile b/Makefile index 8cea675c4..54e87d66d 100644 --- a/Makefile +++ b/Makefile @@ -1,17 +1,5 @@ default: build -test: lint - @echo "==> Running tests..." - @gotestsum --format short-verbose --raw-command go test -v -json -short -coverprofile=coverage.txt ./... - -client-test: - @echo "==> Running tests..." - @gotestsum --format short-verbose --raw-command go test -v -json -short -coverprofile=client-coverage.txt ./client/... - -provider-test: - @echo "==> Running tests..." - @gotestsum --format short-verbose --raw-command go test -v -json -short -coverprofile=provider-coverage.txt ./databricks/... - int: @echo "==> Running tests..." @gotestsum --raw-command go test -v -json -coverprofile=coverage.txt ./... @@ -40,11 +28,24 @@ lint: fmt @echo "==> Linting source code with golangci-lint make sure you run make fmt ..." @golangci-lint run --skip-dirs-use-default --timeout 5m +client-test: + @echo "==> Running tests..." + @gotestsum --format short-verbose --raw-command go test -v -json -short -coverprofile=client-coverage.txt ./client/... + +provider-test: + @echo "==> Running tests..." + @gotestsum --format short-verbose --raw-command go test -v -json -short -coverprofile=provider-coverage.txt ./databricks/... + +test: lint client-test provider-test + @echo "==> Running tests..." + @gotestsum --format short-verbose --raw-command go test -v -json -short -coverprofile=coverage.txt ./... + build: lint test @echo "==> Building source code with go build..." @go build -mod vendor -v -o terraform-provider-databricks install: build + @echo "==> Installing provider into ~/.terraform.d/plugins ..." @rm ~/.terraform.d/plugins/terraform-provider-databricks* @mv terraform-provider-databricks ~/.terraform.d/plugins From 598f87ab18462dad793f10c21e2f3995becc5bf6 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 13:26:39 +0200 Subject: [PATCH 33/42] Added more coverage for edge cases on permissions --- client/model/notebook.go | 4 +- client/service/client.go | 5 +- client/service/clusters_test.go | 4 +- client/service/notebooks.go | 14 +- client/service/notebooks_test.go | 14 +- databricks/resource_databricks_permissions.go | 4 + .../resource_databricks_permissions_test.go | 252 ++++++++++++++++-- databricks/utils_test.go | 41 ++- 8 files changed, 301 insertions(+), 37 deletions(-) diff --git a/client/model/notebook.go b/client/model/notebook.go index fa078836d..e6a28b33a 100644 --- a/client/model/notebook.go +++ b/client/model/notebook.go @@ -32,8 +32,8 @@ const ( LibraryObject ObjectType = "LIBRARY" ) -// NotebookInfo contains information when doing a get request or list request on the workspace api -type NotebookInfo struct { +// WorkspaceObjectStatus contains information when doing a get request or list request on the workspace api +type WorkspaceObjectStatus struct { ObjectID int64 `json:"object_id,omitempty"` ObjectType ObjectType `json:"object_type,omitempty"` Path string `json:"path,omitempty"` diff --git a/client/service/client.go b/client/service/client.go index f80c32bb0..c2538c243 100644 --- a/client/service/client.go +++ b/client/service/client.go @@ -29,7 +29,8 @@ const ( Azure CloudServiceProvider = "Azure" ) -type apiErrorBody struct { +// APIErrorBody maps "proper" databricks rest api errors to a struct +type APIErrorBody struct { ErrorCode string `json:"error_code,omitempty"` Message string `json:"message,omitempty"` // The following two are for scim api only for RFC 7644 Section 3.7.3 https://tools.ietf.org/html/rfc7644#section-3.7.3 @@ -154,7 +155,7 @@ func checkHTTPRetry(ctx context.Context, resp *http.Response, err error) (bool, if err != nil { return false, err } - var errorBody apiErrorBody + var errorBody APIErrorBody err = json.Unmarshal(body, &errorBody) // this is most likely HTML... since un-marshalling JSON failed if err != nil { diff --git a/client/service/clusters_test.go b/client/service/clusters_test.go index 4be6dedb8..b7a46d723 100644 --- a/client/service/clusters_test.go +++ b/client/service/clusters_test.go @@ -459,7 +459,7 @@ func TestClustersAPI_WaitForClusterRunning(t *testing.T) { requestMethod []string args []interface{} wantURI []string - want []model.NotebookInfo + want []model.WorkspaceObjectStatus wantErr bool }{ { @@ -605,7 +605,7 @@ func TestClustersAPI_WaitForClusterTerminated(t *testing.T) { requestMethod []string args []interface{} wantURI []string - want []model.NotebookInfo + want []model.WorkspaceObjectStatus wantErr bool }{ { diff --git a/client/service/notebooks.go b/client/service/notebooks.go index 239124462..507ba86ae 100644 --- a/client/service/notebooks.go +++ b/client/service/notebooks.go @@ -32,8 +32,8 @@ func (a NotebooksAPI) Create(path string, content string, language model.Languag } // Read returns the notebook metadata and not the contents -func (a NotebooksAPI) Read(path string) (model.NotebookInfo, error) { - var notebookInfo model.NotebookInfo +func (a NotebooksAPI) Read(path string) (model.WorkspaceObjectStatus, error) { + var notebookInfo model.WorkspaceObjectStatus notebookGetStatusRequest := struct { Path string `json:"path,omitempty" url:"path,omitempty"` }{} @@ -79,9 +79,9 @@ func (a NotebooksAPI) Mkdirs(path string) error { // List will list all objects in a path on the workspace and with the recursive flag it will recursively list // all the objects -func (a NotebooksAPI) List(path string, recursive bool) ([]model.NotebookInfo, error) { +func (a NotebooksAPI) List(path string, recursive bool) ([]model.WorkspaceObjectStatus, error) { if recursive { - var paths []model.NotebookInfo + var paths []model.WorkspaceObjectStatus err := a.recursiveAddPaths(path, &paths) if err != nil { return nil, err @@ -91,7 +91,7 @@ func (a NotebooksAPI) List(path string, recursive bool) ([]model.NotebookInfo, e return a.list(path) } -func (a NotebooksAPI) recursiveAddPaths(path string, pathList *[]model.NotebookInfo) error { +func (a NotebooksAPI) recursiveAddPaths(path string, pathList *[]model.WorkspaceObjectStatus) error { notebookInfoList, err := a.list(path) if err != nil { return err @@ -109,9 +109,9 @@ func (a NotebooksAPI) recursiveAddPaths(path string, pathList *[]model.NotebookI return err } -func (a NotebooksAPI) list(path string) ([]model.NotebookInfo, error) { +func (a NotebooksAPI) list(path string) ([]model.WorkspaceObjectStatus, error) { var notebookList struct { - Objects []model.NotebookInfo `json:"objects,omitempty" url:"objects,omitempty"` + Objects []model.WorkspaceObjectStatus `json:"objects,omitempty" url:"objects,omitempty"` } listRequest := struct { Path string `json:"path,omitempty" url:"path,omitempty"` diff --git a/client/service/notebooks_test.go b/client/service/notebooks_test.go index d1a6a66d8..3bcc13aab 100644 --- a/client/service/notebooks_test.go +++ b/client/service/notebooks_test.go @@ -119,7 +119,7 @@ func TestNotebooksAPI_ListNonRecursive(t *testing.T) { responseStatus int args args wantURI string - want []model.NotebookInfo + want []model.WorkspaceObjectStatus wantErr bool }{ { @@ -146,7 +146,7 @@ func TestNotebooksAPI_ListNonRecursive(t *testing.T) { Recursive: false, }, wantURI: "/api/2.0/workspace/list?path=%2Ftest%2Fpath", - want: []model.NotebookInfo{ + want: []model.WorkspaceObjectStatus{ { ObjectID: 123, ObjectType: model.Directory, @@ -183,7 +183,7 @@ func TestNotebooksAPI_ListRecursive(t *testing.T) { responseStatus []int args []interface{} wantURI []string - want []model.NotebookInfo + want []model.WorkspaceObjectStatus wantErr bool }{ { @@ -222,7 +222,7 @@ func TestNotebooksAPI_ListRecursive(t *testing.T) { }, }, wantURI: []string{"/api/2.0/workspace/list?path=%2Ftest%2Fpath", "/api/2.0/workspace/list?path=%2FUsers%2Fuser%40example.com%2Fproject"}, - want: []model.NotebookInfo{ + want: []model.WorkspaceObjectStatus{ { ObjectID: 457, ObjectType: model.Notebook, @@ -288,7 +288,7 @@ func TestNotebooksAPI_Read(t *testing.T) { args args responseStatus int wantURI string - want model.NotebookInfo + want model.WorkspaceObjectStatus wantErr bool }{ { @@ -303,7 +303,7 @@ func TestNotebooksAPI_Read(t *testing.T) { Path: "/test/path", }, responseStatus: http.StatusOK, - want: model.NotebookInfo{ + want: model.WorkspaceObjectStatus{ ObjectID: 789, ObjectType: model.Notebook, Path: "/Users/user@example.com/project/ScalaExampleNotebook", @@ -320,7 +320,7 @@ func TestNotebooksAPI_Read(t *testing.T) { Path: "/test/path", }, responseStatus: http.StatusBadRequest, - want: model.NotebookInfo{}, + want: model.WorkspaceObjectStatus{}, wantURI: "/api/2.0/workspace/get-status?path=%2Ftest%2Fpath", wantErr: true, }, diff --git a/databricks/resource_databricks_permissions.go b/databricks/resource_databricks_permissions.go index 914629d96..4b8db33fa 100644 --- a/databricks/resource_databricks_permissions.go +++ b/databricks/resource_databricks_permissions.go @@ -75,6 +75,10 @@ func resourcePermissionsRead(d *schema.ResourceData, m interface{}) error { id := d.Id() client := m.(*service.DBApiClient) objectACL, err := client.Permissions().Read(id) + if e, ok := err.(service.APIError); ok && e.IsMissing() { + d.SetId("") + return nil + } if err != nil { return err } diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index 547146e0a..0b9349e24 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -56,11 +56,10 @@ func TestPermissionsRead(t *testing.T) { UserName: TestingAdminUser, }, }, - }, resourcePermissions, map[string]interface{}{}, - func(d *schema.ResourceData, c interface{}) error { - d.SetId("/clusters/abc") - return resourcePermissionsRead(d, c) - }) + }, resourcePermissions, nil, func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsRead(d, c) + }) assert.NoError(t, err, err) assert.Equal(t, "/clusters/abc", d.Id()) assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) @@ -68,6 +67,72 @@ func TestPermissionsRead(t *testing.T) { assert.Equal(t, 1, d.Get("access_control.#")) } +func TestPermissionsRead_some_error(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: service.APIErrorBody{ + ErrorCode: "INVALID_REQUEST", + Message: "Internal error happened", + }, + Status: 400, + }, + }, resourcePermissions, map[string]interface{}{}, + func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsRead(d, c) + }) + assert.Error(t, err) +} + +func TestPermissionsRead_ErrorOnScimMe(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/clusters/abc?", + Response: model.ObjectACL{ + ObjectID: "/clusters/abc", + ObjectType: "clusters", + AccessControlList: []*model.AccessControl{ + { + UserName: &TestingUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_READ", + Inherited: false, + }, + }, + }, + { + UserName: &TestingAdminUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_MANAGE", + Inherited: false, + }, + }, + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: service.APIErrorBody{ + ErrorCode: "INVALID_REQUEST", + Message: "Internal error happened", + }, + Status: 400, + }, + }, resourcePermissions, map[string]interface{}{}, + func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsRead(d, c) + }) + assert.Error(t, err) +} + func TestPermissionsDelete(t *testing.T) { d, err := ResourceTester(t, []HTTPFixture{ { @@ -75,18 +140,36 @@ func TestPermissionsDelete(t *testing.T) { Resource: "/api/2.0/preview/permissions/clusters/abc", ExpectedRequest: model.ObjectACL{}, }, - }, resourcePermissions, map[string]interface{}{}, - func(d *schema.ResourceData, c interface{}) error { - d.SetId("/clusters/abc") - return resourcePermissionsDelete(d, c) - }) + }, resourcePermissions, nil, func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsDelete(d, c) + }) assert.NoError(t, err, err) assert.Equal(t, "/clusters/abc", d.Id()) } +func TestPermissionsDelete_error(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPut, + Resource: "/api/2.0/preview/permissions/clusters/abc", + ExpectedRequest: model.ObjectACL{}, + Response: service.APIErrorBody{ + ErrorCode: "INVALID_REQUEST", + Message: "Internal error happened", + }, + Status: 400, + }, + }, resourcePermissions, nil, func(d *schema.ResourceData, c interface{}) error { + d.SetId("/clusters/abc") + return resourcePermissionsDelete(d, c) + }) + assert.Error(t, err) +} + func TestPermissionsCreate_invalid(t *testing.T) { - _, err := ResourceTester(t, []HTTPFixture{}, resourcePermissions, map[string]interface{}{}, - resourcePermissionsCreate) + _, err := ResourceTester(t, []HTTPFixture{}, resourcePermissions, + nil, resourcePermissionsCreate) assert.EqualError(t, err, "At least one type of resource identifiers must be set") } @@ -95,7 +178,22 @@ func TestPermissionsCreate_no_access_control(t *testing.T) { map[string]interface{}{ "cluster_id": "abc", }, resourcePermissionsCreate) - assert.EqualError(t, err, "At least one access_control is required") + assert.EqualError(t, err, "Invalid config supplied. access_control: required field is not set") +} + +func TestPermissionsCreate_conflicting_fields(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{}, resourcePermissions, + map[string]interface{}{ + "cluster_id": "abc", + "notebook_path": "/Init", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_READ", + }, + }, + }, resourcePermissionsCreate) + assert.EqualError(t, err, "Invalid config supplied. cluster_id: conflicts with notebook_path. notebook_path: conflicts with cluster_id") } func TestPermissionsCreate(t *testing.T) { @@ -107,7 +205,7 @@ func TestPermissionsCreate(t *testing.T) { AccessControlList: []*model.AccessControlChange{ { UserName: &TestingUser, - PermissionLevel: "CAN_USE", + PermissionLevel: "CAN_READ", }, }, }, @@ -152,7 +250,7 @@ func TestPermissionsCreate(t *testing.T) { "access_control": []interface{}{ map[string]interface{}{ "user_name": TestingUser, - "permission_level": "CAN_USE", + "permission_level": "CAN_READ", }, }, }, resourcePermissionsCreate) @@ -163,6 +261,130 @@ func TestPermissionsCreate(t *testing.T) { assert.Equal(t, 1, d.Get("access_control.#")) } +func TestPermissionsCreate_NotebookPath_NotExists(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodGet, + Resource: "/api/2.0/workspace/get-status?path=%2FDevelopment%2FInit", + Response: service.APIErrorBody{ + ErrorCode: "INVALID_REQUEST", + Message: "Internal error happened", + }, + Status: 400, + }, + }, resourcePermissions, map[string]interface{}{ + "notebook_path": "/Development/Init", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_USE", + }, + }, + }, resourcePermissionsCreate) + + assert.Error(t, err) +} + +func TestPermissionsCreate_NotebookPath(t *testing.T) { + d, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodGet, + Resource: "/api/2.0/workspace/get-status?path=%2FDevelopment%2FInit", + Response: model.WorkspaceObjectStatus{ + ObjectID: 988765, + ObjectType: "NOTEBOOK", + }, + }, + { + Method: http.MethodPatch, + Resource: "/api/2.0/preview/permissions/notebooks/988765", + ExpectedRequest: model.AccessControlChangeList{ + AccessControlList: []*model.AccessControlChange{ + { + UserName: &TestingUser, + PermissionLevel: "CAN_USE", + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/permissions/notebooks/988765?", + Response: model.ObjectACL{ + ObjectID: "/notebooks/988765", + ObjectType: "notebooks", + AccessControlList: []*model.AccessControl{ + { + UserName: &TestingUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_USE", + Inherited: false, + }, + }, + }, + { + UserName: &TestingAdminUser, + AllPermissions: []*model.Permission{ + { + PermissionLevel: "CAN_MANAGE", + Inherited: false, + }, + }, + }, + }, + }, + }, + { + Method: http.MethodGet, + Resource: "/api/2.0/preview/scim/v2/Me?", + Response: model.User{ + UserName: TestingAdminUser, + }, + }, + }, resourcePermissions, map[string]interface{}{ + "notebook_path": "/Development/Init", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_USE", + }, + }, + }, resourcePermissionsCreate) + + assert.NoError(t, err, err) + assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) + assert.Equal(t, "CAN_USE", d.Get("access_control.0.permission_level")) + assert.Equal(t, 1, d.Get("access_control.#")) +} + +func TestPermissionsCreate_error(t *testing.T) { + _, err := ResourceTester(t, []HTTPFixture{ + { + Method: http.MethodPatch, + Resource: "/api/2.0/preview/permissions/clusters/abc", + Response: service.APIErrorBody{ + ErrorCode: "INVALID_REQUEST", + Message: "Internal error happened", + }, + Status: 400, + }, + }, resourcePermissions, map[string]interface{}{ + "cluster_id": "abc", + "access_control": []interface{}{ + map[string]interface{}{ + "user_name": TestingUser, + "permission_level": "CAN_USE", + }, + }, + }, resourcePermissionsCreate) + if assert.Error(t, err) { + if e, ok := err.(service.APIError); ok { + assert.Equal(t, "INVALID_REQUEST", e.ErrorCode) + } + } +} + func TestAccDatabricksPermissionsResourceFullLifecycle(t *testing.T) { var permissions model.ObjectACL randomName := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum) diff --git a/databricks/utils_test.go b/databricks/utils_test.go index 5a571592e..808a5b8e1 100644 --- a/databricks/utils_test.go +++ b/databricks/utils_test.go @@ -8,11 +8,14 @@ import ( "net/http" "net/http/httptest" "os" + "sort" "strconv" + "strings" "testing" "github.com/hashicorp/terraform-plugin-sdk/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/terraform" "github.com/stretchr/testify/assert" "github.com/databrickslabs/databricks-terraform/client/service" @@ -267,6 +270,12 @@ func testVerifyResourceIsMissing(t *testing.T, readFunc func() error) { } } +type errorSlice []error + +func (a errorSlice) Len() int { return len(a) } +func (a errorSlice) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a errorSlice) Less(i, j int) bool { return a[i].Error() < a[j].Error() } + // HTTPFixture defines request structure for test type HTTPFixture struct { Method string @@ -326,8 +335,36 @@ func ResourceTester(t *testing.T, var client service.DBApiClient client.SetConfig(&config) - resource := resouceFunc() - resourceData := schema.TestResourceDataRaw(t, resource.Schema, state) + res := resouceFunc() + + if state != nil { + resourceConfig := terraform.NewResourceConfigRaw(state) + warns, errs := res.Validate(resourceConfig) + if len(warns) > 0 || len(errs) > 0 { + var issues string + if len(warns) > 0 { + sort.Strings(warns) + issues += ". " + strings.Join(warns, ". ") + } + if len(errs) > 0 { + sort.Sort(errorSlice(errs)) + for _, err := range errs { + issues += ". " + err.Error() + } + } + // remove characters that need escaping, it's only tests... + issues = strings.ReplaceAll(issues, "\"", "") + return nil, fmt.Errorf("Invalid config supplied%s", issues) + } + } + + resourceData := schema.TestResourceDataRaw(t, res.Schema, state) + err := res.InternalValidate(res.Schema, true) + if err != nil { + return nil, err + } + + // warns, errs := schemaMap(r.Schema).Validate(c) return resourceData, whatever(resourceData, &client) } From 54d68938e2b88a27c41561581d994d942ec1043b Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Fri, 19 Jun 2020 18:34:47 +0200 Subject: [PATCH 34/42] Added databricks_permissions resource --- client/service/users.go | 1 + 1 file changed, 1 insertion(+) diff --git a/client/service/users.go b/client/service/users.go index 6df7050e7..35a741ff4 100644 --- a/client/service/users.go +++ b/client/service/users.go @@ -91,6 +91,7 @@ func (a UsersAPI) readByPath(userPath string) (model.User, error) { return user, err } + // Update will update the user given the user id, username, display name, entitlements and roles func (a UsersAPI) Update(userID string, userName string, displayName string, entitlements []string, roles []string) error { userPath := fmt.Sprintf("/preview/scim/v2/Users/%v", userID) From 1dd443f5530363157e60bd2126a9df7f97732bcb Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Mon, 22 Jun 2020 21:32:45 +0200 Subject: [PATCH 35/42] Minor commenting tweaks --- client/service/users.go | 1 - 1 file changed, 1 deletion(-) diff --git a/client/service/users.go b/client/service/users.go index 35a741ff4..6df7050e7 100644 --- a/client/service/users.go +++ b/client/service/users.go @@ -91,7 +91,6 @@ func (a UsersAPI) readByPath(userPath string) (model.User, error) { return user, err } - // Update will update the user given the user id, username, display name, entitlements and roles func (a UsersAPI) Update(userID string, userName string, displayName string, entitlements []string, roles []string) error { userPath := fmt.Sprintf("/preview/scim/v2/Users/%v", userID) From 83dca55f11a69db56804a8b5a619fb082084dc22 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Wed, 24 Jun 2020 00:14:04 +0200 Subject: [PATCH 36/42] trigger build --- databricks/resource_databricks_permissions_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index 0b9349e24..bc3b81568 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -254,7 +254,6 @@ func TestPermissionsCreate(t *testing.T) { }, }, }, resourcePermissionsCreate) - assert.NoError(t, err, err) assert.Equal(t, TestingUser, d.Get("access_control.0.user_name")) assert.Equal(t, "CAN_READ", d.Get("access_control.0.permission_level")) From 6e69eb0869e2ebf7fabbc56f72e196035dec0914 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 13:49:19 +0200 Subject: [PATCH 37/42] try fixing build --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index d2a0bbbf4..ae9749ded 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,6 +7,7 @@ jobs: - $HOME/.cache/go-build - $HOME/gopath/pkg/mod script: + - go get golang.org/x/tools/cmd/goimports - curl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsum - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0 - time make build @@ -17,6 +18,7 @@ jobs: - $HOME/.cache/go-build - $HOME/gopath/pkg/mod script: + - go get golang.org/x/tools/cmd/goimports - curl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsum - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0 - time make build From 458b2cf2c0f759265fffc2e4f61d249b7b3ef9a7 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 13:55:09 +0200 Subject: [PATCH 38/42] test linter failing build --- databricks/resource_databricks_permissions.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/databricks/resource_databricks_permissions.go b/databricks/resource_databricks_permissions.go index 4b8db33fa..93652f527 100644 --- a/databricks/resource_databricks_permissions.go +++ b/databricks/resource_databricks_permissions.go @@ -13,7 +13,7 @@ import ( func parsePermissionsFromData(d *schema.ResourceData, client *service.DBApiClient) (*model.AccessControlChangeList, string, error) { - var objectID string + var objectId string acl := new(model.AccessControlChangeList) for _, mapping := range permissionsResourceIDFields() { v, ok := d.GetOk(mapping.field) @@ -24,13 +24,15 @@ func parsePermissionsFromData(d *schema.ResourceData, if err != nil { return nil, "", err } - objectID = fmt.Sprintf("/%s/%s", mapping.resourceType, id) + objectId = fmt.Sprintf( + "/%s/%s", + mapping.resourceType, id) err = d.Set("object_type", mapping.objectType) if err != nil { return nil, "", err } } - if objectID == "" { + if objectId == "" { return nil, "", fmt.Errorf("At least one type of resource identifiers must be set") } changes := 0 @@ -52,9 +54,9 @@ func parsePermissionsFromData(d *schema.ResourceData, } } if changes < 1 { - return nil, "", fmt.Errorf("At least one access_control is required") + return nil, "", fmt.Errorf("At least one access_control is required!") } - return acl, objectID, nil + return acl, objectId, nil } func resourcePermissionsCreate(d *schema.ResourceData, m interface{}) error { From 70b1935ef5bc01b632d315464f7e20734dcfe119 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 13:58:54 +0200 Subject: [PATCH 39/42] remove fmt from lint --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 54e87d66d..1cf770e82 100644 --- a/Makefile +++ b/Makefile @@ -24,7 +24,7 @@ fmt: @gofmt -s -w main.go @go fmt ./... -lint: fmt +lint: @echo "==> Linting source code with golangci-lint make sure you run make fmt ..." @golangci-lint run --skip-dirs-use-default --timeout 5m From 239f2a9ede897ebc597191c4073871c8af845891 Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 14:04:14 +0200 Subject: [PATCH 40/42] format it back --- databricks/resource_databricks_permissions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/databricks/resource_databricks_permissions.go b/databricks/resource_databricks_permissions.go index 93652f527..3dbf87e9a 100644 --- a/databricks/resource_databricks_permissions.go +++ b/databricks/resource_databricks_permissions.go @@ -26,7 +26,7 @@ func parsePermissionsFromData(d *schema.ResourceData, } objectId = fmt.Sprintf( "/%s/%s", - mapping.resourceType, id) + mapping.resourceType, id) err = d.Set("object_type", mapping.objectType) if err != nil { return nil, "", err From 3fee89cf6a850d9b9c6ffe27f9933b9bb36669aa Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 14:30:51 +0200 Subject: [PATCH 41/42] trigger build --- databricks/resource_databricks_permissions_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/databricks/resource_databricks_permissions_test.go b/databricks/resource_databricks_permissions_test.go index bc3b81568..e36c28404 100644 --- a/databricks/resource_databricks_permissions_test.go +++ b/databricks/resource_databricks_permissions_test.go @@ -391,7 +391,6 @@ func TestAccDatabricksPermissionsResourceFullLifecycle(t *testing.T) { Providers: testAccProviders, Steps: []resource.TestStep{ { - // create a resource Config: testClusterPolicyPermissions(randomName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("databricks_permissions.dummy_can_use", From ed678fe203463fb97b52206837dace68d252568e Mon Sep 17 00:00:00 2001 From: Serge Smertin Date: Thu, 25 Jun 2020 14:37:20 +0200 Subject: [PATCH 42/42] remove go get goimports --- .travis.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index ae9749ded..d2a0bbbf4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,7 +7,6 @@ jobs: - $HOME/.cache/go-build - $HOME/gopath/pkg/mod script: - - go get golang.org/x/tools/cmd/goimports - curl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsum - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0 - time make build @@ -18,7 +17,6 @@ jobs: - $HOME/.cache/go-build - $HOME/gopath/pkg/mod script: - - go get golang.org/x/tools/cmd/goimports - curl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsum - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0 - time make build