Merge pull request #143 from jfrog/GH-142-remove-empty-set-validation…

…-for-permission-actions Remove validation for 'users' and 'groups' to allow empty list
jfrog · Oct 17, 2024 · e51644f · e51644f
2 parents d47d191 + 757138b
commit e51644f
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+## 1.15.1 (October 18, 2024). Tested on Artifactory 7.90.14 with Terraform 1.9.8 and OpenTofu 1.8.3
+
+IMPROVEMENTS:
+
+* resource/artifactory_local_repository_multi_replication: Update validation for `actions.users` and `actions.groups` attributes to allow empty list. Issue: [#142](https://github.com/jfrog/terraform-provider-platform/issues/142) PR: [#143](https://github.com/jfrog/terraform-provider-platform/pull/143)
+
 ## 1.15.0 (October 16, 2024). Tested on Artifactory 7.90.14 with Terraform 1.9.7 and OpenTofu 1.8.3
 
 IMPROVEMENTS:

diff --git a/GNUmakefile b/GNUmakefile
@@ -59,7 +59,7 @@ update_pkg_cache:
 	GOPROXY=https://proxy.golang.org GO111MODULE=on go get github.com/jfrog/terraform-provider-${PRODUCT}@v${VERSION}
 
 build: fmt
-	GORELEASER_CURRENT_TAG=${NEXT_VERSION} goreleaser build --clean --snapshot # --single-target
+	GORELEASER_CURRENT_TAG=${NEXT_VERSION} goreleaser build --clean --snapshot --single-target
 
 test:
 	@echo "==> Starting unit tests"

diff --git a/pkg/platform/resource_permission.go b/pkg/platform/resource_permission.go
@@ -67,14 +67,14 @@ var usersGroupsAttributeSchema = func(description string) schema.SetNestedAttrib
 			},
 		},
 		Optional: true,
+		Computed: true,
+		Default:  setdefault.StaticValue(types.SetValueMust(usersGroupsResourceModelAttributeTypes, []attr.Value{})),
 		Validators: []validator.Set{
 			setvalidator.AtLeastOneOf(path.MatchRelative().AtParent().AtName("users"), path.MatchRelative().AtParent().AtName("groups")),
-			setvalidator.SizeAtLeast(1),
 		},
 		PlanModifiers: []planmodifier.Set{
 			setplanmodifier.UseStateForUnknown(),
 		},
-		MarkdownDescription: "Must contain at least one item.",
 	}
 }
 
@@ -449,7 +449,7 @@ func (r *permissionResourceModel) toResourceAPIModel(ctx context.Context, tfReso
 		return
 	}
 
