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

azurerm_virtual_machine_data_disk_attachment: Ignore VM applications #24145

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

jkroepke
Copy link
Contributor

@jkroepke jkroepke commented Dec 7, 2023

If a Virtual Machine has VM application configured from a different subscription, azurerm_virtual_machine_data_disk_attachment can result into a RBAC permission error, if the current SPN does not have read permissions of the VM application

The issue appears, because azurerm_virtual_machine_data_disk_attachment fully override the whole VM resource instead partially patch disk block only.

To mitigate the issue, gallery_application will be set to nil before updating the VM resource. The pattern is already used for the identity property.

% make acctests SERVICE=compute TESTARGS='-run=TestAccVirtualMachineDataDiskAttachment_virtualMachineApplication' TESTTIMEOUT='60m'
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./internal/services/compute -run=TestAccVirtualMachineDataDiskAttachment_virtualMachineApplication -timeout 60m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccVirtualMachineDataDiskAttachment_virtualMachineApplication
=== PAUSE TestAccVirtualMachineDataDiskAttachment_virtualMachineApplication
=== CONT  TestAccVirtualMachineDataDiskAttachment_virtualMachineApplication
--- PASS: TestAccVirtualMachineDataDiskAttachment_virtualMachineApplication (757.75s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/compute       761.263s

@github-actions github-actions bot added the size/L label Dec 7, 2023
@jkroepke jkroepke changed the title azurerm_virtual_machine_data_disk_attachment: Ignore applications azurerm_virtual_machine_data_disk_attachment: Ignore VM applications Dec 7, 2023
@jkroepke jkroepke force-pushed the fix-data-disk-vm-application branch from 441bd19 to 86efa1b Compare December 7, 2023 07:44
@jkroepke jkroepke force-pushed the fix-data-disk-vm-application branch from 86efa1b to 30c237b Compare December 7, 2023 11:31
@jkroepke jkroepke force-pushed the fix-data-disk-vm-application branch 2 times, most recently from 6008c97 to c6cfde3 Compare January 1, 2024 19:54
@jkroepke jkroepke marked this pull request as ready for review January 1, 2024 19:59
@jkroepke jkroepke force-pushed the fix-data-disk-vm-application branch from c6cfde3 to 2e7de78 Compare January 1, 2024 20:00
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 @jkroepke - LGTM 🚋

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.

checking the tests looks like we have a failure:

------- Stdout: -------
=== RUN   TestAccVirtualMachineDataDiskAttachment_virtualMachineApplication
=== PAUSE TestAccVirtualMachineDataDiskAttachment_virtualMachineApplication
=== CONT  TestAccVirtualMachineDataDiskAttachment_virtualMachineApplication
    testcase.go:113: Step 1/2 error: Error running pre-apply refresh: exit status 1
        
        Error: Missing required argument
        
          on terraform_plugin_test.tf line 115, in resource "azurerm_linux_virtual_machine" "test":
         115: resource "azurerm_linux_virtual_machine" "test" {
        
        The argument "admin_username" is required, but no definition was found.
        
        Error: Missing required argument
        
          on terraform_plugin_test.tf line 115, in resource "azurerm_linux_virtual_machine" "test":
         115: resource "azurerm_linux_virtual_machine" "test" {
        
        The argument "size" is required, but no definition was found.
        
        Error: Insufficient os_disk blocks
        
          on terraform_plugin_test.tf line 115, in resource "azurerm_linux_virtual_machine" "test":
         115: resource "azurerm_linux_virtual_machine" "test" {
        
        At least 1 "os_disk" blocks are required.
        
        Error: Unsupported argument
        
          on terraform_plugin_test.tf line 120, in resource "azurerm_linux_virtual_machine" "test":
         120:   vm_size               = "Standard_F4"
        
        An argument named "vm_size" is not expected here.
        
        Error: Unsupported argument
        
          on terraform_plugin_test.tf line 122, in resource "azurerm_linux_virtual_machine" "test":
         122:   delete_os_disk_on_termination    = true
        
        An argument named "delete_os_disk_on_termination" is not expected here.
        
        Error: Unsupported argument
        
          on terraform_plugin_test.tf line 123, in resource "azurerm_linux_virtual_machine" "test":
         123:   delete_data_disks_on_termination = true
        
        An argument named "delete_data_disks_on_termination" is not expected here.
        
        Error: Unsupported block type
        
          on terraform_plugin_test.tf line 125, in resource "azurerm_linux_virtual_machine" "test":
         125:   storage_image_reference {
        
        Blocks of type "storage_image_reference" are not expected here.
        
        Error: Unsupported block type
        
          on terraform_plugin_test.tf line 132, in resource "azurerm_linux_virtual_machine" "test":
         132:   os_profile {
        
        Blocks of type "os_profile" are not expected here.
        
        Error: Unsupported block type
        
          on terraform_plugin_test.tf line 138, in resource "azurerm_linux_virtual_machine" "test":
         138:   storage_os_disk {
        
        Blocks of type "storage_os_disk" are not expected here.
        
        Error: Unsupported block type
        
          on terraform_plugin_test.tf line 145, in resource "azurerm_linux_virtual_machine" "test":
         145:   os_profile_linux_config {
        
        Blocks of type "os_profile_linux_config" are not expected here.
--- FAIL: TestAccVirtualMachineDataDiskAttachment_virtualMachineApplication (8.47s)
FAIL

@jkroepke
Copy link
Contributor Author

jkroepke commented Jan 4, 2024

@katbyte could you check that you are on latest revision? I had the same errors before do a force-push here.

For example the error

    Error: Unsupported argument
   
     on terraform_plugin_test.tf line 123, in resource "azurerm_linux_virtual_machine" "test":
    123:   delete_data_disks_on_termination = true
   
   An argument named "delete_data_disks_on_termination" is not expected here.

Lead to an old revision, since on the whole diff, there is no delete_data_disks_on_termination.

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.

Tests pass now ! thanks LGTM 💾

@katbyte katbyte merged commit 6652e7b into hashicorp:main Jan 8, 2024
32 checks passed
@github-actions github-actions bot added this to the v3.87.0 milestone Jan 8, 2024
katbyte added a commit that referenced this pull request Jan 8, 2024
@jkroepke jkroepke deleted the fix-data-disk-vm-application branch January 8, 2024 09:33
dduportal pushed a commit to jenkins-infra/azure that referenced this pull request Jan 17, 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.86.0&#34; to
&#34;3.87.0&#34; in file &#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>3.87.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.87.0&#xA;ENHANCEMENTS:&#xA;&#xA;*
dependencies: updating to `v0.20240112.1095456` of
`github.com/hashicorp/go-azure-sdk` [GH-24477]&#xA;* dependencies:
updating to `v0.65.1` of `github.com/hashicorp/go-azure-helpers`
[GH-24479]&#xA;* `kusto`: updating to use the base layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest`
[GH-24477]&#xA;* `azurerm_container_group` - support for the `priority`
property [GH-24374]&#xA;* Data Source: `azurerm_application_gateway` -
support for the `trusted_client_certificate.data` property
[GH-24474]&#xA;&#xA;## 3.87.0 (January 11,
2024)&#xA;&#xA;FEATURES:&#xA;&#xA;* New Data Source:
`azurerm_network_manager`
([#24398](hashicorp/terraform-provider-azurerm#24398
New Resource:
`azurerm_security_center_server_vulnerability_assessments_setting`
([#24299](https://github.com/hashicorp/terraform-provider-azurerm/issues/24299))&#xA;&#xA;ENHANCEMENTS:&#xA;&#xA;*
dependencies: updating to `v0.20240111.1094251` of
`github.com/hashicorp/go-azure-sdk`
([#24463](hashicorp/terraform-provider-azurerm#24463
Data Source: `azurerm_mssql_database` - support for `identity`,
`transparent_data_encryption_enabled`,
`transparent_data_encryption_key_vault_key_id` and
`transparent_data_encryption_key_automatic_rotation_enabled`
([#24412](hashicorp/terraform-provider-azurerm#24412
Data Source: `azurerm_mssql_server` - support for
`transparent_data_encryption_key_vault_key_id`
([#24412](hashicorp/terraform-provider-azurerm#24412
`machinelearning`: updating to API Version `2023-10-01`
([#24416](hashicorp/terraform-provider-azurerm#24416
`paloaltonetworks`: updating to API Version `2023-09-01`
([#24290](hashicorp/terraform-provider-azurerm#24290
`azurerm_container_app` - update create time validations for
`ingress.0.traffic_weight`
([#24042](hashicorp/terraform-provider-azurerm#24042
`azurerm_container_app`- support for the `ip_security_restriction` block
([#23870](hashicorp/terraform-provider-azurerm#23870
`azurerm_kubernetes_cluster` - properties in
`default_node_pool.linux_os_config.sysctl_config` are now updateable via
node pool cycling
([#24397](hashicorp/terraform-provider-azurerm#24397
`azurerm_linux_web_app` - support the `VS2022` value for the
`remote_debugging_version` property
([#24407](hashicorp/terraform-provider-azurerm#24407
`azurerm_mssql_database` - support for `identity`,
`transparent_data_encryption_key_vault_key_id` and
`transparent_data_encryption_key_automatic_rotation_enabled`
([#24412](hashicorp/terraform-provider-azurerm#24412
`azurerm_postgres_flexible_server` - the `sku_name` property now
supports being set to `MO_Standard_E96ds_v5`
([#24367](hashicorp/terraform-provider-azurerm#24367
`azurerm_role_assignment` - support for the `principal_type` property
([#24271](hashicorp/terraform-provider-azurerm#24271
`azurerm_windows_web_app` - support the `VS2022` value for the
`remote_debugging_version` property
([#24407](hashicorp/terraform-provider-azurerm#24407
`azurerm_cdn_frontdoor_firewall_policy` - support for
`request_body_check_enabled` property
([#24406](https://github.com/hashicorp/terraform-provider-azurerm/issues/24406))&#xA;&#xA;BUG
FIXES:&#xA;&#xA;* Data Source: `azurerm_role_definition` - fix
`role_definition_id`
([#24418](hashicorp/terraform-provider-azurerm#24418
`azurerm_api_management` - the `sku_name` property can now be updated
([#24431](hashicorp/terraform-provider-azurerm#24431
`azurerm_arc_kubernetes_flux_configuration` - prevent a bug where
certain sensitive properties for `bucket` and `git_repository` were
being overwritten after an update to the resource is made
([#24066](hashicorp/terraform-provider-azurerm#24066
`azurerm_kubernetes_flux_configuration` - prevent a bug where certain
sensitive properties for `bucket` and `git_repository` were being
overwritten after an update to the resource is made
([#24066](hashicorp/terraform-provider-azurerm#24066
`azure_linux_web_app` - prevent a bug in App Service processing of
`application_stack` in updates to `site_config`
([#24424](hashicorp/terraform-provider-azurerm#24424
`azure_linux_web_app_slot` - Fix bug in App Service processing of
`application_stack` in updates to `site_config`
([#24424](hashicorp/terraform-provider-azurerm#24424
`azurerm_network_manager_deployment` - update creation wait logic to
better tolerate the api returning not found
([#24330](hashicorp/terraform-provider-azurerm#24330
`azurerm_virtual_machine_data_disk_attachment` - do not update
applications profile with disks
([#24145](hashicorp/terraform-provider-azurerm#24145
`azure_windows_web_app` - prevent a bug in App Service processing of
`application_stack` in updates to `site_config`
([#24424](hashicorp/terraform-provider-azurerm#24424
`azure_windows_web_app_slot` - prevent a bug in App Service processing
of `application_stack` in updates to `site_config`
([#24424](hashicorp/terraform-provider-azurerm#24424
`azurerm_maintenance_configuration` - set the `reboot` property in
flatten from `AlwaysReboot` to `Always`
([#24376](hashicorp/terraform-provider-azurerm#24376
`azurerm_container_app_environment` - the `workload_profile` property
can now be updated
([#24409](https://github.com/hashicorp/terraform-provider-azurerm/issues/24409))&#xA;&#xA;&#xA;</pre>
            </details>
        </details>
<a
href="https://infra.ci.jenkins.io/job/terraform-jobs/job/azure/job/main/1004/">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]>
Copy link

github-actions bot commented May 1, 2024

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 May 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants