Merge pull request #249 from jfrog/GH-248-fix-policy-state-drift

Fix inconsistent result/state drift in policy resources

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 2.11.2 (September 23, 2024). Tested on Artifactory 7.90.10 and Xray 3.104.11 with Terraform 1.9.6 and OpenTofu 1.8.2
+
+BUG FIXES:
+
+* resource/xray_\*\_policy: Fix "Provider produced inconsistent result after apply" error due to `build_failure_grace_period_in_days` attribute. PR: [#248](https://github.com/jfrog/terraform-provider-xray/pull/248) Issue: [#248](https://github.com/jfrog/terraform-provider-xray/issues/248)
+
 ## 2.11.1 (September 16, 2024). Tested on Artifactory 7.90.10 and Xray 3.104.11 with Terraform 1.9.5 and OpenTofu 1.8.2
 
 IMPROVEMENTS:

pkg/xray/resource/policies.go

@@ -105,7 +105,7 @@ var toActionsAPIModel = func(ctx context.Context, actionsElems []attr.Value) (Po
 		actions.NotifyWatchRecipients = attrs["notify_watch_recipients"].(types.Bool).ValueBool()
 		actions.NotifyDeployer = attrs["notify_deployer"].(types.Bool).ValueBool()
 		actions.CreateJiraTicketEnabled = attrs["create_ticket_enabled"].(types.Bool).ValueBool()
-		actions.FailureGracePeriodDays = attrs["build_failure_grace_period_in_days"].(types.Int64).ValueInt64()
+		actions.FailureGracePeriodDays = attrs["build_failure_grace_period_in_days"].(types.Int64).ValueInt64Pointer()
 	}
 
 	return actions, diags
@@ -223,7 +223,7 @@ var fromActionsAPIModel = func(ctx context.Context, actionsAPIModel PolicyRuleAc
 			"notify_deployer":                    types.BoolValue(actionsAPIModel.NotifyDeployer),
 			"notify_watch_recipients":            types.BoolValue(actionsAPIModel.NotifyWatchRecipients),
 			"create_ticket_enabled":              types.BoolValue(actionsAPIModel.CreateJiraTicketEnabled),
-			"build_failure_grace_period_in_days": types.Int64Value(actionsAPIModel.FailureGracePeriodDays),
+			"build_failure_grace_period_in_days": types.Int64PointerValue(actionsAPIModel.FailureGracePeriodDays),
 		},
 	)
 	if d.HasError() {
@@ -571,7 +571,7 @@ type PolicyRuleActionsAPIModel struct {
 	NotifyWatchRecipients          bool                          `json:"notify_watch_recipients"`
 	NotifyDeployer                 bool                          `json:"notify_deployer"`
 	CreateJiraTicketEnabled        bool                          `json:"create_ticket_enabled"`
-	FailureGracePeriodDays         int64                         `json:"build_failure_grace_period_in_days,omitempty"`
+	FailureGracePeriodDays         *int64                        `json:"build_failure_grace_period_in_days,omitempty"`
 	// License Actions
 	CustomSeverity string `json:"custom_severity,omitempty"`
 }

pkg/xray/resource/resource_xray_license_policy.go

@@ -197,7 +197,7 @@ func (m *LicensePolicyResource) fromActionsAPIModel(ctx context.Context, actions
 			"notify_deployer":                    types.BoolValue(actionsAPIModel.NotifyDeployer),
 			"notify_watch_recipients":            types.BoolValue(actionsAPIModel.NotifyWatchRecipients),
 			"create_ticket_enabled":              types.BoolValue(actionsAPIModel.CreateJiraTicketEnabled),
-			"build_failure_grace_period_in_days": types.Int64Value(actionsAPIModel.FailureGracePeriodDays),
+			"build_failure_grace_period_in_days": types.Int64PointerValue(actionsAPIModel.FailureGracePeriodDays),
 			"custom_severity":                    types.StringValue(actionsAPIModel.CustomSeverity),
 		},
 	)

pkg/xray/resource/resource_xray_operational_risk_policy.go

@@ -197,41 +197,16 @@ func (r *OperationalRiskPolicyResource) fromCriteriaAPIModel(ctx context.Context
 				risk = types.StringValue(criteraAPIModel.OperationalRiskCustom.Risk)
 			}
 
-			releaseDateGreaterThanMonths := types.Int64Null()
-			if criteraAPIModel.OperationalRiskCustom.ReleaseDateGreaterThanMonths != nil {
-				releaseDateGreaterThanMonths = types.Int64PointerValue(criteraAPIModel.OperationalRiskCustom.ReleaseDateGreaterThanMonths)
-			}
-
-			newerVersionsGreaterThan := types.Int64Null()
-			if criteraAPIModel.OperationalRiskCustom.NewerVersionsGreaterThan != nil {
-				newerVersionsGreaterThan = types.Int64PointerValue(criteraAPIModel.OperationalRiskCustom.NewerVersionsGreaterThan)
-			}
-
-			releaseCadencePerYearLessThan := types.Int64Null()
-			if criteraAPIModel.OperationalRiskCustom.ReleaseCadencePerYearLessThan != nil {
-				releaseCadencePerYearLessThan = types.Int64PointerValue(criteraAPIModel.OperationalRiskCustom.ReleaseCadencePerYearLessThan)
-			}
-
-			commitsLessThan := types.Int64Null()
-			if criteraAPIModel.OperationalRiskCustom.CommitsLessThan != nil {
-				commitsLessThan = types.Int64PointerValue(criteraAPIModel.OperationalRiskCustom.CommitsLessThan)
-			}
-
-			committersLessThan := types.Int64Null()
-			if criteraAPIModel.OperationalRiskCustom.CommittersLessThan != nil {
-				committersLessThan = types.Int64PointerValue(criteraAPIModel.OperationalRiskCustom.CommittersLessThan)
-			}
-
 			custom, d := types.ObjectValue(
 				opRiskCustomAttrType,
 				map[string]attr.Value{
 					"use_and_condition":                  types.BoolValue(criteraAPIModel.OperationalRiskCustom.UseAndCondition),
 					"is_eol":                             types.BoolValue(criteraAPIModel.OperationalRiskCustom.IsEOL),
-					"release_date_greater_than_months":   releaseDateGreaterThanMonths,
-					"newer_versions_greater_than":        newerVersionsGreaterThan,
-					"release_cadence_per_year_less_than": releaseCadencePerYearLessThan,
-					"commits_less_than":                  commitsLessThan,
-					"committers_less_than":               committersLessThan,
+					"release_date_greater_than_months":   types.Int64PointerValue(criteraAPIModel.OperationalRiskCustom.ReleaseDateGreaterThanMonths),
+					"newer_versions_greater_than":        types.Int64PointerValue(criteraAPIModel.OperationalRiskCustom.NewerVersionsGreaterThan),
+					"release_cadence_per_year_less_than": types.Int64PointerValue(criteraAPIModel.OperationalRiskCustom.ReleaseCadencePerYearLessThan),
+					"commits_less_than":                  types.Int64PointerValue(criteraAPIModel.OperationalRiskCustom.CommitsLessThan),
+					"committers_less_than":               types.Int64PointerValue(criteraAPIModel.OperationalRiskCustom.CommittersLessThan),
 					"risk":                               risk,
 				},
 			)

pkg/xray/resource/resource_xray_security_policy_test.go

@@ -50,6 +50,7 @@ func TestAccSecurityPolicy_UpgradeFromSDKv2(t *testing.T) {
 	testData["resource_name"] = resourceName
 	testData["policy_name"] = fmt.Sprintf("terraform-security-policy-4-%d", testutil.RandomInt())
 	testData["rule_name"] = fmt.Sprintf("test-security-rule-4-%d", testutil.RandomInt())
+	testData["applicable_cves_only"] = "false"
 
 	template := `
 	resource "xray_security_policy" "{{ .resource_name }}" {
@@ -65,7 +66,6 @@ func TestAccSecurityPolicy_UpgradeFromSDKv2(t *testing.T) {
 					from = {{ .cvss_from }}
 					to = {{ .cvss_to }}
 				}
-				applicable_cves_only = {{ .applicable_cves_only }}
 			}
 			actions {
 				block_release_bundle_distribution = {{ .block_release_bundle_distribution }}
@@ -113,6 +113,61 @@ func TestAccSecurityPolicy_UpgradeFromSDKv2(t *testing.T) {
 	})
 }
 
+func TestAccSecurityPolicy_optional_actions_attribute(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-%d", testutil.RandomInt())
+	testData["rule_name"] = fmt.Sprintf("test-security-rule-%d", testutil.RandomInt())
+
+	templ := `
+	resource "xray_security_policy" "{{ .resource_name }}" {
+		name = "{{ .policy_name }}"
+		description = "{{ .policy_description }}"
+		type = "security"
+		rule {
+			name = "{{ .rule_name }}"
+			priority = 1
+			criteria {
+				cvss_range {
+					from = 7.0
+					to   = 10.0
+				}
+			}
+			actions {
+				block_download {
+					unscanned = {{ .block_unscanned }}
+					active = {{ .block_active }}
+				}
+			}
+		}
+	}`
+
+	config := util.ExecuteTemplate(fqrn, templ, testData)
+
+	resource.Test(t, resource.TestCase{
+		CheckDestroy:             acctest.VerifyDeleted(fqrn, "", acctest.CheckPolicy),
+		ProtoV6ProviderFactories: acctest.ProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config: config,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckNoResourceAttr(fqrn, "build_failure_grace_period_in_days"),
+				),
+			},
+			{
+				Config: config,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(),
+					},
+				},
+			},
+		},
+	})
+}
+
 func TestAccSecurityPolicy_multipleRules(t *testing.T) {
 	_, fqrn, resourceName := testutil.MkNames("policy-", "xray_security_policy")
 	testData := sdk.MergeMaps(testDataSecurity)
@@ -274,6 +329,7 @@ func TestAccSecurityPolicy_withProjectKey(t *testing.T) {
 	testData["project_key"] = projectKey
 	testData["policy_name"] = fmt.Sprintf("terraform-security-policy-4-%d", testutil.RandomInt())
 	testData["rule_name"] = fmt.Sprintf("test-security-rule-4-%d", testutil.RandomInt())
+	testData["applicable_cves_only"] = "false"
 
 	template := `
 	resource "project" "{{ .project_key }}" {
@@ -300,7 +356,6 @@ func TestAccSecurityPolicy_withProjectKey(t *testing.T) {
 					from = {{ .cvss_from }}
 					to = {{ .cvss_to }}
 				}
-				applicable_cves_only = {{ .applicable_cves_only }}
 			}
 			actions {
 				block_release_bundle_distribution = {{ .block_release_bundle_distribution }}
@@ -362,6 +417,7 @@ func TestAccSecurityPolicy_createBlockDownloadTrueCVSS(t *testing.T) {
 	testData["resource_name"] = resourceName
 	testData["policy_name"] = fmt.Sprintf("terraform-security-policy-4-%d", testutil.RandomInt())
 	testData["rule_name"] = fmt.Sprintf("test-security-rule-4-%d", testutil.RandomInt())
+	testData["applicable_cves_only"] = "false"
 
 	resource.Test(t, resource.TestCase{
 		CheckDestroy:             acctest.VerifyDeleted(fqrn, "", acctest.CheckPolicy),
@@ -389,6 +445,7 @@ func TestAccSecurityPolicy_createBlockDownloadFalseCVSS(t *testing.T) {
 	testData["resource_name"] = resourceName
 	testData["policy_name"] = fmt.Sprintf("terraform-security-policy-5-%d", testutil.RandomInt())
 	testData["rule_name"] = fmt.Sprintf("test-security-rule-5-%d", testutil.RandomInt())
+	testData["applicable_cves_only"] = "false"
 	testData["block_unscanned"] = "false"
 	testData["block_active"] = "false"
 
@@ -595,6 +652,7 @@ func TestAccSecurityPolicy_createCVSSFloat(t *testing.T) {
 	testData["resource_name"] = resourceName
 	testData["policy_name"] = fmt.Sprintf("terraform-security-policy-8-%d", testutil.RandomInt())
 	testData["rule_name"] = fmt.Sprintf("test-security-rule-8-%d", testutil.RandomInt())
+	testData["applicable_cves_only"] = "false"
 	testData["cvss_from"] = "1.5"
 	testData["cvss_to"] = "5.3"
 
@@ -1129,7 +1187,6 @@ const securityPolicyCVSS = `resource "xray_security_policy" "{{ .resource_name }
 				from = {{ .cvss_from }}
 				to = {{ .cvss_to }}
 			}
-			applicable_cves_only = {{ .applicable_cves_only }}
 		}
 		actions {
 			block_release_bundle_distribution = {{ .block_release_bundle_distribution }}