Skip to content
This repository has been archived by the owner on Mar 8, 2022. It is now read-only.

auth0_tenant - 400 Bad Request: Payload validation error: 'Too few properties defined (0), minimum 1' #185

Closed
icadariu-talend opened this issue Mar 25, 2020 · 13 comments · Fixed by #194

Comments

@icadariu-talend
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

  • v0.12.20

Auth0 Provider Version

  • v0.8.0

Affected Resource(s)

  • auth0_tenant

Terraform Configuration Files

resource "auth0_tenant" "tenant" {
  friendly_name         = var.tenant_name
  support_email         = var.support_email
  support_url           = var.support_url
  session_lifetime      = var.auth0_session_lifetime
  idle_session_lifetime = var.auth0_idle_session_lifetime

  flags {
    change_pwd_flow_v1                     = false
    disable_clickjack_protection_headers   = false
    enable_apis_section                    = false
    enable_client_connections              = true
    enable_custom_domain_in_emails         = false
    enable_dynamic_client_registration     = false
    enable_legacy_logs_search_v2           = false
    enable_pipeline2                       = false
    enable_public_signup_user_exists_error = false
    universal_login                        = false
  }
}

Debug Output

auth0_tenant.tenant: Modifying... [id=terraform-redacted]
2020/03/25 14:08:33 [DEBUG] auth0_tenant.tenant: applying the planned Update change
2020/03/25 14:08:33 [TRACE] GRPCProvider: ApplyResourceChange
2020-03-25T14:08:33.152+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: 2020/03/25 14:08:33
2020-03-25T14:08:33.152+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: PATCH /api/v2/tenants/settings HTTP/1.1
2020-03-25T14:08:33.152+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: Host: redacted.auth0.com
2020-03-25T14:08:33.152+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: User-Agent: Go-Auth0-SDK/v3; Terraform/1.8.0
2020-03-25T14:08:33.152+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: Content-Length: 3
2020-03-25T14:08:33.152+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: Content-Type: application/json
2020-03-25T14:08:33.152+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: Accept-Encoding: gzip
2020-03-25T14:08:33.152+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4:
2020-03-25T14:08:33.152+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: {}
2020-03-25T14:08:33.152+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4:
2020-03-25T14:08:33.413+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: 2020/03/25 14:08:33
2020-03-25T14:08:33.413+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: HTTP/2.0 400 Bad Request
2020-03-25T14:08:33.413+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: Cache-Control: no-cache
2020-03-25T14:08:33.413+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: Content-Type: application/json; charset=utf-8
2020-03-25T14:08:33.413+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: Date: Wed, 25 Mar 2020 13:08:33 GMT
2020-03-25T14:08:33.413+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: Server: nginx
2020-03-25T14:08:33.413+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: Vary: origin,accept-encoding
2020-03-25T14:08:33.413+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4:
2020-03-25T14:08:33.413+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4: {"statusCode":400,"error":"Bad Request","message":"Payload validation error: 'Too few properties defined (0), minimum 1'.","errorCode":"invalid_body"}
2020-03-25T14:08:33.413+0100 [DEBUG] plugin.terraform-provider-auth0_v0.8.0_x4:
2020/03/25 14:08:33 [DEBUG] auth0_tenant.tenant: apply errored, but we're indicating that via the Error pointer rather than returning it: 400 Bad Request: Payload validation error: 'Too few properties defined (0), minimum 1'.

Panic Output

