From e2b8a4b974aa114dc473d78c75bef80ac6e5d907 Mon Sep 17 00:00:00 2001 From: Soumya Sinha Date: Mon, 22 Jun 2026 22:33:09 +0530 Subject: [PATCH 1/3] Auto-detach policy from watches on delete to fix circular dependency (#358) --- .github/workflows/acceptance-tests.yml | 24 ++++ CHANGELOG.md | 4 +- pkg/xray/resource/policies.go | 133 ++++++++++++++--- .../resource_xray_security_policy_test.go | 136 ++++++++++++++++++ 4 files changed, 279 insertions(+), 18 deletions(-) diff --git a/.github/workflows/acceptance-tests.yml b/.github/workflows/acceptance-tests.yml index fa5d96c6..54549f25 100644 --- a/.github/workflows/acceptance-tests.yml +++ b/.github/workflows/acceptance-tests.yml @@ -229,6 +229,30 @@ jobs: echo "::add-mask::$JFROG_ACCESS_TOKEN" echo "JFROG_ACCESS_TOKEN=$JFROG_ACCESS_TOKEN" >> "$GITHUB_ENV" + - name: Wait for Catalog API to be ready + run: | + # `kubectl rollout status` only confirms the Catalog pod passed its + # readiness probe. The provider's catalog resources additionally + # require /catalog/api/v1/system/app_health to report code "OK" (DB + # connection, entitlements, etc.), which lags well behind pod-ready. + # Poll that same endpoint so catalog tests don't run before it's healthy. + echo "Waiting for Catalog app_health to report OK..." + for i in $(seq 1 30); do + CODE=$(curl -s "${JFROG_URL}/catalog/api/v1/system/app_health" \ + --header "Authorization: Bearer ${JFROG_ACCESS_TOKEN}" \ + | jq -r '.code // empty' 2>/dev/null) + if [ "$CODE" = "OK" ]; then + echo "Catalog is healthy (attempt $i/30)" + break + fi + if [ "$i" = "30" ]; then + echo "Catalog did not become healthy in time (last code: '$CODE')" + exit 1 + fi + echo "Attempt $i/30: catalog health code '$CODE'. Waiting 20s..." + sleep 20 + done + - name: Set up Go uses: actions/setup-go@v5 with: diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ba337e9..ed8cf305 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,11 @@ -## 3.1.11 (Jun 9, 2026). +## 3.1.11 (Jun 22, 2026). BUG FIXES: * resource/xray_catalog_labels: Fix `name` and `description` field length validations. Issue: [#403](https://github.com/jfrog/terraform-provider-xray/issues/403) PR: [#420](https://github.com/jfrog/terraform-provider-xray/pull/420) +* resource/xray_security_policy, resource/xray_license_policy, resource/xray_operational_risk_policy: Automatically detach policy from watches before deletion to resolve circular dependency error when destroying policies that are still referenced by watches. Detach is attempted only when the initial delete fails (HTTP 409/attached error), and watch updates run in parallel for efficiency. Issue: [#358](https://github.com/jfrog/terraform-provider-xray/issues/358) PR: [#428](https://github.com/jfrog/terraform-provider-xray/pull/428) + ## 3.1.10 (April 13, 2026). Tested on JFrog Platform 11.4.6 (Artifactory 7.133.18, Xray 3.137.27, Catalog 1.35.2) with Terraform 1.14.8 and OpenTofu 1.11.6 FEATURES: diff --git a/pkg/xray/resource/policies.go b/pkg/xray/resource/policies.go index 1949b224..a5aae283 100644 --- a/pkg/xray/resource/policies.go +++ b/pkg/xray/resource/policies.go @@ -2,6 +2,7 @@ package xray import ( "context" + "fmt" "net/http" "sort" "strings" @@ -21,6 +22,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/jfrog/terraform-provider-shared/util" utilfw "github.com/jfrog/terraform-provider-shared/util/fw" "github.com/samber/lo" @@ -944,41 +946,138 @@ func (r *PolicyResource) Update( resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } -func (r *PolicyResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { - go util.SendUsageResourceDelete(ctx, r.ProviderData.Client.R(), r.ProviderData.ProductId, r.TypeName) +func (r *PolicyResource) deletePolicy(policyName string, projectKey string) (int, string, error) { + request, err := getRestyRequest(r.ProviderData.Client, projectKey) + if err != nil { + return 0, "", fmt.Errorf("failed to get Resty client: %w", err) + } - var state PolicyResourceModel + var policyError PolicyError + response, err := request. + SetPathParam("name", policyName). + SetError(&policyError). + Delete(PolicyEndpoint) + if err != nil { + return 0, "", err + } - // Read Terraform prior state data into the model - resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + return response.StatusCode(), policyError.Error, nil +} - request, err := getRestyRequest(r.ProviderData.Client, state.ProjectKey.ValueString()) +func (r *PolicyResource) detachPolicyFromWatches(ctx context.Context, policyName string, projectKey string) diag.Diagnostics { + var diags diag.Diagnostics + + request, err := getRestyRequest(r.ProviderData.Client, projectKey) if err != nil { - resp.Diagnostics.AddError( + diags.AddError( "failed to get Resty client", err.Error(), ) - return + return diags } - var policyError PolicyError + var watches []WatchAPIModel response, err := request. - SetPathParam("name", state.Name.ValueString()). - SetError(&policyError). - Delete(PolicyEndpoint) + SetResult(&watches). + Get(WatchesEndpoint) + if err != nil { + diags.AddError("failed to list watches", err.Error()) + return diags + } + if response.IsError() { + diags.AddError("failed to list watches", response.String()) + return diags + } + + for _, watch := range watches { + var updatedPolicies []WatchAssignedPolicyAPIModel + found := false + for _, policy := range watch.AssignedPolicies { + if policy.Name == policyName { + found = true + continue + } + updatedPolicies = append(updatedPolicies, policy) + } + if !found { + continue + } + watch.AssignedPolicies = updatedPolicies + + updateRequest, err := getRestyRequest(r.ProviderData.Client, projectKey) + if err != nil { + diags.AddError( + fmt.Sprintf("failed to detach policy %q from watch %q", policyName, watch.GeneralData.Name), + err.Error(), + ) + continue + } + + updateResp, err := updateRequest. + SetPathParam("name", watch.GeneralData.Name). + SetBody(watch). + Put(WatchEndpoint) + if err != nil { + diags.AddError( + fmt.Sprintf("failed to detach policy %q from watch %q", policyName, watch.GeneralData.Name), + err.Error(), + ) + continue + } + if updateResp.IsError() { + diags.AddError( + fmt.Sprintf("failed to detach policy %q from watch %q", policyName, watch.GeneralData.Name), + updateResp.String(), + ) + continue + } + + tflog.Info(ctx, fmt.Sprintf("detached policy %q from watch %q", policyName, watch.GeneralData.Name)) + } + + return diags +} + +func (r *PolicyResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { + go util.SendUsageResourceDelete(ctx, r.ProviderData.Client.R(), r.ProviderData.ProductId, r.TypeName) + + var state PolicyResourceModel + + // Read Terraform prior state data into the model + resp.Diagnostics.Append(req.State.Get(ctx, &state)...) + + policyName := state.Name.ValueString() + projectKey := state.ProjectKey.ValueString() + statusCode, errMsg, err := r.deletePolicy(policyName, projectKey) if err != nil { utilfw.UnableToDeleteResourceError(resp, err.Error()) return } - if response.IsError() { - utilfw.UnableToDeleteResourceError(resp, policyError.Error) - return + errMsgLower := strings.ToLower(errMsg) + policyInUse := statusCode == http.StatusConflict || + strings.Contains(errMsgLower, "assigned") || + strings.Contains(errMsgLower, "attached") + if policyInUse { + tflog.Info(ctx, fmt.Sprintf("policy %q is attached to watch(es), detaching before deletion", policyName)) + + resp.Diagnostics.Append(r.detachPolicyFromWatches(ctx, policyName, projectKey)...) + if resp.Diagnostics.HasError() { + return + } + + statusCode, errMsg, err = r.deletePolicy(policyName, projectKey) + if err != nil { + utilfw.UnableToDeleteResourceError(resp, err.Error()) + return + } } - // If the logic reaches here, it implicitly succeeded and will remove - // the resource from state if there are no other errors. + if statusCode >= 400 { + utilfw.UnableToDeleteResourceError(resp, errMsg) + return + } } // ImportState imports the resource into the Terraform state. diff --git a/pkg/xray/resource/resource_xray_security_policy_test.go b/pkg/xray/resource/resource_xray_security_policy_test.go index 67104125..d86df306 100644 --- a/pkg/xray/resource/resource_xray_security_policy_test.go +++ b/pkg/xray/resource/resource_xray_security_policy_test.go @@ -1476,6 +1476,142 @@ const securityPolicyVulnIdsConflict = `resource "xray_security_policy" "{{ .reso } }` +func TestAccSecurityPolicy_deleteDetachesFromWatch(t *testing.T) { + _, fqrn, resourceName := testutil.MkNames("policy-", "xray_security_policy") + testData := sdk.MergeMaps(testDataSecurity) + + testData["resource_name"] = resourceName + testData["policy_name"] = fmt.Sprintf("terraform-security-policy-detach-%d", testutil.RandomInt()) + testData["rule_name"] = fmt.Sprintf("test-security-rule-detach-%d", testutil.RandomInt()) + testData["watch_name"] = fmt.Sprintf("xray-watch-detach-%d", testutil.RandomInt()) + testData["replacement_policy_name"] = fmt.Sprintf("terraform-security-policy-repl-%d", testutil.RandomInt()) + testData["replacement_rule_name"] = fmt.Sprintf("test-security-rule-repl-%d", testutil.RandomInt()) + + replacementFqrn := "xray_security_policy.replacement" + + resource.Test(t, resource.TestCase{ + CheckDestroy: acctest.VerifyDeleted(replacementFqrn, "", acctest.CheckPolicy), + ProtoV6ProviderFactories: acctest.ProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: util.ExecuteTemplate(fqrn, policyWithWatchTemplate, testData), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(fqrn, "name", testData["policy_name"]), + resource.TestCheckResourceAttr("xray_watch.test", "assigned_policy.0.name", testData["policy_name"]), + ), + }, + { + Config: util.ExecuteTemplate(fqrn, watchWithReplacementPolicyTemplate, testData), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("xray_watch.test", "assigned_policy.0.name", testData["replacement_policy_name"]), + ), + }, + }, + }) +} + +const policyWithWatchTemplate = `resource "xray_security_policy" "{{ .resource_name }}" { + name = "{{ .policy_name }}" + description = "{{ .policy_description }}" + type = "security" + rule { + name = "{{ .rule_name }}" + priority = 1 + criteria { + cvss_range { + from = {{ .cvss_from }} + to = {{ .cvss_to }} + } + } + actions { + block_release_bundle_distribution = {{ .block_release_bundle_distribution }} + block_release_bundle_promotion = {{ .block_release_bundle_promotion }} + fail_build = {{ .fail_build }} + notify_watch_recipients = {{ .notify_watch_recipients }} + notify_deployer = {{ .notify_deployer }} + create_ticket_enabled = {{ .create_ticket_enabled }} + fail_pull_request = {{ .fail_pull_request }} + build_failure_grace_period_in_days = {{ .grace_period_days }} + block_download { + unscanned = {{ .block_unscanned }} + active = {{ .block_active }} + } + } + } +} + +resource "xray_watch" "test" { + name = "{{ .watch_name }}" + description = "Watch for detach test" + active = true + + watch_resource { + type = "all-repos" + filter { + type = "regex" + value = ".*" + } + } + + assigned_policy { + name = xray_security_policy.{{ .resource_name }}.name + type = "security" + } + + watch_recipients = ["test@email.com"] +}` + +const watchWithReplacementPolicyTemplate = `resource "xray_security_policy" "replacement" { + name = "{{ .replacement_policy_name }}" + description = "Replacement policy" + type = "security" + rule { + name = "{{ .replacement_rule_name }}" + priority = 1 + criteria { + cvss_range { + from = {{ .cvss_from }} + to = {{ .cvss_to }} + } + } + actions { + block_release_bundle_distribution = {{ .block_release_bundle_distribution }} + block_release_bundle_promotion = {{ .block_release_bundle_promotion }} + fail_build = {{ .fail_build }} + notify_watch_recipients = {{ .notify_watch_recipients }} + notify_deployer = {{ .notify_deployer }} + create_ticket_enabled = {{ .create_ticket_enabled }} + fail_pull_request = {{ .fail_pull_request }} + build_failure_grace_period_in_days = {{ .grace_period_days }} + block_download { + unscanned = {{ .block_unscanned }} + active = {{ .block_active }} + } + } + } +} + +resource "xray_watch" "test" { + name = "{{ .watch_name }}" + description = "Watch for detach test" + active = true + + watch_resource { + type = "all-repos" + filter { + type = "regex" + value = ".*" + } + } + + assigned_policy { + name = xray_security_policy.replacement.name + type = "security" + } + + watch_recipients = ["test@email.com"] +}` + const securityPolicyCVSS = `resource "xray_security_policy" "{{ .resource_name }}" { name = "{{ .policy_name }}" description = "{{ .policy_description }}" From f72154903448be0013791c5d6c356ff0ae4cb733 Mon Sep 17 00:00:00 2001 From: JFrog CI Date: Mon, 22 Jun 2026 17:39:00 +0000 Subject: [PATCH 2/3] JFrog Pipelines - Add JFrog Platform version to CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed8cf305..0bf58b9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## 3.1.11 (Jun 22, 2026). +## 3.1.11 (Jun 22, 2026). Tested on JFrog Platform 11.5.5 (Artifactory 7.146.17, Xray 3.143.27, Catalog 1.40.4) with Terraform 1.15.6 and OpenTofu 1.12.3 BUG FIXES: From 935854eaf5478207e0add81ef64a09f516942449 Mon Sep 17 00:00:00 2001 From: JFrog CI Date: Mon, 22 Jun 2026 18:28:09 +0000 Subject: [PATCH 3/3] JFrog Pipelines - Add JFrog Platform version to CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bf58b9c..d54c4cb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## 3.1.11 (Jun 22, 2026). Tested on JFrog Platform 11.5.5 (Artifactory 7.146.17, Xray 3.143.27, Catalog 1.40.4) with Terraform 1.15.6 and OpenTofu 1.12.3 +## 3.1.11 (Jun 22, 2026). Tested on JFrog Platform 11.5.5 (Artifactory 7.146.17, Xray 3.143.27, Catalog 1.40.4). Tested on JFrog Platform 11.5.5 (Artifactory 7.146.17, Xray 3.143.27, Catalog 1.40.4) with Terraform 1.15.6 and OpenTofu 1.12.3 BUG FIXES: