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_app_configuration - omit no permission error in get soft-deleted #19661

Merged
merged 10 commits into from
Jun 7, 2023

Conversation

teowa
Copy link
Contributor

@teowa teowa commented Dec 13, 2022

resolves #19605

Omit no permission error in get soft-deleted.

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

The feature flag here

if !meta.(*clients.Client).Features.AppConfiguration.RecoverSoftDeleted {

Needs to wrap the whole retrieval of the deleted configuration stores which begins on line 249
deletedConfigurationStoresId := deletedconfigurationstores.NewDeletedConfigurationStoreID(subscriptionId, location, name)

@teowa
Copy link
Contributor Author

teowa commented Dec 14, 2022

@stephybun , thanks for reviewing this. I have modified the code. But I find if move the retrieval of deleted into the features toggle scope, we cannot return the optedOutOfRecoveringSoftDeletedAppConfigurationErrorFmt error when the deleted exists and recover deleted features toggle disabled.

And there is test failure, the App Conf resource creation is failed and should not exist but returned by resource group list API, which might related to #18796

TF_ACC=1 go test -v ./internal/services/appconfiguration -parallel 20 -test.run=TestAccAppConfiguration_softDeleteRecoveryDisabled -timeout 1440m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAppConfiguration_softDeleteRecoveryDisabled
=== PAUSE TestAccAppConfiguration_softDeleteRecoveryDisabled
=== CONT  TestAccAppConfiguration_softDeleteRecoveryDisabled
    testing_new.go:84: Error running post-test destroy, there may be dangling resources: exit status 1
        
        Error: deleting Resource Group "acctestRG-appconfig-221214143801923073": the Resource Group still contains Resources.
        
        Terraform is configured to check for Resources within the Resource Group when deleting the Resource Group - and
        raise an error if nested Resources still exist to avoid unintentionally deleting these Resources.
        
        Terraform has detected that the following Resources still exist within the Resource Group:
        
        * `/subscriptions/85b3dbca-5974-4067-9669-67a141095a76/resourceGroups/acctestRG-appconfig-221214143801923073/providers/Microsoft.AppConfiguration/configurationStores/testaccappconf221214143801923073`
        
        This feature is intended to avoid the unintentional destruction of nested Resources provisioned through some
        other means (for example, an ARM Template Deployment) - as such you must either remove these Resources, or
        disable this behaviour using the feature flag `prevent_deletion_if_contains_resources` within the `features`
        block when configuring the Provider, for example:
        
        provider "azurerm" {
          features {
            resource_group {
              prevent_deletion_if_contains_resources = false
            }
          }
        }
        
        When that feature flag is set, Terraform will skip checking for any Resources within the Resource Group and
        delete this using the Azure API directly (which will clear up any nested resources).
        
        More information on the `features` block can be found in the documentation:
        https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs#features
        
        
--- FAIL: TestAccAppConfiguration_softDeleteRecoveryDisabled (830.13s)
FAIL
FAIL    github.com/hashicorp/terraform-provider-azurerm/internal/services/appconfiguration      830.144s
FAIL
make: *** [GNUmakefile:99: acctests] Error 1

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

We should still return an error if the user is missing the necessary permissions for retrieval and is opted into this feature which is enabled by default, the error message should be changed to something like this:

func userIsMissingNecessaryPermission(name, location string) string {
	return fmt.Sprintf(`
An existing soft-deleted App Configuration exists with the Name %q in the location %q, however
the credentials Terraform is using has insufficient permissions to check for an existing soft-deleted App Configuration.
You can opt out of this behaviour by using the "features" block (located within the "provider" block) - more information
can be found here:
https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs#features
Alternatively you can manually recover this (e.g. using the Azure CLI) and then import
this into Terraform via "terraform import".
`, name, location)
}

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks @teowa, there's a test failure for this service, would you mind fixing it? Once that's done this should be good to go!

------- Stdout: -------
=== RUN   TestAccAppConfigurationKeysDataSource_allkeys
=== PAUSE TestAccAppConfigurationKeysDataSource_allkeys
=== CONT  TestAccAppConfigurationKeysDataSource_allkeys
testcase.go:110: Step 1/1 error: Error running apply: exit status 1
Error: checking for presence of existing Key: Configuration Store Id "/subscriptions/*******/resourceGroups/acctestRG-appconfig-230118072400473047/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230118072400473047" / Label "label1" / Key "key1": appconfiguration.BaseClient#GetKeyValue: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error: EOF
with azurerm_app_configuration_key.test,
on terraform_plugin_test.tf line 44, in resource "azurerm_app_configuration_key" "test":
44: resource "azurerm_app_configuration_key" "test" {
checking for presence of existing Key: Configuration Store Id
"/subscriptions/*******/resourceGroups/acctestRG-appconfig-230118072400473047/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230118072400473047"
/ Label "label1" / Key "key1": appconfiguration.BaseClient#GetKeyValue:
Failure responding to request: StatusCode=403 -- Original Error:
autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error:
EOF
Error: checking for presence of existing Key: Configuration Store Id "/subscriptions/*******/resourceGroups/acctestRG-appconfig-230118072400473047/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230118072400473047" / Label "label2" / Key "key1": appconfiguration.BaseClient#GetKeyValue: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error: EOF
with azurerm_app_configuration_key.test2,
on terraform_plugin_test.tf line 52, in resource "azurerm_app_configuration_key" "test2":
52: resource "azurerm_app_configuration_key" "test2" {
checking for presence of existing Key: Configuration Store Id
"/subscriptions/*******/resourceGroups/acctestRG-appconfig-230118072400473047/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230118072400473047"
/ Label "label2" / Key "key1": appconfiguration.BaseClient#GetKeyValue:
Failure responding to request: StatusCode=403 -- Original Error:
autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error:
EOF

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.

Still have test failures:

------- Stdout: -------
=== RUN   TestAccAppConfigurationKeysDataSource_allkeys
=== PAUSE TestAccAppConfigurationKeysDataSource_allkeys
=== CONT  TestAccAppConfigurationKeysDataSource_allkeys
    testcase.go:110: Step 1/1 error: Error running apply: exit status 1
        
        Error: checking for presence of existing Key: Configuration Store Id "/subscriptions/*******/resourceGroups/acctestRG-appconfig-230130193705070873/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230130193705070873" / Label "label1" / Key "key1": appconfiguration.BaseClient#GetKeyValue: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error: EOF
        
          with azurerm_app_configuration_key.test,
          on terraform_plugin_test.tf line 44, in resource "azurerm_app_configuration_key" "test":
          44: resource "azurerm_app_configuration_key" "test" {
        
        checking for presence of existing Key: Configuration Store Id
        "/subscriptions/*******/resourceGroups/acctestRG-appconfig-230130193705070873/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230130193705070873"
        / Label "label1" / Key "key1": appconfiguration.BaseClient#GetKeyValue:
        Failure responding to request: StatusCode=403 -- Original Error:
        autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error:
        EOF
        
        Error: checking for presence of existing Key: Configuration Store Id "/subscriptions/*******/resourceGroups/acctestRG-appconfig-230130193705070873/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230130193705070873" / Label "label2" / Key "key1": appconfiguration.BaseClient#GetKeyValue: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error: EOF
        
          with azurerm_app_configuration_key.test2,
          on terraform_plugin_test.tf line 52, in resource "azurerm_app_configuration_key" "test2":
          52: resource "azurerm_app_configuration_key" "test2" {
        
        checking for presence of existing Key: Configuration Store Id
        "/subscriptions/*******/resourceGroups/acctestRG-appconfig-230130193705070873/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230130193705070873"
        / Label "label2" / Key "key1": appconfiguration.BaseClient#GetKeyValue:
        Failure responding to request: StatusCode=403 -- Original Error:
        autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error:
        EOF
        
        Error: checking for presence of existing Key: Configuration Store Id "/subscriptions/*******/resourceGroups/acctestRG-appconfig-230130193705070873/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230130193705070873" / Label "testlabel" / Key "key2": appconfiguration.BaseClient#GetKeyValue: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error: EOF
        
          with azurerm_app_configuration_key.test3,
          on terraform_plugin_test.tf line 60, in resource "azurerm_app_configuration_key" "test3":
          60: resource "azurerm_app_configuration_key" "test3" {
        
        checking for presence of existing Key: Configuration Store Id
        "/subscriptions/*******/resourceGroups/acctestRG-appconfig-230130193705070873/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230130193705070873"
        / Label "testlabel" / Key "key2": appconfiguration.BaseClient#GetKeyValue:
        Failure responding to request: StatusCode=403 -- Original Error:
        autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error:
        EOF
        
        Error: checking for presence of existing Key: Configuration Store Id "/subscriptions/*******/resourceGroups/acctestRG-appconfig-230130193705070873/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230130193705070873" / Label "testlabel" / Key "key3": appconfiguration.BaseClient#GetKeyValue: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error: EOF
        
          with azurerm_app_configuration_key.test4,
          on terraform_plugin_test.tf line 68, in resource "azurerm_app_configuration_key" "test4":
          68: resource "azurerm_app_configuration_key" "test4" {
        
        checking for presence of existing Key: Configuration Store Id
        "/subscriptions/*******/resourceGroups/acctestRG-appconfig-230130193705070873/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230130193705070873"
        / Label "testlabel" / Key "key3": appconfiguration.BaseClient#GetKeyValue:
        Failure responding to request: StatusCode=403 -- Original Error:
        autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error:
        EOF
        
        Error: checking for presence of existing Key: Configuration Store Id "/subscriptions/*******/resourceGroups/acctestRG-appconfig-230130193705070873/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230130193705070873" / Label "" / Key "key3": appconfiguration.BaseClient#GetKeyValue: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: error response cannot be parsed: {"" '\x00' '\x00'} error: EOF
        
          with azurerm_app_configuration_key.test5,
          on terraform_plugin_test.tf line 76, in resource "azurerm_app_configuration_key" "test5":
          76: resource "azurerm_app_configuration_key" "test5" {
        
        checking for presence of existing Key: Configuration Store Id
        "/subscriptions/*******/resourceGroups/acctestRG-appconfig-230130193705070873/providers/Microsoft.AppConfiguration/configurationStores/testacc-appconf230130193705070873"
        / Label "" / Key "key3": appconfiguration.BaseClient#GetKeyValue: Failure
        responding to request: StatusCode=403 -- Original Error: autorest/azure:
        error response cannot be parsed: {"" '\x00' '\x00'} error: EOF
--- FAIL: TestAccAppConfigurationKeysDataSource_allkeys (97.43s)
FAIL

@stephybun
Copy link
Member

@teowa any update on this?

@teowa
Copy link
Contributor Author

teowa commented Feb 22, 2023

The PR should be OK, the failed test is related to #20324

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.

the tests are still failing:

image

@katbyte
Copy link
Collaborator

katbyte commented Mar 20, 2023

@brental - while the GH CI is passing the acctests are failing stil.

@teowa - we are still seeing a test failure:

------- Stdout: -------
=== RUN   TestAccAppConfiguration_softDeletePurgeThenRecreate
=== PAUSE TestAccAppConfiguration_softDeletePurgeThenRecreate
=== CONT  TestAccAppConfiguration_softDeletePurgeThenRecreate
    testcase.go:110: Step 4/5 error: Error running apply: exit status 1
        
        Error: creating Configuration Store (Subscription: "*******"
        Resource Group Name: "acctestRG-appconfig-230320083720637210"
        Configuration Store Name: "testaccappconf230320083720637210"): polling after Create: Code="NameUnavailable" Message="The specified name is already in use." AdditionalInfo=[{"info":{"activityId":"c857dfe2-fe66-4c04-b2bc-04839b5b19d0"},"type":"ActivityId"}]
        

@teowa
Copy link
Contributor Author

teowa commented Apr 6, 2023

Fixing of the failed test TestAccAppConfiguration_softDeletePurgeThenRecreate is blocked by hashicorp/pandora#1373

@stephybun
Copy link
Member

@teowa this should have been unblocked by hashicorp/pandora#2363

@teowa
Copy link
Contributor Author

teowa commented May 11, 2023

HI @stephybun , I have submit another PR #21750 to fix the failed test case.

@ahmar-husain
Copy link

Does anyone know, when this PR will be merged?

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

@teowa looks like a rebase is required

------- Stdout: -------
=== RUN   TestAccAppConfiguration_softDeletePurgeThenRecreate
=== PAUSE TestAccAppConfiguration_softDeletePurgeThenRecreate
=== CONT  TestAccAppConfiguration_softDeletePurgeThenRecreate
testcase.go:110: Step 4/5 error: Error running apply: exit status 1
Error: creating Configuration Store (Subscription: "*******"
Resource Group Name: "acctestRG-appconfig-230601085823840487"
Configuration Store Name: "testaccappconf230601085823840487"): polling after Create: internal-error: a polling status of `Failed` should be surfaced as a PollingFailedError
with azurerm_app_configuration.test,
on terraform_plugin_test.tf line 26, in resource "azurerm_app_configuration" "test":
26: resource "azurerm_app_configuration" "test" {
--- FAIL: TestAccAppConfiguration_softDeletePurgeThenRecreate (659.92s)
FAIL

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Tests are passing now, thanks @teowa LGTM 👍

@stephybun stephybun merged commit 3f6f8ac into hashicorp:main Jun 7, 2023
@github-actions github-actions bot added this to the v3.60.0 milestone Jun 7, 2023
stephybun added a commit that referenced this pull request Jun 7, 2023
katbyte added a commit that referenced this pull request Jun 9, 2023
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 May 28, 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.

Support for disabling detection of soft deleted azurerm_app_configuration resources
6 participants