Expected Behavior

  • plan/apply should work. This is plan output

  ~ resource "auth0_tenant" "tenant" {
        allowed_logout_urls   = []
        friendly_name         = "redacted"
        id                    = "terraform-123"
        idle_session_lifetime = 24
        sandbox_version       = "8"
        session_lifetime      = 168
        support_email         = "support@example.com"
        support_url           = "https://www.example.com/support"

      - change_password {}

      - error_page {}

        flags {
            change_pwd_flow_v1                     = false
            disable_clickjack_protection_headers   = false
            enable_apis_section                    = false
            enable_client_connections              = true
            enable_custom_domain_in_emails         = false
            enable_dynamic_client_registration     = false
            enable_legacy_logs_search_v2           = false
            enable_pipeline2                       = false
            enable_public_signup_user_exists_error = false
            universal_login                        = false
            use_scope_descriptions_for_consent     = false
        }

      - guardian_mfa_page {}

      - universal_login {
        }
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Actual Behavior

Steps to Reproduce

  1. terraform apply

Important Factoids

References

  • apply error:
auth0_tenant.tenant: Modifying... [id=terraform-redacted]

Error: 400 Bad Request: Payload validation error: 'Too few properties defined (0), minimum 1'.

  on auth0_tenant.tf line 3, in resource "auth0_tenant" "tenant":
   3: resource "auth0_tenant" "tenant" {
@yinzara
Copy link
Contributor

yinzara commented Mar 26, 2020

Yeah I got the same issue. This happens when you make a change that removes elements that were already present and do not change any other attributes in the same apply. Unfortunately this is really hard to handle as the logic is not completely clear. Do you want to ignore the "change_password" and "error_page" attributes (as you have not specified them) or do you want to set the values of all the properties to empty string?

If you want the former behavior (i.e. where you want terraform to not manage those elements and let the Auth0 Management console specify them), I would suggest adding the four fields you've removed to the "lifecycle" "ignore_changes" section as well.

If you would actually like to set their values to empty string, keep the elements but set all of their properties to empty string.

@yinzara
Copy link
Contributor

yinzara commented Mar 26, 2020

I've actually rethought my original statement and I actually think this is a bug.

How the provider generally works, if a resource isn't being created, it won't send a key on an update unless that key itself has changed. Unfortunately auth0 assumes in some of these cases that not sending a key implies the key should be set to empty string.

Additionally if you change one value inside one of the mentioned blocks, but not the other value, you can often set the others to empty string.

The specific big culprits are the ones you mention (the custom html blocks in the auth0_tenant resource) and the "options" in the "auth0_connection" resource.

I've also discovered that the new v4 merge introduced a host of defects with the auth0_connection changes that I'm working on fixing right now. All of them are related to this issue.

@alexkappa
Copy link
Owner

Thanks for jumping in @yinzara. The analysis makes sense really as this seems to appear with the tenant resource a lot the last couple of days. v4 shouldn't have had a huge impact on tenant but it had unfortunately and at this point I'm not 100% sure why. To start with, I think we need some tests to verify the broken behaviour 🤔

@alexkappa
Copy link
Owner

@icadariu-talend just so I can understand the exact situation, did you include change_password, error_page, guardian_mfa_page universal_login and then removed them or they were not included to begin with?

@yinzara
Copy link
Contributor

yinzara commented Apr 3, 2020

Honestly I think we may need to think about the whole "Only include a field if it's changed" paradigm across the whole repo. I'm finding a host of small defects all related to using that methodology generally that only happen when we have specific changes in state that include some fields and not others.

I'm curious, in the "Update" functions, why do we only send fields that have changed? I don't see any drawback to just sending the entire entity as long as all our fields are mapped correctly (there are some issues with the Connection entity I know of already with the "options" of incorrectly mapped fields because of issues in upper/lower casing). The amount of tests we'd have to cover every possible original state and then change is very error prone.

@yinzara
Copy link
Contributor

yinzara commented Apr 3, 2020

I think the reason we're not catching a lot of these bugs is one of two reasons.

  1. Test cases should all end with the following test step. This will verify that the "read" functions build the same state as the state you created after the last update operation.
,
			{
			    ImportState: true,
			    ImportStateVerify: true,
            },
  1. Because we only transmit fields that were changed as part of the most recent update, we would need test cases to cover changing every field individually to ensure we cover every use case. I propose instead we change the library to transmit ALL fields. As long as our test cases all have import state verify steps above and we have at least one test case that covers every field, all fields are marked as computed and optional (unless they're required), then we can be sure everything works as intended.

Fields that the user has not specified we be calculated as part of the read/refresh operation. Fields that are specified will be updated in the state to match and terraform will consider the resource unchanged unless an actual change is made to the resource.

@nitrag
Copy link

nitrag commented Apr 3, 2020

I imported an existing tenant. Clearly, the parameters are not all optional as per the docs.

@alexkappa
Copy link
Owner

Honestly it goes a while back so I don't remember the exact reason why we send patches only. Perhaps due to the API relying on PATCH operations as the main method to update a resource...

As to why we only get fields if they have changed, surely it's something we can take a step back and verify. We can pick a resource and re-write it to contain the entire payload instead of just changes. I know certain things are read-only so we can't be sending those fields (e.g. connection strategy, id's etc) but we can account for that too.

The helper methods which check for d.IsNewResource() || d.HasChange(key) are a product of many iterations and it's the most robust way so far. However within the community no one seems to be doing such a thing and I'm unsure if its the best way to go. But the alternative is writing extremely tedious, repetitive and hard to read code.

Example: https://github.com/terraform-providers/terraform-provider-kubernetes/blob/master/kubernetes/structures_pod.go

On the other hand, the author of the kubernetes provider has written https://github.com/radeksimko/terraform-gen which should help generate most of the work, leaving some final touches to the developer.

@icadariu-talend
Copy link
Author

@icadariu-talend just so I can understand the exact situation, did you include change_password, error_page, guardian_mfa_page universal_login and then removed them or they were not included to begin with?
@alexkappa they were not included in the first place.

@icadariu-talend
Copy link
Author

icadariu-talend commented Apr 6, 2020

I've also noticed(please let me know if I should open a new issue) on auth0_connection that regardless of what we change on configuration it's not triggering an update so the only way I was able to deploy a change was to destroy the connection resouce and recreate it. Here is the example from the docs which is relevant to test also:

resource "auth0_connection" "my_connection" {
  name = "Example-Connection"
  strategy = "auth0"
  options {
    password_policy = "excellent"
    password_history {
      enable = true
      size = 3
    }
    brute_force_protection = "true"
    enabled_database_customization = "true"
    custom_scripts = {
      get_user = <<EOF
function getByEmail (email, callback) {
  return callback(new Error("Whoops!"))
}
EOF
    }

    configuration = {
      foo = "bar"
      bar = "baz"
    }
  }
}

PS: This last issue was discovered on 0.6.0 version of auth0 provider. Because of time constraint I had to use a version that was more stable than the last one and this did the job if we ignore the problems with configuration not triggering a change in auth0.

@alexkappa
Copy link
Owner

I've also noticed(please let me know if I should open a new issue) on auth0_connection that regardless of what we change on configuration it's not triggering an update so the only way I was able to deploy a change was to destroy the connection resouce and recreate it.

Yes I think that is a separate issue.

@icadariu-talend
Copy link
Author

icadariu-talend commented Apr 6, 2020

Yes I think that is a separate issue.

Ok, opened #195

@alexkappa
Copy link
Owner

I'm curious, in the "Update" functions, why do we only send fields that have changed?

After playing around a little with trying to send all fields instead of those which have changed, I think I have an answer to this question.

The Auth0 API only offers PATCH as a method to modify resources. With these semantics, the resource is not replaced, but modified in-place (only the fields that were set).

Feel free to check the result of the prototype here: https://github.com/alexkappa/terraform-provider-auth0/pull/194/files .

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants