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

Bugfix: Error creating VM with private IP in DTL. #3704

Closed
wants to merge 10 commits into from

Conversation

kmsisco
Copy link
Contributor

@kmsisco kmsisco commented Jun 19, 2019

Creating a VM in a DTL that doesn't allow public IPs results in the following error messages even when "disallow_public_ip_address" is true.

Error: Error waiting for creation/update of DevTest Linux Virtual Machine "terraform-test" (Lab "xxx" / Resource Group "..."): Code="RequestDisallowedByPolicy" Message="Resource 'xxx' was disallowed by policy. Policy identifiers: '[{"policyAssignment":{"name":"No Public IP Policy","id":"/subscriptions/xxx/providers/Microsoft.Authorization/policyAssignments/xxx"},"policyDefinition":{"name":"NoPublicIPPolicyDefinition","id":"/subscriptions/xxx/providers/Microsoft.Authorization/policyDefinitions/NoPublicIPPolicyDefinition"}}]'."

This change only sets the NetworkInterface property if inbound_nat_rules was defined to allow Azure to use its defaults.

@ghost ghost added the size/XS label Jun 19, 2019
@katbyte katbyte self-assigned this Jun 19, 2019
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.

Hi @kmsisco,

Would moving line 181 into the if statement so ‘nic’ Is nil when disable public up is true work too?

@kmsisco
Copy link
Contributor Author

kmsisco commented Jun 19, 2019

@katbyte Moving 181 into the if has the same effect as the current code. I am setting disallow_public_ip_address to true to get a private IP but I don't want it to initialize the nic. When the structure is passed Azure tries to allocate a public IP for some reason.

I removed the comment about reproducing issue with Azure REST API. Further testing showed that was not correct.

@ghost ghost removed the waiting-response label Jun 19, 2019
@kmsisco kmsisco marked this pull request as ready for review June 19, 2019 21:28
@katbyte
Copy link
Collaborator

katbyte commented Jun 20, 2019

Ah, could you write a test with that specific configuration the fails before the fix, and passes after?

Thanks @kmsisco

@ghost ghost added size/XL and removed size/XS labels Jun 22, 2019
@kmsisco
Copy link
Contributor Author

kmsisco commented Jun 22, 2019

Added acceptance tests.

These changes depend on #3717

@ghost ghost removed the waiting-response label Jun 22, 2019
@katbyte katbyte modified the milestones: v1.31.0, v1.32.0 Jun 28, 2019
@ghost ghost added size/L and removed size/XL labels Jul 3, 2019
@tombuildsstuff tombuildsstuff modified the milestones: v1.32.0, v1.33.0 Jul 22, 2019
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.

@kmsisco,

Tried to run the acceptance tests today and got:

------- Stdout: -------
=== RUN   TestAccAzureRMDevTestLinuxVirtualMachine_privatePolicy
--- FAIL: TestAccAzureRMDevTestLinuxVirtualMachine_privatePolicy (1263.37s)
    testing.go:568: Step 0 error: After applying this step, the plan was not empty:
        
        DIFF:
        
        UPDATE: azurerm_dev_test_linux_virtual_machine.test
          allow_claim:                         "true" => "true"
          disallow_public_ip_address:          "true" => "true"
          fqdn:                                "" => ""
          gallery_image_reference.#:           "1" => "1"
          gallery_image_reference.0.offer:     "UbuntuServer" => "UbuntuServer"
          gallery_image_reference.0.publisher: "Canonical" => "Canonical"
          gallery_image_reference.0.sku:       "18.04-LTS" => "18.04-LTS"
          gallery_image_reference.0.version:   "latest" => "latest"
          id:                                  "/subscriptions/1a6092a6-137e-4025-9a7c-ef77f76f2c02/resourcegroups/acctestrg-190802152219228149/providers/microsoft.devtestlab/labs/acctestdtl190802152219228149/virtualmachines/acctestvm-vm190802152219228149" => "/subscriptions/1a6092a6-137e-4025-9a7c-ef77f76f2c02/resourcegroups/acctestrg-190802152219228149/providers/microsoft.devtestlab/labs/acctestdtl190802152219228149/virtualmachines/acctestvm-vm190802152219228149"
          inbound_nat_rule.#:                  "0" => "0"
          lab_name:                            "acctestdtl190802152219228149" => "acctestdtl190802152219228149"
          lab_subnet_name:                     "acctestdtvn190802152219228149Subnet" => "acctestdtvn190802152219228149Subnet"
          lab_virtual_network_id:              "/subscriptions/1a6092a6-137e-4025-9a7c-ef77f76f2c02/resourcegroups/acctestrg-190802152219228149/providers/microsoft.devtestlab/labs/acctestdtl190802152219228149/virtualnetworks/acctestdtvn190802152219228149" => "/subscriptions/1a6092a6-137e-4025-9a7c-ef77f76f2c02/resourcegroups/acctestrg-190802152219228149/providers/microsoft.devtestlab/labs/acctestdtl190802152219228149/virtualnetworks/acctestdtvn190802152219228149"
          location:                            "westeurope" => "westeurope"
          name:                                "acctestvm-vm190802152219228149" => "acctestvm-vm190802152219228149"
          notes:                               "" => ""
          password:                            "Pa$$w0rd1234!" => "Pa$$w0rd1234!"
          resource_group_name:                 "acctestrg-190802152219228149" => "acctestrg-190802152219228149"
          size:                                "Standard_F2" => "Standard_F2"
          storage_type:                        "Premium" => "Standard"
          tags.Acceptance:                     "Test" => "Test"
          unique_identifier:                   "66f0ec63-f2bb-43f4-99f2-e5f4fae3f012" => "66f0ec63-f2bb-43f4-99f2-e5f4fae3f012"
          username:                            "acct5stU5er" => "acct5stU5er"
        

so i changed the storage type and got a new error:


        Error: Error creating/updating DevTest Linux Virtual Machine "acctestvm-vm190802114613370723" (Lab "acctestdtl190802114613370723" / Resource Group "acctestRG-190802114613370723"): dtl.VirtualMachinesClient#CreateOrUpdate: Failure sending request: StatusCode=400 -- Original Error: Code="InvalidSubnetUsed" Message="Subnet acctestdtvn190802114613370723Subnet either is not enabled or is not part of specified virtual network /subscriptions/1a6092a6-137e-4025-9a7c-ef77f76f2c02/resourcegroups/acctestrg-190802114613370723/providers/microsoft.devtestlab/labs/acctestdtl190802114613370723/virtualnetworks/acctestdtvn190802114613370723."

@kmsisco
Copy link
Contributor Author

kmsisco commented Aug 4, 2019

@katbyte

I merged the changes that fixed the virtual network issue and now the tests pass.

@ghost ghost removed the waiting-response label Aug 4, 2019
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.

@kmsisco,

I am still getting a diff on the test for:

=== RUN   TestAccAzureRMDevTestLinuxVirtualMachine_privatePolicy
--- FAIL: TestAccAzureRMDevTestLinuxVirtualMachine_privatePolicy (1261.89s)
    testing.go:568: Step 0 error: After applying this step, the plan was not empty:
        
        DIFF:
        
        UPDATE: azurerm_dev_test_linux_virtual_machine.test
 ...
          storage_type:                        "Premium" => "Standard"
        
```,

I think you'll need to change the storage type to `Premium` for some reason?

@katbyte
Copy link
Collaborator

katbyte commented Aug 6, 2019

@kmsisco,

The test is now failing with:

 DIFF:
        
        CREATE: azurerm_policy_assignment.test
          description:          "" => "No Public IP Policy Test"
          display_name:         "" => "No Public IP Policy"
          id:                   "" => "<computed>"
          identity:             "" => "<computed>"
          name:                 "" => "test-policy-assignment"
          policy_definition_id: "" => "<computed>"
          scope:                "" => "/subscriptions/1a6092a6-137e-4025-9a7c-ef77f76f2c02"
        CREATE: azurerm_policy_definition.test
          display_name: "" => "No Public IP Policy"
          id:           "" => "<computed>"
          metadata:     "" => "<computed>"
          mode:         "" => "Indexed"
          name:         "" => "NoPublicIPPolicyDefinition"
          policy_rule:  "" => "  {\n    \"if\": {\n      \"anyOf\": [{\n        \"source\": \"action\",\n        \"like\": \"Microsoft.Network/publicIPAddresses/*\"\n      }]\n    },\n    \"then\": {\n      \"effect\": \"deny\"\n    }\n  }\n"
          policy_type:  "" => "Custom"
        

Is this something unique to our environment maybe 🤔

@tombuildsstuff tombuildsstuff modified the milestones: v1.33.0, v1.34.0 Aug 21, 2019
@tombuildsstuff tombuildsstuff modified the milestones: v1.34.0, v1.35.0 Sep 12, 2019
@tombuildsstuff tombuildsstuff modified the milestones: v1.35.0, v1.36.0 Oct 1, 2019
@tombuildsstuff tombuildsstuff modified the milestones: v1.36.0, v1.37.0 Oct 24, 2019
@tombuildsstuff
Copy link
Contributor

hey @kmsisco

Apologies for the delayed re-review here!

Since we've not heard back from you here and some of the test cases here are failing - rather than leaving this open languishing (since our team are currently focused on other areas) I'm going to suggest we close this PR for the moment.

Should you (or somebody else) be able to resolve the failing test then we're more than happy to take another look - however whilst I'd like to thank you for this contribution I'm going to close this PR for the moment.

Thanks!

@tombuildsstuff tombuildsstuff removed this from the v1.37.0 milestone Oct 30, 2019
@ghost
Copy link

ghost commented Nov 30, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 30, 2019
@ghost ghost removed the waiting-response label Nov 30, 2019
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.

3 participants