-
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
new resource: azurerm_spring_cloud_dynatrace_application_performance_monitoring
#23889
new resource: azurerm_spring_cloud_dynatrace_application_performance_monitoring
#23889
Conversation
ms-henglu
commented
Nov 14, 2023
cd8d3e7
to
79a6208
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 PR!
I've taken a look through and left some comments inline, but this is mostly looking good to me 👍
website/docs/r/spring_cloud_dynatrace_application_performance_monitoring.html.markdown
Outdated
Show resolved
Hide resolved
website/docs/r/spring_cloud_dynatrace_application_performance_monitoring.html.markdown
Outdated
Show resolved
Hide resolved
website/docs/r/spring_cloud_dynatrace_application_performance_monitoring.html.markdown
Show resolved
Hide resolved
...vices/springcloud/spring_cloud_dynatrace_application_performance_monitoring_resource_test.go
Outdated
Show resolved
Hide resolved
...vices/springcloud/spring_cloud_dynatrace_application_performance_monitoring_resource_test.go
Show resolved
Hide resolved
...l/services/springcloud/spring_cloud_dynatrace_application_performance_monitoring_resource.go
Show resolved
Hide resolved
...l/services/springcloud/spring_cloud_dynatrace_application_performance_monitoring_resource.go
Outdated
Show resolved
Hide resolved
...l/services/springcloud/spring_cloud_dynatrace_application_performance_monitoring_resource.go
Show resolved
Hide resolved
...l/services/springcloud/spring_cloud_dynatrace_application_performance_monitoring_resource.go
Show resolved
Hide resolved
...l/services/springcloud/spring_cloud_dynatrace_application_performance_monitoring_resource.go
Show resolved
Hide resolved
ValidateFunc: validation.StringIsNotEmpty, | ||
}, | ||
|
||
"tenant": { |
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.
should this be
"tenant": { | |
"tenant_id": { |
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.
or is this different then a standard azure tennant id?
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.
Yes, this is not a standard azure tenant id.
tenant_token = "example-tenant-token" | ||
connection_point = "example-connection-endpoint" |
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.
can you use examples that are more realistic here?
Type: pluginsdk.TypeString, | ||
Required: true, | ||
Sensitive: true, | ||
ValidateFunc: validation.StringIsNotEmpty, |
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.
can we do a better job validating this?
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.
I'm afraid that we can't
Type: pluginsdk.TypeString, | ||
Required: true, | ||
Sensitive: true, | ||
ValidateFunc: validation.StringIsNotEmpty, |
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.
and here
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.
This is the token used to authenticated, so we can't validate it.
"api_url": { | ||
Type: pluginsdk.TypeString, | ||
Optional: true, | ||
ValidateFunc: validation.StringIsNotEmpty, |
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.
this should be validate as a url?
Type: pluginsdk.TypeString, | ||
Optional: true, | ||
Sensitive: true, | ||
ValidateFunc: validation.StringIsNotEmpty, |
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.
can we properly validate this?
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.
This is a token used to authenticated, so we can't validate it.
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.
Could we also apply the more realitic values to the docs?
environment_id = "test-environment-id" | ||
tenant = "test-tenant" |
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 about these values?
@@ -161,10 +161,10 @@ resource "azurerm_spring_cloud_dynatrace_application_performance_monitoring" "te | |||
spring_cloud_service_id = azurerm_spring_cloud_service.test.id | |||
globally_enabled = true | |||
api_url = "https://test-api-url.com" | |||
api_token = "test-api-token" | |||
api_token = "dt0s01.ST2EY72KQINMH574WMNVI7YN.G3DFPBEJYMODIDAEX454M7YWBUVEFOWKPRVMWFASS64NFH52PX6BNDVFFM572RZM" |
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.
this might be better as
api_token = "dt0s01.AAAAAAAAAAAAAAAAAAAAAAAA.BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"
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.
Could we also apply the more realitic values to the docs?
Hi @katbyte , Thanks for the code review! I've updated the tests and doc as suggested. But for the unchanged fields, the realistic values could be random string. |
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, LGTM 🌴
<Actions> <action id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>"hashicorp/azurerm" updated from "3.87.0" to "3.88.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.88.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.88.0
FEATURES:

* New Data Source: `azurerm_nginx_deployment` ([#24492](hashicorp/terraform-provider-azurerm#24492 New Resource: `azurerm_spring_cloud_dynatrace_application_performance_monitoring` ([#23889](hashicorp/terraform-provider-azurerm#23889 New Resource: `azurerm_virtual_machine_run_command` ([#23377](https://github.com/hashicorp/terraform-provider-azurerm/issues/23377))

ENHANCEMENTS:

* dependencies: updating to `v0.20240117.1163544` of `github.com/hashicorp/go-azure-sdk` ([#24481](hashicorp/terraform-provider-azurerm#24481 dependencies: updating to `v0.65.1` of `github.com/hashicorp/go-azure-helpers` ([#24479](hashicorp/terraform-provider-azurerm#24479 `datashare`: updating to use the base layer from `hashicorp/go-azure-sdk` rather than `Azure/go-autorest` ([#24481](hashicorp/terraform-provider-azurerm#24481 `kusto`: updating to use the base layer from `hashicorp/go-azure-sdk` rather than `Azure/go-autorest` ([#24477](hashicorp/terraform-provider-azurerm#24477 Data Source: `azurerm_application_gateway` - support for the `trusted_client_certificate.data` property ([#24474](hashicorp/terraform-provider-azurerm#24474 `azurerm_service_plan`: refactoring to use `hashicorp/go-azure-sdk` ([#24483](hashicorp/terraform-provider-azurerm#24483 `azurerm_container_group` - support for the `priority` property ([#24374](hashicorp/terraform-provider-azurerm#24374 `azurerm_mssql_managed_database` - support for the `point_in_time_restore` property ([#24535](hashicorp/terraform-provider-azurerm#24535 `azurerm_mssql_managed_instance` - now exports the `dns_zone` attribute ([#24435](hashicorp/terraform-provider-azurerm#24435 `azurerm_linux_web_app_slot` - support for setting `python_version` to `3.12` ([#24363](hashicorp/terraform-provider-azurerm#24363 `azurerm_linux_web_app` - support for setting `python_version` to `3.12` ([#24363](hashicorp/terraform-provider-azurerm#24363 `azurerm_linux_function_app_slot` - support for setting `python_version` to `3.12` ([#24363](hashicorp/terraform-provider-azurerm#24363 `azurerm_linux_function_app` - support for setting `python_version` to `3.12` ([#24363](https://github.com/hashicorp/terraform-provider-azurerm/issues/24363))

BUG FIXES:

* `azurerm_application_gateway` - the `components` property within the `url` block is no longer computed ([#24480](hashicorp/terraform-provider-azurerm#24480 `azurerm_cdn_frontdoor_route` - prevent an issue where `cdn_frontdoor_origin_path` gets removed on update if unchanged. ([#24488](hashicorp/terraform-provider-azurerm#24488 `azurerm_cognitive_account` - fixing support for the `DC0` SKU ([#24526](https://github.com/hashicorp/terraform-provider-azurerm/issues/24526))


</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/terraform-jobs/job/azure/job/main/1017/">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]>
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. |