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

Separate auth0_connection options blocks into provider-specific option blocks #234

Closed
reify-tanner-stirrat opened this issue Jul 12, 2022 · 3 comments
Labels
📚 documentation Improvements or additions to documentation resource/auth0_connection 🌱 feature New feature or request

Comments

@reify-tanner-stirrat
Copy link
Contributor

Describe the problem you'd like to have solved

When I go to the options documentation for this provider, literally every option is listed as "(Optional)." My sense is that this is because no one option is common to all providers, so they're all technically "optional" because there's only a single options block that's used to configure every different provider.

It'd be much nicer from a DX standpoint to have options blocks that have clues to which values are actually necessary for a given provider, and it'd be nice to be able to discover this through means other than going through the Management UI for adding a particular kind of connection and manually discovering which values are and aren't required.

Describe the ideal solution

Rather than a single options block:

options {
  foo = bar
  another_optional_thing = baz
}

I'd rather have top-level optional blocks, and then each of those optional blocks clearly delineate which values are and aren't optional:

# for an AAD connection
aad_options {
  client_id = "whatever"
}

# for a SAML connection
saml_options {
  signing_cert = "whatever"
}

Alternatives and current workarounds

Workarounds

I can find the characteristics of these values through manual inspection of the Management UI. I can also plan something and let it fail and 400 because the management API rejects things and iterate that way. Neither is very nice.

Alternatives

I'm not versed enough in terraform to know whether there are architectural alternatives to what I'm describing above.

Additional context

I wouldn't call myself an expert in Terraform, so if there are good reasons for the API being the way that it is currently, I'm game to hear them and then document them.

@sergiught sergiught added 📚 documentation Improvements or additions to documentation 🌱 feature New feature or request resource/auth0_connection labels Jul 13, 2022
@sergiught
Copy link
Contributor

Hey @reify-tanner-stirrat appreciate you opening this issue with us and for the detailed explanation.

I reflect your sentiment regarding the connection resource. It's causing quite a bit of pain for us as well and definitely not the best DX. Between passwordless, social, enterprise etc. connections we probably have around 50+ different strategy types.

Some options are common among multiple connection types and others are specific just to SAML for example. If we'd end up having a specific options block for each connection type we'd be left with 50+ different blocks that have quite a big amount of duplication between them.

You could argue that most of the social ones are simply OAuth2 connection types, so that would reduce everything by a lot. This is something we've been discussing internally recently, but we need to do a bit more extensive research on it.

What we can do in the meantime is improve the docs and add more safeguards using the RequiredWith and ConflictsWith fields on the schema. What we would need is something like "require when field = X", but as far as I know (hoping to get corrected here) the terraform SDK doesn't offer this at the moment cuz then based on the strategy value we could say that the following fields are required.

As always any suggestions/ideas/contributions towards improving the DX are more than welcome so please keep them coming:)

@reify-tanner-stirrat
Copy link
Contributor Author

we'd be left with 50+ different blocks that have quite a big amount of duplication between them

Is the concern that this would make the provider (i.e. the backing code) hard to maintain? If there are ways to share the definitions between different kinds of blocks on the backend, that still seems like a sane tradeoff.

I agree that I wouldn't want to have to maintain all of the duplicated definitions separately.

@sergiught
Copy link
Contributor

Hey @reify-tanner-stirrat as mentioned in #238 (comment) as well, a v3 of the API is on the roadmap and the connections resource will be reworked. As we aim at being as close as possible with the behavior of the API we'll defer this kind of work for then. As such we'll close this issue down, but rest assured feedback has been taken into account and is highly appreciated!

In the meantime we'll improve our documentation to specify in the examples all the fields that can be set for each type of connection and as well attach strategy options on the options fields in https://registry.terraform.io/providers/auth0/auth0/latest/docs/resources/connection#nestedblock--options.

I created #365 to focus just on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 documentation Improvements or additions to documentation resource/auth0_connection 🌱 feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants