Refactor databricks_permissions to common.Resource (#1253)

Author: nfx

access/resource_ipaccesslist_test.go

@@ -313,25 +313,23 @@ func TestIPACLDelete(t *testing.T) {
 }
 
 func TestIPACLDelete_Error(t *testing.T) {
-	d, err := qa.ResourceFixture{
+	qa.ResourceFixture{
 		Fixtures: []qa.HTTPFixture{
 			{
 				Method:   http.MethodDelete,
 				Resource: "/api/2.0/ip-access-lists/" + TestingID,
 				Response: common.APIErrorBody{
-					ErrorCode: "FEATURE_DISABLE",
-					Message:   "IP access list is not available in the pricing tier of this workspace",
+					ErrorCode: "INVALID_STATE",
+					Message:   "Something went wrong",
 				},
-				Status: 404,
+				Status: 400,
 			},
 		},
 		Resource: ResourceIPAccessList(),
 		Delete:   true,
+		Removed:  true,
 		ID:       TestingID,
-	}.Apply(t)
-	assert.Error(t, err)
-	qa.AssertErrorStartsWith(t, err, "IP access list is not available in ")
-	assert.Equal(t, TestingID, d.Id())
+	}.ExpectError(t, "Something went wrong")
 }
 
 func TestListIpAccessLists(t *testing.T) {

common/resource.go

@@ -129,7 +129,14 @@ func (r Resource) ToResource() *schema.Resource {
 		UpdateContext: update,
 		DeleteContext: func(ctx context.Context, d *schema.ResourceData,
 			m interface{}) diag.Diagnostics {
-			if err := recoverable(r.Delete)(ctx, d, m.(*DatabricksClient)); err != nil {
+			err := recoverable(r.Delete)(ctx, d, m.(*DatabricksClient))
+			if IsMissing(err) {
+				log.Printf("[INFO] %s[id=%s] is removed on backend",
+					ResourceName.GetOrUnknown(ctx), d.Id())
+				d.SetId("")
+				return nil
+			}
+			if err != nil {
 				err = nicerError(ctx, err, "delete")
 				return diag.FromErr(err)
 			}

common/resource_test.go

@@ -37,6 +37,55 @@ func TestImportingCallsRead(t *testing.T) {
 	assert.Equal(t, 1, d.Get("foo"))
 }
 
+func TestHTTP404TriggersResourceRemovalForReadAndDelete(t *testing.T) {
+	nope := func(ctx context.Context,
+		d *schema.ResourceData,
+		c *DatabricksClient) error {
+		return NotFound("nope")
+	}
+	r := Resource{
+		Create: nope,
+		Read: nope,
+		Update: nope,
+		Delete: nope,
+		Schema: map[string]*schema.Schema{
+			"foo": {
+				Type:     schema.TypeInt,
+				Required: true,
+			},
+		},
+	}.ToResource()
+
+	client := &DatabricksClient{}
+	ctx := context.Background()
+	d := r.TestResourceData()
+	
+	// Create propagates 404 error
+	diags := r.CreateContext(ctx, d, client)
+	assert.True(t, diags.HasError())
+	assert.Equal(t, "nope", diags[0].Summary)
+
+	// Read removes the resource and swallows 404 error (clears ID)
+	d.SetId("a")
+	diags = r.ReadContext(ctx, d, client)
+	assert.False(t, diags.HasError())
+	assert.Equal(t, "", d.Id())
+
+	// Update propagates 404 error (keeps ID)
+	d.SetId("b")
+	diags = r.UpdateContext(ctx, d, client)
+	assert.True(t, diags.HasError())
+	assert.Equal(t, "nope", diags[0].Summary)
+	assert.Equal(t, "b", d.Id())
+
+	// Delete removes the resource and swallows 404 error
+	// if it was removed on backend (clears ID)
+	d.SetId("c")
+	diags = r.DeleteContext(ctx, d, client)
+	assert.False(t, diags.HasError())
+	assert.Equal(t, "", d.Id())
+}
+
 func TestUpdate(t *testing.T) {
 	r := Resource{
 		Update: func(ctx context.Context,
@@ -49,6 +98,11 @@ func TestUpdate(t *testing.T) {
 			c *DatabricksClient) error {
 			return NotFound("nope")
 		},
+		Delete: func(ctx context.Context,
+			d *schema.ResourceData,
+			c *DatabricksClient) error {
+			return NotFound("nope")
+		},
 		Schema: map[string]*schema.Schema{
 			"foo": {
 				Type:     schema.TypeInt,

permissions/resource_permissions.go

@@ -208,7 +208,7 @@ func (a PermissionsAPI) Read(objectID string) (objectACL ObjectACL, err error) {
 	// platform propagates INVALID_STATE error for auto-purged clusters in 
 	// the permissions api. this adds "a logical fix" also here, not to introduce
 	// cross-package dependency on "clusters".
-	if ok && strings.Contains(apiErr.Message, "Cannot access cluster") {
+	if ok && strings.Contains(apiErr.Message, "Cannot access cluster") && apiErr.StatusCode == 400 {
 		apiErr.StatusCode = 404
 		err = apiErr
 		return
@@ -292,11 +292,7 @@ func (oa *ObjectACL) ToPermissionsEntity(d *schema.ResourceData, me string) (Per
 			return entity, nil
 		}
 		identifier := path.Base(oa.ObjectID)
-		err := d.Set(mapping.field, identifier)
-		if err != nil {
-			return entity, err
-		}
-		return entity, nil
+		return entity, d.Set(mapping.field, identifier)
 	}
 	return entity, fmt.Errorf("unknown object type %s", oa.ObjectType)
 }
@@ -346,37 +342,7 @@ func ResourcePermissions() *schema.Resource {
 		}
 		return s
 	})
-	readContext := func(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
-		id := d.Id()
-		objectACL, err := NewPermissionsAPI(ctx, m).Read(id)
-		if common.IsMissing(err) {
-			log.Printf("[INFO] %s is removed on backend", d.Id())
-			d.SetId("")
-			return nil
-		}
-		if err != nil {
-			return diag.FromErr(err)
-		}
-		me, err := scim.NewUsersAPI(ctx, m).Me()
-		if err != nil {
-			return diag.FromErr(err)
-		}
-		entity, err := objectACL.ToPermissionsEntity(d, me.UserName)
-		if err != nil {
-			return diag.FromErr(err)
-		}
-		if len(entity.AccessControlList) == 0 {
-			// empty "modifiable" access control list is the same as resource absence
-			d.SetId("")
-			return nil
-		}
-		err = common.StructToData(entity, s, d)
-		if err != nil {
-			return diag.FromErr(err)
-		}
-		return nil
-	}
-	return &schema.Resource{
+	return common.Resource{
 		Schema: s,
 		CustomizeDiff: func(ctx context.Context, diff *schema.ResourceDiff, c interface{}) error {
 			client := c.(*common.DatabricksClient)
@@ -407,53 +373,58 @@ func ResourcePermissions() *schema.Resource {
 			}
 			return nil
 		},
-		ReadContext: readContext,
-		CreateContext: func(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
+		Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
+			id := d.Id()
+			objectACL, err := NewPermissionsAPI(ctx, c).Read(id)
+			if err != nil {
+				return err
+			}
+			me, err := scim.NewUsersAPI(ctx, c).Me()
+			if err != nil {
+				return err
+			}
+			entity, err := objectACL.ToPermissionsEntity(d, me.UserName)
+			if err != nil {
+				return err
+			}
+			if len(entity.AccessControlList) == 0 {
+				// empty "modifiable" access control list is the same as resource absence
+				d.SetId("")
+				return nil
+			}
+			return common.StructToData(entity, s, d)
+		},
+		Create: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
 			var entity PermissionsEntity
 			common.DataToStructPointer(d, s, &entity)
 			for _, mapping := range permissionsResourceIDFields() {
 				if v, ok := d.GetOk(mapping.field); ok {
-					id, err := mapping.idRetriever(ctx, m.(*common.DatabricksClient), v.(string))
+					id, err := mapping.idRetriever(ctx, c, v.(string))
 					if err != nil {
-						return diag.FromErr(err)
+						return err
 					}
 					objectID := fmt.Sprintf("/%s/%s", mapping.resourceType, id)
-					err = NewPermissionsAPI(ctx, m).Update(objectID, AccessControlChangeList{
+					err = NewPermissionsAPI(ctx, c).Update(objectID, AccessControlChangeList{
 						AccessControlList: entity.AccessControlList,
 					})
 					if err != nil {
-						return diag.FromErr(err)
+						return err
 					}
 					d.SetId(objectID)
-					return readContext(ctx, d, m)
+					return nil
 				}
 			}
-			return diag.Errorf("At least one type of resource identifiers must be set")
+			return errors.New("At least one type of resource identifiers must be set")
 		},
-		UpdateContext: func(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
+		Update: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
 			var entity PermissionsEntity
 			common.DataToStructPointer(d, s, &entity)
-			err := NewPermissionsAPI(ctx, m).Update(d.Id(), AccessControlChangeList{
+			return NewPermissionsAPI(ctx, c).Update(d.Id(), AccessControlChangeList{
 				AccessControlList: entity.AccessControlList,
 			})
-			if err != nil {
-				return diag.FromErr(err)
-			}
-			return readContext(ctx, d, m)
 		},
-		DeleteContext: func(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
-			err := NewPermissionsAPI(ctx, m).Delete(d.Id())
-			if common.IsMissing(err) {
-				log.Printf("[INFO] %s is already removed on backend", d.Id())
-				return nil
-			}
-			if err != nil {
-				return diag.FromErr(err)
-			}
-			return nil
+		Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
+			return NewPermissionsAPI(ctx, c).Delete(d.Id())
 		},
-		Importer: &schema.ResourceImporter{
-			StateContext: schema.ImportStatePassthroughContext,
-		},
-	}
+	}.ToResource()
 }