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

Allow empty values on certain fields #14

Closed
sergiught opened this issue Feb 21, 2022 · 3 comments
Closed

Allow empty values on certain fields #14

sergiught opened this issue Feb 21, 2022 · 3 comments
Assignees
Labels
🪲 bug Something isn't working

Comments

@sergiught
Copy link
Contributor

sergiught commented Feb 21, 2022

Describe the problem

I'm opening this issue to capture several related issues opened on https://github.com/alexkappa/terraform-provider-auth0.

Under the hood, this Provider uses the Auth0 Go SDK to make calls to the Auth0 Management API.

It turns out that several struct fields makes use of the omitempty json tag. This means that any fields that are empty or nil will be omitted from the request that is sent to the Management API.

The Management API returns this error only for POST and PUT requests, but not for PATCH requests.

On PATCH certain values need to be empty depending on use case.

Currently it’s not possible to do this through the provider. It is only possible through the dashboard or calling the API manually.

Related Issues

@sergiught sergiught added the 🪲 bug Something isn't working label Feb 21, 2022
@mbarrien
Copy link

One possible solution is switching the underlying Auth0 library to use Jettison for its JSON encoding, and use the omitnil tag instead of omitempty (which would omit nils, but keep empty arrays/strings). It is supposed to be mostly compatible with encoding/json. See wI2L/jettison@v0.6.0...v0.7.0. According to golang/go#34701 (comment) it's unlikely such a feature would get added to encoding/json, and there seems to be no way to patch the encoding.

The other option would be to change the underlying library to remove all omitempty, and either accept larger request payloads, or to manually filter out the empty fields in most cases except PATCH.

@sergiught
Copy link
Contributor Author

Hey folks, this is currently what our team is focusing on and a fix for this will be available shortly.

However for posterity we'll explain a bit better what exactly the issue was and the approach we took.

The main issue was found to be within the Terraform SDK and not the go-auth0 SDK.

Let's dive into it.


Can go-auth0 send empty values to the API?

For our analysis let’s consider we want to send a PATCH request to update a Client 's CustomLoginPage property to be an empty string "".

The property is defined as follows:

type Client struct {
  CustomLoginPage  *string  `json:"custom_login_page,omitempty"`
}

If we call the Update() func to update our client to have an empty custom login page, the client will get marshaled as follows:

customLoginPage := ""
client := &Client{CustomLoginPage: &customLoginPage}
management.Client.Update(client.GetID(), client)

We will notice that the following payload is sent to the API

{
  "custom_login_page": ""
}

So it seems we can actually send empty values to the API from the go-auth0 SDK.

Why?

To fully understand how this is possible let’s review what the omitempty JSON tag does.

When building JSON Structs we often have fields that are optional. To reduce the unnecessary transmission size in case we are calling some JSON/REST APIs these fields don’t need to be sent so we make use of the omitempty tag to omit the fields if they are empty.

In Go the empty value for a bool field is false, for int is 0, for a string is "" for structs, slices, interfaces and pointers the default is nil (full list https://golangbyexample.com/go-default-zero-value-all-types/).

Let’s review another example.

type Client struct {
    Name              string    `json:"name,omitempty"`
    AllowedLogoutURLs []string  `json:"allowed_logout_urls,omitempty"`
}

If we encode the following into JSON

client := &Client{Name: "", AllowedLogoutURLs: []string{}}

The payload created will look like {} as we’ve set empty values to both fields.

client := &Client{Name: "test"}

Will encode into {"name": "test"}.

What happens if instead we use pointer fields to types?

The empty value of a pointer is actually nil and if we set the pointer to anything else the type of the property will end up not being empty any longer and thus it won’t get omitted when using the omitempty tag. Let’s see this with an example:

type Client struct {
    Name              *string    `json:"name,omitempty"`
    AllowedLogoutURLs *[]string  `json:"allowed_logout_urls,omitempty"`
}

name := ""
allowedLogoutURLs := []string{}
client := &Client{Name: &name, AllowedLogoutURLs: &allowedLogoutURLs}

After encoding the client we get

{"name": "", "allowed_logout_urls": []}

Exactly what we want.

Here’s a short reproducible you can run and play with: https://go.dev/play/p/LhZL8oFqZDT.

But then what happens inside the terraform provider?

So we’ve established that we can actually send empty values using the go-auth0 SDK, but if we try to set a string schema field to an empty string for example it seems the value doesn’t get sent to the API.

The terraform provider makes use of the following helper func to set String data to the payload to be sent to the API

func String(d ResourceData, key string, conditions ...Condition) (s *string) {
	v, ok := d.GetOk(key)
	if ok && Any(conditions...).Eval(d, key) {
		s = auth0.String(v.(string))
	}
	return
}

And the problem is actually with the GetOk func from the terraform-plugin-sdk because the ok value returned by it to notify if the field contains any data will return false when the field is not set but also if it set to an empty go type value (string = ""). Because of this the conditional on line 3 evaluates always to false in such cases and thus the value of s returned is nil and the omitempty tag will strip it from the payload.

There are already relevant issues opened on the terraform-plugin-sdk that surface this:

Particular relevance to the following comment: hashicorp/terraform-plugin-sdk#261 (comment) where it is suggested to switch over to the new and not yet stable terraform-plugin-framework to get this fixed or use the experimental dependency introduced (cty pkg) that will give access to the raw terraform config.

https://discuss.hashicorp.com/t/differentiate-between-null-and-empty-value/8134/3

So how are we going to fix this?

We decided to add better typing into some of the go-auth0 SDK and use the go-cty package to differentiate between an attribute not present in the config, an attribute present but the default value of its type and the attribute being set to any value of the type.

@sergiught
Copy link
Contributor Author

This has been fixed in the latest release of the provider https://github.com/auth0/terraform-provider-auth0/releases/tag/v0.38.0. Closing this down now. Thanks for everyones patience! 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪲 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants