Merge pull request #952 from PatrickRice-KSC/update_managed_license_values

Update managed_license resources to deprecate existing values and add GitLab 15+ values

Author: timofurrer

docs/resources/managed_license.md

@@ -28,7 +28,7 @@ resource "gitlab_project" "foo" {
 resource "gitlab_managed_license" "mit" {
   project         = gitlab_project.foo.id
   name            = "MIT license"
-  approval_status = "approved"
+  approval_status = "allowed"
 }
 ```
 
@@ -37,7 +37,9 @@ resource "gitlab_managed_license" "mit" {
 
 ### Required
 
-- **approval_status** (String) Whether the license is approved or not. Only 'approved' or 'blacklisted' allowed.
+- **approval_status** (String) The approval status of the license. Valid values are: `approved`, `blacklisted`, `allowed`, `denied`. "approved" and "blacklisted" 
+				have been deprecated in favor of "allowed" and "denied"; use "allowed" and "denied" for GitLab versions 15.0 and higher. 
+				Prior to version 15.0 and after 14.6, the values are equivalent.
 - **name** (String) The name of the managed license (I.e., 'Apache License 2.0' or 'MIT license')
 - **project** (String) The ID of the project under which the managed license will be created.
 

examples/resources/gitlab_managed_license/resource.tf

@@ -7,5 +7,5 @@ resource "gitlab_project" "foo" {
 resource "gitlab_managed_license" "mit" {
   project         = gitlab_project.foo.id
   name            = "MIT license"
-  approval_status = "approved"
+  approval_status = "allowed"
 }

internal/provider/helper_test.go

@@ -15,6 +15,8 @@ import (
 	"github.com/xanzy/go-gitlab"
 )
 
+type SkipFunc = func() (bool, error)
+
 func init() {
 	// We are using the gomega package for its matchers only, but it requires us to register a handler anyway.
 	gomega.RegisterFailHandler(func(_ string, _ ...int) {
@@ -71,6 +73,22 @@ func testAccCheck(t *testing.T) {
 	}
 }
 
+// orSkipFunc accepts many skipFunc and returns "true" if any returns true.
+func orSkipFunc(input ...SkipFunc) SkipFunc {
+	return func() (bool, error) {
+		for _, item := range input {
+			result, err := item()
+			if err != nil {
+				return false, err
+			}
+			if result {
+				return result, nil
+			}
+		}
+		return false, nil
+	}
+}
+
 // testAccCheckEE is a test helper that skips the current test if the GitLab version is not GitLab Enterprise.
 // This is useful when the version needs to be checked during setup, before the Terraform acceptance test starts.
 func testAccCheckEE(t *testing.T) {

internal/provider/resource_gitlab_managed_license.go

@@ -12,6 +12,10 @@ import (
 	"github.com/xanzy/go-gitlab"
 )
 
+var managedLicenseAllowedValues = []string{
+	"approved", "blacklisted", "allowed", "denied",
+}
+
 var _ = registerResource("gitlab_managed_license", func() *schema.Resource {
 	return &schema.Resource{
 		Description: `The ` + "`" + `gitlab_managed_license` + "`" + ` resource allows to manage the lifecycle of a managed license.
@@ -44,11 +48,14 @@ var _ = registerResource("gitlab_managed_license", func() *schema.Resource {
 				Description: "The name of the managed license (I.e., 'Apache License 2.0' or 'MIT license')",
 			},
 			"approval_status": {
-				Type:         schema.TypeString,
-				Required:     true,
-				ForceNew:     false,
-				ValidateFunc: validation.StringInSlice([]string{"approved", "blacklisted"}, true),
-				Description:  "Whether the license is approved or not. Only 'approved' or 'blacklisted' allowed.",
+				Type:             schema.TypeString,
+				Required:         true,
+				ForceNew:         false,
+				ValidateFunc:     validation.StringInSlice(managedLicenseAllowedValues, true),
+				DiffSuppressFunc: checkDeprecatedValuesForDiff,
+				Description: fmt.Sprintf(`The approval status of the license. Valid values are: %s. "approved" and "blacklisted" 
+				have been deprecated in favor of "allowed" and "denied"; use "allowed" and "denied" for GitLab versions 15.0 and higher. 
+				Prior to version 15.0 and after 14.6, the values are equivalent.`, renderValueListForDocs(managedLicenseAllowedValues)),
 			},
 		},
 	}
@@ -149,6 +156,11 @@ func stringToApprovalStatus(s string) *gitlab.LicenseApprovalStatusValue {
 	lookup := map[string]gitlab.LicenseApprovalStatusValue{
 		"approved":    gitlab.LicenseApproved,
 		"blacklisted": gitlab.LicenseBlacklisted,
+
+		// This is counter-intuitive, but currently the API response from the non-deprecated
+		// values is the deprecated values. So we have to map them here.
+		"allowed": gitlab.LicenseApproved,
+		"denied":  gitlab.LicenseBlacklisted,
 	}
 
 	value, ok := lookup[s]
@@ -171,3 +183,20 @@ func projectIdAndLicenseIdFromId(id string) (string, int, error) {
 
 	return projectId, licenseId, nil
 }
+
+func checkDeprecatedValuesForDiff(k, oldValue, newValue string, d *schema.ResourceData) bool {
+	approvedValues := []string{"approved", "allowed"}
+	deniedValues := []string{"blacklisted", "denied"}
+
+	// While we could technically combine these two "if" blocks, this seems more readable
+	// and should have the same execution pattern.
+	if contains(approvedValues, oldValue) && contains(approvedValues, newValue) {
+		return true
+	}
+
+	if contains(deniedValues, oldValue) && contains(deniedValues, newValue) {
+		return true
+	}
+
+	return false
+}

internal/provider/resource_gitlab_managed_license_test.go

@@ -21,19 +21,20 @@ func TestAccGitlabManagedLicense_basic(t *testing.T) {
 		CheckDestroy:      testAccCheckManagedLicenseDestroy,
 		Steps: []resource.TestStep{
 			{
-				// Create a managed license with an "approved" state
+				// Create a managed license with an "allowed" state
 				SkipFunc: isRunningInCE,
-				Config:   testManagedLicenseConfig(rInt, "approved"),
+				Config:   testManagedLicenseConfig(rInt, "allowed"),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckGitlabManagedLicenseExists("gitlab_managed_license.fixme", &managedLicense),
 				),
 			},
 			{
-				// Update the managed license to have a blacklisted state
+				// Update the managed license to have a denied state
 				SkipFunc: isRunningInCE,
-				Config:   testManagedLicenseConfig(rInt, "blacklisted"),
+				Config:   testManagedLicenseConfig(rInt, "denied"),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckGitlabManagedLicenseStatus("gitlab_managed_license.fixme", &managedLicense),
+					//Even though the API accepts "denied", it still returns "blacklisted" until 15.0
+					testAccCheckGitlabManagedLicenseStatus("gitlab_managed_license.fixme", "blacklisted", &managedLicense),
 				),
 			},
 			{
@@ -46,14 +47,79 @@ func TestAccGitlabManagedLicense_basic(t *testing.T) {
 	})
 }
 
-func testAccCheckGitlabManagedLicenseStatus(n string, license *gitlab.ManagedLicense) resource.TestCheckFunc {
+func TestAccGitlabManagedLicense_deprecatedConfigValues(t *testing.T) {
+	var managedLicense gitlab.ManagedLicense
+	rInt := acctest.RandInt()
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:          func() {},
+		ProviderFactories: providerFactories,
+		CheckDestroy:      testAccCheckManagedLicenseDestroy,
+		Steps: []resource.TestStep{
+			{
+				// Create a managed license with an "approved" state
+				SkipFunc: orSkipFunc(
+					isRunningInCE,
+					isGitLabVersionLessThan(testGitlabClient, "15.0"),
+				),
+				Config: testManagedLicenseConfig(rInt, "approved"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGitlabManagedLicenseExists("gitlab_managed_license.fixme", &managedLicense),
+				),
+			},
+			{
+				// Update the managed license to have a blacklisted state
+				SkipFunc: orSkipFunc(
+					isRunningInCE,
+					isGitLabVersionLessThan(testGitlabClient, "15.0"),
+				),
+				Config: testManagedLicenseConfig(rInt, "blacklisted"),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckGitlabManagedLicenseStatus("gitlab_managed_license.fixme", "blacklisted", &managedLicense),
+				),
+			},
+			{
+				SkipFunc: orSkipFunc(
+					isRunningInCE,
+					isGitLabVersionLessThan(testGitlabClient, "15.0"),
+				),
+				ResourceName:      "gitlab_managed_license.fixme",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestCheckManagedLicenseStatusDiffFunc(t *testing.T) {
+	if !checkDeprecatedValuesForDiff("", "approved", "allowed", nil) {
+		t.Log("approved and allowed should be suppressed")
+		t.Fail()
+	}
+
+	if !checkDeprecatedValuesForDiff("", "denied", "blacklisted", nil) {
+		t.Log("denied and blacklisted should be suppressed")
+		t.Fail()
+	}
+
+	if checkDeprecatedValuesForDiff("", "denied", "approved", nil) {
+		t.Log("denied and approved should not be suppressed")
+		t.Fail()
+	}
+
+	if checkDeprecatedValuesForDiff("", "allowed", "blacklisted", nil) {
+		t.Log("allowed and blacklisted should not be suppressed")
+		t.Fail()
+	}
+}
+
+func testAccCheckGitlabManagedLicenseStatus(resource string, status string, license *gitlab.ManagedLicense) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
-		rs, ok := s.RootModule().Resources[n]
+		rs, ok := s.RootModule().Resources[resource]
 		if !ok {
-			return fmt.Errorf("not Found: %s", n)
+			return fmt.Errorf("not Found: %s", resource)
 		}
 
-		licenseStatus := rs.Primary.Attributes["approval_status"]
 		project := rs.Primary.Attributes["project"]
 		if project == "" {
 			return fmt.Errorf("no project ID is set")
@@ -65,7 +131,7 @@ func testAccCheckGitlabManagedLicenseStatus(n string, license *gitlab.ManagedLic
 		}
 
 		for _, gotLicense := range licenses {
-			if gotLicense.ApprovalStatus == *stringToApprovalStatus(licenseStatus) {
+			if gotLicense.ApprovalStatus == *stringToApprovalStatus(status) {
 				*license = *gotLicense
 				return nil
 			}