Various fixes in SAML group link resource

docs/resources/group_saml_link.md

@@ -0,0 +1,46 @@
+---
+# generated by https://github.com/hashicorp/terraform-plugin-docs
+page_title: "gitlab_group_saml_link Resource - terraform-provider-gitlab"
+subcategory: ""
+description: |-
+  The gitlab_group_saml_link resource allows to manage the lifecycle of an SAML integration with a group.
+  Upstream API: GitLab REST API docs https://docs.gitlab.com/ee/api/groups.html#saml-group-links
+---
+
+# gitlab_group_saml_link (Resource)
+
+The `gitlab_group_saml_link` resource allows to manage the lifecycle of an SAML integration with a group.
+
+**Upstream API**: [GitLab REST API docs](https://docs.gitlab.com/ee/api/groups.html#saml-group-links)
+
+## Example Usage
+
+```terraform
+resource "gitlab_group_saml_link" "test" {
+  group_id        = "12345"
+  access_level    = "Developer"
+  saml_group_name = "samlgroupname1"
+}
+```
+
+<!-- schema generated by tfplugindocs -->
+## Schema
+
+### Required
+
+- `access_level` (String) Access level for members of the SAML group. Valid values are: `Guest`, `Reporter`, `Developer`, `Maintainer`, `Owner`.
+- `group` (String) The ID or path of the group to add the SAML Group Link to.
+- `saml_group_name` (String) The name of the SAML group.
+
+### Read-Only
+
+- `id` (String) The ID of this resource.
+
+## Import
+
+Import is supported using the following syntax:
+
+```shell
+# GitLab group saml links can be imported using an id made up of `group_id:saml_group_name`, e.g.
+terraform import gitlab_group_saml_link.test "12345:samlgroupname1"
+```

internal/provider/resource_gitlab_group_saml_link.go

@@ -11,15 +11,15 @@ import (
 	gitlab "github.com/xanzy/go-gitlab"
 )
 
-var validGroupSamlLinkAccessLevelNames = []string{
-	"Guest",
-	"Reporter",
-	"Developer",
-	"Maintainer",
-	"Owner",
-}
-
 var _ = registerResource("gitlab_group_saml_link", func() *schema.Resource {
+	validGroupSamlLinkAccessLevelNames := []string{
+		"Guest",
+		"Reporter",
+		"Developer",
+		"Maintainer",
+		"Owner",
+	}
+
 	return &schema.Resource{
 		Description: `The ` + "`gitlab_group_saml_link`" + ` resource allows to manage the lifecycle of an SAML integration with a group.
 
@@ -39,19 +39,19 @@ var _ = registerResource("gitlab_group_saml_link", func() *schema.Resource {
 				Required:    true,
 				ForceNew:    true,
 			},
-			"access_level": {
-				Description:      fmt.Sprintf("Minimum access level for members of the SAML group. Valid values are: %s", renderValueListForDocs(validGroupSamlLinkAccessLevelNames)),
-				Type:             schema.TypeString,
-				ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice(validGroupSamlLinkAccessLevelNames, false)),
-				Required:         true,
-				ForceNew:         true,
-			},
 			"saml_group_name": {
 				Description: "The name of the SAML group.",
 				Type:        schema.TypeString,
 				Required:    true,
 				ForceNew:    true,
 			},
+			"access_level": {
+				Description:      fmt.Sprintf("Access level for members of the SAML group. Valid values are: %s.", renderValueListForDocs(validGroupSamlLinkAccessLevelNames)),
+				Type:             schema.TypeString,
+				ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice(validGroupSamlLinkAccessLevelNames, false)),
+				Required:         true,
+				ForceNew:         true,
+			},
 		},
 	}
 })
@@ -60,22 +60,21 @@ func resourceGitlabGroupSamlLinkCreate(ctx context.Context, d *schema.ResourceDa
 	client := meta.(*gitlab.Client)
 
 	group := d.Get("group").(string)
-	accessLevel := d.Get("access_level").(string)
 	samlGroupName := d.Get("saml_group_name").(string)
+	accessLevel := accessLevelNameToValue[d.Get("access_level").(string)]
 
 	options := &gitlab.AddGroupSAMLLinkOptions{
-		AccessLevel:   gitlab.String(accessLevel),
-		SamlGroupName: gitlab.String(samlGroupName),
+		SAMLGroupName: gitlab.String(samlGroupName),
+		AccessLevel:   gitlab.AccessLevel(accessLevel),
 	}
 
-	log.Printf("[DEBUG] Create GitLab group SamlLink %s", d.Id())
+	log.Printf("[DEBUG] Create GitLab Group SAML Link for group %q with name %q", group, samlGroupName)
 	SamlLink, _, err := client.Groups.AddGroupSAMLLink(group, options, gitlab.WithContext(ctx))
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
 	d.SetId(buildTwoPartID(&group, &SamlLink.Name))
-
 	return resourceGitlabGroupSamlLinkRead(ctx, d, meta)
 }
 
@@ -87,21 +86,21 @@ func resourceGitlabGroupSamlLinkRead(ctx context.Context, d *schema.ResourceData
 	}
 
 	// Try to fetch all group links from GitLab
-	log.Printf("[DEBUG] Read GitLab group SamlLinks %s", group)
+	log.Printf("[DEBUG] Read GitLab Group SAML Link for group %q", group)
 	samlLink, _, err := client.Groups.GetGroupSAMLLink(group, samlGroupName, nil, gitlab.WithContext(ctx))
 	if err != nil {
 		if is404(err) {
-			log.Printf("[DEBUG] GitLab SAML Group Link %d, group ID %s not found, removing from state", samlGroupName, group)
+			log.Printf("[DEBUG] GitLab SAML Group Link %s for group ID %s not found, removing from state", samlGroupName, group)
 			d.SetId("")
 			return nil
 		}
 		return diag.FromErr(err)
 	}
 
 	d.Set("group", group)
-	d.Set("access_level", samlLink.AccessLevel)
+	d.Set("access_level", accessLevelValueToName[samlLink.AccessLevel])
 	d.Set("saml_group_name", samlLink.Name)
-	
+
 	return nil
 }
 
@@ -112,7 +111,7 @@ func resourceGitlabGroupSamlLinkDelete(ctx context.Context, d *schema.ResourceDa
 		return diag.FromErr(parse_err)
 	}
 
-	log.Printf("[DEBUG] Delete GitLab group SamlLink %s", d.Id())
+	log.Printf("[DEBUG] Delete GitLab Group SAML Link for group %q with name %q", group, samlGroupName)
 	_, err := client.Groups.DeleteGroupSAMLLink(group, samlGroupName, gitlab.WithContext(ctx))
 	if err != nil {
 		if is404(err) {

internal/provider/resource_gitlab_group_saml_link_test.go

@@ -4,122 +4,70 @@
 package provider
 
 import (
-	"errors"
 	"fmt"
 	"testing"
 
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
-	"github.com/xanzy/go-gitlab"
 )
 
 func TestAccGitlabGroupSamlLink_basic(t *testing.T) {
-	rInt := acctest.RandInt()
-	resourceName := "gitlab_group_saml_link.foo"
+	testAccCheckEE(t)
 
-	// PreCheck runs after Config so load test data here
-	var samlLink gitlab.SAMLGroupLink
-	testSamlLink := gitlab.SAMLGroupLink{
-		Name: "test_saml_group",
-	}
+	testGroup := testAccCreateGroups(t, 1)[0]
 
 	resource.ParallelTest(t, resource.TestCase{
 		ProviderFactories: providerFactories,
 		CheckDestroy:      testAccCheckGitlabGroupSamlLinkDestroy,
 		Steps: []resource.TestStep{
 
-			// Create a group SAML link as a developer (uses testAccGitlabGroupLdapSamlCreateConfig for Config)
+			// Create a group SAML link as a developer
 			{
-				SkipFunc: isRunningInCE,
 				Config: fmt.Sprintf(`
-				resource "gitlab_group_saml_link" "foo" {
-					group_id 		= "%d"
-					access_level 	= "Developer"
-					saml_group_name = "%s"
-				
-				}`, rInt, rInt, testSamlLink.Name),
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckGitlabGroupSamlLinkExists(resourceName, &samlLink)),
-			},
+					resource "gitlab_group_saml_link" "this" {
+						group   		= "%d"
+						access_level 	= "Developer"
+						saml_group_name = "test_saml_group"
 
-			// Import the group SAML link (re-uses testAccGitlabGroupSamlLinkCreateConfig for Config)
+					}
+				`, testGroup.ID),
+			},
+			// Verify Import
 			{
-				SkipFunc:          isRunningInCE,
-				ResourceName:      resourceName,
-				ImportStateIdFunc: getGitlabGroupSamlLinkImportID(resourceName),
+				ResourceName:      "gitlab_group_saml_link.this",
 				ImportState:       true,
 				ImportStateVerify: true,
 			},
-
-			// Update the group SAML link to change the access level (uses testAccGitlabGroupSamlLinkUpdateConfig for Config)
+			// Update the group SAML link to change the access level
 			{
-				SkipFunc: isRunningInCE,
 				Config: fmt.Sprintf(`
-				resource "gitlab_group_saml_link" "foo" {
-					group_id 		= "%d"
-					access_level 	= "Maintainer"
-					saml_group_name = "%s"
-				}`, rInt, rInt, testSamlLink.Name),
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckGitlabGroupSamlLinkExists(resourceName, &samlLink)),
+					resource "gitlab_group_saml_link" "this" {
+						group   		= "%d"
+						access_level 	= "Maintainer"
+						saml_group_name = "test_saml_group"
+
+					}
+				`, testGroup.ID),
 			},
 		},
 	})
 }
 
-func getGitlabGroupSamlLinkImportID(resourceName string) resource.ImportStateIdFunc {
-	return func(s *terraform.State) (string, error) {
-		rs, ok := s.RootModule().Resources[resourceName]
-		if !ok {
-			return "", fmt.Errorf("Not Found: %s", resourceName)
-		}
-
-		groupID := rs.Primary.Attributes["group_id"]
-		if groupID == "" {
-			return "", fmt.Errorf("No group ID is set")
-		}
-		samlGroupName := rs.Primary.Attributes["saml_group_name"]
-		if samlGroupName == "" {
-			return "", fmt.Errorf("No SAML group name is set")
-		}
-
-		return fmt.Sprintf("%s:%s", groupID, samlGroupName), nil
-	}
-}
-
-func testAccCheckGitlabGroupSamlLinkExists(resourceName string, samlLink *gitlab.SAMLGroupLink) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		// Clear the "found" SAML link before checking for existence
-		*samlLink = gitlab.SAMLGroupLink{}
-
-		resourceState, ok := s.RootModule().Resources[resourceName]
-		if !ok {
-			return fmt.Errorf("Not found: %s", resourceName)
+func testAccCheckGitlabGroupSamlLinkDestroy(s *terraform.State) error {
+	for _, resourceState := range s.RootModule().Resources {
+		if resourceState.Type != "gitlab_group_saml_link" {
+			continue
 		}
 
-		err := testAccGetGitlabGroupSamlLink(samlLink, resourceState)
+		group, samlGroupName, err := parseTwoPartID(resourceState.Primary.ID)
 		if err != nil {
 			return err
 		}
 
-		return nil
-	}
-}
-
-func testAccCheckGitlabGroupSamlLinkDestroy(s *terraform.State) error {
-	// Can't check for links if the group is destroyed so make sure all groups are destroyed instead
-	for _, resourceState := range s.RootModule().Resources {
-		if resourceState.Type != "gitlab_group" {
-			continue
-		}
-
-		group, _, err := testGitlabClient.Groups.GetGroup(resourceState.Primary.ID, nil)
+		samlGroupLink, _, err := testGitlabClient.Groups.GetGroupSAMLLink(group, samlGroupName)
 		if err == nil {
-			if group != nil && fmt.Sprintf("%d", group.ID) == resourceState.Primary.ID {
-				if group.MarkedForDeletionOn == nil {
-					return fmt.Errorf("Group still exists")
-				}
+			if samlGroupLink != nil {
+				return fmt.Errorf("SAML Group Link still exists")
 			}
 		}
 		if !is404(err) {
@@ -129,41 +77,3 @@ func testAccCheckGitlabGroupSamlLinkDestroy(s *terraform.State) error {
 	}
 	return nil
 }
-
-func testAccGetGitlabGroupSamlLink(samlLink *gitlab.SAMLGroupLink, resourceState *terraform.ResourceState) error {
-	groupId := resourceState.Primary.Attributes["group_id"]
-	if groupId == "" {
-		return fmt.Errorf("No group ID is set")
-	}
-
-	// Construct our desired SAML Link from the config values
-	desiredSamlLink := gitlab.SAMLGroupLink{
-		AccessLevel: resourceState.Primary.Attributes["access_level"],
-		Name:        resourceState.Primary.Attributes["saml_group_name"],
-	}
-
-	desiredSamlLinkId := buildTwoPartID(&groupId, &desiredSamlLink.Name)
-
-	// Try to fetch all group links from GitLab
-	currentSamlLinks, _, err := testGitlabClient.Groups.ListGroupSamlLinks(groupId, nil)
-	if err != nil {
-		return err
-	}
-
-	found := false
-
-	// Check if the SAML link exists in the returned list of links
-	for _, currentSamlLink := range currentSamlLinks {
-		if buildTwoPartID(&groupId, &currentSamlLink.Name) == desiredSamlLinkId {
-			found = true
-			*samlLink = *currentSamlLink
-			break
-		}
-	}
-
-	if !found {
-		return errors.New(fmt.Sprintf("SamlLink %s does not exist.", desiredSamlLinkId)) // nolint // TODO: Resolve this golangci-lint issue: S1028: should use fmt.Errorf(...) instead of errors.New(fmt.Sprintf(...)) (gosimple)
-	}
-
-	return nil
-}