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

new resource: azurerm_spring_cloud_dynatrace_application_performance_monitoring #23889

Merged

Conversation

ms-henglu
Copy link
Contributor

image

@ms-henglu ms-henglu force-pushed the ticket-24187298-spring-cloud-apms-dynatrace branch from cd8d3e7 to 79a6208 Compare November 14, 2023 07:32
Copy link
Collaborator

@magodo magodo left a 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 👍

@jackofallops jackofallops self-assigned this Dec 5, 2023
ValidateFunc: validation.StringIsNotEmpty,
},

"tenant": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be

Suggested change
"tenant": {
"tenant_id": {

Copy link
Collaborator

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?

Copy link
Contributor Author

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.

Comment on lines 42 to 43
tenant_token = "example-tenant-token"
connection_point = "example-connection-endpoint"
Copy link
Collaborator

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,
Copy link
Collaborator

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?

Copy link
Contributor Author

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,
Copy link
Collaborator

Choose a reason for hiding this comment

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

and here

Copy link
Contributor Author

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,
Copy link
Collaborator

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,
Copy link
Collaborator

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?

Copy link
Contributor Author

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.

Copy link
Collaborator

@katbyte katbyte left a 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?

Comment on lines 165 to 166
environment_id = "test-environment-id"
tenant = "test-tenant"
Copy link
Collaborator

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"
Copy link
Collaborator

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"

Copy link
Collaborator

@katbyte katbyte left a 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?

@ms-henglu
Copy link
Contributor Author

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.

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM 🌴

@katbyte katbyte merged commit 48783ef into hashicorp:main Jan 15, 2024
33 checks passed
katbyte added a commit that referenced this pull request Jan 15, 2024
@github-actions github-actions bot added this to the v3.88.0 milestone Jan 15, 2024
dduportal pushed a commit to jenkins-infra/azure that referenced this pull request Jan 19, 2024
<Actions>
<action
id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8">
        <h3>Bump Terraform `azurerm` provider version</h3>
<details
id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24">
            <summary>Update Terraform lock file</summary>
<p>&#34;hashicorp/azurerm&#34; updated from &#34;3.87.0&#34; to
&#34;3.88.0&#34; in file &#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>3.88.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.88.0&#xA;FEATURES:&#xA;&#xA;*
New Data Source: `azurerm_nginx_deployment`
([#24492](https://github.com/hashicorp/terraform-provider-azurerm/issues/24492))&#xA;*
New Resource:
`azurerm_spring_cloud_dynatrace_application_performance_monitoring`
([#23889](https://github.com/hashicorp/terraform-provider-azurerm/issues/23889))&#xA;*
New Resource: `azurerm_virtual_machine_run_command`
([#23377](https://github.com/hashicorp/terraform-provider-azurerm/issues/23377))&#xA;&#xA;ENHANCEMENTS:&#xA;&#xA;*
dependencies: updating to `v0.20240117.1163544` of
`github.com/hashicorp/go-azure-sdk`
([#24481](https://github.com/hashicorp/terraform-provider-azurerm/issues/24481))&#xA;*
dependencies: updating to `v0.65.1` of
`github.com/hashicorp/go-azure-helpers`
([#24479](https://github.com/hashicorp/terraform-provider-azurerm/issues/24479))&#xA;*
`datashare`: updating to use the base layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest`
([#24481](https://github.com/hashicorp/terraform-provider-azurerm/issues/24481))&#xA;*
`kusto`: updating to use the base layer from `hashicorp/go-azure-sdk`
rather than `Azure/go-autorest`
([#24477](https://github.com/hashicorp/terraform-provider-azurerm/issues/24477))&#xA;*
Data Source: `azurerm_application_gateway` - support for the
`trusted_client_certificate.data` property
([#24474](https://github.com/hashicorp/terraform-provider-azurerm/issues/24474))&#xA;*
`azurerm_service_plan`: refactoring to use `hashicorp/go-azure-sdk`
([#24483](https://github.com/hashicorp/terraform-provider-azurerm/issues/24483))&#xA;*
`azurerm_container_group` - support for the `priority` property
([#24374](https://github.com/hashicorp/terraform-provider-azurerm/issues/24374))&#xA;*
`azurerm_mssql_managed_database` - support for the
`point_in_time_restore` property
([#24535](https://github.com/hashicorp/terraform-provider-azurerm/issues/24535))&#xA;*
`azurerm_mssql_managed_instance` - now exports the `dns_zone` attribute
([#24435](https://github.com/hashicorp/terraform-provider-azurerm/issues/24435))&#xA;*
`azurerm_linux_web_app_slot` - support for setting `python_version` to
`3.12`
([#24363](https://github.com/hashicorp/terraform-provider-azurerm/issues/24363))&#xA;*
`azurerm_linux_web_app` - support for setting `python_version` to `3.12`
([#24363](https://github.com/hashicorp/terraform-provider-azurerm/issues/24363))&#xA;*
`azurerm_linux_function_app_slot` - support for setting `python_version`
to `3.12`
([#24363](https://github.com/hashicorp/terraform-provider-azurerm/issues/24363))&#xA;*
`azurerm_linux_function_app` - support for setting `python_version` to
`3.12`
([#24363](https://github.com/hashicorp/terraform-provider-azurerm/issues/24363))&#xA;&#xA;BUG
FIXES:&#xA;&#xA;* `azurerm_application_gateway` - the `components`
property within the `url` block is no longer computed
([#24480](https://github.com/hashicorp/terraform-provider-azurerm/issues/24480))&#xA;*
`azurerm_cdn_frontdoor_route` - prevent an issue where
`cdn_frontdoor_origin_path` gets removed on update if unchanged.
([#24488](https://github.com/hashicorp/terraform-provider-azurerm/issues/24488))&#xA;*
`azurerm_cognitive_account` - fixing support for the `DC0` SKU
([#24526](https://github.com/hashicorp/terraform-provider-azurerm/issues/24526))&#xA;&#xA;&#xA;</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) <60776566+jenkins-infra-bot@users.noreply.github.com>
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 30, 2024
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.

4 participants