Merge pull request #940 from jfrog/bump-user-access-api-version-check

Bump Artifactory version check for Access user API

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 10.7.1 (Apr 22, 2024)
+
+BUG FIXES:
+
+resource/artifactory_managed_user, resource/artifactory_unmanaged_user, resource/artifactory_user: Toggle between using (old) Artifactory Security API and (new) Access API based on Artifactory version 7.84.3 due to Access API bug in updating user without password field. Issue: [#931](https://github.com/jfrog/terraform-provider-artifactory/issues/931) PR: [#940](https://github.com/jfrog/terraform-provider-artifactory/pull/940)
+
 ## 10.7.0 (Apr 18, 2024)
 
 FEATURES:

pkg/acctest/test.go

@@ -341,15 +341,16 @@ func DeleteUser(t *testing.T, name string) error {
 }
 
 func CreateUserUpdatable(t *testing.T, name string, email string) {
-	userObj := user.User{
+	internalPasswordDisabled := false
+	userObj := user.ArtifactoryUserResourceAPIModel{
 		Name:                     name,
 		Email:                    email,
 		Password:                 "Lizard123!",
 		Admin:                    false,
 		ProfileUpdatable:         true,
 		DisableUIAccess:          false,
-		InternalPasswordDisabled: false,
-		Groups:                   []string{"readers"},
+		InternalPasswordDisabled: &internalPasswordDisabled,
+		Groups:                   &[]string{"readers"},
 	}
 
 	restyClient := GetTestResty(t)

pkg/artifactory/datasource/security/datasource_artifactory_permission_target_test.go

@@ -56,7 +56,10 @@ func createPermissionTarget(targetName string, userName string, t *testing.T) {
 
 	restyClient := acctest.GetTestResty(t)
 	postPermissionTarget := security.PermissionsEndPoint + permissionTarget.Name
-	if _, err := restyClient.R().AddRetryCondition(repository.Retry400).SetBody(permissionTarget).Post(postPermissionTarget); err != nil {
+	if _, err := restyClient.R().
+		AddRetryCondition(repository.Retry400).
+		SetBody(permissionTarget).
+		Post(postPermissionTarget); err != nil {
 		t.Fatal(err)
 	}
 

pkg/artifactory/datasource/user/datasource_artifactory_user.go

@@ -3,6 +3,7 @@ package user
 import (
 	"context"
 
+	"github.com/go-resty/resty/v2"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
@@ -71,9 +72,9 @@ func DataSourceArtifactoryUser() *schema.Resource {
 		d := &utilsdk.ResourceData{ResourceData: rd}
 
 		userName := d.Get("name").(string)
-		var userObj user.User
+		var userObj User
 		var artifactoryError artifactory.ArtifactoryErrorsResponse
-		resp, err := user.ReadUser(
+		resp, err := readUser(
 			m.(util.ProviderMetadata).Client.R(),
 			m.(util.ProviderMetadata).ArtifactoryVersion,
 			userName,
@@ -90,7 +91,7 @@ func DataSourceArtifactoryUser() *schema.Resource {
 
 		d.SetId(userObj.Name)
 
-		return user.PackUser(userObj, rd)
+		return packUser(userObj, rd)
 	}
 
 	return &schema.Resource{
@@ -99,3 +100,74 @@ func DataSourceArtifactoryUser() *schema.Resource {
 		Description: "Provides the Artifactory User data source. ",
 	}
 }
+
+type User struct {
+	Name                     string   `json:"username"`
+	Email                    string   `json:"email"`
+	Password                 string   `json:"password,omitempty"`
+	Admin                    bool     `json:"admin"`
+	ProfileUpdatable         bool     `json:"profile_updatable"`
+	DisableUIAccess          bool     `json:"disable_ui_access"`
+	InternalPasswordDisabled *bool    `json:"internal_password_disabled"`
+	Groups                   []string `json:"groups,omitempty"`
+}
+
+func readUser(req *resty.Request, artifactoryVersion, name string, result *User, artifactoryError *artifactory.ArtifactoryErrorsResponse) (*resty.Response, error) {
+	endpoint := user.GetUserEndpointPath(artifactoryVersion)
+
+	// 7.83.1 or later, use Access API
+	if ok, err := util.CheckVersion(artifactoryVersion, user.AccessAPIArtifactoryVersion); err == nil && ok {
+		return req.
+			SetPathParam("name", name).
+			SetResult(&result).
+			SetError(&artifactoryError).
+			Get(endpoint)
+	}
+
+	// else use old Artifactory API, which has a slightly differect JSON payload!
+	var artifactoryResult user.ArtifactoryUserAPIModel
+	res, err := req.
+		SetPathParam("name", name).
+		SetResult(&artifactoryResult).
+		SetError(artifactoryError).
+		Get(endpoint)
+
+	var groups []string
+	if artifactoryResult.Groups != nil {
+		groups = *artifactoryResult.Groups
+	}
+
+	*result = User{
+		Name:                     artifactoryResult.Name,
+		Email:                    artifactoryResult.Email,
+		Admin:                    artifactoryResult.Admin,
+		ProfileUpdatable:         artifactoryResult.ProfileUpdatable,
+		DisableUIAccess:          artifactoryResult.DisableUIAccess,
+		InternalPasswordDisabled: artifactoryResult.InternalPasswordDisabled,
+		Groups:                   groups,
+	}
+
+	return res, err
+}
+
+func packUser(user User, d *schema.ResourceData) diag.Diagnostics {
+
+	setValue := utilsdk.MkLens(d)
+
+	setValue("name", user.Name)
+	setValue("email", user.Email)
+	setValue("admin", user.Admin)
+	setValue("profile_updatable", user.ProfileUpdatable)
+	setValue("disable_ui_access", user.DisableUIAccess)
+	errors := setValue("internal_password_disabled", user.InternalPasswordDisabled)
+
+	if user.Groups != nil {
+		errors = setValue("groups", schema.NewSet(schema.HashString, utilsdk.CastToInterfaceArr(user.Groups)))
+	}
+
+	if len(errors) > 0 {
+		return diag.Errorf("failed to pack user %q", errors)
+	}
+
+	return nil
+}

pkg/artifactory/provider/framework.go

@@ -194,9 +194,10 @@ func (p *ArtifactoryProvider) Configure(ctx context.Context, req provider.Config
 func (p *ArtifactoryProvider) Resources(ctx context.Context) []func() resource.Resource {
 	return []func() resource.Resource{
 		rs.NewArtifactResource,
-		user.NewUserResource,
-		user.NewManagedUserResource,
 		user.NewAnonymousUserResource,
+		user.NewManagedUserResource,
+		user.NewUnmanagedUserResource,
+		user.NewUserResource,
 		security.NewGroupResource,
 		security.NewScopedTokenResource,
 		security.NewGlobalEnvironmentResource,

pkg/artifactory/provider/resources.go

@@ -12,7 +12,6 @@ import (
 	"github.com/jfrog/terraform-provider-artifactory/v10/pkg/artifactory/resource/repository/remote"
 	"github.com/jfrog/terraform-provider-artifactory/v10/pkg/artifactory/resource/repository/virtual"
 	"github.com/jfrog/terraform-provider-artifactory/v10/pkg/artifactory/resource/security"
-	"github.com/jfrog/terraform-provider-artifactory/v10/pkg/artifactory/resource/user"
 	"github.com/jfrog/terraform-provider-artifactory/v10/pkg/artifactory/resource/webhook"
 	utilsdk "github.com/jfrog/terraform-provider-shared/util/sdk"
 )
@@ -77,7 +76,6 @@ func resourcesMap() map[string]*schema.Resource {
 		"artifactory_virtual_nuget_repository":                virtual.ResourceArtifactoryVirtualNugetRepository(),
 		"artifactory_virtual_oci_repository":                  virtual.ResourceArtifactoryVirtualOciRepository(),
 		"artifactory_virtual_rpm_repository":                  virtual.ResourceArtifactoryVirtualRpmRepository(),
-		"artifactory_unmanaged_user":                          user.ResourceArtifactoryUser(), // alias of artifactory_user
 		"artifactory_permission_target":                       security.ResourceArtifactoryPermissionTarget(),
 		"artifactory_pull_replication":                        replication.ResourceArtifactoryPullReplication(),
 		"artifactory_push_replication":                        replication.ResourceArtifactoryPushReplication(),

pkg/artifactory/resource/configuration/resource_artifactory_backup_test.go

@@ -2,7 +2,9 @@ package configuration_test
 
 import (
 	"fmt"
+	"os"
 	"regexp"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
@@ -14,6 +16,11 @@ import (
 )
 
 func TestAccBackup_full(t *testing.T) {
+	jfrogURL := os.Getenv("JFROG_URL")
+	if strings.HasSuffix(jfrogURL, "jfrog.io") {
+		t.Skipf("env var JFROG_URL '%s' is a cloud instance.", jfrogURL)
+	}
+
 	_, fqrn, resourceName := testutil.MkNames("backup-", "artifactory_backup")
 	_, _, repoResourceName1 := testutil.MkNames("test-backup-local-", "artifactory_local_generic_repository")
 	_, _, repoResourceName2 := testutil.MkNames("test-backup-local-", "artifactory_local_generic_repository")
@@ -93,6 +100,11 @@ resource "artifactory_backup" "{{ .resourceName }}" {
 }
 
 func TestAccBackup_importNotFound(t *testing.T) {
+	jfrogURL := os.Getenv("JFROG_URL")
+	if strings.HasSuffix(jfrogURL, "jfrog.io") {
+		t.Skipf("env var JFROG_URL '%s' is a cloud instance.", jfrogURL)
+	}
+
 	config := `
 		resource "artifactory_backup" "not-exist-test" {
 		  enabled                = false
@@ -146,6 +158,11 @@ func TestAccBackup_invalid_cron(t *testing.T) {
 }
 
 func TestAccBackup_CronExpressions(t *testing.T) {
+	jfrogURL := os.Getenv("JFROG_URL")
+	if strings.HasSuffix(jfrogURL, "jfrog.io") {
+		t.Skipf("env var JFROG_URL '%s' is a cloud instance.", jfrogURL)
+	}
+
 	cronExpressions := [...]string{
 		"10/20 15 14 5-10 * ? *",
 		"* 5,7,9 14-16 * * ? *",

pkg/artifactory/resource/configuration/resource_artifactory_general_security_test.go

@@ -2,6 +2,8 @@ package configuration_test
 
 import (
 	"fmt"
+	"os"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
@@ -17,6 +19,11 @@ resource "artifactory_general_security" "security" {
 }`
 
 func TestAccGeneralSecurity_full(t *testing.T) {
+	jfrogURL := os.Getenv("JFROG_URL")
+	if strings.HasSuffix(jfrogURL, "jfrog.io") {
+		t.Skipf("env var JFROG_URL '%s' is a cloud instance.", jfrogURL)
+	}
+
 	fqrn := "artifactory_general_security.security"
 	resource.Test(t, resource.TestCase{
 		PreCheck:          func() { acctest.PreCheck(t) },

pkg/artifactory/resource/configuration/resource_artifactory_ldap_group_setting_test.go

@@ -84,6 +84,10 @@ resource "artifactory_ldap_group_setting" "ldapgrouptest" {
 }
 
 func TestAccLdapGroupSetting_importNotFound(t *testing.T) {
+	var onOrAfterVersion7571 = func() (bool, error) {
+		return acctest.CompareArtifactoryVersions(t, "7.57.1")
+	}
+
 	config := `
 		resource "artifactory_ldap_group_setting" "not-exist-test" {
 			name = "not-exist-test"
@@ -102,6 +106,7 @@ func TestAccLdapGroupSetting_importNotFound(t *testing.T) {
 		ProviderFactories: acctest.ProviderFactories,
 		Steps: []resource.TestStep{
 			{
+				SkipFunc:      onOrAfterVersion7571,
 				Config:        config,
 				ResourceName:  "artifactory_ldap_group_setting.not-exist-test",
 				ImportStateId: "not-exist-test",

pkg/artifactory/resource/configuration/resource_artifactory_ldap_setting_test.go

@@ -99,11 +99,17 @@ func TestAccLdapSetting_importNotFound(t *testing.T) {
 			manager_password = "testmgrpaswd"
 		}
 	`
+
+	var onOrAfterVersion7571 = func() (bool, error) {
+		return acctest.CompareArtifactoryVersions(t, "7.57.1")
+	}
+
 	resource.Test(t, resource.TestCase{
 		PreCheck:          func() { acctest.PreCheck(t) },
 		ProviderFactories: acctest.ProviderFactories,
 		Steps: []resource.TestStep{
 			{
+				SkipFunc:      onOrAfterVersion7571,
 				Config:        config,
 				ResourceName:  "artifactory_ldap_setting.not-exist-test",
 				ImportStateId: "not-exist-test",

pkg/artifactory/resource/configuration/resource_artifactory_mail_server_test.go

@@ -2,7 +2,9 @@ package configuration_test
 
 import (
 	"fmt"
+	"os"
 	"regexp"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
@@ -14,6 +16,11 @@ import (
 )
 
 func TestAccMailServer_full(t *testing.T) {
+	jfrogURL := os.Getenv("JFROG_URL")
+	if strings.HasSuffix(jfrogURL, "jfrog.io") {
+		t.Skipf("env var JFROG_URL '%s' is a cloud instance.", jfrogURL)
+	}
+
 	_, fqrn, resourceName := testutil.MkNames("mailserver-", "artifactory_mail_server")
 
 	const mailServerTemplate = `

pkg/artifactory/resource/configuration/resource_artifactory_oauth_settings_test.go

@@ -2,6 +2,8 @@ package configuration_test
 
 import (
 	"fmt"
+	"os"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
@@ -30,6 +32,11 @@ resource "artifactory_oauth_settings" "oauth" {
 }`
 
 func TestAccOauthSettings_full(t *testing.T) {
+	jfrogURL := os.Getenv("JFROG_URL")
+	if strings.HasSuffix(jfrogURL, "jfrog.io") {
+		t.Skipf("env var JFROG_URL '%s' is a cloud instance.", jfrogURL)
+	}
+
 	fqrn := "artifactory_oauth_settings.oauth"
 	resource.Test(t, resource.TestCase{
 		PreCheck:          func() { acctest.PreCheck(t) },
@@ -87,6 +94,11 @@ resource "artifactory_oauth_settings" "oauth" {
 `
 
 func TestAccOauthSettings_multipleProviders(t *testing.T) {
+	jfrogURL := os.Getenv("JFROG_URL")
+	if strings.HasSuffix(jfrogURL, "jfrog.io") {
+		t.Skipf("env var JFROG_URL '%s' is a cloud instance.", jfrogURL)
+	}
+
 	fqrn := "artifactory_oauth_settings.oauth"
 	resource.Test(t, resource.TestCase{
 		PreCheck:          func() { acctest.PreCheck(t) },