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=