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.

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

Fix auth0 tenant flags so it only sends set values #144

Merged
merged 1 commit into from
Apr 22, 2022

Conversation

sergiught
Copy link
Contributor

@sergiught sergiught commented Apr 20, 2022

Description

Fixes: #137
Fixes: #127
Fixes: #63
Fixes #54

Fix auth0 tenant flags so it only sends set values.

Checklist

Note: Checklist required to be completed before a PR is considered to be reviewable.

Auth0 Code of Conduct

Auth0 General Contribution Guidelines

Changes include test coverage?

  • Yes
  • Not needed

Does the description provide the correct amount of context?

  • Yes, the description provides enough context for the reviewer to understand what these changes accomplish

Have you updated the documentation?

  • Yes, I've updated the appropriate docs
  • Not needed

Is this code ready for production?

  • Yes, all code changes are intentional and no debugging calls are left over

@sergiught sergiught self-assigned this Apr 20, 2022
@sergiught sergiught force-pushed the patch/tenant-flags branch from 32d963e to 9ba43e8 Compare April 20, 2022 14:13
func Flag(configFlags cty.Value, key string) *bool {
configFlag := configFlags.GetAttr(key)

if configFlag.IsNull() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Terraform Plugin SDK v2 uses github.com/hashicorp/go-cty/cty under the hood and because of that we are now able to distinguish between values that are set in the config (true/false) and nil values (not present in the config) correctly making sure we also omit them from the PATCH payload if not present in the config.

flags.EnablePublicSignupUserExistsError = Bool(d, "enable_public_signup_user_exists_error")
flags.UseScopeDescriptionsForConsent = Bool(d, "use_scope_descriptions_for_consent")
func expandTenantFlags(flagsList cty.Value) *management.TenantFlags {
var tenantFlags *management.TenantFlags
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By changing this to a pointer and only setting the flags if we have the flags list set in the terraform config, we won't force the flags to be sent as an empty object any more to avoid the 400 bad requests error we get from the API in case we don't have any flags sent within. This truly makes the flags now optional as the docs indicate.

@@ -257,13 +258,27 @@ func Float64(d ResourceData, key string, conditions ...Condition) (f *float64) {
// Bool accesses the value held by key and type asserts it to a pointer to a
// bool.
func Bool(d ResourceData, key string, conditions ...Condition) (b *bool) {
// We need to keep using this for bool values even if it's marked as deprecated.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue with the flags was that they were using GetOkExists to check if the bool value was set or not which actually defaults to false when the flag is not set forcing the entire payload to be sent as the flags are pointers to bool and omitempty will only omit nil values.

Even tho GetOkExists is deprecated in the sdkv2 we need to keep using it for all the other bool fields we have on other structs. See hashicorp/terraform-plugin-sdk#817 for more details.

@sergiught sergiught marked this pull request as ready for review April 20, 2022 14:24
@sergiught sergiught requested a review from a team as a code owner April 20, 2022 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment