Skip to content

Commit

Permalink
Merge pull request #932 from jfrog/GH-931-fix-unable-to-update-user-w…
Browse files Browse the repository at this point in the history
…ith-no-password

Explicitly set state value for 'password' attribute
  • Loading branch information
alexhung authored Apr 11, 2024
2 parents 04722eb + 48076bc commit 6296a0e
Show file tree
Hide file tree
Showing 13 changed files with 94 additions and 44 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 10.5.1 (Apr 10, 2024)

BUG FIXES:

* resource/artifactory_user, resource/artifactory_managed_user: Fix error when updating resource without `password` attribute defined (i.e. relying on auto generated password). Issue: [#931](https://github.com/jfrog/terraform-provider-artifactory/issues/931) PR: [#932](https://github.com/jfrog/terraform-provider-artifactory/pull/932)

## 10.5.0 (Apr 9, 2024)

FEATURES:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestAccDataSourceUser_basic(t *testing.T) {
temp := `
resource "artifactory_managed_user" "{{ .name }}" {
name = "{{ .name }}"
password = "Password1!"
password = "Passw0rd!123"
email = "{{ .email }}"
groups = ["readers"]
admin = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestAccAccessTokenAudienceBad(t *testing.T) {
email = "[email protected]"
admin = false
groups = ["readers"]
password = "Passw0rd!"
password = "Passw0rd!123"
}
resource "artifactory_access_token" "foobar" {
Expand Down Expand Up @@ -52,7 +52,7 @@ func TestAccAccessTokenAudienceGood(t *testing.T) {
email = "[email protected]"
admin = false
groups = ["readers"]
password = "Passw0rd!"
password = "Passw0rd!123"
}
resource "artifactory_access_token" "foobar" {
Expand Down Expand Up @@ -95,7 +95,7 @@ resource "artifactory_managed_user" "existinguser" {
email = "[email protected]"
admin = false
groups = ["readers"]
password = "Passw0rd!"
password = "Passw0rd!123"
}
resource "artifactory_access_token" "foobar" {
Expand Down Expand Up @@ -142,7 +142,7 @@ resource "artifactory_managed_user" "existinguser" {
email = "[email protected]"
admin = false
groups = ["readers"]
password = "Passw0rd!"
password = "Passw0rd!123"
}
resource "artifactory_access_token" "foobar" {
Expand Down Expand Up @@ -185,7 +185,7 @@ resource "artifactory_managed_user" "existinguser" {
email = "[email protected]"
admin = false
groups = ["readers"]
password = "Passw0rd!"
password = "Passw0rd!123"
}
resource "artifactory_access_token" "foobar" {
Expand Down Expand Up @@ -216,7 +216,7 @@ resource "artifactory_managed_user" "existinguser" {
email = "[email protected]"
admin = false
groups = ["readers"]
password = "Passw0rd!"
password = "Passw0rd!123"
}
resource "artifactory_access_token" "foobar" {
Expand Down Expand Up @@ -249,7 +249,7 @@ resource "artifactory_managed_user" "existinguser" {
email = "[email protected]"
admin = false
groups = ["readers"]
password = "Passw0rd!"
password = "Passw0rd!123"
}
resource "artifactory_access_token" "foobar" {
Expand Down Expand Up @@ -354,7 +354,7 @@ resource "artifactory_managed_user" "existinguser" {
email = "[email protected]"
admin = false
groups = ["readers"]
password = "Passw0rd!"
password = "Passw0rd!123"
}
resource "artifactory_access_token" "foobar" {
Expand Down Expand Up @@ -386,7 +386,7 @@ resource "artifactory_managed_user" "existinguser" {
email = "[email protected]"
admin = false
groups = ["readers"]
password = "Passw0rd!"
password = "Passw0rd!123"
}
resource "artifactory_access_token" "foobar" {
Expand Down Expand Up @@ -433,7 +433,7 @@ resource "artifactory_managed_user" "existinguser" {
email = "[email protected]"
admin = false
groups = ["readers"]
password = "Passw0rd!"
password = "Passw0rd!123"
}
resource "artifactory_access_token" "foobar" {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ const permissionFull = `
resource "artifactory_managed_user" "test-user" {
name = "terraform"
email = "[email protected]"
password = "Passsw0rd!"
password = "Passsw0rd!12"
}
resource "artifactory_permission_target" "{{ .permission_name }}" {
Expand Down Expand Up @@ -250,7 +250,7 @@ func TestAccPermissionTarget_GitHubIssue126(t *testing.T) {
admin = false
disable_ui_access = true
internal_password_disabled = false
password = "Passw0rd!"
password = "Passw0rd!123"
}
resource "artifactory_permission_target" "{{ .perm_name }}" {
Expand Down Expand Up @@ -500,7 +500,7 @@ func TestAccPermissionTarget_MissingRepositories(t *testing.T) {
admin = false
disable_ui_access = true
internal_password_disabled = true
password = "Passw0rd!"
password = "Passw0rd!123"
}
resource "artifactory_permission_target" "{{ .perm_name }}" {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"golang.org/x/exp/maps"
"github.com/samber/lo"
)

func NewManagedUserResource() resource.Resource {
Expand All @@ -32,7 +32,7 @@ func (r *ArtifactoryManagedUserResource) Schema(ctx context.Context, req resourc
},
}

maps.Copy(managedUserSchemaFramework, baseUserSchemaFramework)
managedUserSchemaFramework = lo.Assign(baseUserSchemaFramework, managedUserSchemaFramework)

resp.Schema = schema.Schema{
MarkdownDescription: "Provides an Artifactory managed user resource. This can be used to create and manage Artifactory users. For example, service account where password is known and managed externally.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestAccManagedUser_UpgradeFromSDKv2(t *testing.T) {
resource "artifactory_managed_user" "{{ .name }}" {
name = "{{ .name }}"
email = "{{ .email }}"
password = "Passsw0rd!"
password = "Passsw0rd!12"
}
`, params)

Expand Down Expand Up @@ -64,7 +64,7 @@ func TestAccManagedUser_no_groups(t *testing.T) {
resource "artifactory_managed_user" "%s" {
name = "%s"
email = "dummy%[email protected]"
password = "Passsw0rd!"
password = "Passsw0rd!12"
}
`
id, fqrn, name := testutil.MkNames("foobar-", "artifactory_managed_user")
Expand Down Expand Up @@ -97,7 +97,7 @@ func TestAccManagedUser_empty_groups(t *testing.T) {
resource "artifactory_managed_user" "%s" {
name = "%s"
email = "dummy%[email protected]"
password = "Passsw0rd!"
password = "Passsw0rd!12"
groups = []
}
`
Expand Down Expand Up @@ -138,16 +138,17 @@ func TestAccManagedUser_invalidName(t *testing.T) {
}

for _, tc := range testCase {
t.Run(tc.name, testAccManagedUserInvalidName(t, tc.username, tc.errorRegex))
t.Run(tc.name, testAccManagedUserInvalidName(tc.username, tc.errorRegex))
}
}

func testAccManagedUserInvalidName(t *testing.T, username, errorRegex string) func(t *testing.T) {
func testAccManagedUserInvalidName(username, errorRegex string) func(t *testing.T) {
return func(t *testing.T) {
const userNoGroups = `
resource "artifactory_managed_user" "%s" {
name = "%s"
email = "dummy%[email protected]"
password = "Passsw0rd!12"
}
`
id, fqrn, name := testutil.MkNames("test-", "artifactory_managed_user")
Expand Down Expand Up @@ -187,7 +188,7 @@ func TestAccManagedUser_basic(t *testing.T) {
resource "artifactory_managed_user" "{{ .name }}" {
name = "{{ .username }}"
email = "{{ .email }}"
password = "Passsw0rd!"
password = "Passsw0rd!12"
admin = true
profile_updatable = true
disable_ui_access = false
Expand All @@ -205,7 +206,7 @@ func TestAccManagedUser_basic(t *testing.T) {
resource "artifactory_managed_user" "{{ .name }}" {
name = "{{ .username }}"
email = "{{ .email }}"
password = "Passsw0rd!"
password = "Passsw0rd!12"
admin = false
profile_updatable = false
groups = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ func ResourceArtifactoryUser() *schema.Resource {
Type: schema.TypeString,
Sensitive: true,
Optional: true,
Computed: true,
Description: "(Optional, Sensitive) Password for the user. When omitted, a random password is generated using the following password policy: " +
"12 characters with 1 digit, 1 symbol, with upper and lower case letters",
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,17 @@ func TestAccUnmanagedUser_basic(t *testing.T) {
}

func TestAccUnmanagedUserShouldCreateWithoutPassword(t *testing.T) {
const userBasic = `
const config = `
resource "artifactory_unmanaged_user" "%s" {
name = "%s"
email = "dummy_user%[email protected]"
}
`
const updatedConfig = `
resource "artifactory_unmanaged_user" "%s" {
name = "%s"
email = "dummy_user%[email protected]"
profile_updatable = false
}
`
id := testutil.RandomInt()
Expand All @@ -134,13 +141,19 @@ func TestAccUnmanagedUserShouldCreateWithoutPassword(t *testing.T) {
CheckDestroy: testAccCheckUserDestroy(fqrn),
Steps: []resource.TestStep{
{
Config: fmt.Sprintf(userBasic, name, username, id),
Config: fmt.Sprintf(config, name, username, id),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(fqrn, "name", username),
resource.TestCheckResourceAttr(fqrn, "email", fmt.Sprintf("dummy_user%[email protected]", id)),
resource.TestCheckNoResourceAttr(fqrn, "groups"),
),
},
{
Config: fmt.Sprintf(updatedConfig, name, username, id),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(fqrn, "profile_updatable", "false"),
),
},
{
ResourceName: fqrn,
ImportState: true,
Expand Down Expand Up @@ -283,11 +296,11 @@ func TestAccUnmanagedUser_invalidName(t *testing.T) {
}

for _, tc := range testCase {
t.Run(tc.name, testAccUnmanagedUserInvalidName(t, tc.username, tc.errorRegex))
t.Run(tc.name, testAccUnmanagedUserInvalidName(tc.username, tc.errorRegex))
}
}

func testAccUnmanagedUserInvalidName(t *testing.T, username, errorRegex string) func(t *testing.T) {
func testAccUnmanagedUserInvalidName(username, errorRegex string) func(t *testing.T) {
return func(t *testing.T) {
const userNoGroups = `
resource "artifactory_unmanaged_user" "%s" {
Expand Down
10 changes: 8 additions & 2 deletions pkg/artifactory/resource/user/resource_artifactory_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import (

"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"golang.org/x/exp/maps"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/samber/lo"
)

func NewUserResource() resource.Resource {
Expand All @@ -27,10 +29,14 @@ func (r *ArtifactoryUserResource) Schema(ctx context.Context, req resource.Schem
"12 characters with 1 digit, 1 symbol, with upper and lower case letters",
Optional: true,
Sensitive: true,
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
}

maps.Copy(userSchemaFramework, baseUserSchemaFramework)
userSchemaFramework = lo.Assign(baseUserSchemaFramework, userSchemaFramework)

resp.Schema = schema.Schema{
MarkdownDescription: "Provides an Artifactory user resource. This can be used to create and manage Artifactory users. The password is a required field by the [Artifactory API](https://www.jfrog.com/confluence/display/JFROG/Artifactory+REST+API#ArtifactoryRESTAPI-CreateorReplaceUser), but we made it optional in this resource to accommodate the scenario where the password is not needed and will be reset by the actual user later. When the optional attribute `password` is omitted, a random password is generated according to current Artifactory password policy.",
Expand Down
32 changes: 25 additions & 7 deletions pkg/artifactory/resource/user/resource_artifactory_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestAccUser_UpgradeFromSDKv2(t *testing.T) {
resource "artifactory_user" "{{ .name }}" {
name = "{{ .name }}"
email = "{{ .email }}"
password = "Passsw0rd!"
password = "Passsw0rd!12"
}
`, params)

Expand Down Expand Up @@ -81,7 +81,7 @@ func TestAccUser_full_groups(t *testing.T) {
resource "artifactory_user" "{{ .name }}" {
name = "{{ .name }}"
email = "{{ .email }}"
password = "Passsw0rd!"
password = "Passsw0rd!12"
admin = false
groups = [
artifactory_group.{{ .groupName1 }}.name,
Expand All @@ -101,7 +101,7 @@ func TestAccUser_full_groups(t *testing.T) {
resource "artifactory_user" "{{ .name }}" {
name = "{{ .name }}"
email = "{{ .email }}"
password = "Passsw0rd!"
password = "Passsw0rd!12"
admin = false
groups = [
artifactory_group.{{ .groupName1 }}.name,
Expand Down Expand Up @@ -161,11 +161,21 @@ func TestAccUser_no_password(t *testing.T) {
"username": username,
"email": email,
}
userNoGroups := util.ExecuteTemplate("TestAccUserBasic", `
config := util.ExecuteTemplate("TestAccUserBasic", `
resource "artifactory_user" "{{ .name }}" {
name = "{{ .name }}"
email = "{{ .email }}"
admin = false
groups = [ "readers" ]
}
`, params)

updatedConfig := util.ExecuteTemplate("TestAccUserBasic", `
resource "artifactory_user" "{{ .name }}" {
name = "{{ .name }}"
email = "{{ .email }}"
admin = false
profile_updatable = false
groups = [ "readers" ]
}
`, params)
Expand All @@ -176,9 +186,17 @@ func TestAccUser_no_password(t *testing.T) {
CheckDestroy: testAccCheckManagedUserDestroy(fqrn),
Steps: []resource.TestStep{
{
Config: userNoGroups,
Config: config,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(fqrn, "name", fmt.Sprintf("foobar-%d", id)),
resource.TestCheckResourceAttr(fqrn, "groups.#", "1"),
),
},
{
Config: updatedConfig,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(fqrn, "name", fmt.Sprintf("foobar-%d", id)),
resource.TestCheckResourceAttr(fqrn, "profile_updatable", "false"),
resource.TestCheckResourceAttr(fqrn, "groups.#", "1"),
),
},
Expand Down Expand Up @@ -207,7 +225,7 @@ func TestAccUser_no_groups(t *testing.T) {
resource "artifactory_user" "{{ .name }}" {
name = "{{ .name }}"
email = "{{ .email }}"
password = "Passsw0rd!"
password = "Passsw0rd!12"
admin = false
}
`, params)
Expand Down Expand Up @@ -249,7 +267,7 @@ func TestAccUser_empty_groups(t *testing.T) {
resource "artifactory_user" "{{ .name }}" {
name = "{{ .name }}"
email = "{{ .email }}"
password = "Passsw0rd!"
password = "Passsw0rd!12"
admin = false
groups = []
}
Expand Down
Loading

0 comments on commit 6296a0e

Please sign in to comment.