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_palo_alto_next_generation_firewall_* - support the property trustedRanges #24459

Merged
merged 8 commits into from
Jan 31, 2024

Conversation

neil-yechenwei
Copy link
Contributor

@neil-yechenwei neil-yechenwei commented Jan 11, 2024

Service team is requesting this new feature.

image

Note: Above test cases are also failed in Teamcity Daily Run with same error so that it's not related with this PR.

@github-actions github-actions bot added size/M and removed size/S labels Jan 11, 2024
Copy link
Member

@catriona-m catriona-m left a comment

Choose a reason for hiding this comment

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

Hi @neil-yechenwei - it looks like the test has a fail related to this PR in the latest run:

------- Stdout: -------
=== RUN   TestAccPaloAltoNextGenerationFirewallLocalRulestackVNet_update
=== PAUSE TestAccPaloAltoNextGenerationFirewallLocalRulestackVNet_update
=== CONT  TestAccPaloAltoNextGenerationFirewallLocalRulestackVNet_update
    testcase.go:113: Step 3/8 error: Error running pre-apply refresh: exit status 1
        Error: expected "network_profile.0.trusted_ranges.0" to be a valid CIDR Value, got 20.22.92.11: invalid CIDR address: 20.22.92.11
          with azurerm_palo_alto_next_generation_firewall_virtual_network_local_rulestack.test,
          on terraform_plugin_test.tf line 138, in resource "azurerm_palo_alto_next_generation_firewall_virtual_network_local_rulestack" "test":
         138:     trusted_ranges            = ["20.22.92.11", "20.23.92.11"]
        Error: expected network_profile.0.trusted_ranges.0 to contain a valid IP range, got: 20.22.92.11
          with azurerm_palo_alto_next_generation_firewall_virtual_network_local_rulestack.test,
          on terraform_plugin_test.tf line 138, in resource "azurerm_palo_alto_next_generation_firewall_virtual_network_local_rulestack" "test":
         138:     trusted_ranges            = ["20.22.92.11", "20.23.92.11"]
        Error: expected "network_profile.0.trusted_ranges.1" to be a valid CIDR Value, got 20.23.92.11: invalid CIDR address: 20.23.92.11
          with azurerm_palo_alto_next_generation_firewall_virtual_network_local_rulestack.test,
          on terraform_plugin_test.tf line 138, in resource "azurerm_palo_alto_next_generation_firewall_virtual_network_local_rulestack" "test":
         138:     trusted_ranges            = ["20.22.92.11", "20.23.92.11"]
        Error: expected network_profile.0.trusted_ranges.1 to contain a valid IP range, got: 20.23.92.11
          with azurerm_palo_alto_next_generation_firewall_virtual_network_local_rulestack.test,
          on terraform_plugin_test.tf line 138, in resource "azurerm_palo_alto_next_generation_firewall_virtual_network_local_rulestack" "test":
         138:     trusted_ranges            = ["20.22.92.11", "20.23.92.11"]
--- FAIL: TestAccPaloAltoNextGenerationFirewallLocalRulestackVNet_update (2307.17s)
FAIL

@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Jan 12, 2024

@catriona-m , thanks for the comment. I updated PR. Please take another look. Thanks.

@@ -67,6 +70,18 @@ func VnetNetworkProfileSchema() *pluginsdk.Schema {
},
},

"trusted_ranges": {
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
"trusted_ranges": {
"trusted_addresses_ranges": {

or

Suggested change
"trusted_ranges": {
"trusted_addresses": {

to both be more clear what it is looking for as well match other propertyes int he provider?

Copy link
Contributor Author

@neil-yechenwei neil-yechenwei Jan 18, 2024

Choose a reason for hiding this comment

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

Updated to trusted_address_ranges

@neil-yechenwei
Copy link
Contributor Author

@katbyte , thanks for the comments. I updated PR. Please take another look. Thanks.

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 @neil-yechenwei - checked tests and they are all failing?

------- Stdout: -------
=== RUN   TestAccPaloAltoNextGenerationFirewallVHubLocalRulestack_basic
=== PAUSE TestAccPaloAltoNextGenerationFirewallVHubLocalRulestack_basic
=== CONT  TestAccPaloAltoNextGenerationFirewallVHubLocalRulestack_basic
    testcase.go:113: Step 1/2 error: Error running apply: exit status 1
        
        Error: waiting for Virtual Hub: (Name "acctestVHUB-240118040419856605" / Resource Group "acctestRG-PANGFWVH-240118040419856605") provisioning route: failed to provision routing on Virtual Hub "acctestRG-PANGFWVH-240118040419856605" (Resource Group "acctestVHUB-240118040419856605")
        
          with azurerm_virtual_hub.test,
          on terraform_plugin_test.tf line 49, in resource "azurerm_virtual_hub" "test":
          49: resource "azurerm_virtual_hub" "test" {
        
    testing_new.go:90: Error running post-test destroy, there may be dangling resources: exit status 1
        
        Error: deleting Virtual Wan: (Name "acctestvwan-240118040419856605" / Resource Group "acctestRG-PANGFWVH-240118040419856605"): network.VirtualWansClient#Delete: Failure sending request: StatusCode=400 -- Original Error: Code="InUseVirtualWanCannotBeDeleted" Message="Cannot proceed with the operation because Virtual WAN has references. The WAN is referenced by 1 Virtual Hubs and 0 VPN Sites." Details=[]
        
--- FAIL: TestAccPaloAltoNextGenerationFirewallVHubLocalRulestack_basic (473.16s)
FAIL

could we plase fix these up so we can ensure this works as intended?

@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Jan 22, 2024

Hi @katbyte , the test cases you mentioned are also failed in both Network and PaloAlto in Teamcity Daily Run with same error. Per the error message returned by API in teamcity, it's service API issue not TF issue. It depends on the fix from service team, so I can't fix it in the short term. I suggest to review/merge this PR first since the failure is not related with this PR. In the meanwhile, I have already filed azure support ticket and IcM ticket to ask service team to fix it. Does it make sense?

Note: As our test service principal and test subscription have insufficient permission for this RP, so it's hard to us to run some test cases on my local for this RP. I assume jackofallops has full permission for this RP since he added them in TF.

@katbyte
Copy link
Collaborator

katbyte commented Jan 30, 2024

@neil-yechenwei - is the problem causing the tests to fail described anywhere? and is there an ETA for the fix?

@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Jan 30, 2024

@katbyte , for Virtual Hub issue, Virtual Wan Service team confirmed that the issue has been fixed last Friday and I found the Virtual Hub issue has gone in the latest Teamcity Daily Run. For PaloAlto issue, seems it is also fixed by the related service team. Below is the latest test result.

image

@neil-yechenwei
Copy link
Contributor Author

Below is the latest test result.

image

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.

Great thanks @neil-yechenwei - LGTM 🪟

@katbyte katbyte merged commit f4227ed into hashicorp:main Jan 31, 2024
33 checks passed
@github-actions github-actions bot added this to the v3.90.0 milestone Jan 31, 2024
katbyte added a commit that referenced this pull request Jan 31, 2024
dduportal pushed a commit to jenkins-infra/azure that referenced this pull request Feb 5, 2024
<Actions>
<action
id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8">
        <h3>Bump Terraform `azurerm` provider version</h3>
<details
id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24">
            <summary>Update Terraform lock file</summary>
<p>changes detected:&#xA;&#x9;&#34;hashicorp/azurerm&#34; updated from
&#34;3.89.0&#34; to &#34;3.90.0&#34; in file
&#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>3.90.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.90.0&#xA;UPGRADE
NOTES:&#xA;&#xA;* provider - The provider will now automatically
register the `AppConfiguration`, `DataFactory`, and `SignalRService`
Resource Providers. When running Terraform with limited permissions,
note that you [must disable automatic Resource Provider Registration and
ensure that any Resource Providers Terraform requires are
registered]([XXX](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs#skip_provider_registration)).
([#24645](hashicorp/terraform-provider-azurerm#24645;
&#xA;FEATURES:&#xA;&#xA;* **New Data Source**:
`azurerm_nginx_configuration`
([#24642](hashicorp/terraform-provider-azurerm#24642
**New Data Source**: `azurerm_virtual_desktop_workspace`
([#24732](hashicorp/terraform-provider-azurerm#24732
**New Resource**: `azurerm_kubernetes_fleet_update_strategy`
([#24328](hashicorp/terraform-provider-azurerm#24328
**New Resource**: `azurerm_site_recovery_vmware_replicated_vm`
([#22477](hashicorp/terraform-provider-azurerm#22477
**New Resource**:
`azurerm_spring_cloud_new_relic_application_performance_monitoring`
([#24699](https://github.com/hashicorp/terraform-provider-azurerm/issues/24699))&#xA;&#xA;ENHANCEMENTS:&#xA;&#xA;*
provider: registering the Resource Provider `Microsoft.AppConfiguration`
([#24645](hashicorp/terraform-provider-azurerm#24645
provider: registering the Resource Provider `Microsoft.DataFactory`
([#24645](hashicorp/terraform-provider-azurerm#24645
provider: registering the Resource Provider `Microsoft.SignalRService`
([#24645](hashicorp/terraform-provider-azurerm#24645
provider: the Provider is now built using Go 1.21.6
([#24653](hashicorp/terraform-provider-azurerm#24653
dependencies: the dependency `github.com/hashicorp/go-azure-sdk` has
been split into multiple Go Modules - and as such will be referred to by
those paths going forwards
([#24636](hashicorp/terraform-provider-azurerm#24636
dependencies: updating to ``v0.20240201.1064937` of
`github.com/hashicorp/go-azure-sdk/resource-manager`
([#24738](hashicorp/terraform-provider-azurerm#24738
dependencies: updating to `v0.20240201.1064937` of
`github.com/hashicorp/go-azure-sdk/sdk`
([#24738](hashicorp/terraform-provider-azurerm#24738
`appservice`: update to `go-azure-sdk` and API version `2023-01-01`
([#24688](hashicorp/terraform-provider-azurerm#24688
`datafactory`: updating to use `tombuildsstuff/kermit`
([#24675](hashicorp/terraform-provider-azurerm#24675
`hdinsight`: refactoring to use
`github.com/hashicorp/go-azure-sdk/resource-manager`
([#24011](hashicorp/terraform-provider-azurerm#24011
`hdinsight`: updating to API Version `2021-06-01`
([#24011](hashicorp/terraform-provider-azurerm#24011
`loadbalancer`: updating to use `hashicorp/go-azure-sdk`
([#24291](hashicorp/terraform-provider-azurerm#24291
`nginx`: updating to API Version `2023-09-01`
([#24640](hashicorp/terraform-provider-azurerm#24640
`servicefabricmanagedcluster`: updating to use the base layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest`
([#24654](hashicorp/terraform-provider-azurerm#24654
`springcloud`: updating to use API Version `2023-11-01-preview`
([#24690](hashicorp/terraform-provider-azurerm#24690
`subscriptions`: refactoring to use `hashicorp/go-azure-sdk`
([#24663](hashicorp/terraform-provider-azurerm#24663
Data Source: `azurerm_stream_analytics_job` - support for User Assigned
Identities
([#24738](hashicorp/terraform-provider-azurerm#24738
`azurerm_cosmosdb_account` - support for the `gremlin_database` and
`tables_to_restore` properties
([#24627](hashicorp/terraform-provider-azurerm#24627
`azurerm_bot_channel_email` - support for the `magic_code` property
([#23129](hashicorp/terraform-provider-azurerm#23129
`azurerm_cosmosdb_account` - support for the `partition_merge_enabled`
property
([#24615](hashicorp/terraform-provider-azurerm#24615
`azurerm_mssql_managed_database` - support for the
`immutable_backups_enabled` property
([#24745](hashicorp/terraform-provider-azurerm#24745
`azurerm_mssql_database` - support for the `immutable_backups_enabled`
property
([#24745](hashicorp/terraform-provider-azurerm#24745
`azurerm_palo_alto_next_generation_firewall_virtual_hub_panorama` -
support for the `trusted_address_ranges` property
([#24459](hashicorp/terraform-provider-azurerm#24459
`azurerm_palo_alto_next_generation_firewall_virtual_network_local_rulestack`
- support for the `trusted_address_ranges` property
([#24459](hashicorp/terraform-provider-azurerm#24459
`azurerm_palo_alto_next_generation_firewall_virtual_network_panorama` -
support for the `trusted_address_ranges` property
([#24459](hashicorp/terraform-provider-azurerm#24459
`azurerm_servicebus_namespace` - updating to use API Version
`2022-10-01-preview`
([#24650](hashicorp/terraform-provider-azurerm#24650
`azurerm_spring_cloud_api_portal` - support for the
`api_try_out_enabled` property
([#24696](hashicorp/terraform-provider-azurerm#24696
`azurerm_spring_cloud_gateway` - support for the
`local_response_cache_per_route` and `local_response_cache_per_instance`
properties
([#24697](hashicorp/terraform-provider-azurerm#24697
`azurerm_stream_analytics_job` - support for User Assigned Identities
([#24738](hashicorp/terraform-provider-azurerm#24738
`azurerm_subscription` - refactoring to use `hashicorp/go-azure-sdk` to
set tags on the subscription
([#24734](hashicorp/terraform-provider-azurerm#24734
`azurerm_virtual_desktop_workspace` - correctly validate the `name`
property
([#24668](https://github.com/hashicorp/terraform-provider-azurerm/issues/24668))&#xA;&#xA;&#xA;BUG
FIXES:&#xA;&#xA;* provider: skip registration for resource providers
that are unavailable
([#24571](hashicorp/terraform-provider-azurerm#24571
`azurerm_app_configuration` - no longer require
`lifecycle_ignore_changes` for the `value` property when using a key
vault reference
([#24702](hashicorp/terraform-provider-azurerm#24702
`azurerm_app_service_managed_certificate` - fix casing issue in
`app_service_plan_id` by parsing insensitively
([#24664](hashicorp/terraform-provider-azurerm#24664
`azurerm_cognitive_deployment` - updates now include the `version`
property
([#24700](hashicorp/terraform-provider-azurerm#24700
`azurerm_dns_cname_record` - prevent casing issue in
`target_resource_id` by parsing the ID insensitively
([#24181](hashicorp/terraform-provider-azurerm#24181
`azurerm_mssql_managed_instance_failover_group` - prevent an issue when
trying to create a failover group with a managed instance from a
different subscription
([#24646](hashicorp/terraform-provider-azurerm#24646
`azurerm_storage_account` - conditionally update properties only when
needed
([#24669](hashicorp/terraform-provider-azurerm#24669
`azurerm_storage_account` - change update order for `access_tier`to
prevent errors when uploading blobs to the archive tier
([#22250](https://github.com/hashicorp/terraform-provider-azurerm/issues/22250))&#xA;&#xA;&#xA;</pre>
            </details>
        </details>
<a
href="https://infra.ci.jenkins.io/job/terraform-jobs/job/azure/job/main/1075/">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 25, 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.

3 participants