-
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
azurerm_resource_deployment_script_azure_power_shell
azurerm_resource_deployment_script_azure_cli
- loosen version validation
#23370
azurerm_resource_deployment_script_azure_power_shell
azurerm_resource_deployment_script_azure_cli
- loosen version validation
#23370
Conversation
5926fba
to
031d629
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 working on this @teowa. I agree that maintaining a list of versions is infeasible given the release cadence of both engines, but we should do better than only checking for a nonempty string. Please see my inline comment below, thanks!
Type: pluginsdk.TypeString, | ||
Required: true, | ||
ForceNew: 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.
Rather than dropping the validation, we should make sure the version string is in the expected format, e.g. x.y
for PowerShell or x.y.z
for Azure CLI.
3d137e5
to
4e3e412
Compare
4e3e412
to
b845f4a
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.
hey @teowa
Thanks for this PR - I've taken a look through and left a couple of comments inline around the documentation, but this otherwise LGTM 👍
@@ -62,7 +62,7 @@ The following arguments are supported: | |||
|
|||
* `location` - (Required) Specifies the Azure Region where the Resource Deployment Script should exist. Changing this forces a new Resource Deployment Script to be created. | |||
|
|||
* `version` - (Required) Azure CLI module version to be used. The supported versions are `2.0.77`, `2.0.78`, `2.0.79`, `2.0.80`, `2.0.81`, `2.1.0`, `2.10.0`, `2.10.1`, `2.11.0`, `2.11.1`, `2.12.0`, `2.12.1`, `2.13.0`, `2.14.0`, `2.14.1`, `2.14.2`, `2.15.0`, `2.15.1`, `2.16.0`, `2.17.0`, `2.17.1`, `2.18.0`, `2.19.0`, `2.19.1`, `2.2.0`, `2.20.0`, `2.21.0`, `2.22.0`, `2.22.1`, `2.23.0`, `2.24.0`, `2.24.1`, `2.24.2`, `2.25.0`, `2.26.0`, `2.26.1`, `2.27.0`, `2.27.1`, `2.27.2`, `2.28.0`, `2.29.0`, `2.29.1`, `2.29.2`, `2.3.0`, `2.3.1`, `2.30.0`, `2.31.0`, `2.32.0`, `2.33.0`, `2.33.1`, `2.34.0`, `2.34.1`, `2.35.0`, `2.36.0`, `2.37.0`, `2.38.0`, `2.39.0`, `2.4.0`, `2.40.0`, `2.41.0`, `2.5.0`, `2.5.1`, `2.6.0`, `2.7.0`, `2.8.0`, `2.9.0`, `2.9.1`. Changing this forces a new Resource Deployment Script to be created. | |||
* `version` - (Required) Azure CLI module version to be used. The supported versions include but not limited to `2.0.77`, `2.0.78`, `2.0.79`, `2.0.80`, `2.0.81`, `2.1.0`, `2.10.0`, `2.10.1`, `2.11.0`, `2.11.1`, `2.12.0`, `2.12.1`, `2.13.0`, `2.14.0`, `2.14.1`, `2.14.2`, `2.15.0`, `2.15.1`, `2.16.0`, `2.17.0`, `2.17.1`, `2.18.0`, `2.19.0`, `2.19.1`, `2.2.0`, `2.20.0`, `2.21.0`, `2.22.0`, `2.22.1`, `2.23.0`, `2.24.0`, `2.24.1`, `2.24.2`, `2.25.0`, `2.26.0`, `2.26.1`, `2.27.0`, `2.27.1`, `2.27.2`, `2.28.0`, `2.29.0`, `2.29.1`, `2.29.2`, `2.3.0`, `2.3.1`, `2.30.0`, `2.31.0`, `2.32.0`, `2.33.0`, `2.33.1`, `2.34.0`, `2.34.1`, `2.35.0`, `2.36.0`, `2.37.0`, `2.38.0`, `2.39.0`, `2.4.0`, `2.40.0`, `2.41.0`, `2.5.0`, `2.5.1`, `2.6.0`, `2.7.0`, `2.8.0`, `2.9.0`, `2.9.1`. Please refer to https://aka.ms/DeploymentScriptsTroubleshoot for more details. Changing this forces a new Resource Deployment Script to be created. |
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.
* `version` - (Required) Azure CLI module version to be used. The supported versions include but not limited to `2.0.77`, `2.0.78`, `2.0.79`, `2.0.80`, `2.0.81`, `2.1.0`, `2.10.0`, `2.10.1`, `2.11.0`, `2.11.1`, `2.12.0`, `2.12.1`, `2.13.0`, `2.14.0`, `2.14.1`, `2.14.2`, `2.15.0`, `2.15.1`, `2.16.0`, `2.17.0`, `2.17.1`, `2.18.0`, `2.19.0`, `2.19.1`, `2.2.0`, `2.20.0`, `2.21.0`, `2.22.0`, `2.22.1`, `2.23.0`, `2.24.0`, `2.24.1`, `2.24.2`, `2.25.0`, `2.26.0`, `2.26.1`, `2.27.0`, `2.27.1`, `2.27.2`, `2.28.0`, `2.29.0`, `2.29.1`, `2.29.2`, `2.3.0`, `2.3.1`, `2.30.0`, `2.31.0`, `2.32.0`, `2.33.0`, `2.33.1`, `2.34.0`, `2.34.1`, `2.35.0`, `2.36.0`, `2.37.0`, `2.38.0`, `2.39.0`, `2.4.0`, `2.40.0`, `2.41.0`, `2.5.0`, `2.5.1`, `2.6.0`, `2.7.0`, `2.8.0`, `2.9.0`, `2.9.1`. Please refer to https://aka.ms/DeploymentScriptsTroubleshoot for more details. Changing this forces a new Resource Deployment Script to be created. | |
* `version` - (Required) Specifies the version of the Azure CLI that should be used in the format `X.Y.Z` (e.g. `2.30.0`). Changing this forces a new Resource Deployment Script to be created. | |
-> A canonical list of versions [is available from the Microsoft Container Registry API](https://mcr.microsoft.com/v2/azure-cli/tags/list) |
@@ -66,7 +66,7 @@ The following arguments are supported: | |||
|
|||
* `location` - (Required) Specifies the Azure Region where the Resource Deployment Script should exist. Changing this forces a new Resource Deployment Script to be created. | |||
|
|||
* `version` - (Required) Azure PowerShell module version to be used. The supported versions are `2.7`, `2.8`, `3.0`, `3.1`, `3.2`, `3.3`, `3.4`, `3.5`, `3.6`, `3.7`, `3.8`, `4.1`, `4.2`, `4.3`, `4.4`, `4.5`, `4.6`, `4.7`, `4.8`, `5.0`, `5.1`, `5.2`, `5.3`, `5.4`, `5.5`, `5.6`, `5.7`, `5.8`, `5.9`, `6.0`, `6.1`, `6.2`, `6.3`, `6.4`, `6.5`, `6.6`, `7.0`, `7.1`, `7.2`, `7.3`, `7.4`, `7.5`, `8.0`, `8.1`, `8.2`, `8.3`, `9.0`. Changing this forces a new Resource Deployment Script to be created. | |||
* `version` - (Required) Azure PowerShell module version to be used. The supported versions include but not limited to `2.7`, `2.8`, `3.0`, `3.1`, `3.2`, `3.3`, `3.4`, `3.5`, `3.6`, `3.7`, `3.8`, `4.1`, `4.2`, `4.3`, `4.4`, `4.5`, `4.6`, `4.7`, `4.8`, `5.0`, `5.1`, `5.2`, `5.3`, `5.4`, `5.5`, `5.6`, `5.7`, `5.8`, `5.9`, `6.0`, `6.1`, `6.2`, `6.3`, `6.4`, `6.5`, `6.6`, `7.0`, `7.1`, `7.2`, `7.3`, `7.4`, `7.5`, `8.0`, `8.1`, `8.2`, `8.3`, `9.0`. Please refer to https://aka.ms/DeploymentScriptsTroubleshoot for more details. Changing this forces a new Resource Deployment Script to be created. |
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.
* `version` - (Required) Azure PowerShell module version to be used. The supported versions include but not limited to `2.7`, `2.8`, `3.0`, `3.1`, `3.2`, `3.3`, `3.4`, `3.5`, `3.6`, `3.7`, `3.8`, `4.1`, `4.2`, `4.3`, `4.4`, `4.5`, `4.6`, `4.7`, `4.8`, `5.0`, `5.1`, `5.2`, `5.3`, `5.4`, `5.5`, `5.6`, `5.7`, `5.8`, `5.9`, `6.0`, `6.1`, `6.2`, `6.3`, `6.4`, `6.5`, `6.6`, `7.0`, `7.1`, `7.2`, `7.3`, `7.4`, `7.5`, `8.0`, `8.1`, `8.2`, `8.3`, `9.0`. Please refer to https://aka.ms/DeploymentScriptsTroubleshoot for more details. Changing this forces a new Resource Deployment Script to be created. | |
* `version` - (Required) Specifies the version of Azure PowerShell that should be used in the format `X.Y.Z` (e.g. `2.30.0`). Changing this forces a new Resource Deployment Script to be created. | |
-> A canonical list of versions [is available from the Microsoft Container Registry API](https://mcr.microsoft.com/v2/azuredeploymentscripts-powershell/tags/list) |
40b6633
to
5bd5146
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 @teowa, this LGTM 👍
<Actions> <action id="4a39167e811ac038e4a588362092472c27cfbe9e4929ae61d035f708a093a669"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>"hashicorp/azurerm" updated from "3.76.0" to "3.77.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.77.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.77.0
FEATURES:

* New Resources: `azurerm_application_load_balancer_frontend` ([#23411](hashicorp/terraform-provider-azurerm#23411 New Resources: `azurerm_dev_center` ([#23538](hashicorp/terraform-provider-azurerm#23538 New Resources: `azurerm_dev_center_project` ([#23538](https://github.com/hashicorp/terraform-provider-azurerm/issues/23538))

ENHANCEMENTS:

* dependencies: updating to `v0.62.0` of `github.com/hashicorp/go-azure-helpers` ([#23581](hashicorp/terraform-provider-azurerm#23581 dependencies: updating Kusto SDK from `2023-05-02` to `2023-08-15` ([#23598](hashicorp/terraform-provider-azurerm#23598 dependencies: updating nginx from `2022-08-01` to `2023-04-01` ([#23583](hashicorp/terraform-provider-azurerm#23583 `netapp`: updating to use API Version `2023-05-01` ([#23576](hashicorp/terraform-provider-azurerm#23576 `springcloud`: updating to use API Version `2023-09-01-preview` ([#23544](hashicorp/terraform-provider-azurerm#23544 `storage`: updating to use API Version `2023-01-01` ([#23543](hashicorp/terraform-provider-azurerm#23543 `internal/sdk`: fixing an issue where struct fields containing `removedInNextMajorVersion` wouldn't be decoded correctly ([#23564](hashicorp/terraform-provider-azurerm#23564 `internal/sdk`: struct tag parsing is now handled consistently during both encoding and decoding ([#23568](hashicorp/terraform-provider-azurerm#23568 provider: the `roll_instances_when_required` provider feature in the `virtual_machine_scale_set` block is now optional ([#22976](hashicorp/terraform-provider-azurerm#22976 Data Source: `azurerm_automation_account`: refactoring the remaining usage of `Azure/azure-sdk-for-go` to use `hashicorp/go-azure-sdk` ([#23555](hashicorp/terraform-provider-azurerm#23555 `azurerm_automation_account`: refactoring the remaining usage of `Azure/azure-sdk-for-go` to use `hashicorp/go-azure-sdk` ([#23555](hashicorp/terraform-provider-azurerm#23555 `azurerm_resource_deployment_script_azure_cli` - improve validation for the `version` property to support newer versions ([#23370](hashicorp/terraform-provider-azurerm#23370 `azurerm_resource_deployment_script_azure_power_shell` - improve validation for the `version` property to support newer versions ([#23370](hashicorp/terraform-provider-azurerm#23370 `azurerm_nginx_deployment` - support for the `capacity` and `email` properties ([#23596](https://github.com/hashicorp/terraform-provider-azurerm/issues/23596))

BUG FIXES:

* Data Source: `azurerm_virtual_hub_connection` - export the `inbound_route_map_id`, `outbound_route_map_id`, and `static_vnet_local_route_override_criteria` attributes in the `routing` block, and fix a bug where these attributes could not be set ([#23491](hashicorp/terraform-provider-azurerm#23491 `azurerm_cdn_frontdoor_rule` - the `url_filename_condition` properties `match_values` is now optional if `operator` is set to `Any` ([#23541](hashicorp/terraform-provider-azurerm#23541 `azurerm_shared_image_gallery` - added the `Private` and `Groups` options for the `sharing.permission` property ([#23570](hashicorp/terraform-provider-azurerm#23570 `azurerm_redis_cache` - fixed incorrect ssl values for `redis_primary_connection_string` and `secondary_connection_string` ([#23575](hashicorp/terraform-provider-azurerm#23575 `azurerm_monitor_activity_log_alert` - the `recommend_category` property now can be set to `HighAvailability` ([#23605](hashicorp/terraform-provider-azurerm#23605 `azurerm_recovery_services_vault` - the `encryption` property can now be used with the `cross_region_restore_enabled` property ([#23618](hashicorp/terraform-provider-azurerm#23618 `azurerm_storage_account_customer_managed_key` - prevent a panic when the keyvault id is empty ([#23599](https://github.com/hashicorp/terraform-provider-azurerm/issues/23599))


</pre> </details> </details> </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>
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. |
fix #23352
loose the version validation for newly released PSH/CLI version can be used. And the error message from API is helpful: