Merge pull request #1005 from jfrog/GH-1004-fix-external-dependencies…

…-patterns-force-new Remove 'ForceNew' setting from external_dependencies_patterns attribute
jfrog · Jun 25, 2024 · a7929ae · a7929ae
2 parents cb46b35 + 19a5ee0
commit a7929ae
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,4 @@
-## 11.1.0 (June 25, 2024). Tested on Artifactory 7.84.15 with Terraform 1.8.5 and OpenTofu 1.7.2
+## 11.1.0 (June 26, 2024). Tested on Artifactory 7.84.15 with Terraform 1.8.5 and OpenTofu 1.7.2
 
 NOTES:
 
@@ -10,6 +10,10 @@ IMPROVEMENTS:
 
 Issue: [#999](https://github.com/jfrog/terraform-provider-artifactory/issues/999) PR: [#1003](https://github.com/jfrog/terraform-provider-artifactory/pull/1003)
 
+BUG FIXES:
+
+* resource/artifactory_virtual_bower_repository, resource/artifactory_virtual_npm_repository: Fix `external_dependencies_patterns` attribute set to be force new, which causes the resource to be recreated when attribute value changes. Issue: [#1004](https://github.com/jfrog/terraform-provider-artifactory/issues/1004) PR: [#1005](https://github.com/jfrog/terraform-provider-artifactory/pull/1005)
+
 ## 11.0.0 (June 6, 2024). Tested on Artifactory 7.84.14 with Terraform 1.8.5 and OpenTofu 1.7.2
 
 BREAKING CHANGES:

diff --git a/pkg/acctest/test.go b/pkg/acctest/test.go
@@ -371,7 +371,7 @@ func CompareArtifactoryVersions(t *testing.T, instanceVersions string) (bool, er
 
 	skipTest := runtimeVersion.GreaterThanOrEqual(fixedVersion)
 	if skipTest {
-		t.Skip(fmt.Printf("Test skip because: runtime version %s is same or later than %s\n", runtimeVersion.String(), fixedVersion.String()))
+		t.Skipf("Test skip because: runtime version %s is same or later than %s\n", runtimeVersion.String(), fixedVersion.String())
 	}
 	return skipTest, nil
 }
diff --git a/pkg/artifactory/resource/repository/virtual/resource_artifactory_virtual_repository_test.go b/pkg/artifactory/resource/repository/virtual/resource_artifactory_virtual_repository_test.go
@@ -874,7 +874,7 @@ func TestAccVirtualBowerExternalDependenciesRepository(t *testing.T) {
 		"name":           name,
 		"remoteRepoName": remoteRepoName,
 	}
-	var virtualBowerRepository = util.ExecuteTemplate("TestAccVirtualBower", `
+	config := util.ExecuteTemplate("TestAccVirtualBower", `
 		resource "artifactory_remote_bower_repository" "bower-remote" {
 			key = "{{ .remoteRepoName }}"
 			url = "https://registry.npmjs.org"
@@ -891,14 +891,31 @@ func TestAccVirtualBowerExternalDependenciesRepository(t *testing.T) {
 		}
 	`, params)
 
+	updatedConfig := util.ExecuteTemplate("TestAccVirtualBower", `
+		resource "artifactory_remote_bower_repository" "bower-remote" {
+			key = "{{ .remoteRepoName }}"
+			url = "https://registry.npmjs.org"
+		}
+
+		resource "artifactory_virtual_bower_repository" "{{ .name }}" {
+			key                               = "{{ .name }}"
+			repositories                      = ["{{ .remoteRepoName }}"]
+			external_dependencies_enabled     = true
+			external_dependencies_patterns    = ["**/go.googlesource.com/**"]
+			external_dependencies_remote_repo = "{{ .remoteRepoName }}"
+
+			depends_on = ["artifactory_remote_bower_repository.bower-remote"]
+		}
+	`, params)
+
 	resource.Test(t, resource.TestCase{
 		PreCheck:          func() { acctest.PreCheck(t) },
 		ProviderFactories: acctest.ProviderFactories,
 		CheckDestroy:      acctest.VerifyDeleted(fqrn, acctest.CheckRepo),
 
 		Steps: []resource.TestStep{
 			{
-				Config: virtualBowerRepository,
+				Config: config,
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(fqrn, "key", name),
 					resource.TestCheckResourceAttr(fqrn, "package_type", "bower"),
@@ -910,6 +927,18 @@ func TestAccVirtualBowerExternalDependenciesRepository(t *testing.T) {
 					resource.TestCheckTypeSetElemAttr(fqrn, "external_dependencies_patterns.*", "**/go.googlesource.com/**"),
 				),
 			},
+			{
+				Config: updatedConfig,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(fqrn, "key", name),
+					resource.TestCheckResourceAttr(fqrn, "package_type", "bower"),
+					resource.TestCheckResourceAttr(fqrn, "repositories.#", "1"),
+					resource.TestCheckResourceAttr(fqrn, "repositories.0", remoteRepoName),
+					resource.TestCheckResourceAttr(fqrn, "external_dependencies_enabled", "true"),
+					resource.TestCheckResourceAttr(fqrn, "external_dependencies_patterns.#", "1"),
+					resource.TestCheckTypeSetElemAttr(fqrn, "external_dependencies_patterns.*", "**/go.googlesource.com/**"),
+				),
+			},
 			{
 				ResourceName:      fqrn,
 				ImportState:       true,
@@ -930,7 +959,7 @@ func TestAccVirtualNpmExternalDependenciesRepository(t *testing.T) {
 		"name":           name,
 		"remoteRepoName": remoteRepoName,
 	}
-	var virtualNpmRepository = util.ExecuteTemplate("TestAccVirtualNpm", `
+	config := util.ExecuteTemplate("TestAccVirtualNpm", `
 		resource "artifactory_remote_npm_repository" "npm-remote" {
 			key = "{{ .remoteRepoName }}"
 			url = "https://registry.npmjs.org"
@@ -948,14 +977,32 @@ func TestAccVirtualNpmExternalDependenciesRepository(t *testing.T) {
 		}
 	`, params)
 
+	updatedConfig := util.ExecuteTemplate("TestAccVirtualNpm", `
+		resource "artifactory_remote_npm_repository" "npm-remote" {
+			key = "{{ .remoteRepoName }}"
+			url = "https://registry.npmjs.org"
+		}
+
+		resource "artifactory_virtual_npm_repository" "{{ .name }}" {
+			key                               = "{{ .name }}"
+			repositories                      = ["{{ .remoteRepoName }}"]
+			external_dependencies_enabled     = true
+			retrieval_cache_period_seconds    = 650
+			external_dependencies_patterns    = ["**/go.googlesource.com/**"]
+			external_dependencies_remote_repo = "{{ .remoteRepoName }}"
+
+			depends_on = ["artifactory_remote_npm_repository.npm-remote"]
+		}
+	`, params)
+
 	resource.Test(t, resource.TestCase{
 		PreCheck:          func() { acctest.PreCheck(t) },
 		ProviderFactories: acctest.ProviderFactories,
 		CheckDestroy:      acctest.VerifyDeleted(fqrn, acctest.CheckRepo),
 
 		Steps: []resource.TestStep{
 			{
-				Config: virtualNpmRepository,
+				Config: config,
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(fqrn, "key", name),
 					resource.TestCheckResourceAttr(fqrn, "package_type", "npm"),
@@ -968,6 +1015,19 @@ func TestAccVirtualNpmExternalDependenciesRepository(t *testing.T) {
 					resource.TestCheckTypeSetElemAttr(fqrn, "external_dependencies_patterns.*", "**/go.googlesource.com/**"),
 				),
 			},
+			{
+				Config: updatedConfig,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(fqrn, "key", name),
+					resource.TestCheckResourceAttr(fqrn, "package_type", "npm"),
+					resource.TestCheckResourceAttr(fqrn, "repositories.#", "1"),
+					resource.TestCheckResourceAttr(fqrn, "repositories.0", remoteRepoName),
+					resource.TestCheckResourceAttr(fqrn, "retrieval_cache_period_seconds", "650"),
+					resource.TestCheckResourceAttr(fqrn, "external_dependencies_enabled", "true"),
+					resource.TestCheckResourceAttr(fqrn, "external_dependencies_patterns.#", "1"),
+					resource.TestCheckTypeSetElemAttr(fqrn, "external_dependencies_patterns.*", "**/go.googlesource.com/**"),
+				),
+			},
 			{
 				ResourceName:      fqrn,
 				ImportState:       true,

diff --git a/pkg/artifactory/resource/repository/virtual/virtual.go b/pkg/artifactory/resource/repository/virtual/virtual.go
@@ -122,7 +122,6 @@ var externalDependenciesSchema = map[string]*schema.Schema{
 	"external_dependencies_patterns": {
 		Type:     schema.TypeList,
 		Optional: true,
-		ForceNew: true,
 		Elem: &schema.Schema{
 			Type: schema.TypeString,
 		},