alekc / terraform-provider-auth0

Mozilla Public License 2.0
18 stars 7 forks source link

Add better support for managing client connections. #21

Open mvanderlee opened 2 years ago

mvanderlee commented 2 years ago

Description

Auth0 links clients and connections on the connection object, which makes it very difficult to add and remove a single client from a connection. We should add a way to link a client and connection. Something similar to AWS's role policy attachment: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/iam_role_policy_attachment

New or Affected Resource(s)

Potential Terraform Configuration

resource "auth0_client_connection" "this" {
  client_id = "obvious_fake"
  connection_id = "con_fake"
}

References

Community Note

alekc commented 2 years ago

Hi, thanks for opening this. Just to be sure that we are talking about the same thing:

Currently, if we want to link connection and client, we have to use the property enabled_clients, which is declared as following:

"enabled_clients": {
        Type:        schema.TypeSet,
        Elem:        &schema.Schema{Type: schema.TypeString},
        Optional:    true,
        Computed:    true,
        Description: "IDs of the clients for which the connection is enabled",
    },

as the things are right now, in theory we can add an additional object creating such link, and it would update the enabled clients (as long as it's empty).

But if we leave the optional: true and computed: true, due to limitations of the terraform, once you set that field to an empty value, terraform won't patch it since it will assume that the field is computed.

I have a v2 version coming soon (due to breaking changes which will be introduced on auth_client (i.e. https://github.com/alekc/terraform-provider-auth0/issues/20 and also dropping the options and separating them into individual entities like I did on the email), where I'd rather set enabled_clients as explicit and not computed value. That way we would have a stronger IAC implementation.

If we were to introduce a datasource for the clients so that you can easily find and reference them here, would that alleviate the issue?

mvanderlee commented 2 years ago

I think we're talking about different use-cases?

My use case is as follows. Our product is a multi-tenanted SAAS offering. We have 2 Auth0 Connections, 'dev' and 'prod'. To create a new tenant, we're setting up a number of resources that need single sign on via Auth0. So I create an Auth0 Client per application per tenant. i.e.: 'Kibana_Tenant1'. When I run terraform apply it should add 'Kibana_Tenant1' to the Auth0 Connection. When I run terraform destroy it should remove 'Kibana_Tenant1' from the Auth0 Connection.

I don't want the tenant module to be able to destroy or modify any other fields of the connection. It should only add or remove itself to the enabled_clients field. So I don't think that a datasource will do the trick. It should be a separate resources.

mvanderlee commented 2 years ago

I suppose that when you destroy a client, it would automatically be detached from a connection. So that's a bad example. A better example is when we'd want to attach to a different connection. Because now we'd have to remove the existing attachment, then attach the client to the new connection.

alekc commented 2 years ago

Ok, maybe I am beginning to get what you are aiming for (apologies, it looks like my brain is a bit fried today :p)

so: what you want is to be able to perform such connections without necessarily having to deal with the root resource (i.e. create auth0_client_connection by hand on the webui) and be able to link it without having to manage the connection resource from the same terraform stack. Is that right?

I am not opposed to the idea (if I got it right), my only concern is that my intention for the v2 was to remove "computed: true" from the connection.

If the connection resource is not inside the same terraform stack, it's not a problem, however if they are, there might be a "race condition" where 2 resources trying to achieve different actions on the same object.

I guess we can solve it in the same way as aws provider (by stating _For a given role, this resource is incompatible with using the aws_iam_role resource managed_policyarns argument. When using that argument and this resource, both will attempt to manage the role's managed policy attachments and Terraform will show a permanent difference.)

I have mostly done with the refactoring of the connection object, once that ticket is closed we can see how to integrate the link.

mvanderlee commented 2 years ago

You got it.

And agreed on the race condition concern. I think the documentation should also mention that if it's used with this new resource, we recommend ignoring lifecycle changes to the enabled_clients attribute. https://www.terraform.io/docs/language/meta-arguments/lifecycle.html#ignore_changes

alekc commented 2 years ago

And I see that you have found the biggest underlying issue: https://github.com/go-auth0/auth0/issues/241 where we cannot unset a value :/

For our purposes (terraform provider), I think it would be safe enough to remove the omitempty field since we are sending off pretty much everything. I will have a look at that once I am done with my current ticket

btilford commented 2 years ago

Would this solve the scenario where you have internal clients managed with Terraform and external ones managed through another application which results with any client not being managed by Terraform being removed from the connection every time Terraform runs?

mvanderlee commented 2 years ago

@btilford yes. I've already used the source branch in PR #22 in production to do exactly that.