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

r/storage_account: making default_action required #4037

Merged
merged 3 commits into from Aug 9, 2019

Conversation

tombuildsstuff
Copy link
Member

Fixes #4006

@@ -205,7 +206,7 @@ func resourceArmStorageAccount() *schema.Resource {

"default_action": {
Type: schema.TypeString,
Optional: true,
Required: true,

Choose a reason for hiding this comment

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

Is this not a breaking change?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The title says "changing default to allow" and the change is to make it required? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦‍♂ my bad - this took a different approach in the end

This is currently set to Required in the documentation: https://www.terraform.io/docs/providers/azurerm/r/storage_account.html#default_action - as such this PR updates it to match.. I'll update the title.

Choose a reason for hiding this comment

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

I'll defer to you and @katbyte on whether you want to call this a breaking change or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

given this block is now Computed (and the API returns a default response here) I think we're safe to not call this a breaking change here

@tombuildsstuff tombuildsstuff changed the title r/storage_account: defaulting the default_action to 'allow' r/storage_account: making default_action required Aug 8, 2019
@tombuildsstuff
Copy link
Member Author

Tests pass:

Screenshot 2019-08-08 at 15 37 57

@tombuildsstuff tombuildsstuff force-pushed the f/storage-account-default-action branch from fa87039 to c67c666 Compare August 8, 2019 14:57
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.

LGTM 👍

@tombuildsstuff tombuildsstuff merged commit bf975b3 into master Aug 9, 2019
@tombuildsstuff tombuildsstuff deleted the f/storage-account-default-action branch August 9, 2019 09:22
tombuildsstuff added a commit that referenced this pull request Aug 9, 2019
@tombuildsstuff
Copy link
Member Author

Tests passed:

Screenshot 2019-08-09 at 10 23 35

metacpp pushed a commit to VSChina/terraform-provider-azurerm that referenced this pull request Aug 15, 2019
* r/storage_blob: refactoring the existing tests

* Updating to use hashicorp#3857

* Add metadata to example usage

* r/storage_blob: rewriting the acceptance tests

* r/storage_blob: test fixes

* Adding a todo

* r/private_dns_a_record: fixing comments from code review

* Updating to include hashicorp#3847

* Updating to include hashicorp#3849

* Add schedule backup functionality.

* Add doc in app_service.html.markdown file

* Add deafult value to backup_schedule_enabled

* Add default value to backup_schedule_enabled parameter

* delete backup-schedule_enabled parameter

* Delete backupScheduleEnabled param

* Add condition err in resourceArmDeleteScheduleBackup

* In deleteConfiguration function delete check

* Add HasChange in update function

* Fix errcheck in DeleteBackupConfiguration

* Frequency interval validation range limit

* Delete backupSchedule in create function

* Update azurerm/resource_arm_app_service.go

Co-Authored-By: Matthew Frahry <mbfrahry@gmail.com>

* Validate retention period range limit

* Fix tabulation and add validate to backup_name

* Add frequency_unit validation

* azurerm_app_service: fixing the code comments from hashicorp#3330

```
$ acctests azurerm TestAccAzureRMAppService_backup
=== RUN   TestAccAzureRMAppService_backup
=== PAUSE TestAccAzureRMAppService_backup
=== CONT  TestAccAzureRMAppService_backup
--- PASS: TestAccAzureRMAppService_backup (660.26s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	660.313s
```

```
$ acctests azurerm TestAccAzureRMAppService_basic
=== RUN   TestAccAzureRMAppService_basic
=== PAUSE TestAccAzureRMAppService_basic
=== RUN   TestAccAzureRMAppService_basicWindowsContainer
=== PAUSE TestAccAzureRMAppService_basicWindowsContainer
=== CONT  TestAccAzureRMAppService_basic
--- PASS: TestAccAzureRMAppService_basic (189.59s)
=== CONT  TestAccAzureRMAppService_basicWindowsContainer
--- PASS: TestAccAzureRMAppService_basicWindowsContainer (369.53s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	559.155s
```

* fwixing comments from code review

* Updating to include hashicorp#3804

* [azurerm_recovery_services_protected_vm] - changing `backup_pol… (hashicorp#3822)

Refer to hashicorp#3441 now changing the behaviour of backup_policy_id so it should not trigger recreation of resource when value changed

(fixes hashicorp#3441)

* Update CHANGELOG.md to include hashicorp#3822

* storage: using the environment as available

* Fix documentation. Real resource name is azurerm_application_in… (hashicorp#3877)

* Fix azurerm_redis_cache non-SSL port in docs

* fix merging problem

* change firewall test to explicit config
fix bug of returning nil

* fix some errors; enable adminUsers test but not working currently

* fix linting error

* fix linting error

* fix admin users test

* r/storage_share: switching the state migration over to 0.12 format

```
$ go test -v ./azurerm/ -run=TestAzureRMStorageShareMigrateState
=== RUN   TestAzureRMStorageShareMigrateStateV0ToV1
2019/07/19 13:33:46 [DEBUG] Updating ID from "share1" to "share1/group1/account1"
2019/07/19 13:33:46 [DEBUG] Updating ID from "share1" to "share1/group1/account1"
2019/07/19 13:33:46 [DEBUG] Updating ID from "share1" to "share1/group1/account1"
2019/07/19 13:33:46 [DEBUG] Updating ID from "share1" to "share1/group1/account1"
--- PASS: TestAzureRMStorageShareMigrateStateV0ToV1 (0.00s)
    resource_arm_storage_share_migration_test.go:20: [DEBUG] Testing with Cloud "AzureChinaCloud"
    resource_arm_storage_share_migration_test.go:49: [DEBUG] Ok!
    resource_arm_storage_share_migration_test.go:20: [DEBUG] Testing with Cloud "AzureGermanCloud"
    resource_arm_storage_share_migration_test.go:49: [DEBUG] Ok!
    resource_arm_storage_share_migration_test.go:20: [DEBUG] Testing with Cloud "AzurePublicCloud"
    resource_arm_storage_share_migration_test.go:49: [DEBUG] Ok!
    resource_arm_storage_share_migration_test.go:20: [DEBUG] Testing with Cloud "AzureUSGovernmentCloud"
    resource_arm_storage_share_migration_test.go:49: [DEBUG] Ok!
=== RUN   TestAzureRMStorageShareMigrateStateV1ToV2
2019/07/19 13:33:46 [DEBUG] Updating Resource ID from "share1/group1/account1" to "https://account1.file.core.chinacloudapi.cn/share1"
2019/07/19 13:33:46 [DEBUG] Updating Resource ID from "share1/group1/account1" to "https://account1.file.core.cloudapi.de/share1"
2019/07/19 13:33:46 [DEBUG] Updating Resource ID from "share1/group1/account1" to "https://account1.file.core.windows.net/share1"
2019/07/19 13:33:46 [DEBUG] Updating Resource ID from "share1/group1/account1" to "https://account1.file.core.usgovcloudapi.net/share1"
--- PASS: TestAzureRMStorageShareMigrateStateV1ToV2 (0.00s)
    resource_arm_storage_share_migration_test.go:62: [DEBUG] Testing with Cloud "AzureChinaCloud"
    resource_arm_storage_share_migration_test.go:91: [DEBUG] Ok!
    resource_arm_storage_share_migration_test.go:62: [DEBUG] Testing with Cloud "AzureGermanCloud"
    resource_arm_storage_share_migration_test.go:91: [DEBUG] Ok!
    resource_arm_storage_share_migration_test.go:62: [DEBUG] Testing with Cloud "AzurePublicCloud"
    resource_arm_storage_share_migration_test.go:91: [DEBUG] Ok!
    resource_arm_storage_share_migration_test.go:62: [DEBUG] Testing with Cloud "AzureUSGovernmentCloud"
    resource_arm_storage_share_migration_test.go:91: [DEBUG] Ok!
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	0.038s
```

* r/kubernetes_cluster: fixing comments from code review

* r/kubernetes_cluster: ordering the sections alphabetically

* adjust pr comments

* r/kubernetes_cluster: switching to an import test

* fixing comments from code review

* sidebar: fixing the ordering

* Updating to include hashicorp#3785

* add function app auth settings

* docs

* bug: missing link to azurerm_api_management_subscription (hashicorp#3888)

* r/analysis_services_server: ensuring the ID is set in the create

* updating to include hashicorp#3721

* updating to include hashicorp#3893

* Add auth_settings support to app service slot

* Add auth_settings tests to app service slot

* auth_settings block in app service slot docs

* Missed import modules for test

* return value for UpdateAuthSettingsSlot

* Wrong resource name in tests

* r/app_service_slot: wrapping the errors

* r/app_service_slot: renaming the CreateUpdate method to Create since it's been split

* r/storage_container: metadata should be computed

```
$ acctests azurerm TestAccAzureRMStorageContainer_metaData
=== RUN   TestAccAzureRMStorageContainer_metaData
=== PAUSE TestAccAzureRMStorageContainer_metaData
=== CONT  TestAccAzureRMStorageContainer_metaData
--- PASS: TestAccAzureRMStorageContainer_metaData (165.53s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	165.571s
```

* [Feature] Azure Maps Account Support

Adding Azure Map Accounts support to Terraform.

Adds azurerm_maps_account data source.
Adds azurerm_maps_account resource type.
Adds website documentation for data source and resource.
Adds data source and resource acceptance tests.

* vendoring the maps sdk

* r/maps_account: changes from code review

* docs: adding to the sidebar

* updating to include hashicorp#3897

* r/maps: removing dead code

* d/cosmosdb_account: handling the document endpoint being nil (hashicorp#3899)

* maps: making the sku lower-case to match the api

* r/storage_container: support for containers named `$web` (hashicorp#3896)

Fixes hashicorp#3894

* Update CHANGELOG.md to include hashicorp#3896

* maps: fixing the tests

```
$ acctests azurerm TestAccAzureRMMapsAccount_
=== RUN   TestAccAzureRMMapsAccount_basic
=== PAUSE TestAccAzureRMMapsAccount_basic
=== RUN   TestAccAzureRMMapsAccount_sku
=== PAUSE TestAccAzureRMMapsAccount_sku
=== RUN   TestAccAzureRMMapsAccount_tags
=== PAUSE TestAccAzureRMMapsAccount_tags
=== CONT  TestAccAzureRMMapsAccount_basic
=== CONT  TestAccAzureRMMapsAccount_tags
--- PASS: TestAccAzureRMMapsAccount_basic (80.43s)
--- PASS: TestAccAzureRMMapsAccount_tags (111.23s)
=== CONT  TestAccAzureRMMapsAccount_sku
--- PASS: TestAccAzureRMMapsAccount_sku (82.26s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	273.956s
```

* [azurerm_kubernetes_cluster ] - support for the `windows_profil… (hashicorp#3519)

fixes hashicorp#3508

* Update CHANGELOG.md to include hashicorp#3519

* updating to include hashicorp#3698

* sorting/reordering the changelog

* decrease gogc to prevent linting OOM

* website: Fix a sidebar name (hashicorp#3911)

* `azurerm_storage_account`: set `queue_properties` (hashicorp#3859)

* Update CHANGELOG.md

* azurerm_cosmosdb_account: validate max_interval_in_seconds and… (hashicorp#3906)

fixes hashicorp#3551

* Update CHANGELOG.md to include hashicorp#3906

* kubernetes_cluster: support specifying load balancer sku (hashicorp#3890)

Fixes hashicorp#3726

* Update CHANGELOG.md to include hashicorp#3890

* Update mysql_server.html.markdown (hashicorp#3914)

* azurerm_application_gateway  - Support for Managed Identity (resolves hashicorp#3640) (hashicorp#3648)

resolves hashicorp#3640

* Update CHANGELOG.md to include hashicorp#3648

* v1.32.0

* Cleanup after v1.32.0 release

* Sending nil certificate when no certificate is specified (hashicorp#3931)

* azurerm_kubernetes_cluster - Remove duplicated line in docs (hashicorp#3940)

* Bug Fix: `azurerm_api_managment_product` - Additional validation for `approval_required` (hashicorp#3945)

* Update CHANGELOG.md

* Bug Fix: `azurerm_storage_table` - Migrate id to new format hashicorp#3932

* Bug Fix: `azurerm_kubernetes_cluster` - suppress case difference on `load_balancer_sku` (hashicorp#3958)

* Fix crash (hashicorp#3966)

* Update CHANGELOG.md

* Fix the typo in the application gateway name (hashicorp#3953)

The application gateway was referenced by name `test` while it's defined as `network`

* azurerm_storage_account - fix `enable_advanced_threat_protectio… (hashicorp#3947)

fixes hashicorp#3920

* Update CHANGELOG.md to include hashicorp#3947

* Update CHANGELOG.md

* Bug Fix - `azurerm_iot_dps` fix deletion issue with service principal hashicorp#3973

* Update CHANGELOG.md

* Update hash function for os_profile_linux_config (hashicorp#3837)

* Add new acc test to verify ssh key issue

This commit adds a new acceptance test to verify that the vmss is not
calculating correctly the hashcode for the `os_profile_linux_config`

Related: hashicorp#3836

* Update OsProfileLinuxConfigHash generator

Update the `resourceArmVirtualMachineScaleSetOsProfileLinuxConfigHash` method to
include in the hash function the fields from the `ssh_keys` subelements.

Fixes: hashicorp#3836

* Update CHANGELOG.md to include hashicorp#3837

* Bugfix: Fixes the acceptance tests for resource_arm_dev_test_*_virtual_machine_test.go (hashicorp#3717)

* Update CHANGELOG.md to include hashicorp#3717

* azurerm_network_ddos_protection_plan - correctly decodes the re… (hashicorp#3975)

Fixes hashicorp#3974

* Update CHANGELOG.md to include hashicorp#3975

* azurerm_virtual_machine_scale_set  - changes made to `network… (hashicorp#3821)

The acceptance tests added here are describing the issue as reported by
this ticket hashicorp#3812

Initially tried to remove the custom Set method which calculates the hashcode similar to what is suggested by hashicorp/terraform#10520 and hashicorp/terraform#15420 but then we got into a position that the test framework was generating a perpetual diff.

So the current solution was to include the other fields in the hashcode calculation.

Many thanks to @kwilczynski

Fixes: hashicorp#3812
Co-authored-by: Krzysztof Wilczynski kw@linux.com

* Update CHANGELOG.md to include hashicorp#3821

* azurerm_postgresql_server  - support for version `11.0` (hashicorp#3970)

Fixes hashicorp#3969, and relates to hashicorp#3327 even so it doesn't solve the problem for 10.X versions. The Azure SDK for go doesn't provide version 10.7 for example, see here. But fortunately it has a version 11.

* Update CHANGELOG.md to include hashicorp#3970

* Update CHANGELOG.md to include hashicorp#3986

* storage: always nil check resource group before deref (hashicorp#3986)

* v1.32.1

* Cleanup after v1.32.1 release

* Added LogAnalyticsDestinationType

* Updated the docs

* Added the attribute to the doc

*  Added the value to the error string

* New Resource azurerm_dev_test_lab_schedule - Fix for issue 2334  (hashicorp#3554)

This pull request is to fix issue hashicorp#2334, It has changes to add new resource provider **azurerm_dev_test_lab_schedule**.

**Additional Points**
* Resource provider has been created based on [microsoft.devtestlab/2016-05-15/schedules](https://docs.microsoft.com/en-us/azure/templates/microsoft.devtestlab/2016-05-15/schedules)

(fixes hashicorp#2334)

* Update CHANGELOG.md to include hashicorp#3554

* Bug Fix: `azurerm_app_service_plan` - workaround for missing error on 404 hashicorp#3990

* Update CHANGELOG.md

* Fixing typo (hashicorp#3992)

* New Feature: `azrurerm_iot_dps` - add support for `linked_hub` (hashicorp#3922)

* Update CHANGELOG.md

* azurerm_postgresql_server  - remove unsupported version `10.2` (hashicorp#3915)

Closes hashicorp#3327

* Update CHANGELOG.md to include hashicorp#3915

* Switch more client registrations to new format (hashicorp#3895)

* Added property read and simplified validation

* Added check that the log analytics workspace id is provided

* Added log_analytics_destination_type to the logAnalyticsWorkspace test

* Data source for azurerm_dev_test_virtual_network (hashicorp#3746)

* Update CHANGELOG.md to include hashicorp#3554

* azurerm_function_app - support for cors (hashicorp#3949)

* Update CHANGELOG.md to include hashicorp#3949

* azurerm_role_definition - enture role_definition_id is correctl… (hashicorp#3913)

We encountered the issue where role_definition_id remained nil after create, returning 409s on RoleDefinitionsClient.CreateOrUpdate with code RoleDefinitionWithSameNameExists.

Updating role definition resource was failing because resourceArmRoleDefinitionCreateUpdate checks if role_definition_id is nil and, if so, creates a new uuid and calls client.CreateOrUpdate which attempts to create a new resource instead of updating.

* Update CHANGELOG.md to include hashicorp#3913

* Updated api_management_api_policy for XML file explanation (hashicorp#3994)

Added a note for those seeking to utilize -PolicyFilePath from docs.microsoft.com/en-us/powershell/module/azurerm.apimanagement/set-azurermapimanagementpolicy?view=azurermps-6.13.0#parameters

Tested in Azure, this works and would prevent work on implementing PolicyFilePath as another parameter.

* `azurerm_traffic_manager_profile`  - support for the `interval_… (hashicorp#3473)

* Update CHANGELOG.md to include hashicorp#3473

* Migrate more clients to new registration pattern (hashicorp#3999)

* azurerm_batch_certificate - fix thumbprint casing in tests (hashicorp#3977)

* update CHANGELOG.md to include hashicorp#3977

* Update azurerm/resource_arm_cosmosdb_account.go

* Update azurerm/resource_arm_cosmosdb_account.go

* quote echo

* update echo

* change GOGC to 15

* Support for registering Data Sources and Resources from packages

* tags: refactoring out to a new package

    ```
    $ go test -v ./azurerm/internal/tags/
    === RUN   TestExpand
    --- PASS: TestExpand (0.00s)
    === RUN   TestFilter
    --- PASS: TestFilter (0.00s)
    === RUN   TestFlatten
    --- PASS: TestFlatten (0.00s)
        flatten_test.go:46: [DEBUG] Test "Empty"
        flatten_test.go:46: [DEBUG] Test "One Item"
        flatten_test.go:46: [DEBUG] Test "Multiple Items"
    === RUN   TestValidateMaximumNumberOfTags
    --- PASS: TestValidateMaximumNumberOfTags (0.00s)
    === RUN   TestValidateTagMaxKeyLength
    --- PASS: TestValidateTagMaxKeyLength (0.00s)
    === RUN   TestValidateTagMaxValueLength
    --- PASS: TestValidateTagMaxValueLength (0.00s)
    PASS
    ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/tags 0.021s

* tags: moving the existing methods to deprecated.go

* refactor: moving the top level methods

* refactor: moving the networking specific method out

* azurerm_analysis_services_server - correct import documentation (server -> servers) (hashicorp#4005)

* change GOGC to 20

* Moved test changes to new test TestAccAzureRMMonitorDiagnosticSetting_logAnalyticsWorkspaceDedicated.

* Update storage_account.html.markdown

* azurerm_container_group - allow empty log_type

This is not documented anywhere, as far as I can tell, but if you leave
out `logType` when creating the container, you get the behavior of
both `ContainerInsights` and `ContainerInstanceLogs` at once.

If `logType` is not set, metadata can't be set either.

* refactor: moving the validation functions into the validate package

* refactor: moving into the deprecated.go

* f/service-registration: updating the checklist needed to refactor into packages

* azurem_container_group - add a test for unset log_type

* fixing the linting

* fixing the linting

* refactor: moving locks into the internal package

* Fix typo in private_dns_a_record docs

* provider: add the disable_correlation_request_id option and red… (hashicorp#4023)

* resolve conflicts

* Add resource_arm_private_dns_cname_record

* updating to include hashicorp#4024

* Fix review comments

Co-Authored-By: Tom Harvey <tombuildsstuff@users.noreply.github.com>

* Fix lint issue

* dns_zone: deprecatiog the `zone_type` field

* updating to include hashicorp#4028

* Update postgresql_server.html.markdown

Clarified what geo_redundant_backup means.  Source: https://docs.microsoft.com/en-us/azure/postgresql/concepts-backup

* updating to include hashicorp#4033

* r/container_group: polling to ensure the NIC is removed from the Network Profile

* removing a todo

* provider: ensure test logging has date/time

* updating to include hashicorp#3716

* r/storage_account: sorting the docs alphabtically

* r/storage_account: defaulting the default_action field to 'Allow'

* r/storage_account: moving the 'not applicable' text under the top level block definition

* provider: misc golang inspection cleanup (hashicorp#4039)

* Updating to include hashicorp#4013

* clarification on which API to use (hashicorp#3946)

* go get github.com/hashicorp/terraform@v0.12.6; go mod tidy; go mod vendor;

* updating to include hashicorp#4041

* Updating to include hashicorp#4037

* r/monitor_diagnostic_setting: fixing the formatting of the note

* updating to include hashicorp#3987

* change graphrabc client registrations to new pattern

* change compute client registrations to new pattern

* change datalake client registrations to new pattern

* change monitor  client registrations to new pattern

* Update servicebus_subscription_rule.html.markdown (hashicorp#4051)

The import example for the subscription filter is incorrect- the resource type is 'azurerm_servicebus_subscription', where it should be 'azurerm_servicebus_subscription_rule'

* change network client registrations to new pattern

* change resource client registrations to new pattern

* change sql client registrations to new pattern

* Upped Storage share limit to 100TB; updated docs (hashicorp#4054)

* Update CHANGELOG.md to include hashicorp#4054

* I noticed at the top of the page, it still said azurerm_dns_a_record instead of azurerm_private_dns_a_record

* Update function_app.html.markdown (hashicorp#4073)

* Update function_app.html.markdown

I have run into hours of headaches with Azure in particular -- unless some default app settings are specified / seeded within my terraform code. Please include note specified under app settings (and/or something similar). We deploy with Azure DevOps release pipeline, and without these defaults, we cannot successfully deploy to a terraformed function app.

* Update function_app.html.markdown

* provider: increase linter deadline to 10m (hashicorp#4077)

* New Resource: azurerm_mariadb_configuration (hashicorp#4060)

* Update to include hashicorp#4060

* [azurerm_role_assignment] Add support for Management Groups (hashicorp#4063)

Solves hashicorp#4040

* update CHANGELOG.md to include hashicorp#4063

* [resource_arm_function_app] Add missing vnet_name property (hashicorp#4078)

* Update CHANGELOG.md to include hashicorp#4078
@ghost
Copy link

ghost commented Aug 22, 2019

This has been released in version 1.33.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 1.33.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Sep 8, 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!

@hashicorp hashicorp locked and limited conversation to collaborators Sep 8, 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.

Omitting default_action should result in a deployment failure.
3 participants