-	actions := permissionActionsAPIModel{}
+	var actions *permissionActionsAPIModel
 	if !resource.Actions.IsNull() {
 		var actionsResource permissionActionsResourceModel
 		ds.Append(resource.Actions.As(ctx, &actionsResource, basetypes.ObjectAsOptions{})...)
@@ -458,7 +458,7 @@ func (r *permissionResourceModel) toResourceAPIModel(ctx context.Context, tfReso
 		}
 
 		users := make(map[string][]string)
-		if !actionsResource.Users.IsNull() {
+		if !actionsResource.Users.IsNull() || len(actionsResource.Users.Elements()) > 0 {
 			var usersResources []permissionUsersGroupsResourceModel
 			ds.Append(actionsResource.Users.ElementsAs(ctx, &usersResources, false)...)
 			if ds.HasError() {
@@ -472,7 +472,7 @@ func (r *permissionResourceModel) toResourceAPIModel(ctx context.Context, tfReso
 		}
 
 		groups := make(map[string][]string)
-		if !actionsResource.Groups.IsNull() {
+		if !actionsResource.Groups.IsNull() || len(actionsResource.Groups.Elements()) > 0 {
 			var groupsResources []permissionUsersGroupsResourceModel
 			ds.Append(actionsResource.Groups.ElementsAs(ctx, &groupsResources, false)...)
 			if ds.HasError() {
@@ -485,7 +485,7 @@ func (r *permissionResourceModel) toResourceAPIModel(ctx context.Context, tfReso
 			}
 		}
 
-		actions = permissionActionsAPIModel{
+		actions = &permissionActionsAPIModel{
 			Users:  users,
 			Groups: groups,
 		}
@@ -511,7 +511,7 @@ func (r *permissionResourceModel) toResourceAPIModel(ctx context.Context, tfReso
 	}
 
 	*apiResource = permissionActionsTargetsAPIModel{
-		Actions: &actions,
+		Actions: actions,
 		Targets: targets,
 	}
 
@@ -591,11 +591,6 @@ var resourceResourceModelAttributeTypes map[string]attr.Type = map[string]attr.T
 }
 
 func (r *permissionResourceModel) fromUsersGroupsAPIModel(ctx context.Context, usersGroups map[string][]string) (set types.Set, ds diag.Diagnostics) {
-	if len(usersGroups) == 0 {
-		set = types.SetNull(usersGroupsResourceModelAttributeTypes)
-		return
-	}
-
 	userGroupSet := lo.MapToSlice(
 		usersGroups,
 		func(name string, permissions []string) attr.Value {
@@ -645,8 +640,7 @@ func (r *permissionResourceModel) fromResourceAPIModel(ctx context.Context, reso
 	}
 
 	actions := types.ObjectNull(actionsResourceModelAttributeTypes)
-	if resourceAPIModel.Actions != nil &&
-		(len(resourceAPIModel.Actions.Users) > 0 || len(resourceAPIModel.Actions.Groups) > 0) {
+	if resourceAPIModel.Actions != nil {
 		usersSet, d := r.fromUsersGroupsAPIModel(ctx, resourceAPIModel.Actions.Users)
 		if d != nil {
 			ds = append(ds, d...)
@@ -776,13 +770,13 @@ type PermissionAPIModel struct {
 }
 
 type permissionActionsTargetsAPIModel struct {
-	Actions *permissionActionsAPIModel           `json:"actions"`
-	Targets map[string]permissionTargetsAPIModel `json:"targets"`
+	Actions *permissionActionsAPIModel           `json:"actions,omitempty"`
+	Targets map[string]permissionTargetsAPIModel `json:"targets,omitempty"`
 }
 
 type permissionActionsAPIModel struct {
-	Users  map[string][]string `json:"users"`
-	Groups map[string][]string `json:"groups"`
+	Users  map[string][]string `json:"users,omitempty"`
+	Groups map[string][]string `json:"groups,omitempty"`
 }
 
 type permissionTargetsAPIModel struct {

diff --git a/pkg/platform/resource_permission_test.go b/pkg/platform/resource_permission_test.go
@@ -91,6 +91,8 @@ func TestAccPermission_full(t *testing.T) {
 						permissions = ["READ", "WRITE"]
 					}
 				]
+
+				groups = []
 			}
 
 			targets = [
@@ -115,6 +117,11 @@ func TestAccPermission_full(t *testing.T) {
 		}
 
 		build = {
+			actions = {
+				users = []
+				groups = []
+			}
+
 			targets = [
 				{
 					name = "artifactory-build-info"
@@ -245,9 +252,9 @@ func TestAccPermission_full(t *testing.T) {
 					resource.TestCheckResourceAttr(fqrn, "artifact.actions.users.0.permissions.#", "2"),
 					resource.TestCheckTypeSetElemAttr(fqrn, "artifact.actions.users.0.permissions.*", "READ"),
 					resource.TestCheckTypeSetElemAttr(fqrn, "artifact.actions.users.0.permissions.*", "WRITE"),
-					resource.TestCheckNoResourceAttr(fqrn, "artifact.actions.groups"),
+					resource.TestCheckResourceAttr(fqrn, "artifact.actions.groups.#", "0"),
 					resource.TestCheckResourceAttr(fqrn, "artifact.targets.#", "4"),
-					resource.TestCheckNoResourceAttr(fqrn, "build.actions"),
+					resource.TestCheckResourceAttr(fqrn, "build.actions.users.#", "0"),
 					resource.TestCheckResourceAttr(fqrn, "build.targets.#", "1"),
 					resource.TestCheckResourceAttr(fqrn, "build.targets.0.name", "artifactory-build-info"),
 					resource.TestCheckResourceAttr(fqrn, "build.targets.0.include_patterns.#", "1"),
@@ -506,15 +513,15 @@ func TestAccPermission_empty_users_state_migration(t *testing.T) {
 				},
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(fqrn, "name", testData["name"]),
-					resource.TestCheckNoResourceAttr(fqrn, "artifact.actions.users"),
+					resource.TestCheckResourceAttr(fqrn, "artifact.actions.users.#", "0"),
 					resource.TestCheckResourceAttr(fqrn, "artifact.actions.groups.#", "1"),
-					resource.TestCheckNoResourceAttr(fqrn, "build.actions.users"),
+					resource.TestCheckResourceAttr(fqrn, "build.actions.users.#", "0"),
 					resource.TestCheckResourceAttr(fqrn, "build.actions.groups.#", "1"),
-					resource.TestCheckNoResourceAttr(fqrn, "release_bundle.actions.users"),
+					resource.TestCheckResourceAttr(fqrn, "release_bundle.actions.users.#", "0"),
 					resource.TestCheckResourceAttr(fqrn, "release_bundle.actions.groups.#", "1"),
-					resource.TestCheckNoResourceAttr(fqrn, "destination.actions.users"),
+					resource.TestCheckResourceAttr(fqrn, "destination.actions.users.#", "0"),
 					resource.TestCheckResourceAttr(fqrn, "destination.actions.groups.#", "1"),
-					resource.TestCheckNoResourceAttr(fqrn, "pipeline_source.actions.users"),
+					resource.TestCheckResourceAttr(fqrn, "pipeline_source.actions.users.#", "0"),
 					resource.TestCheckResourceAttr(fqrn, "pipeline_source.actions.groups.#", "1"),
 				),
 			},
@@ -587,7 +594,7 @@ func TestAccPermission_no_users_state_migration(t *testing.T) {
 				},
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(fqrn, "name", testData["name"]),
-					resource.TestCheckNoResourceAttr(fqrn, "artifact.actions.users"),
+					resource.TestCheckResourceAttr(fqrn, "artifact.actions.users.#", "0"),
 					resource.TestCheckResourceAttr(fqrn, "artifact.actions.groups.#", "1"),
 				),
 			},
@@ -805,15 +812,15 @@ func TestAccPermission_empty_groups_state_migration(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(fqrn, "name", testData["name"]),
 					resource.TestCheckResourceAttr(fqrn, "artifact.actions.users.#", "1"),
-					resource.TestCheckNoResourceAttr(fqrn, "artifact.actions.groups"),
+					resource.TestCheckResourceAttr(fqrn, "artifact.actions.groups.#", "0"),
 					resource.TestCheckResourceAttr(fqrn, "build.actions.users.#", "1"),
-					resource.TestCheckNoResourceAttr(fqrn, "build.actions.groups"),
+					resource.TestCheckResourceAttr(fqrn, "build.actions.groups.#", "0"),
 					resource.TestCheckResourceAttr(fqrn, "release_bundle.actions.users.#", "1"),
-					resource.TestCheckNoResourceAttr(fqrn, "release_bundle.actions.groups"),
+					resource.TestCheckResourceAttr(fqrn, "release_bundle.actions.groups.#", "0"),
 					resource.TestCheckResourceAttr(fqrn, "destination.actions.users.#", "1"),
-					resource.TestCheckNoResourceAttr(fqrn, "destination.actions.groups"),
+					resource.TestCheckResourceAttr(fqrn, "destination.actions.groups.#", "0"),
 					resource.TestCheckResourceAttr(fqrn, "pipeline_source.actions.users.#", "1"),
-					resource.TestCheckNoResourceAttr(fqrn, "pipeline_source.actions.groups"),
+					resource.TestCheckResourceAttr(fqrn, "pipeline_source.actions.groups.#", "0"),
 				),
 			},
 		},
@@ -888,7 +895,7 @@ func TestAccPermission_no_groups_state_migration(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(fqrn, "name", testData["name"]),
 					resource.TestCheckResourceAttr(fqrn, "artifact.actions.users.#", "1"),
-					resource.TestCheckNoResourceAttr(fqrn, "artifact.actions.groups"),
+					resource.TestCheckResourceAttr(fqrn, "artifact.actions.groups.#", "0"),
 				),
 			},
 		},