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

Add cloudflare_account_roles data source #1238

Conversation

ouranos
Copy link
Contributor

@ouranos ouranos commented Oct 7, 2021

Closes #1161

I'm not experienced with Go and didn't want to run make testacc against our account so I'm not too sure about the tests.
However, I've tested with the terraform snippet I used in the documentation and it's working as expected.

I haven't mapped the permissions field but this would be enough for the use case mentioned in #1161.

@jacobbednarz
Copy link
Member

solid start! can you please also:

ouranos and others added 2 commits October 8, 2021 10:21
Co-authored-by: Jacob Bednarz <jacob.bednarz@hey.com>
@ouranos
Copy link
Contributor Author

ouranos commented Oct 7, 2021

Applied feedback. Let me know if I need to do anything else and if you want me to squash the commits.

@@ -25,6 +25,9 @@
<li<%= sidebar_current("docs-cloudflare-datasource") %>>
<a href="#">Data Sources</a>
<ul class="nav nav-visible">
<li<%= sidebar_current("docs-cloudflare-account-roles") %>>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<li<%= sidebar_current("docs-cloudflare-account-roles") %>>
<li<%= sidebar_current("docs-cloudflare-datasource-account-roles") %>>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this line also incorrect then sidebar_current("docs-cloudflare-api-token-permission-groups") ?

Copy link
Member

Choose a reason for hiding this comment

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

no, the value in sidebar_current() needs to match the sidebar_current value in the YAML which this case does. they match and work but they should include datasource somewhere in the value but you don't have to worry about fixing that here.

@ouranos
Copy link
Contributor Author

ouranos commented Oct 8, 2021

Let me know if I need to do anything else.

@jacobbednarz
Copy link
Member

this looks good! acceptance tests are all passing locally!

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./...) -v -run "^TestAccCloudflareAccountRoles" -count 1 -parallel 1 -timeout 120m -parallel 1
?   	github.com/cloudflare/terraform-provider-cloudflare	[no test files]
=== RUN   TestAccCloudflareAccountRoles
--- PASS: TestAccCloudflareAccountRoles (10.55s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/cloudflare	11.069s
?   	github.com/cloudflare/terraform-provider-cloudflare/version	[no test files

@jacobbednarz jacobbednarz merged commit dd1837f into cloudflare:master Oct 8, 2021
@jacobbednarz
Copy link
Member

solid effort on this one! thanks for your help getting this one over the line. 👏

@ouranos ouranos deleted the add-support-for-account-roles-data-source branch March 14, 2023 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for cloudflare_account_roles Data Source
2 participants