-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
New Data Source: azurerm_automation_variables
#22216
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR @oWretch.
I have a few suggestions and questions which need looking over. Once those are resolved I can proceed with my review.
I also wanted to call out that all new resources and data sources should be typed (this was copied from an untyped data source within the provider). We have some contributor docs on what that means here, there are also guides in that folder on how to write typed resources/data sources.
For the moment this is fine, but just something to keep in mind for any future resources/data sources you choose to contribute here - having them typed would help us out a lot!
internal/services/automation/automation_variables_data_source.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_variables_data_source.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_variables_data_source.go
Outdated
Show resolved
Hide resolved
internal/services/automation/automation_variables_data_source.go
Outdated
Show resolved
Hide resolved
}, | ||
}, | ||
|
||
"null": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What variable type is this? I don't see a resource for this variable type - so in what cases would this be populated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Microsoft offer a null
type variable in an automation account. Currently there is no TF resource for it, but it can be populated from the Azure portal. I added to this data source to try and get all the different variables that might be created available.
If we want consistency, I could add a resource for the null variable type so we can create them in TF.
Co-authored-by: stephybun <steph@hashicorp.com>
Updated based on comments.
I'll try to update to be a typed source… |
@stephybun I've converted to a typed data source now too
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for converting this to a typed resource @oWretch! LGTM 🦆
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Create a data resource for fetching all the variables in an automation account. Modelled after
azurerm_key_vault_secrets
.--- PASS: TestAccDataSourceAzureRMAutomationVariables_basic (143.66s)