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

DXCDT-20: Client data source #511

Merged
merged 7 commits into from
Feb 4, 2022
Merged

Conversation

willvedd
Copy link
Collaborator

@willvedd willvedd commented Feb 3, 2022

Proposed Changes

Reintroducing the client data source that was originally introduced by #363 by @yinzara. This PR however, it more focused in that it only pertains to the client data source. This PR can serve as a template for future data source contributions.

I've identified a couple points of interest below, mostly around the exclusion of certain properties and the testing inefficiencies.

Acceptance Test Output

$ make testacc TESTS=TestAccXXX

=== RUN   TestAccDataClientById
--- PASS: TestAccDataClientById (4.30s)
=== RUN   TestAccDataClientByName
--- PASS: TestAccDataClientByName (4.87s)
...

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request


func newDataClient() *schema.Resource {
clientSchema := datasourceSchemaFromResourceSchema(newClient().Schema)
delete(clientSchema, "client_secret_rotation_trigger")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We're currently excluding the client_secret_rotation_trigger property when building from the client resource schema. This was proposed in #363 and I'm assuming that the exclusion is still valid but need to verify that no other properties should also be excluded.

Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrSet("data.auth0_client.test", "client_id"),
resource.TestCheckResourceAttr("data.auth0_client.test", "name", fmt.Sprintf("Acceptance Test - %v", rand)),
resource.TestCheckResourceAttr("data.auth0_client.test", "app_type", "non_interactive"),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The check for against the app_type property is only arbitrary and a naive way of checking that properties outside the ID and name exist on the schema.

PR #363 introduced a more comprehensive method of comparing the data source against the resource configuration, but I think that functionality needs to be looked at closer before we fully integrate.

Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrSet("data.auth0_client.test", "id"),
resource.TestCheckResourceAttrSet("data.auth0_client.test", "name"),
resource.TestCheckNoResourceAttr("data.auth0_client.test", "client_secret_rotation_trigger"),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just checking for the absence of this particular property because we explicitly excluded it from the schema definition.


## Attribute Reference

The client data source possesses the same attributes as the `auth0_client` resource, with the exception of `client_secret_rotation_trigger`. Refer to the [auth0_client resource documentation](../resources/client.md) for a list of returned attributes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This seems more reasonable than just duplicating the properties again, it'll just become more of a maintenance liability. However, I'm unsure if this will interfere with other documentation, for example the TF registry docs.

Copy link
Collaborator

@sergiught sergiught left a comment

Choose a reason for hiding this comment

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

Looking good 👍🏻 just one small comment.

Also as a suggestion but not necessarily for now we could make use of wiremock to skip having to create the resource in the future.

auth0/data_source_auth0_client_test.go Outdated Show resolved Hide resolved
@willvedd willvedd merged commit 67c9f64 into master Feb 4, 2022
@willvedd willvedd deleted the DXCDT-20-client-data-source branch February 4, 2022 21:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants