Merge pull request #171 from jfrog/GH-168-fix-ldap-group-settings-con…

…version-error Fix 'Value Conversion Error' for platform_saml_settings
jfrog · Dec 3, 2024 · 4a023cb · 4a023cb
2 parents 78ebe0e + c639021
commit 4a023cb
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,11 +1,15 @@
-## 1.19.0 (December 2, 2024). Tested on Artifactory 7.98.9 with Terraform 1.10.0 and OpenTofu 1.8.6
+## 1.19.0 (December 3, 2024). Tested on Artifactory 7.98.9 with Terraform 1.10.0 and OpenTofu 1.8.6
 
 FEATURES:
 
 **New Resource:**
 
 * `platform_crowd_settings` - Resource to manage Crowd/JIRA authentication provider. PR: [#167](https://github.com/jfrog/terraform-provider-platform/pull/167)
 
+BUG FIXES:
+
+* resource/platform_saml_settings: Fix `Value Conversion Error` for attribute `ldap_group_settings`. Issue: [#168](https://github.com/jfrog/terraform-provider-platform/issues/168) PR: [#171](https://github.com/jfrog/terraform-provider-platform/pull/171)
+
 ## 1.18.2 (November 27, 2024). Tested on Artifactory 7.98.9 with Terraform 1.9.8 and OpenTofu 1.8.6
 
 BUG FIXES:

diff --git a/pkg/platform/resource_crowd_settings.go b/pkg/platform/resource_crowd_settings.go
@@ -45,7 +45,7 @@ type CrowdSettingsResourceModel struct {
 	OverrideAllGroupsUponLogin types.Bool   `tfsdk:"override_all_groups_upon_login"`
 }
 
-func (r *CrowdSettingsResourceModel) toAPIModel(ctx context.Context, apiModel *CrowdSettingsAPIModel) diag.Diagnostics {
+func (r *CrowdSettingsResourceModel) toAPIModel(_ context.Context, apiModel *CrowdSettingsAPIModel) diag.Diagnostics {
 	diags := diag.Diagnostics{}
 
 	apiModel.Enable = r.Enable.ValueBool()
@@ -62,7 +62,7 @@ func (r *CrowdSettingsResourceModel) toAPIModel(ctx context.Context, apiModel *C
 	return diags
 }
 
-func (r *CrowdSettingsResourceModel) fromAPIModel(ctx context.Context, apiModel *CrowdSettingsAPIModel) (ds diag.Diagnostics) {
+func (r *CrowdSettingsResourceModel) fromAPIModel(_ context.Context, apiModel *CrowdSettingsAPIModel) (ds diag.Diagnostics) {
 	r.Enable = types.BoolValue(apiModel.Enable)
 	r.ServerURL = types.StringValue(apiModel.ServerURL)
 	r.ApplicationName = types.StringValue(apiModel.ApplicationName)

diff --git a/pkg/platform/resource_saml_settings.go b/pkg/platform/resource_saml_settings.go
@@ -86,11 +86,13 @@ func (r *SAMLSettingsResourceModelV1) toAPIModel(ctx context.Context, apiModel *
 	apiModel.VerifyAudienceRestriction = r.VerifyAudienceRestriction.ValueBool()
 	apiModel.UseEncryptedAssertion = r.UseEncryptedAssertion.ValueBool()
 
-	var ldapGroupSettings []string
-	d := r.LDAPGroupSettings.ElementsAs(ctx, &ldapGroupSettings, false)
-	if d.HasError() {
-		diags.Append(d...)
-		return diags
+	ldapGroupSettings := []string{} // API treats absent or null value as noop so needs empty array to reset
+	if !r.LDAPGroupSettings.IsNull() {
+		d := r.LDAPGroupSettings.ElementsAs(ctx, &ldapGroupSettings, false)
+		if d.HasError() {
+			diags.Append(d...)
+			return diags
+		}
 	}
 
 	apiModel.LDAPGroupSettings = ldapGroupSettings
@@ -159,7 +161,7 @@ type SAMLSettingsAPIModel struct {
 	GroupAttribute            string   `json:"group_attribute"`
 	EmailAttribute            string   `json:"email_attribute"`
 	NameIDAttribute           string   `json:"name_id_attribute"`
-	LDAPGroupSettings         []string `json:"ldap_group_settings,omitempty"`
+	LDAPGroupSettings         []string `json:"ldap_group_settings"`
 }
 
 var samlSettingsSchemaV0 = map[string]schema.Attribute{

diff --git a/pkg/platform/resource_saml_settings_test.go b/pkg/platform/resource_saml_settings_test.go
@@ -59,6 +59,27 @@ func TestAccSAMLSettings_full(t *testing.T) {
 
 	updatedConfig := util.ExecuteTemplate(name, temp, updatedTestData)
 
+	temp2 := `
+	resource "platform_saml_settings" "{{ .name }}" {
+		name                         = "{{ .name }}"
+		enable                       = true
+		certificate                  = "{{ .certificate }}"
+		email_attribute              = "{{ .email_attribute }}"
+		group_attribute              = "{{ .group_attribute }}"
+		name_id_attribute            = "{{ .name_id_attribute }}"
+		login_url                    = "http://tempurl.org/login"
+		logout_url                   = "http://tempurl.org/logout"
+		auto_user_creation           = {{ .auto_user_creation }}
+		service_provider_name        = "okta"
+		allow_user_to_access_profile = true
+		auto_redirect                = true
+		sync_groups                  = true
+		verify_audience_restriction  = true
+		use_encrypted_assertion      = false
+	}`
+
+	updatedConfig2 := util.ExecuteTemplate(name, temp2, updatedTestData)
+
 	resource.Test(t, resource.TestCase{
 		PreCheck:                 func() { testAccPreCheck(t) },
 		ProtoV6ProviderFactories: testAccProviders(),
@@ -109,6 +130,27 @@ func TestAccSAMLSettings_full(t *testing.T) {
 					resource.TestCheckResourceAttr(fqrn, "ldap_group_settings.1", "test-group-2"),
 				),
 			},
+			{
+				Config: updatedConfig2,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(fqrn, "name", updatedTestData["name"]),
+					resource.TestCheckResourceAttr(fqrn, "enable", "true"),
+					resource.TestCheckResourceAttr(fqrn, "certificate", updatedTestData["certificate"]),
+					resource.TestCheckResourceAttr(fqrn, "email_attribute", updatedTestData["email_attribute"]),
+					resource.TestCheckResourceAttr(fqrn, "group_attribute", updatedTestData["group_attribute"]),
+					resource.TestCheckResourceAttr(fqrn, "name_id_attribute", updatedTestData["name_id_attribute"]),
+					resource.TestCheckResourceAttr(fqrn, "login_url", "http://tempurl.org/login"),
+					resource.TestCheckResourceAttr(fqrn, "logout_url", "http://tempurl.org/logout"),
+					resource.TestCheckResourceAttr(fqrn, "auto_user_creation", updatedTestData["auto_user_creation"]),
+					resource.TestCheckResourceAttr(fqrn, "service_provider_name", "okta"),
+					resource.TestCheckResourceAttr(fqrn, "allow_user_to_access_profile", "true"),
+					resource.TestCheckResourceAttr(fqrn, "auto_redirect", "true"),
+					resource.TestCheckResourceAttr(fqrn, "sync_groups", "true"),
+					resource.TestCheckResourceAttr(fqrn, "verify_audience_restriction", "true"),
+					resource.TestCheckResourceAttr(fqrn, "use_encrypted_assertion", "false"),
+					resource.TestCheckNoResourceAttr(fqrn, "ldap_group_settings"),
+				),
+			},
 			{
 				ResourceName:                         fqrn,
 				ImportState:                          true,