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

add auth0_client and auth0_global_client data sources and tests #363

Merged
merged 4 commits into from
Apr 14, 2021

Conversation

yinzara
Copy link
Contributor

@yinzara yinzara commented Apr 3, 2021

Proposed Changes

  • Add "auth0_client" data source
  • Add "auth0_global_client" data source

Fixes #279

Acceptance Test Output

$ make testacc TESTS=TestAccData
==> Checking that code complies with gofmt requirements...
?       github.com/alexkappa/terraform-provider-auth0   [no test files]
=== RUN   TestAccDataClientByName
--- PASS: TestAccDataClientByName (3.94s)
=== RUN   TestAccDataClientById
--- PASS: TestAccDataClientById (9.42s)
=== RUN   TestAccDataGlobalClient
--- PASS: TestAccDataGlobalClient (2.92s)
PASS
coverage: 20.6% of statements
ok      github.com/alexkappa/terraform-provider-auth0/auth0     16.549s coverage: 20.6% of statements
?       github.com/alexkappa/terraform-provider-auth0/auth0/internal/debug      [no test files]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok      github.com/alexkappa/terraform-provider-auth0/auth0/internal/random     0.233s  coverage: 0.0% of statements [no tests to run]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok      github.com/alexkappa/terraform-provider-auth0/auth0/internal/validation 0.367s  coverage: 0.0% of statements [no tests to run]
?       github.com/alexkappa/terraform-provider-auth0/version   [no test files]

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

@yvovandoorn
Copy link
Contributor

Hi @yinzara! Thank you for this contribution.

I think the PR looks good, but do you mind changing it from data_<resource> to data_source_<resource>, to match (for example) the AWS provider (e.g. https://github.com/hashicorp/terraform-provider-aws/blob/main/aws/data_source_aws_acm_certificate.go).

@yinzara
Copy link
Contributor Author

yinzara commented Apr 7, 2021

Absolutely, I'll submit an update as soon as I'm available.

@yinzara yinzara changed the title add auth_client and auth0_global_client data sources and tests add auth0_client and auth0_global_client data sources and tests Apr 7, 2021
@yvovandoorn
Copy link
Contributor

One more thing that is missing is if you can add a bit of docs about this data source!

Another PR, #281, has a bit of documentation (https://github.com/alexkappa/terraform-provider-auth0/blob/7f443aca26b23b7bf5d89d71799679ffc00bf7c2/docs/datasources/user.md). Would be great to get that here as well so that the two PRs that start providing data sources have supporting documentation.

Thanks again @yinzara for your contribution!

Copy link
Owner

@alexkappa alexkappa left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @yinzara!

The only thing I would suggest is to follow some of the practices found in other providers around sharing schemas between resources and data sources. Many providers re-define the entire schema for data sources which I find repetitive and tedious. Others, however, like the google-beta provider may have some practices we can borrow.

For example:

@yinzara
Copy link
Contributor Author

yinzara commented Apr 12, 2021

Funny you mention that, I actually created makeComputed for this purpose, but datasourceSchemaFromResourceSchema is even better so I just copied it exactly from google.

Since their project is also Mozilla v2 licensed, there is no reason we can't do that as don't even need to include the license.

I've also added the docs requested and missing docs for the auth0_global_client and auth0_log_stream resources and the new "secrets" attributes of hooks.

Here is the result of rerunning the acceptance tests:

make testacc TESTS=TestAccData
==> Checking that code complies with gofmt requirements...
?       github.com/alexkappa/terraform-provider-auth0   [no test files]
=== RUN   TestAccDataClientByName
--- PASS: TestAccDataClientByName (3.95s)
=== RUN   TestAccDataClientById
--- PASS: TestAccDataClientById (9.49s)
=== RUN   TestAccDataGlobalClient
--- PASS: TestAccDataGlobalClient (2.95s)
PASS
coverage: 20.6% of statements
ok      github.com/alexkappa/terraform-provider-auth0/auth0     16.655s coverage: 20.6% of statements
?       github.com/alexkappa/terraform-provider-auth0/auth0/internal/debug      [no test files]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok      github.com/alexkappa/terraform-provider-auth0/auth0/internal/random     0.226s  coverage: 0.0% of statements [no tests to run]
testing: warning: no tests to run
PASS
coverage: 0.0% of statements
ok      github.com/alexkappa/terraform-provider-auth0/auth0/internal/validation 0.147s  coverage: 0.0% of statements [no tests to run]
?       github.com/alexkappa/terraform-provider-auth0/version   [no test files]

@yvovandoorn
Copy link
Contributor

Hi @yinzara, unfortunately, while passing locally for me in testing at first. Upon merge, it is epicly failing on expecting custom_login_page_on being a different value. I quickly took a look at the test and I think its missing an include string in the test.

For now I've reverted it.

@yvovandoorn
Copy link
Contributor

Hi @yinzara do you mind submitting the updated test to re-open this PR (or submit a new one?)

Thanks!

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.

Support Terraform data sources
3 participants