Merge pull request jfrog#804 from jfrog/remove-repos-project-key-default

Remove repos project key default

Author: chb0github

CHANGELOG.md

@@ -1,4 +1,14 @@
+## 9.0.0 (Sep 15, 2023). Tested on Artifactory 7.68.7 with Terraform CLI v1.5.7
+
+IMPROVEMENTS:
+* resource/artifactory_\*\_repository: remove default value of "default" from `project_key` attribute. This is a REST API bug fix that is part of Artifactory v7.68.7 (self-hosted) and v7.67.0 (cloud). Existing Terraform state with "default" value should be automatically migrated to "" on `terraform apply`. No state drift should occurs on `terraform plan`. Issue: [#779](https://github.com/jfrog/terraform-provider-artifactory/issues/779) 
+* Fix incorrect description for remote repository attribute `block_mismatching_mime_types`. Issue: [#799](https://github.com/jfrog/terraform-provider-artifactory/pull/799)
+* Add multiple users and groups HCL example for `artifactory_permission_target` resource. Issue: [#800](https://github.com/jfrog/terraform-provider-artifactory/pull/800)
+
+PR: [#804](https://github.com/jfrog/terraform-provider-artifactory/pull/804)
+
 ## 8.9.1 (Sep 11, 2023). Tested on Artifactory 7.63.14 with Terraform CLI v1.5.7
+
 BUG FIX:
 * resource/artifactory_local_\*\_repository, resource/artifactory_remote_\*\_repository, resource/artifactory_virtual_\*\_repository, resource/artifactory_federated_\*\_repository: fix unable to set `description` and `notes` attributes with empty text.
 

docs/resources/local.md

@@ -24,10 +24,7 @@ The following arguments are supported:
 contain spaces or special characters.
 * `description` - (Optional)
 * `notes` - (Optional)
-* `project_key` - (Optional) Project key for assigning this repository to. Must be 2 - 20 lowercase alphanumeric and hyphen characters.
-  When assigning repository to a project, repository key must be prefixed with project key, separated by a dash.
-  We don't recommend using this attribute to assign the repository to the project. Use the `repos` attribute in Project provider
-  to manage the list of repositories.  Default value - `default`.
+* `project_key` - (Optional) Project key for assigning this repository to. Must be 2 - 20 lowercase alphanumeric and hyphen characters. When assigning repository to a project, repository key must be prefixed with project key, separated by a dash. We don't recommend using this attribute to assign the repository to the project. Use the `repos` attribute in Project provider to manage the list of repositories.
 * `project_environments` - (Optional) Project environment for assigning this repository to. Allow values: `DEV` or `PROD`.
   Before Artifactory 7.53.1, up to 2 values (`DEV` and `PROD`) are allowed. From 7.53.1 onward, only one value is allowed.
   The attribute should only be used if the repository is already assigned to the existing project.

docs/resources/permission_target.md

@@ -23,10 +23,20 @@ resource "artifactory_permission_target" "test-perm" {
         permissions = ["read", "write"]
       }
 
+      users {
+        name        = "user1"
+        permissions = ["read", "write"]
+      }
+
       groups {
         name        = "readers"
         permissions = ["read"]
       }
+
+      groups {
+        name        = "dev"
+        permissions = ["read", "write"]
+      }
     }
   }
 
@@ -37,6 +47,11 @@ resource "artifactory_permission_target" "test-perm" {
     actions {
       users {
         name        = "anonymous"
+        permissions = ["read"]
+      }
+
+      users {
+        name        = "user1"
         permissions = ["read", "write"]
       }
     }

docs/resources/remote.md

@@ -37,10 +37,7 @@ All generic repo arguments are supported, in addition to:
 * `key` - (Required) A mandatory identifier for the repository that must be unique. It cannot begin with a number or contain spaces or special characters.
 * `description` - (Optional) Public description.
 * `notes` - (Optional) Internal description.
-* `project_key` - (Optional) Project key for assigning this repository to. Must be 2 - 20 lowercase alphanumeric and hyphen characters.
-  When assigning repository to a project, repository key must be prefixed with project key, separated by a dash.
-  We don't recommend using this attribute to assign the repository to the project. Use the `repos` attribute in Project provider
-  to manage the list of repositories. Default value - `default`.
+* `project_key` - (Optional) Project key for assigning this repository to. Must be 2 - 20 lowercase alphanumeric and hyphen characters. When assigning repository to a project, repository key must be prefixed with project key, separated by a dash. We don't recommend using this attribute to assign the repository to the project. Use the `repos` attribute in Project provider to manage the list of repositories.
 * `project_environments` - (Optional) Project environment for assigning this repository to. Allow values: `DEV` or `PROD`.
   Before Artifactory 7.53.1, up to 2 values (`DEV` and `PROD`) are allowed. From 7.53.1 onward, only one value is allowed.
   The attribute should only be used if the repository is already assigned to the existing project.
@@ -68,7 +65,7 @@ All generic repo arguments are supported, in addition to:
 * `assumed_offline_period_secs` - (Optional, Default: `300`) The number of seconds the repository stays in assumed offline state after a connection error. At the end of this time, an online check is attempted in order to reset the offline status. A value of 0 means the repository is never assumed offline.
 * `share_configuration` - (Optional) The attribute is 'Computed', so it's not managed by the Provider. There is no corresponding field in the UI, but the attribute is returned by Get.
 * `synchronize_properties` - (Optional, Default: `false`) When set, remote artifacts are fetched along with their properties.
-* `block_mismatching_mime_types` - (Optional, Default: `true`) Before caching an artifact, Artifactory first sends a HEAD request to the remote resource. In some remote resources, HEAD requests are disallowed and therefore rejected, even though downloading the artifact is allowed. When checked, Artifactory will bypass the HEAD request and cache the artifact directly using a GET request. Note: dafault value in the UI is `true`, but it is `false` if the repo was created using the API call. We are copying the UI behavior.
+* `block_mismatching_mime_types` - (Optional, Default: `true`) If set, artifacts will fail to download if a mismatch is detected between requested and received mimetype, according to the list specified in the system properties file under blockedMismatchingMimeTypes. You can override by adding mimetypes to the override list `mismatching_mime_types_override_list`.
 * `mismatching_mime_types_override_list` - (Optional) The set of mime types that should override the block_mismatching_mime_types setting. Eg: "application/json,application/xml". Default value is empty.
 * `property_sets` - (Optional) List of property set names.
 * `allow_any_host_auth` - (Optional, Default: `false`) Also known as 'Lenient Host Authentication', Allow credentials of this repository to be used on requests redirected to any other host.

docs/resources/virtual.md

@@ -28,10 +28,7 @@ The following arguments are supported:
 * `key` - (Required) A mandatory identifier for the repository that must be unique. It cannot begin with a number or
   contain spaces or special characters.
 * `repositories` - (Optional) The effective list of actual repositories included in this virtual repository.
-* `project_key` - (Optional) Project key for assigning this repository to. Must be 2 - 20 lowercase alphanumeric and hyphen characters. 
-  When assigning repository to a project, repository key must be prefixed with project key, separated by a dash.
-  We don't recommend using this attribute to assign the repository to the project. Use the `repos` attribute in Project provider 
-  to manage the list of repositories. Default value - `default`.
+* `project_key` - (Optional) Project key for assigning this repository to. Must be 2 - 20 lowercase alphanumeric and hyphen characters. When assigning repository to a project, repository key must be prefixed with project key, separated by a dash. We don't recommend using this attribute to assign the repository to the project. Use the `repos` attribute in Project provider to manage the list of repositories.
 * `project_environments` - (Optional) Project environment for assigning this repository to. Allow values: `DEV` or `PROD`.
   Before Artifactory 7.53.1, up to 2 values (`DEV` and `PROD`) are allowed. From 7.53.1 onward, only one value is allowed.
   The attribute should only be used if the repository is already assigned to the existing project. 

pkg/artifactory/resource/repository/federated/federated.go

@@ -182,7 +182,22 @@ func mkResourceSchema(skeema map[string]*schema.Schema, packer packer.PackFunc,
 		},
 
 		Schema:        skeema,
-		SchemaVersion: 2,
+		SchemaVersion: 3,
+		StateUpgraders: []schema.StateUpgrader{
+			{
+				// this only works because the schema hasn't changed, except the removal of default value
+				// from `project_key` attribute.
+				Type:    resourceV2(skeema).CoreConfigSchema().ImpliedType(),
+				Upgrade: repository.ResourceUpgradeProjectKey,
+				Version: 2,
+			},
+		},
 		CustomizeDiff: repository.ProjectEnvironmentsDiff,
 	}
 }
+
+func resourceV2(skeema map[string]*schema.Schema) *schema.Resource {
+	return &schema.Resource{
+		Schema: skeema,
+	}
+}

pkg/artifactory/resource/repository/local/resource_artifactory_local_generic_repository.go

@@ -8,19 +8,19 @@ import (
 )
 
 func GetGenericRepoSchema(repoType string) map[string]*schema.Schema {
-	return utilsdk.MergeMaps(BaseLocalRepoSchema, repository.RepoLayoutRefSchema("local", repoType))
+	return utilsdk.MergeMaps(BaseLocalRepoSchema, repository.RepoLayoutRefSchema(rclass, repoType))
 }
 
 func ResourceArtifactoryLocalGenericRepository(repoType string) *schema.Resource {
 	constructor := func() (interface{}, error) {
 		return &RepositoryBaseParams{
 			PackageType: repoType,
-			Rclass:      "local",
+			Rclass:      rclass,
 		}, nil
 	}
 
 	unpack := func(data *schema.ResourceData) (interface{}, string, error) {
-		repo := UnpackBaseRepo("local", data, repoType)
+		repo := UnpackBaseRepo(rclass, data, repoType)
 		return repo, repo.Id(), nil
 	}
 

pkg/artifactory/resource/repository/remote/remote.go

@@ -555,6 +555,10 @@ var resourceV1 = &schema.Resource{
 	Schema: baseRemoteRepoSchemaV1,
 }
 
+var resourceV2 = &schema.Resource{
+	Schema: baseRemoteRepoSchemaV2,
+}
+
 func mkResourceSchema(skeema map[string]*schema.Schema, packer packer.PackFunc, unpack unpacker.UnpackFunc, constructor repository.Constructor) *schema.Resource {
 	var reader = repository.MkRepoRead(packer, constructor)
 	return &schema.Resource{
@@ -572,10 +576,17 @@ func mkResourceSchema(skeema map[string]*schema.Schema, packer packer.PackFunc,
 				Upgrade: ResourceStateUpgradeV1,
 				Version: 1,
 			},
+			{
+				// this only works because the schema hasn't changed, except the removal of default value
+				// from `project_key` attribute.
+				Type:    resourceV2.CoreConfigSchema().ImpliedType(),
+				Upgrade: repository.ResourceUpgradeProjectKey,
+				Version: 2,
+			},
 		},
 
 		Schema:        skeema,
-		SchemaVersion: 2,
+		SchemaVersion: 3,
 		CustomizeDiff: customdiff.All(
 			repository.ProjectEnvironmentsDiff,
 			verifyExternalDependenciesDockerAndHelm,

pkg/artifactory/resource/repository/remote/resource_artifactory_remote_repository_test.go

@@ -47,7 +47,8 @@ func TestAccRemoteUpgradeFromVersionWithNoDisableProxyAttr(t *testing.T) {
 						Source:            "registry.terraform.io/jfrog/artifactory",
 					},
 				},
-				Config: config,
+				Config:             config,
+				ExpectNonEmptyPlan: true,
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(fqrn, "key", name),
 					resource.TestCheckResourceAttr(fqrn, "repo_layout_ref", "go-default"),

pkg/artifactory/resource/repository/repository.go

@@ -22,8 +22,6 @@ import (
 	"github.com/jfrog/terraform-provider-shared/validator"
 )
 
-const defaultProjectKey = "default"
-
 var BaseRepoSchema = map[string]*schema.Schema{
 	"key": {
 		Type:         schema.TypeString,
@@ -35,7 +33,6 @@ var BaseRepoSchema = map[string]*schema.Schema{
 	"project_key": {
 		Type:             schema.TypeString,
 		Optional:         true,
-		Default:          "default",
 		ValidateDiagFunc: validator.ProjectKey,
 		Description:      "Project key for assigning this repository to. Must be 2 - 20 lowercase alphanumeric and hyphen characters. When assigning repository to a project, repository key must be prefixed with project key, separated by a dash.",
 	},
@@ -198,8 +195,8 @@ func MkRepoUpdate(unpack unpacker.UnpackFunc, read schema.ReadContextFunc) schem
 			newProjectKey := newProject.(string)
 			tflog.Debug(ctx, fmt.Sprintf("oldProjectKey: %v, newProjectKey: %v", oldProjectKey, newProjectKey))
 
-			assignToProject := oldProjectKey == defaultProjectKey && len(newProjectKey) > 0
-			unassignFromProject := len(oldProjectKey) > 0 && newProjectKey == defaultProjectKey
+			assignToProject := oldProjectKey == "" && len(newProjectKey) > 0
+			unassignFromProject := len(oldProjectKey) > 0 && newProjectKey == ""
 			tflog.Debug(ctx, fmt.Sprintf("assignToProject: %v, unassignFromProject: %v", assignToProject, unassignFromProject))
 
 			var err error
@@ -252,13 +249,6 @@ func Retry400(response *resty.Response, _ error) bool {
 	return response.StatusCode() == http.StatusBadRequest
 }
 
-func repoExists(d *schema.ResourceData, m interface{}) (bool, error) {
-	_, err := CheckRepo(d.Id(), m.(utilsdk.ProvderMetadata).Client.R().AddRetryCondition(Retry400))
-	return err == nil, err
-}
-
-var repoTypeValidator = validation.StringInSlice(RepoTypesSupported, false)
-
 var RepoKeyValidator = validation.All(
 	validation.StringDoesNotMatch(regexp.MustCompile("^[0-9].*"), "repo key cannot start with a number"),
 	validation.StringDoesNotContainAny(" !@#$%^&*()+={}[]:;<>,/?~`|\\"),
@@ -341,12 +331,12 @@ func ProjectEnvironmentsDiff(ctx context.Context, diff *schema.ResourceDiff, met
 
 		isSupported, err := utilsdk.CheckVersion(providerMetadata.ArtifactoryVersion, CustomProjectEnvironmentSupportedVersion)
 		if err != nil {
-			return fmt.Errorf("Failed to check version %s", err)
+			return fmt.Errorf("failed to check version %s", err)
 		}
 
 		if isSupported {
 			if len(projectEnvironments) == 2 {
-				return fmt.Errorf("For Artifactory %s or later, only one environment can be assigned to a repository.", CustomProjectEnvironmentSupportedVersion)
+				return fmt.Errorf("for Artifactory %s or later, only one environment can be assigned to a repository", CustomProjectEnvironmentSupportedVersion)
 			}
 		} else { // Before 7.53.1
 			projectEnvironments := data.(*schema.Set).List()
@@ -373,10 +363,35 @@ func MkResourceSchema(skeema map[string]*schema.Schema, packer packer.PackFunc,
 		},
 
 		Schema:        skeema,
+		SchemaVersion: 1,
+		StateUpgraders: []schema.StateUpgrader{
+			{
+				// this only works because the schema hasn't changed, except the removal of default value
+				// from `project_key` attribute. Future common schema changes that involve attributes should
+				// figure out a way to create a previous and new version.
+				Type:    resourceV0(skeema).CoreConfigSchema().ImpliedType(),
+				Upgrade: ResourceUpgradeProjectKey,
+				Version: 0,
+			},
+		},
 		CustomizeDiff: ProjectEnvironmentsDiff,
 	}
 }
 
+func resourceV0(skeema map[string]*schema.Schema) *schema.Resource {
+	return &schema.Resource{
+		Schema: skeema,
+	}
+}
+
+func ResourceUpgradeProjectKey(ctx context.Context, rawState map[string]any, meta any) (map[string]any, error) {
+	if rawState["project_key"] == "default" {
+		rawState["project_key"] = ""
+	}
+
+	return rawState, nil
+}
+
 const RepositoriesEndpoint = "artifactory/api/repositories/{key}"
 
 func CheckRepo(id string, request *resty.Request) (*resty.Response, error) {