SCIM patch to remove resources must not contain a value field (#3374)

* SCIM patch to remove resources must not contain a value field * simplification * remove accidental extra test
databricks · Mar 15, 2024 · 11bceaa · 11bceaa
1 parent d032b9d
commit 11bceaa
Show file tree

Hide file tree

Showing 15 changed files with 34 additions and 33 deletions.
diff --git a/aws/resource_group_instance_profile.go b/aws/resource_group_instance_profile.go
@@ -27,11 +27,11 @@ func ResourceGroupInstanceProfile() common.Resource {
 			return err
 		},
 		CreateContext: func(ctx context.Context, groupID, roleARN string, c *common.DatabricksClient) error {
-			return scim.NewGroupsAPI(ctx, c).Patch(groupID, scim.PatchRequest("add", "roles", roleARN))
+			return scim.NewGroupsAPI(ctx, c).Patch(groupID, scim.PatchRequestWithValue("add", "roles", roleARN))
 		},
 		DeleteContext: func(ctx context.Context, groupID, roleARN string, c *common.DatabricksClient) error {
 			return scim.NewGroupsAPI(ctx, c).Patch(groupID, scim.PatchRequest(
-				"remove", fmt.Sprintf(`roles[value eq "%s"]`, roleARN), ""))
+				"remove", fmt.Sprintf(`roles[value eq "%s"]`, roleARN)))
 		},
 	})
 	r.DeprecationMessage = "Please migrate to `databricks_group_role`"

diff --git a/aws/resource_group_instance_profile_test.go b/aws/resource_group_instance_profile_test.go
@@ -16,7 +16,7 @@ func TestResourceGroupInstanceProfileCreate(t *testing.T) {
 			{
 				Method:   "PATCH",
 				Resource: "/api/2.0/preview/scim/v2/Groups/abc",
-				ExpectedRequest: scim.PatchRequest(
+				ExpectedRequest: scim.PatchRequestWithValue(
 					"add",
 					"roles",
 					"arn:aws:iam::999999999999:instance-profile/my-fake-instance-profile"),
@@ -215,8 +215,7 @@ func TestResourceGroupInstanceProfileDelete(t *testing.T) {
 				Resource: "/api/2.0/preview/scim/v2/Groups/abc",
 				ExpectedRequest: scim.PatchRequest(
 					"remove",
-					`roles[value eq "arn:aws:iam::999999999999:instance-profile/my-fake-instance-profile"]`,
-					""),
+					`roles[value eq "arn:aws:iam::999999999999:instance-profile/my-fake-instance-profile"]`),
 			},
 		},
 		Resource: ResourceGroupInstanceProfile(),

diff --git a/aws/resource_service_principal_role.go b/aws/resource_service_principal_role.go
@@ -13,7 +13,7 @@ import (
 func ResourceServicePrincipalRole() common.Resource {
 	r := common.NewPairID("service_principal_id", "role").BindResource(common.BindResource{
 		CreateContext: func(ctx context.Context, servicePrincipalID, role string, c *common.DatabricksClient) error {
-			return scim.NewServicePrincipalsAPI(ctx, c).Patch(servicePrincipalID, scim.PatchRequest("add", "roles", role))
+			return scim.NewServicePrincipalsAPI(ctx, c).Patch(servicePrincipalID, scim.PatchRequestWithValue("add", "roles", role))
 		},
 		ReadContext: func(ctx context.Context, servicePrincipalID, roleARN string, c *common.DatabricksClient) error {
 			servicePrincipal, err := scim.NewServicePrincipalsAPI(ctx, c).Read(servicePrincipalID, "roles")
@@ -25,7 +25,7 @@ func ResourceServicePrincipalRole() common.Resource {
 		},
 		DeleteContext: func(ctx context.Context, servicePrincipalID, roleARN string, c *common.DatabricksClient) error {
 			return scim.NewServicePrincipalsAPI(ctx, c).Patch(servicePrincipalID, scim.PatchRequest(
-				"remove", fmt.Sprintf(`roles[value eq "%s"]`, roleARN), ""))
+				"remove", fmt.Sprintf(`roles[value eq "%s"]`, roleARN)))
 		},
 	})
 	return r

diff --git a/aws/resource_service_principal_role_test.go b/aws/resource_service_principal_role_test.go
@@ -15,7 +15,7 @@ func TestResourceServicePrincipalRoleCreate(t *testing.T) {
 			{
 				Method:   "PATCH",
 				Resource: "/api/2.0/preview/scim/v2/ServicePrincipals/abc",
-				ExpectedRequest: scim.PatchRequest(
+				ExpectedRequest: scim.PatchRequestWithValue(
 					"add",
 					"roles",
 					"arn:aws:iam::999999999999:instance-profile/my-fake-instance-profile"),

diff --git a/aws/resource_user_instance_profile.go b/aws/resource_user_instance_profile.go
@@ -19,7 +19,7 @@ func ResourceUserInstanceProfile() common.Resource {
 		return m
 	}).BindResource(common.BindResource{
 		CreateContext: func(ctx context.Context, userID, roleARN string, c *common.DatabricksClient) error {
-			return scim.NewUsersAPI(ctx, c).Patch(userID, scim.PatchRequest("add", "roles", roleARN))
+			return scim.NewUsersAPI(ctx, c).Patch(userID, scim.PatchRequestWithValue("add", "roles", roleARN))
 		},
 		ReadContext: func(ctx context.Context, userID, roleARN string, c *common.DatabricksClient) error {
 			user, err := scim.NewUsersAPI(ctx, c).Read(userID, "roles")
@@ -31,7 +31,7 @@ func ResourceUserInstanceProfile() common.Resource {
 		},
 		DeleteContext: func(ctx context.Context, userID, roleARN string, c *common.DatabricksClient) error {
 			return scim.NewUsersAPI(ctx, c).Patch(userID, scim.PatchRequest(
-				"remove", fmt.Sprintf(`roles[value eq "%s"]`, roleARN), ""))
+				"remove", fmt.Sprintf(`roles[value eq "%s"]`, roleARN)))
 		},
 	})
 	r.DeprecationMessage = "Please migrate to `databricks_user_role`. This resource will be removed in v0.5.x"

diff --git a/aws/resource_user_instance_profile_test.go b/aws/resource_user_instance_profile_test.go
@@ -16,7 +16,7 @@ func TestResourceUserInstanceProfileCreate(t *testing.T) {
 			{
 				Method:   "PATCH",
 				Resource: "/api/2.0/preview/scim/v2/Users/abc",
-				ExpectedRequest: scim.PatchRequest(
+				ExpectedRequest: scim.PatchRequestWithValue(
 					"add",
 					"roles",
 					"arn:aws:iam::999999999999:instance-profile/my-fake-instance-profile"),
@@ -181,8 +181,7 @@ func TestResourceUserInstanceProfileDelete(t *testing.T) {
 				Resource: "/api/2.0/preview/scim/v2/Users/abc",
 				ExpectedRequest: scim.PatchRequest(
 					"remove",
-					`roles[value eq "arn:aws:iam::999999999999:instance-profile/my-fake-instance-profile"]`,
-					""),
+					`roles[value eq "arn:aws:iam::999999999999:instance-profile/my-fake-instance-profile"]`),
 			},
 		},
 		Resource: ResourceUserInstanceProfile(),

diff --git a/aws/resource_user_role.go b/aws/resource_user_role.go
@@ -12,7 +12,7 @@ import (
 func ResourceUserRole() common.Resource {
 	return common.NewPairID("user_id", "role").BindResource(common.BindResource{
 		CreateContext: func(ctx context.Context, userID, role string, c *common.DatabricksClient) error {
-			return scim.NewUsersAPI(ctx, c).Patch(userID, scim.PatchRequest("add", "roles", role))
+			return scim.NewUsersAPI(ctx, c).Patch(userID, scim.PatchRequestWithValue("add", "roles", role))
 		},
 		ReadContext: func(ctx context.Context, userID, roleARN string, c *common.DatabricksClient) error {
 			user, err := scim.NewUsersAPI(ctx, c).Read(userID, "roles")
@@ -24,7 +24,7 @@ func ResourceUserRole() common.Resource {
 		},
 		DeleteContext: func(ctx context.Context, userID, roleARN string, c *common.DatabricksClient) error {
 			return scim.NewUsersAPI(ctx, c).Patch(userID, scim.PatchRequest(
-				"remove", fmt.Sprintf(`roles[value eq "%s"]`, roleARN), ""))
+				"remove", fmt.Sprintf(`roles[value eq "%s"]`, roleARN)))
 		},
 	})
 }
diff --git a/aws/resource_user_role_test.go b/aws/resource_user_role_test.go
@@ -19,7 +19,7 @@ func TestUserRoleCreate_AndGetResourceDrift(t *testing.T) {
 			{
 				Method:          "PATCH",
 				Resource:        "/api/2.0/preview/scim/v2/Users/a",
-				ExpectedRequest: scim.PatchRequest("add", "roles", "b"),
+				ExpectedRequest: scim.PatchRequestWithValue("add", "roles", "b"),
 			},
 			{
 				Method:   "GET",

diff --git a/scim/resource_group_member.go b/scim/resource_group_member.go
@@ -12,7 +12,7 @@ import (
 func ResourceGroupMember() common.Resource {
 	return common.NewPairID("group_id", "member_id").BindResource(common.BindResource{
 		CreateContext: func(ctx context.Context, groupID, memberID string, c *common.DatabricksClient) error {
-			return NewGroupsAPI(ctx, c).Patch(groupID, PatchRequest("add", "members", memberID))
+			return NewGroupsAPI(ctx, c).Patch(groupID, PatchRequestWithValue("add", "members", memberID))
 		},
 		ReadContext: func(ctx context.Context, groupID, memberID string, c *common.DatabricksClient) error {
 			group, err := NewGroupsAPI(ctx, c).Read(groupID, "members")
@@ -24,7 +24,7 @@ func ResourceGroupMember() common.Resource {
 		},
 		DeleteContext: func(ctx context.Context, groupID, memberID string, c *common.DatabricksClient) error {
 			return NewGroupsAPI(ctx, c).Patch(groupID, PatchRequest(
-				"remove", fmt.Sprintf(`members[value eq "%s"]`, memberID), ""))
+				"remove", fmt.Sprintf(`members[value eq "%s"]`, memberID)))
 		},
 	})
 }
diff --git a/scim/resource_group_member_test.go b/scim/resource_group_member_test.go
@@ -14,7 +14,7 @@ func TestResourceGroupMemberCreate(t *testing.T) {
 			{
 				Method:          "PATCH",
 				Resource:        "/api/2.0/preview/scim/v2/Groups/abc",
-				ExpectedRequest: PatchRequest("add", "members", "bcd"),
+				ExpectedRequest: PatchRequestWithValue("add", "members", "bcd"),
 				Response: Group{
 					ID: "abc",
 				},
@@ -164,8 +164,7 @@ func TestResourceGroupMemberDelete(t *testing.T) {
 				Resource: "/api/2.0/preview/scim/v2/Groups/abc",
 				ExpectedRequest: PatchRequest(
 					"remove",
-					`members[value eq "bcd"]`,
-					""),
+					`members[value eq "bcd"]`),
 			},
 		},
 		Resource: ResourceGroupMember(),

diff --git a/scim/resource_group_role.go b/scim/resource_group_role.go
@@ -12,7 +12,7 @@ import (
 func ResourceGroupRole() common.Resource {
 	return common.NewPairID("group_id", "role").BindResource(common.BindResource{
 		CreateContext: func(ctx context.Context, groupID, role string, c *common.DatabricksClient) error {
-			return NewGroupsAPI(ctx, c).Patch(groupID, PatchRequest("add", "roles", role))
+			return NewGroupsAPI(ctx, c).Patch(groupID, PatchRequestWithValue("add", "roles", role))
 		},
 		ReadContext: func(ctx context.Context, groupID, role string, c *common.DatabricksClient) error {
 			group, err := NewGroupsAPI(ctx, c).Read(groupID, "roles")
@@ -24,7 +24,7 @@ func ResourceGroupRole() common.Resource {
 		},
 		DeleteContext: func(ctx context.Context, groupID, role string, c *common.DatabricksClient) error {
 			return NewGroupsAPI(ctx, c).Patch(groupID, PatchRequest(
-				"remove", fmt.Sprintf(`roles[value eq "%s"]`, role), ""))
+				"remove", fmt.Sprintf(`roles[value eq "%s"]`, role)))
 		},
 	})
 }
diff --git a/scim/resource_group_role_test.go b/scim/resource_group_role_test.go
@@ -13,7 +13,7 @@ func TestResourceGroupRoleCreate(t *testing.T) {
 			{
 				Method:          "PATCH",
 				Resource:        "/api/2.0/preview/scim/v2/Groups/abc",
-				ExpectedRequest: PatchRequest("add", "roles", "arn:aws:iam::000000000000:role/test-role"),
+				ExpectedRequest: PatchRequestWithValue("add", "roles", "arn:aws:iam::000000000000:role/test-role"),
 				Response: Group{
 					ID: "abc",
 				},
@@ -155,8 +155,7 @@ func TestResourceGroupRoleDelete(t *testing.T) {
 				Resource: "/api/2.0/preview/scim/v2/Groups/abc",
 				ExpectedRequest: PatchRequest(
 					"remove",
-					`roles[value eq "arn:aws:iam::000000000000:role/test-role"]`,
-					""),
+					`roles[value eq "arn:aws:iam::000000000000:role/test-role"]`),
 			},
 		},
 		Resource: ResourceGroupRole(),

diff --git a/scim/resource_service_principal.go b/scim/resource_service_principal.go
@@ -214,7 +214,7 @@ func ResourceServicePrincipal() common.Resource {
 			}
 			// Disable or delete
 			if isDisable {
-				r := PatchRequest("replace", "active", "false")
+				r := PatchRequestWithValue("replace", "active", "false")
 				err = spAPI.Patch(d.Id(), r)
 			} else {
 				err = spAPI.Delete(d.Id())

diff --git a/scim/resource_user.go b/scim/resource_user.go
@@ -140,7 +140,7 @@ func ResourceUser() common.Resource {
 			}
 			// Disable or delete
 			if isDisable {
-				r := PatchRequest("replace", "active", "false")
+				r := PatchRequestWithValue("replace", "active", "false")
 				err = user.Patch(d.Id(), r)
 			} else {
 				err = user.Delete(d.Id())

diff --git a/scim/scim.go b/scim/scim.go
@@ -175,14 +175,19 @@ type patchRequest struct {
 	Operations []patchOperation `json:"Operations,omitempty"`
 }
 
-func PatchRequest(op, path, value string) patchRequest {
+func PatchRequest(op, path string) patchRequest {
+	o := patchOperation{
+		Op:   op,
+		Path: path,
+	}
+	return PatchRequestComplexValue([]patchOperation{o})
+}
+
+func PatchRequestWithValue(op, path, value string) patchRequest {
 	o := patchOperation{
 		Op:    op,
 		Path:  path,
-		Value: value,
-	}
-	if value != "" {
-		o.Value = []ComplexValue{{Value: value}}
+		Value: []ComplexValue{{Value: value}},
 	}
 	return PatchRequestComplexValue([]patchOperation{o})
 }