Merge pull request #921 from jfrog/GH-886-fix-state-drift-issue

Potentially fix state drift issue
jfrog · Mar 27, 2024 · bf73ce6 · bf73ce6
2 parents ec85e1c + e4d3f0f
commit bf73ce6
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 22 deletions.
diff --git a/pkg/artifactory/resource/configuration/resource_artifactory_ldap_setting.go b/pkg/artifactory/resource/configuration/resource_artifactory_ldap_setting.go
@@ -159,10 +159,13 @@ func ResourceArtifactoryLdapSetting() *schema.Resource {
 		key := data.GetString("key", false)
 
 		ldapConfigs := XmlLdapConfig{}
-		_, err := m.(util.ProvderMetadata).Client.R().SetResult(&ldapConfigs).Get("artifactory/api/system/configuration")
+		resp, err := m.(util.ProvderMetadata).Client.R().SetResult(&ldapConfigs).Get("artifactory/api/system/configuration")
 		if err != nil {
 			return diag.Errorf("failed to retrieve data from API: /artifactory/api/system/configuration during Read")
 		}
+		if resp.IsError() {
+			return diag.Errorf("%s", resp.String())
+		}
 
 		matchedLdapSetting := FindConfigurationById[LdapSetting](ldapConfigs.Security.LdapSettings.LdapSettingArr, key)
 		if matchedLdapSetting == nil {

diff --git a/pkg/artifactory/resource/replication/replication.go b/pkg/artifactory/resource/replication/replication.go
@@ -50,12 +50,16 @@ type repoConfiguration struct {
 
 func getRepositoryRclass(repoKey string, m interface{}) (string, error) {
 	repoConfig := repoConfiguration{}
-	_, err := m.(util.ProvderMetadata).Client.R().
+	resp, err := m.(util.ProvderMetadata).Client.R().
 		SetResult(&repoConfig).
 		Get("artifactory/api/repositories/" + repoKey)
 	if err != nil {
 		return "", err
 	}
+	if resp.IsError() {
+		return "", fmt.Errorf("%s", resp.String())
+	}
+
 	return repoConfig.Rclass, err
 }
 

diff --git a/pkg/artifactory/resource/repository/federated/federated.go b/pkg/artifactory/resource/repository/federated/federated.go
@@ -158,7 +158,7 @@ func configSync(ctx context.Context, repoKey string, m interface{}) diag.Diagnos
 			"repoKey": repoKey,
 		},
 	)
-	_, restErr := m.(util.ProvderMetadata).Client.R().
+	resp, restErr := m.(util.ProvderMetadata).Client.R().
 		SetPathParam("repositoryKey", repoKey).
 		Post("artifactory/api/federation/configSync/{repositoryKey}")
 	if restErr != nil {
@@ -168,6 +168,13 @@ func configSync(ctx context.Context, repoKey string, m interface{}) diag.Diagnos
 			Detail:   restErr.Error(),
 		})
 	}
+	if resp.IsError() {
+		ds = append(ds, diag.Diagnostic{
+			Severity: diag.Warning,
+			Summary:  "failed to trigger synchronization of the federated member configuration",
+			Detail:   resp.String(),
+		})
+	}
 
 	return ds
 }

diff --git a/pkg/artifactory/resource/repository/repository.go b/pkg/artifactory/resource/repository/repository.go
@@ -184,12 +184,16 @@ func MkRepoRead(pack packer.PackFunc, construct Constructor) schema.ReadContextF
 			Get(RepositoriesEndpoint)
 
 		if err != nil {
-			if resp != nil && (resp.StatusCode() == http.StatusBadRequest || resp.StatusCode() == http.StatusNotFound) {
-				d.SetId("")
-				return nil
-			}
 			return diag.FromErr(err)
 		}
+		if resp.StatusCode() == http.StatusBadRequest || resp.StatusCode() == http.StatusNotFound {
+			d.SetId("")
+			return nil
+		}
+		if resp.IsError() {
+			return diag.Errorf("%s", resp.String())
+		}
+
 		return diag.FromErr(pack(repo, d))
 	}
 }

diff --git a/pkg/artifactory/resource/security/resource_artifactory_access_token.go b/pkg/artifactory/resource/security/resource_artifactory_access_token.go
@@ -292,6 +292,11 @@ func resourceAccessTokenDelete(ctx context.Context, d *schema.ResourceData, m in
 			d.SetId("")
 			return nil
 		}
