Merge pull request #1799 from okta/1797_okta_group_match_multiple_flaw

Release v4.6.1

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## 4.6.1 (November 2, 2023)
+
+### BUG FIXES
+
+*  Correct flaw in data source `okta_group` where name query matches multiple groups but did not consider exact match [#1799](https://github.com/okta/terraform-provider-okta/pull/1799). Thanks, [@monde](https://github.com/monde)!
+*  For resource `okta_idp_saml` set `status`, `sso_binding`, `sso_destination`, and `sso_url` during read context for proper import [#1558](https://github.com/okta/terraform-provider-okta/pull/1558). Thanks, [@monde](https://github.com/monde)!
+
 ## 4.6.0 (November 1, 2023)
 
 ### IMPROVEMENTS
@@ -16,7 +23,6 @@
 * Fix a panic in resource okta_resource_set [#1786](https://github.com/okta/terraform-provider-okta/pull/1786). Thanks, [@monde](https://github.com/monde)!
 * Correct change detection on resources okta_app_oauth_post_logout_redirect_uri and okta_app_oauth_redirect_uri [#1793](https://github.com/okta/terraform-provider-okta/pull/1793). Thanks, [@monde](https://github.com/monde)!
 
-
 ## 4.5.0 (October 17, 2023)
 
 ### NEW - RESOURCES, DATA SOURCES, PROPERTIES, ATTRIBUTES, ENV VARS:
@@ -34,7 +40,6 @@
 ### IMPROVEMENTS
 * Add track all users argument to okta_group_memberships import [#1766](https://github.com/okta/terraform-provider-okta/pull/1766). Thanks, [@arvindkrishnakumar-okta](https://github.com/arvindkrishnakumar-okta)!
 
-
 ## 4.4.3 (October 09, 2023)
 
 ### BUG FIXES

okta/config.go

@@ -25,7 +25,7 @@ import (
 )
 
 const (
-	OktaTerraformProviderVersion   = "4.6.0"
+	OktaTerraformProviderVersion   = "4.6.1"
 	OktaTerraformProviderUserAgent = "okta-terraform/" + OktaTerraformProviderVersion
 )
 

okta/data_source_okta_group.go

@@ -92,31 +92,55 @@ func findGroup(ctx context.Context, name string, d *schema.ResourceData, m inter
 		// NOTE: Uncertain of the OKTA /api/v1/groups API drifted during Classic
 		// (when this data source was originally created) to OIE migration.
 		// Currently, Okta API enforces unique names on all groups regardless of
-		// type so type is essentially a meaningless parameter in OIE.
+		// type so type is essentially a meaningless parameter in OIE.  There
+		// may be a case where imported groups allow duplicate names.
 		t, okType := d.GetOk("type")
 		if okType {
 			searchParams.Filter = fmt.Sprintf("type eq \"%s\"", t.(string))
 		}
 
 		logger(m).Info("looking for data source group", "query", searchParams.String())
 		groups, _, err := getOktaClientFromMetadata(m).Group.ListGroups(ctx, searchParams)
-		switch {
-		case err != nil:
+		if err != nil {
 			return diag.Errorf("failed to query for groups: %v", err)
-		case len(groups) > 1:
-			if okType {
-				return diag.Errorf("group starting with name %q and type %q matches %d groups, select a more precise name parameter", name, d.Get("type").(string), len(groups))
+		}
+		if len(groups) > 1 {
+			logger(m).Warn("data source group query matches", len(groups), "groups")
+			for _, g := range groups {
+				// exact match on name
+				if g.Profile.Name == name {
+					if okType && t.(string) == g.Type {
+						// data source has type argument so take that into consideration also
+						group = g
+						break
+					}
+					if !okType {
+						// otherwise consider name only
+						group = g
+						break
+					}
+				}
+			}
+			if group == nil {
+				if okType {
+					return diag.Errorf("group starting with name %q and type %q matches %d groups, select a more precise name parameter", name, d.Get("type").(string), len(groups))
+				}
+				return diag.Errorf("group starting with name %q matches %d groups, select a more precise name parameter", name, len(groups))
 			}
-			return diag.Errorf("group starting with name %q matches %d groups, select a more precise name parameter", name, len(groups))
-		case len(groups) < 1:
+		}
+		if len(groups) < 1 {
 			if okType {
 				return diag.Errorf("group with name %q and type %q does not exist", name, d.Get("type").(string))
 			}
 			return diag.Errorf("group with name %q does not exist", name)
-		case groups[0].Profile.Name != name:
-			logger(m).Warn("group with exact name match was not found: using partial match which contains name as a substring", "name", groups[0].Profile.Name)
 		}
-		group = groups[0]
+		if len(groups) == 1 {
+			group = groups[0]
+			if group.Profile.Name != name {
+				// keep old behavior that a fuzzy match is acceptable if query only returns one group
+				logger(m).Warn("group with exact name match was not found: using partial match which contains name as a substring", "name", group.Profile.Name)
+			}
+		}
 	}
 	d.SetId(group.Id)
 	_ = d.Set("description", group.Profile.Description)

okta/data_source_okta_group_test.go

@@ -1,6 +1,7 @@
 package okta
 
 import (
+	"fmt"
 	"regexp"
 	"testing"
 
@@ -41,32 +42,51 @@ func TestAccDataSourceOktaGroup_read(t *testing.T) {
 // source surfaces the correct error when more than one group matches the name
 // argument.
 func TestAccDataSourceOktaGroup_read_multiple_groups(t *testing.T) {
+	resourceName := fmt.Sprintf("data.%s.test", group)
 	mgr := newFixtureManager("data-sources", group, t.Name())
-	config := `
+
+	baseConfig := `
 resource "okta_group" "test_1" {
-  name        = "testAcc_1_replace_with_uuid"
-  description = "testing, testing"
+  name        = "testAcc_replace_with_uuid_MORE"
+  description = "testing, more"
 }
 resource "okta_group" "test_2" {
-  name        = "testAcc_2_replace_with_uuid"
+  name        = "testAcc_replace_with_uuid"
   description = "testing, testing"
-}
+}`
+
+	step1Config := `
+# error, name is substring and matches both groups
 data "okta_group" "test" {
   name          = "testAcc"
   depends_on = [okta_group.test_1, okta_group.test_2]
 }`
+
+	step2Config := `
+# ok, name is substring and matches both groups, but is exact match for one group
+data "okta_group" "test" {
+  name        = "testAcc_replace_with_uuid"
+  depends_on = [okta_group.test_1, okta_group.test_2]
+}`
 	oktaResourceTest(t, resource.TestCase{
 		PreCheck:                 testAccPreCheck(t),
 		ErrorCheck:               testAccErrorChecks(t),
 		ProtoV5ProviderFactories: testAccMergeProvidersFactories,
 		Steps: []resource.TestStep{
 			{
-				Config: mgr.ConfigReplace(config),
+				Config: mgr.ConfigReplace(fmt.Sprintf("%s\n%s", step1Config, baseConfig)),
 				// NOTE: there might be dangling test groups on the org starting
 				// with "testAcc" so just make sure the error message is correct
 				// besides the count of groups
 				ExpectError: regexp.MustCompile(`group starting with name "testAcc" matches (\d+) groups, select a more precise name parameter`),
 			},
+			{
+				Config: mgr.ConfigReplace(fmt.Sprintf("%s\n%s", step2Config, baseConfig)),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("testAcc_%d", mgr.Seed)),
+					resource.TestCheckResourceAttr(resourceName, "description", "testing, testing"),
+				),
+			},
 		},
 	})
 }