Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2/2] #35 Fix infinite plan on user metadata #250

Merged
merged 1 commit into from
Jul 20, 2022
Merged

[2/2] #35 Fix infinite plan on user metadata #250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 43 additions & 12 deletions auth0/resource_auth0_user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,9 @@ func readUser(ctx context.Context, d *schema.ResourceData, m interface{}) diag.D
api := m.(*management.Management)
user, err := api.User.Read(d.Id())
if err != nil {
if mErr, ok := err.(management.Error); ok {
if mErr.Status() == http.StatusNotFound {
d.SetId("")
return nil
}
if err, ok := err.(management.Error); ok && err.Status() == http.StatusNotFound {
d.SetId("")
return nil
}
return diag.FromErr(err)
}
Expand Down Expand Up @@ -250,19 +248,52 @@ func expandUser(d *schema.ResourceData) (*management.User, error) {
Blocked: Bool(d, "blocked"),
}

userMeta, err := JSON(d, "user_metadata")
if d.HasChange("user_metadata") {
userMetadata, err := expandMetadata(d, "user")
if err != nil {
return nil, err
}
user.UserMetadata = &userMetadata
}

if d.HasChange("app_metadata") {
appMetadata, err := expandMetadata(d, "app")
if err != nil {
return nil, err
}
user.AppMetadata = &appMetadata
}

return user, nil
}

func expandMetadata(d *schema.ResourceData, metadataType string) (map[string]interface{}, error) {
oldMetadata, newMetadata := d.GetChange(metadataType + "_metadata")
if oldMetadata == "" {
return JSON(d, metadataType+"_metadata")
}

if newMetadata == "" {
return map[string]interface{}{}, nil
}

oldMap, err := structure.ExpandJsonFromString(oldMetadata.(string))
if err != nil {
return nil, err
return map[string]interface{}{}, err
}
user.UserMetadata = &userMeta

appMeta, err := JSON(d, "app_metadata")
newMap, err := structure.ExpandJsonFromString(newMetadata.(string))
if err != nil {
return nil, err
return map[string]interface{}{}, err
}
user.AppMetadata = &appMeta

return user, nil
for key := range oldMap {
if _, ok := newMap[key]; !ok {
newMap[key] = nil
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're applying the following explained logic in alexkappa/terraform-provider-auth0#392 (comment), where in order to properly remove something we need to send it's value as null.

This is because the metadata fields are merged instead of being replaced but the merge only occurs on the first level during a PATCH.

}
}

return newMap, nil
}

func flattenUserRoles(roleList *management.RoleList) []interface{} {
Expand Down
145 changes: 76 additions & 69 deletions auth0/resource_auth0_user_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,22 +75,10 @@ resource auth0_user user {
family_name = "Lastname"
nickname = "{{.testName}}"
picture = "https://www.example.com/picture.jpg"
user_metadata = <<EOF
{
"foo": "bar",
"bar": { "baz": "qux" }
}
EOF
app_metadata = <<EOF
{
"foo": "bar",
"bar": { "baz": "qux" }
}
EOF
}
`

const testAccUserAddRole = `
const testAccUserUpdateWithRolesAndMetadata = `
resource auth0_user user {
depends_on = [auth0_role.owner, auth0_role.admin]
connection_name = "Username-Password-Authentication"
Expand All @@ -104,18 +92,14 @@ resource auth0_user user {
nickname = "{{.testName}}"
picture = "https://www.example.com/picture.jpg"
roles = [ auth0_role.owner.id, auth0_role.admin.id ]
user_metadata = <<EOF
{
"foo": "bar",
"bar": { "baz": "qux" }
}
EOF
app_metadata = <<EOF
{
"foo": "bar",
"bar": { "baz": "qux" }
}
EOF
user_metadata = jsonencode({
"foo": "bar",
"baz": "qux"
})
app_metadata = jsonencode({
"foo": "bar",
"baz": "qux"
})
}

resource auth0_role owner {
Expand All @@ -130,7 +114,7 @@ resource auth0_role admin {
}
`

const testAccUserRemoveRole = `
const testAccUserUpdateRemovingOneRoleAndUpdatingMetadata = `
resource auth0_user user {
depends_on = [auth0_role.admin]
connection_name = "Username-Password-Authentication"
Expand All @@ -144,18 +128,12 @@ resource auth0_user user {
nickname = "{{.testName}}"
picture = "https://www.example.com/picture.jpg"
roles = [ auth0_role.admin.id ]
user_metadata = <<EOF
{
"foo": "bar",
"bar": { "baz": "qux" }
}
EOF
app_metadata = <<EOF
{
"foo": "bar",
"bar": { "baz": "qux" }
}
EOF
user_metadata = jsonencode({
"foo": "bars",
})
app_metadata = jsonencode({
"foo": "bars",
})
}

resource auth0_role admin {
Expand All @@ -164,6 +142,44 @@ resource auth0_role admin {
}
`

const testAccUserUpdateRemovingAllRolesAndUpdatingMetadata = `
resource auth0_user user {
connection_name = "Username-Password-Authentication"
username = "{{.testName}}"
user_id = "{{.testName}}"
email = "{{.testName}}@acceptance.test.com"
password = "passpass$12$12"
name = "Firstname Lastname"
given_name = "Firstname"
family_name = "Lastname"
nickname = "{{.testName}}"
picture = "https://www.example.com/picture.jpg"
user_metadata = jsonencode({
"foo": "barss",
"foo2": "bar2",
})
app_metadata = jsonencode({
"foo": "barss",
"foo2": "bar2",
})
}
`

const testAccUserUpdateRemovingMetadata = `
resource auth0_user user {
connection_name = "Username-Password-Authentication"
username = "{{.testName}}"
user_id = "{{.testName}}"
email = "{{.testName}}@acceptance.test.com"
password = "passpass$12$12"
name = "Firstname Lastname"
given_name = "Firstname"
family_name = "Lastname"
nickname = "{{.testName}}"
picture = "https://www.example.com/picture.jpg"
}
`

func TestAccUser(t *testing.T) {
httpRecorder := configureHTTPRecorder(t)

Expand All @@ -173,62 +189,53 @@ func TestAccUser(t *testing.T) {
{
Config: template.ParseTestName(testAccUserCreate, strings.ToLower(t.Name())),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_user.user", "connection_name", "Username-Password-Authentication"),
resource.TestCheckResourceAttr("auth0_user.user", "username", strings.ToLower(t.Name())),
resource.TestCheckResourceAttr("auth0_user.user", "user_id", fmt.Sprintf("auth0|%s", strings.ToLower(t.Name()))),
resource.TestCheckResourceAttr("auth0_user.user", "email", fmt.Sprintf("%s@acceptance.test.com", strings.ToLower(t.Name()))),
resource.TestCheckResourceAttr("auth0_user.user", "name", "Firstname Lastname"),
resource.TestCheckResourceAttr("auth0_user.user", "family_name", "Lastname"),
resource.TestCheckResourceAttr("auth0_user.user", "given_name", "Firstname"),
resource.TestCheckResourceAttr("auth0_user.user", "family_name", "Lastname"),
resource.TestCheckResourceAttr("auth0_user.user", "nickname", strings.ToLower(t.Name())),
resource.TestCheckResourceAttr("auth0_user.user", "connection_name", "Username-Password-Authentication"),
resource.TestCheckResourceAttr("auth0_user.user", "roles.#", "0"),
resource.TestCheckResourceAttr("auth0_user.user", "picture", "https://www.example.com/picture.jpg"),
resource.TestCheckResourceAttr("auth0_user.user", "user_metadata", ""),
resource.TestCheckResourceAttr("auth0_user.user", "app_metadata", ""),
resource.TestCheckResourceAttr("auth0_user.user", "roles.#", "0"),
),
},
{
Config: template.ParseTestName(testAccUserAddRole, strings.ToLower(t.Name())),
Config: template.ParseTestName(testAccUserUpdateWithRolesAndMetadata, strings.ToLower(t.Name())),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_user.user", "roles.#", "2"),
resource.TestCheckResourceAttr("auth0_role.owner", "name", "owner"),
resource.TestCheckResourceAttr("auth0_role.admin", "name", "admin"),
resource.TestCheckResourceAttr("auth0_user.user", "user_metadata", `{"baz":"qux","foo":"bar"}`),
resource.TestCheckResourceAttr("auth0_user.user", "app_metadata", `{"baz":"qux","foo":"bar"}`),
),
},
{
Config: template.ParseTestName(testAccUserRemoveRole, strings.ToLower(t.Name())),
Config: template.ParseTestName(testAccUserUpdateRemovingOneRoleAndUpdatingMetadata, strings.ToLower(t.Name())),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_user.user", "roles.#", "1"),
resource.TestCheckResourceAttr("auth0_user.user", "user_metadata", `{"foo":"bars"}`),
resource.TestCheckResourceAttr("auth0_user.user", "app_metadata", `{"foo":"bars"}`),
),
},
},
})
}

const testAccUserCanSerializeEmptyMetadataFields = `
resource auth0_user auth0_user_issue_218 {
connection_name = "Username-Password-Authentication"
user_id = "id_{{.testName}}"
username = "user_{{.testName}}"
email = "issue.218.{{.testName}}@acceptance.test.com"
email_verified = true
password = "MyPass123$"
}
`

func TestAccUserCanSerializeEmptyMetadataFields(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're removing this as we added it as a last test case for the TestAccUser scenario.

httpRecorder := configureHTTPRecorder(t)

resource.Test(t, resource.TestCase{
ProviderFactories: testProviders(httpRecorder),
Steps: []resource.TestStep{
{
Config: template.ParseTestName(testAccUserCanSerializeEmptyMetadataFields, "issue#218"),
Config: template.ParseTestName(testAccUserUpdateRemovingAllRolesAndUpdatingMetadata, strings.ToLower(t.Name())),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_user.auth0_user_issue_218", "user_id", "auth0|id_issue#218"),
resource.TestCheckResourceAttr("auth0_user.auth0_user_issue_218", "username", "user_issue#218"),
resource.TestCheckResourceAttr("auth0_user.auth0_user_issue_218", "email", "issue.218.issue#218@acceptance.test.com"),
resource.TestCheckResourceAttr("auth0_user.user", "roles.#", "0"),
resource.TestCheckResourceAttr("auth0_user.user", "user_metadata", `{"foo":"barss","foo2":"bar2"}`),
resource.TestCheckResourceAttr("auth0_user.user", "app_metadata", `{"foo":"barss","foo2":"bar2"}`),
),
},
{
Config: template.ParseTestName(testAccUserCanSerializeEmptyMetadataFields, "issue#218"),
Config: template.ParseTestName(testAccUserUpdateRemovingMetadata, strings.ToLower(t.Name())),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("auth0_user.user", "roles.#", "0"),
resource.TestCheckResourceAttr("auth0_user.user", "user_metadata", ""),
resource.TestCheckResourceAttr("auth0_user.user", "app_metadata", ""),
),
},
},
})
Expand Down
Loading