-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 azurerm_nginx_certificate
data source
#24577
Conversation
puneetsarna
commented
Jan 22, 2024
b7e20b7
to
29ebfd5
Compare
29ebfd5
to
61908ef
Compare
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 @puneetsarna.
I left a few minor comments that should be fixed up for consistency across the provider. Once that's done we can run the tests and this should be good to go!
} | ||
deploymentId, err := nginxdeployment.ParseNginxDeploymentID(model.NginxDeploymentId) | ||
if err != nil { | ||
return fmt.Errorf("error parsing NGINX deployment ID %s: %+v", deploymentId, err) |
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.
The error returned by the parsing function already contains the information we need. We don't need to wrap it further.
return fmt.Errorf("error parsing NGINX deployment ID %s: %+v", deploymentId, err) | |
return err |
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.
👍 made the change.
if response.WasNotFound(result.HttpResponse) { | ||
return fmt.Errorf("%s was not found", id) | ||
} | ||
return fmt.Errorf("reading %s: %+v", id, err) |
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.
return fmt.Errorf("reading %s: %+v", id, err) | |
return fmt.Errorf("retrieving %s: %+v", id, err) |
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.
done 👍
output.KeyVirtualPath = pointer.ToString(prop.KeyVirtualPath) | ||
output.KeyVaultSecretId = pointer.ToString(prop.KeyVaultSecretId) | ||
output.CertificateVirtualPath = pointer.ToString(prop.CertificateVirtualPath) |
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.
We should be using the generic pointer.From
here and not .ToString
output.KeyVirtualPath = pointer.ToString(prop.KeyVirtualPath) | |
output.KeyVaultSecretId = pointer.ToString(prop.KeyVaultSecretId) | |
output.CertificateVirtualPath = pointer.ToString(prop.CertificateVirtualPath) | |
output.KeyVirtualPath = pointer.From(prop.KeyVirtualPath) | |
output.KeyVaultSecretId = pointer.From(prop.KeyVaultSecretId) | |
output.CertificateVirtualPath = pointer.From(prop.CertificateVirtualPath) |
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.
👍 made the change.
|
||
* `id` - The ID of the Nginx Certificate. | ||
|
||
* `certificate_virtual_path` - The path to the cert file of this certificate. |
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.
* `certificate_virtual_path` - The path to the cert file of this certificate. | |
* `certificate_virtual_path` - The path to the certificate file of this certificate. |
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.
👍 made the change.
61908ef
to
3f0cb47
Compare
@stephybun Thank you for the review. I have made the suggested changes and have rerun the acceptance test as well. |
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 @puneetsarna LGTM 🍕
This functionality has been released in v3.89.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
<Actions> <action id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>changes detected:
	"hashicorp/azurerm" updated from "3.88.0" to "3.89.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.89.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.89.0
FEATURES:

* New Data Source: `azurerm_data_factory_trigger_schedule` ([#24572](hashicorp/terraform-provider-azurerm#24572 New Data Source: `azurerm_data_factory_trigger_schedules` ([#24572](hashicorp/terraform-provider-azurerm#24572 New Data Source: `azurerm_ip_groups` ([#24540](hashicorp/terraform-provider-azurerm#24540 New Data Source: `azurerm_nginx_certificate` ([#24577](hashicorp/terraform-provider-azurerm#24577 New Resource: `azurerm_chaos_studio_target` ([#24580](hashicorp/terraform-provider-azurerm#24580 New Resource: `azurerm_elastic_san_volume_group` ([#24166](hashicorp/terraform-provider-azurerm#24166 New Resource: `azurerm_netapp_account_encryption` ([#23733](hashicorp/terraform-provider-azurerm#23733 New Resource: `azurerm_redhat_openshift_cluster` ([#24375](https://github.com/hashicorp/terraform-provider-azurerm/issues/24375))

ENHANCEMENTS:

* dependencies: updating to `v0.66.1` of `github.com/hashicorp/go-azure-helpers` ([#24561](hashicorp/terraform-provider-azurerm#24561 dependencies: updating to `v0.20240124.1115501` of `github.com/hashicorp/go-azure-sdk` ([#24619](hashicorp/terraform-provider-azurerm#24619 `bot`: updating to API Version `2021-05-01-preview` ([#24555](hashicorp/terraform-provider-azurerm#24555 `containerservice`: the SDK Clients now support logging ([#24564](hashicorp/terraform-provider-azurerm#24564 `cosmosdb`: updating to API Version `2023-04-15` ([#24541](hashicorp/terraform-provider-azurerm#24541 `loadtestservice`: updating to use the base layer from `hashicorp/go-azure-sdk` rather than `Azure/go-autorest` (and support logging) ([#24578](hashicorp/terraform-provider-azurerm#24578 `managedidentity`: updating to use the base layer from `hashicorp/go-azure-sdk` rather than `Azure/go-autorest` (and support logging) ([#24578](hashicorp/terraform-provider-azurerm#24578 `azurerm_api_management_api` - change the `id` format so specific `revision`s can be managed by Terraform ([#23031](hashicorp/terraform-provider-azurerm#23031 `azurerm_data_protection_backup_vault` - the `redundancy` propety can now be set to `ZoneRedundant` ([#24556](hashicorp/terraform-provider-azurerm#24556 `azurerm_data_factory_integration_runtime_azure_ssis` - support for the `credential_name` property ([#24458](hashicorp/terraform-provider-azurerm#24458 `azurerm_orchestrated_virtual_machine_scale_set` - support `2022-datacenter-azure-edition-hotpatch` and `2022-datacenter-azure-edition-hotpatch-smalldisk` hotpatching images ([#23500](hashicorp/terraform-provider-azurerm#23500 `azurerm_stream_analytics_job` - support for the `sku_name` property ([#24554](https://github.com/hashicorp/terraform-provider-azurerm/issues/24554))

BUG FIXES:

* Data Source: `azurerm_app_service` - parsing the API Response for `app_service_plan_id` case-insensitively ([#24626](hashicorp/terraform-provider-azurerm#24626 Data Source: `azurerm_function_app` - parsing the API Response for `app_service_plan_id` case-insensitively ([#24626](hashicorp/terraform-provider-azurerm#24626 `azurerm_app_configuration_key` - the value for the `value` property can now be removed/emptied ([#24582](https://github.com/hashicorp/terraform-provider-azurerm/issues/24582))

* `azurerm_app_service` - parsing the API Response for `app_service_plan_id` case-insensitively ([#24626](hashicorp/terraform-provider-azurerm#24626 `azurerm_app_service_plan` - fix casing in `serverFarms` due to ID update ([#24562](hashicorp/terraform-provider-azurerm#24562 `azurerm_app_service_slot` - parsing the API Response for `app_service_plan_id` case-insensitively ([#24626](hashicorp/terraform-provider-azurerm#24626 `azurerm_automation_schedule` - only one `monthly_occurence` block can now be specified ([#24614](hashicorp/terraform-provider-azurerm#24614 `azurerm_cognitive_deployment` - the `model.version` property is no longer required ([#24264](hashicorp/terraform-provider-azurerm#24264 `azurerm_container_app` - multiple `custom_scale_rule` can not be updated ([#24509](hashicorp/terraform-provider-azurerm#24509 `azurerm_container_registry_task_schedule_run_now` - prevent issue where the incorrect scheduled run in tracked if there have been multiple ([#24592](hashicorp/terraform-provider-azurerm#24592 `azurerm_function_app` - parsing the API Response for `app_service_plan_id` case-insensitively ([#24626](hashicorp/terraform-provider-azurerm#24626 `azurerm_function_app_slot` - parsing the API Response for `app_service_plan_id` case-insensitively ([#24626](hashicorp/terraform-provider-azurerm#24626 `azurerm_logic_app_standard` - now will parse the app service ID insensitively ([#24562](hashicorp/terraform-provider-azurerm#24562 `azurerm_logic_app_workflow` - the `workflow_parameters` will now correctly handle information specified by `$connections` ([#24141](hashicorp/terraform-provider-azurerm#24141 `azurerm_mssql_managed_instance_security_alert_policy` - can not update empty storage attributes ([#24553](hashicorp/terraform-provider-azurerm#24553 `azurerm_network_interface` - the `ip_configuration` properties are no longer added to a Load Balancer Backend if one of those `ip_configurations` is associated with a backend ([#24470](https://github.com/hashicorp/terraform-provider-azurerm/issues/24470))


</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/terraform-jobs/job/azure/job/main/1052/">Jenkins pipeline link</a> </action> </Actions> --- <table> <tr> <td width="77"> <img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli logo" width="50" height="50"> </td> <td> <p> Created automatically by <a href="https://www.updatecli.io/">Updatecli</a> </p> <details><summary>Options:</summary> <br /> <p>Most of Updatecli configuration is done via <a href="https://www.updatecli.io/docs/prologue/quick-start/">its manifest(s)</a>.</p> <ul> <li>If you close this pull request, Updatecli will automatically reopen it, the next time it runs.</li> <li>If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.</li> </ul> <p> Feel free to report any issues at <a href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br /> If you find this tool useful, do not hesitate to star <a href="https://github.com/updatecli/updatecli/stargazers">our GitHub repository</a> as a sign of appreciation, and/or to tell us directly on our <a href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>! </p> </details> </td> </tr> </table> Co-authored-by: Jenkins Infra Bot (updatecli) <[email protected]> Co-authored-by: Damien Duportal <[email protected]>
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. |