+
+		if resp.IsError() {
+			return diag.Errorf("%s", resp.String())
+		}
+
 		// the original atlassian code considered any error code fine. However, expiring tokens can't be revoked
 		regex := regexp.MustCompile(`.*AccessToken not revocable.*`)
 		if regex.MatchString(string(resp.Body()[:])) {

diff --git a/pkg/artifactory/resource/security/resource_artifactory_api_key.go b/pkg/artifactory/resource/security/resource_artifactory_api_key.go
@@ -59,10 +59,13 @@ func packApiKey(apiKey string, d *schema.ResourceData) diag.Diagnostics {
 func resourceApiKeyCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
 	data := ApiKey{}
 
-	_, err := m.(util.ProvderMetadata).Client.R().SetResult(&data).Post(ApiKeyEndpoint)
+	resp, err := m.(util.ProvderMetadata).Client.R().SetResult(&data).Post(ApiKeyEndpoint)
 	if err != nil {
 		return diag.FromErr(err)
 	}
+	if resp.IsError() {
+		return diag.Errorf("%s", resp.String())
+	}
 
 	if len(data.ApiKey) > 0 {
 		d.SetId(strconv.Itoa(schema.HashString(data.ApiKey)))
@@ -73,10 +76,13 @@ func resourceApiKeyCreate(ctx context.Context, d *schema.ResourceData, m interfa
 
 func resourceApiKeyRead(_ context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
 	data := ApiKey{}
-	_, err := m.(util.ProvderMetadata).Client.R().SetResult(&data).Get(ApiKeyEndpoint)
+	resp, err := m.(util.ProvderMetadata).Client.R().SetResult(&data).Get(ApiKeyEndpoint)
 	if err != nil {
 		return diag.FromErr(err)
 	}
+	if resp.IsError() {
+		return diag.Errorf("%s", resp.String())
+	}
 	if data.ApiKey == "" {
 		d.SetId("")
 		return nil
@@ -85,6 +91,13 @@ func resourceApiKeyRead(_ context.Context, d *schema.ResourceData, m interface{}
 }
 
 func apiKeyRevoke(_ context.Context, _ *schema.ResourceData, m interface{}) diag.Diagnostics {
-	_, err := m.(util.ProvderMetadata).Client.R().Delete(ApiKeyEndpoint)
-	return diag.FromErr(err)
+	resp, err := m.(util.ProvderMetadata).Client.R().Delete(ApiKeyEndpoint)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+	if resp.IsError() {
+		return diag.Errorf("%s", resp.String())
+	}
+
+	return nil
 }
diff --git a/pkg/artifactory/resource/security/resource_artifactory_permission_target.go b/pkg/artifactory/resource/security/resource_artifactory_permission_target.go
@@ -341,9 +341,10 @@ func resourcePermissionTargetCreate(ctx context.Context, d *schema.ResourceData,
 		SetBody(permissionTarget).
 		Post(PermissionsEndPoint + permissionTarget.Name)
 	if err != nil {
-		if !(resp.StatusCode() == http.StatusConflict && conflictRegex.Match(resp.Body())) {
-			return diag.FromErr(err)
-		}
+		return diag.FromErr(err)
+	}
+	if resp.IsError() && !(resp.StatusCode() == http.StatusConflict && conflictRegex.Match(resp.Body())) {
+		return diag.Errorf("%s", resp.String())
 	}
 
 	d.SetId(permissionTarget.Name)
@@ -357,34 +358,47 @@ func resourcePermissionTargetRead(_ context.Context, d *schema.ResourceData, m i
 		SetResult(&permissionTarget).
 		Get(PermissionsEndPoint + d.Id())
 	if err != nil {
-		if resp != nil && resp.StatusCode() == http.StatusNotFound {
-			d.SetId("")
-			return nil
-		}
 		return diag.FromErr(err)
 	}
+	if resp.StatusCode() == http.StatusNotFound {
+		d.SetId("")
+		return nil
+	}
+	if resp.IsError() {
+		return diag.Errorf("%s", resp.String())
+	}
 
 	return PackPermissionTarget(&permissionTarget, d)
 }
 
 func resourcePermissionTargetUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
 	permissionTarget := unpackPermissionTarget(ctx, d)
 
-	_, err := m.(util.ProvderMetadata).Client.R().
+	resp, err := m.(util.ProvderMetadata).Client.R().
 		SetBody(permissionTarget).
 		Put(PermissionsEndPoint + d.Id())
 	if err != nil {
 		return diag.FromErr(err)
 	}
+	if resp.IsError() {
+		return diag.Errorf("%s", resp.String())
+	}
 
 	d.SetId(permissionTarget.Name)
 	return resourcePermissionTargetRead(ctx, d, m)
 }
 
 func resourcePermissionTargetDelete(_ context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
-	_, err := m.(util.ProvderMetadata).Client.R().Delete(PermissionsEndPoint + d.Id())
+	resp, err := m.(util.ProvderMetadata).Client.R().Delete(PermissionsEndPoint + d.Id())
+
+	if err != nil {
+		return diag.FromErr(err)
+	}
+	if resp.IsError() {
+		return diag.Errorf("%s", resp.String())
+	}
 
-	return diag.FromErr(err)
+	return nil
 }
 
 func PermTargetExists(id string, m interface{}) (bool, error) {

diff --git a/scripts/run-artifactory.sh b/scripts/run-artifactory.sh
@@ -3,7 +3,7 @@
 SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" > /dev/null && pwd )"
 source "${SCRIPT_DIR}/get-access-key.sh"
 source "${SCRIPT_DIR}/wait-for-rt.sh"
-export ARTIFACTORY_VERSION=${ARTIFACTORY_VERSION:-7.71.4}
+export ARTIFACTORY_VERSION=${ARTIFACTORY_VERSION:-7.77.7}
 echo "ARTIFACTORY_VERSION=${ARTIFACTORY_VERSION}"
 
 set -euf