Merge pull request #363 from jfrog/gh-259-ext-dep-pat-upd

GH-259 Setting default value for external_dependencies_patterns field…

CHANGELOG.md

@@ -1,3 +1,10 @@
+## 2.23.1 (Mar 15, 2022)
+
+IMPROVEMENTS:
+
+* resource/artifactory_remote_docker_repository: Setting default value '**' for external_dependencies_patterns field. [GH-363]
+* resource/artifactory_remote_helm_repository: Setting default value '**' for external_dependencies_patterns field. [GH-363]
+
 ## 2.23.0 (Mar 11, 2022)
 
 FEATURES:

docs/resources/artifactory_remote_helm_repository.md

@@ -43,9 +43,8 @@ All generic repo arguments are supported, in addition to:
 * `helm_charts_base_url` - (Optional) - No documentation is available. Hopefully you know what this means
 * `external_dependencies_enabled` - (Optional) When set, external dependencies are rewritten.
 * `external_dependencies_patterns` - (Optional) An Allow List of Ant-style path expressions that specify where external
-  dependencies may be downloaded from. By default, this is an empty list which means that no dependencies may be downloaded
-  from external sources. Note that the official documentation states the default is '**', which is correct when creating
-  repositories in the UI, but incorrect for the API.
+  dependencies may be downloaded from. By default, this is set to ** which means that dependencies may be downloaded
+  from any external source.
 * `xray_index` - (Optional, Default: false)  Enable Indexing In Xray. Repository will be indexed with the default retention period. You will be able to change it via Xray settings.
 * `store_artifacts_locally` - (Optional) When set, the repository should store cached artifacts locally. When not set, artifacts are not stored locally, and direct repository-to-client streaming is used. This can be useful for multi-server setups over a high-speed LAN, with one Artifactory caching certain data on central storage, and streaming it directly to satellite pass-though Artifactory servers.
 * `socket_timeout_millis` - (Optional) Network timeout (in ms) to use when establishing a connection and for unanswered requests. Timing out on a network operation is considered a retrieval failure.

pkg/artifactory/resource_artifactory_remote_docker_repository.go

@@ -56,10 +56,21 @@ func resourceArtifactoryRemoteDockerRepository() *schema.Resource {
 			BlockPushingSchema1:          d.getBool("block_pushing_schema1", false),
 			ExternalDependenciesPatterns: d.getList("external_dependencies_patterns"),
 		}
+		if len(repo.ExternalDependenciesPatterns) == 0 {
+			repo.ExternalDependenciesPatterns = []string{"**"}
+		}
 		return repo, repo.Id(), nil
 	}
 
-	return mkResourceSchema(dockerRemoteSchema, defaultPacker, unpackDockerRemoteRepo, func() interface{} {
+	// Special handling for "external_dependencies_patterns" attribute to match default value behavior in UI.
+	dockerRemoteRepoPacker := universalPack(
+		allHclPredicate(
+			ignoreHclPredicate("class", "rclass", "external_dependencies_patterns"),
+			schemaHasKey(dockerRemoteSchema),
+		),
+	)
+
+	return mkResourceSchema(dockerRemoteSchema, dockerRemoteRepoPacker, unpackDockerRemoteRepo, func() interface{} {
 		return &DockerRemoteRepository{
 			RemoteRepositoryBaseParams: RemoteRepositoryBaseParams{
 				Rclass:      "remote",

pkg/artifactory/resource_artifactory_remote_helm_repository.go

@@ -32,9 +32,7 @@ func resourceArtifactoryRemoteHelmRepository() *schema.Resource {
 			},
 			RequiredWith: []string{"external_dependencies_enabled"},
 			Description: "An Allow List of Ant-style path expressions that specify where external dependencies may be downloaded from. " +
-				"By default, this is an empty list which means that no dependencies may be downloaded from external sources. " +
-				"Note that the official documentation states the default is '**', " +
-				"which is correct when creating repositories in the UI, but incorrect for the API.",
+				"By default, this is set to ** which means that dependencies may be downloaded from any external source.",
 		},
 	}, repoLayoutRefSchema("remote", packageType))
 
@@ -53,10 +51,21 @@ func resourceArtifactoryRemoteHelmRepository() *schema.Resource {
 			ExternalDependenciesEnabled:  d.getBool("external_dependencies_enabled", false),
 			ExternalDependenciesPatterns: d.getList("external_dependencies_patterns"),
 		}
+		if len(repo.ExternalDependenciesPatterns) == 0 {
+			repo.ExternalDependenciesPatterns = []string{"**"}
+		}
 		return repo, repo.Id(), nil
 	}
 
-	return mkResourceSchema(helmRemoteSchema, defaultPacker, unpackHelmRemoteRepo, func() interface{} {
+	// Special handling for "external_dependencies_patterns" attribute to match default value behavior in UI.
+	helmRemoteRepoPacker := universalPack(
+		allHclPredicate(
+			ignoreHclPredicate("class", "rclass", "external_dependencies_patterns"),
+			schemaHasKey(helmRemoteSchema),
+		),
+	)
+
+	return mkResourceSchema(helmRemoteSchema, helmRemoteRepoPacker, unpackHelmRemoteRepo, func() interface{} {
 		return &HelmRemoteRepo{
 			RemoteRepositoryBaseParams: RemoteRepositoryBaseParams{
 				Rclass:      "remote",

pkg/artifactory/resource_artifactory_remote_repository_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/go-resty/resty/v2"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 )
 
 func TestAccLocalAllowDotsUnderscorersAndDashesInKeyGH129(t *testing.T) {
@@ -175,6 +176,60 @@ func TestAccRemoteHelmRepositoryWithAdditionalCheckFunctions(t *testing.T) {
 	}))
 }
 
+func TestAccRemoteRepository_ExternalDependenciesDefaults(t *testing.T) {
+	repoTypes := []string{"helm", "docker"}
+	const remoteRepoExternalDependenciesDefaults = `
+		resource "artifactory_remote_%s_repository" "%s" {
+			key 				            = "%s"
+            url                             = "https://testrepo.io/"
+            external_dependencies_enabled   = true
+		}
+	    `
+	type ExternalDependenciesRemoteRepo struct {
+		ExternalDependenciesEnabled  bool     `json:"externalDependenciesEnabled"`
+		ExternalDependenciesPatterns []string `json:"externalDependenciesPatterns"`
+	}
+
+	for _, repoType := range repoTypes {
+		id := rand.Int()
+		name := fmt.Sprintf("terraform-remote-test-%s-repo%d", repoType, id)
+		fqrn := fmt.Sprintf("artifactory_remote_%s_repository.%s", repoType, name)
+
+		var externalDependenciesCheck = func(state *terraform.State) error {
+			restyClient := getTestResty(t)
+			queryRepoResponse := &ExternalDependenciesRemoteRepo{}
+			_, err := restyClient.R().SetResult(&queryRepoResponse).Get("artifactory/api/repositories/" + name)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if queryRepoResponse.ExternalDependenciesPatterns == nil ||
+				len(queryRepoResponse.ExternalDependenciesPatterns) != 1 ||
+				queryRepoResponse.ExternalDependenciesPatterns[0] != "**" {
+				return fmt.Errorf("error: external_dependencies_patterns attribute = %v instead of default '**'", queryRepoResponse.ExternalDependenciesPatterns)
+			}
+			return err
+		}
+
+		t.Run(fmt.Sprintf("TestRemote%sRepo_ExternalDependenciesDefaults", strings.Title(strings.ToLower(repoType))), func(t *testing.T) {
+			resource.Test(t, resource.TestCase{
+				PreCheck:          func() { testAccPreCheck(t) },
+				CheckDestroy:      verifyDeleted(fqrn, testCheckRepo),
+				ProviderFactories: testAccProviders,
+				Steps: []resource.TestStep{
+					{
+						Config: fmt.Sprintf(remoteRepoExternalDependenciesDefaults, repoType, name, name),
+						Check: resource.ComposeTestCheckFunc(
+							resource.TestCheckResourceAttr(fqrn, "key", name),
+							resource.TestCheckResourceAttr(fqrn, "external_dependencies_enabled", "true"),
+							externalDependenciesCheck,
+						),
+					},
+				},
+			})
+		})
+	}
+}
+
 func TestAccRemoteNpmRepository(t *testing.T) {
 	const packageType = "npm"
 	resource.Test(mkNewRemoteTestCase(packageType, t, map[string]interface{}{