From 4b95409b0f5f70c4661fccebfb01238a9607e58a Mon Sep 17 00:00:00 2001 From: teowa <104055472+teowa@users.noreply.github.com> Date: Wed, 18 Sep 2024 05:49:21 +0000 Subject: [PATCH 1/4] fix replace in update --- .../app_configuration_key_resource_test.go | 2 +- .../app_configuration_resource_test.go | 23 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/internal/services/appconfiguration/app_configuration_key_resource_test.go b/internal/services/appconfiguration/app_configuration_key_resource_test.go index e81ff630c342..6989a5c68cbe 100644 --- a/internal/services/appconfiguration/app_configuration_key_resource_test.go +++ b/internal/services/appconfiguration/app_configuration_key_resource_test.go @@ -157,7 +157,7 @@ func TestAccAppConfigurationKey_basicNoValue(t *testing.T) { r := AppConfigurationKeyResource{} data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.basic(data), + Config: r.basicNoLabel(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), diff --git a/internal/services/appconfiguration/app_configuration_resource_test.go b/internal/services/appconfiguration/app_configuration_resource_test.go index cd7c9ea2e3bc..c559364ddc54 100644 --- a/internal/services/appconfiguration/app_configuration_resource_test.go +++ b/internal/services/appconfiguration/app_configuration_resource_test.go @@ -147,7 +147,7 @@ func TestAccAppConfiguration_update(t *testing.T) { data.ResourceTest(t, r, []acceptance.TestStep{ { - Config: r.standard(data), + Config: r.standardWithSoftDeleteRetentionDays(data), Check: acceptance.ComposeTestCheckFunc( check.That(data.ResourceName).ExistsInAzure(r), ), @@ -499,6 +499,27 @@ resource "azurerm_app_configuration" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomInteger) } +func (AppConfigurationResource) standardWithSoftDeleteRetentionDays(data acceptance.TestData) string { + return fmt.Sprintf(` +provider "azurerm" { + features {} +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-appconfig-%d" + location = "%s" +} + +resource "azurerm_app_configuration" "test" { + name = "testaccappconf%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + sku = "standard" + soft_delete_retention_days = 1 +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger) +} + func (AppConfigurationResource) replica(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurerm" { From 0ee3cecc100321ec7e0cb2c444c76505c23c1bc6 Mon Sep 17 00:00:00 2001 From: teowa <104055472+teowa@users.noreply.github.com> Date: Wed, 16 Oct 2024 02:20:43 +0000 Subject: [PATCH 2/4] app_configuration_key/feature wait synced in udpate --- .../appconfiguration/app_configuration.go | 60 +++++++++++++ .../app_configuration_feature_resource.go | 52 ++++++++---- ...app_configuration_feature_resource_test.go | 1 + .../app_configuration_key_resource.go | 85 ++++++++++++------- 4 files changed, 153 insertions(+), 45 deletions(-) diff --git a/internal/services/appconfiguration/app_configuration.go b/internal/services/appconfiguration/app_configuration.go index 15a94babf315..5b23b5af765a 100644 --- a/internal/services/appconfiguration/app_configuration.go +++ b/internal/services/appconfiguration/app_configuration.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "fmt" + "log" "github.com/Azure/go-autorest/autorest" "github.com/hashicorp/go-azure-helpers/lang/pointer" @@ -65,6 +66,7 @@ func resourceConfigurationStoreReplicaHash(input interface{}) int { func appConfigurationGetKeyRefreshFunc(ctx context.Context, client *appconfiguration.BaseClient, key, label string) pluginsdk.StateRefreshFunc { return func() (interface{}, string, error) { + log.Printf("[DEBUG] Refresh App Configuration status") res, err := client.GetKeyValue(ctx, key, label, "", "", "", []appconfiguration.KeyValueFields{}) if err != nil { if v, ok := err.(autorest.DetailedError); ok { @@ -81,3 +83,61 @@ func appConfigurationGetKeyRefreshFunc(ctx context.Context, client *appconfigura return res, "Exists", nil } } + +func appConfigurationGetKeyRefreshFuncForUpdate(ctx context.Context, client *appconfiguration.BaseClient, key, label string, model appconfiguration.KeyValue) pluginsdk.StateRefreshFunc { + return func() (interface{}, string, error) { + log.Printf("[DEBUG] Refresh App Configuration to ensure all properties are synced") + res, err := client.GetKeyValue(ctx, key, label, "", "", "", []appconfiguration.KeyValueFields{}) + if err != nil { + if v, ok := err.(autorest.DetailedError); ok { + if response.WasForbidden(v.Response) { + return "Forbidden", "Forbidden", nil + } + if response.WasNotFound(v.Response) { + return "NotFound", "NotFound", nil + } + } + return res, "Error", nil + } + + if !appConfigurationKeyValueEuqals(res, model) { + return "Syncing", "Syncing", nil + } + + return res, "Synced", nil + } +} + +func appConfigurationKeyValueEuqals(kv1, kv2 appconfiguration.KeyValue) bool { + if (kv1.ContentType == nil) != (kv2.ContentType == nil) || pointer.From(kv1.ContentType) != pointer.From(kv2.ContentType) { + log.Printf("[DEBUG] Syncing App Configuration Key `content_type`: one with value %q, another with value %q", pointer.From(kv1.ContentType), pointer.From(kv2.ContentType)) + return false + } + + if (kv1.Locked == nil) != (kv2.Locked == nil) || pointer.From(kv1.Locked) != pointer.From(kv2.Locked) { + log.Printf("[DEBUG] Syncing App Configuration Key `locked`: one with value %q, another with value %q", pointer.From(kv1.ContentType), pointer.From(kv2.ContentType)) + return false + } + + if (kv1.Value == nil) != (kv2.Value == nil) || pointer.From(kv1.Value) != pointer.From(kv2.Value) { + log.Printf("[DEBUG] Syncing App Configuration Key `value` field: one with value %q, another with value %q", pointer.From(kv1.Value), pointer.From(kv2.Value)) + return false + } + + if (kv1.Tags == nil) != (kv2.Tags == nil) || len(kv1.Tags) != len(kv2.Tags) { + log.Printf("[DEBUG] Syncing App Configuration Key `tags` field: one with value %q, another with value %q", kv1.Tags, kv2.Tags) + return false + } + + for k, v := range kv1.Tags { + if v != nil { + v2, ok := kv2.Tags[k] + if !ok || v2 == nil || *v != *v2 { + log.Printf("[DEBUG] Syncing App Configuration Key `tags` field: one with value %q, another with value %q", kv1.Tags, kv2.Tags) + return false + } + } + } + + return true +} diff --git a/internal/services/appconfiguration/app_configuration_feature_resource.go b/internal/services/appconfiguration/app_configuration_feature_resource.go index c105f1dc8e95..da0032cd70f9 100644 --- a/internal/services/appconfiguration/app_configuration_feature_resource.go +++ b/internal/services/appconfiguration/app_configuration_feature_resource.go @@ -44,6 +44,7 @@ type FeatureResourceModel struct { ConfigurationStoreId string `tfschema:"configuration_store_id"` Description string `tfschema:"description"` Enabled bool `tfschema:"enabled"` + Etag string `tfschema:"etag"` Key string `tfschema:"key"` Name string `tfschema:"name"` Label string `tfschema:"label"` @@ -383,7 +384,7 @@ func (k FeatureResource) Read() sdk.ResourceFunc { } var fv FeatureValue - err = json.Unmarshal([]byte(utils.NormalizeNilableString(kv.Value)), &fv) + err = json.Unmarshal([]byte(pointer.From(kv.Value)), &fv) if err != nil { return fmt.Errorf("while unmarshalling underlying key's value: %+v", err) } @@ -392,16 +393,14 @@ func (k FeatureResource) Read() sdk.ResourceFunc { ConfigurationStoreId: configurationStoreId.ID(), Description: fv.Description, Enabled: fv.Enabled, - Key: strings.TrimPrefix(utils.NormalizeNilableString(kv.Key), fmt.Sprintf("%s/", FeatureKeyPrefix)), + Etag: pointer.From(kv.Etag), + Key: strings.TrimPrefix(pointer.From(kv.Key), fmt.Sprintf("%s/", FeatureKeyPrefix)), Name: fv.ID, - Label: utils.NormalizeNilableString(kv.Label), + Label: pointer.From(kv.Label), + Locked: pointer.From(kv.Locked), Tags: tags.Flatten(kv.Tags), } - if kv.Locked != nil { - model.Locked = *kv.Locked - } - if len(fv.Conditions.ClientFilters.Filters) > 0 { for _, f := range fv.Conditions.ClientFilters.Filters { switch f := f.(type) { @@ -444,7 +443,7 @@ func (k FeatureResource) Update() sdk.ResourceFunc { } var fv FeatureValue - err = json.Unmarshal([]byte(utils.NormalizeNilableString(kv.Value)), &fv) + err = json.Unmarshal([]byte(pointer.From(kv.Value)), &fv) if err != nil { return fmt.Errorf("while unmarshalling underlying key's value: %+v", err) } @@ -552,16 +551,39 @@ func (k FeatureResource) Update() sdk.ResourceFunc { return err } - if model.Locked { - if _, err = client.PutLock(ctx, nestedItemId.Key, model.Label, "", ""); err != nil { - return fmt.Errorf("while locking key/label pair %s/%s: %+v", model.Name, model.Label, err) - } - } else { - if _, err = client.DeleteLock(ctx, nestedItemId.Key, model.Label, "", ""); err != nil { - return fmt.Errorf("while unlocking key/label pair %s/%s: %+v", model.Name, model.Label, err) + if metadata.ResourceData.HasChange("locked") { + kv.Locked = pointer.To(model.Locked) + if model.Locked { + if _, err = client.PutLock(ctx, nestedItemId.Key, model.Label, "", ""); err != nil { + return fmt.Errorf("while locking key/label pair %s/%s: %+v", model.Name, model.Label, err) + } + } else { + if _, err = client.DeleteLock(ctx, nestedItemId.Key, model.Label, "", ""); err != nil { + return fmt.Errorf("while unlocking key/label pair %s/%s: %+v", model.Name, model.Label, err) + } } } + deadline, ok := ctx.Deadline() + if !ok { + return fmt.Errorf("internal-error: context had no deadline") + } + + // https://github.com/Azure/AppConfiguration/issues/763 + metadata.Logger.Infof("[DEBUG] Waiting for App Configuration Feature %q to be synced", model.Key) + stateConf := &pluginsdk.StateChangeConf{ + Pending: []string{"Syncing"}, + Target: []string{"Synced"}, + Refresh: appConfigurationGetKeyRefreshFuncForUpdate(ctx, client, nestedItemId.Key, model.Label, kv), + PollInterval: 5 * time.Second, + ContinuousTargetOccurence: 2, + Timeout: time.Until(deadline), + } + + if _, err = stateConf.WaitForStateContext(ctx); err != nil { + return fmt.Errorf("waiting for App Configuration Feature %q to be synced: %+v", nestedItemId.Key, err) + } + return nil }, Timeout: 30 * time.Minute, diff --git a/internal/services/appconfiguration/app_configuration_feature_resource_test.go b/internal/services/appconfiguration/app_configuration_feature_resource_test.go index 3a75c397ccba..7acad45e511e 100644 --- a/internal/services/appconfiguration/app_configuration_feature_resource_test.go +++ b/internal/services/appconfiguration/app_configuration_feature_resource_test.go @@ -77,6 +77,7 @@ func TestAccAppConfigurationFeature_percentFilter(t *testing.T) { data.ImportStep(), }) } + func TestAccAppConfigurationFeature_basicNoLabel(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_app_configuration_feature", "test") r := AppConfigurationFeatureResource{} diff --git a/internal/services/appconfiguration/app_configuration_key_resource.go b/internal/services/appconfiguration/app_configuration_key_resource.go index 564e8062ac42..0862159902ae 100644 --- a/internal/services/appconfiguration/app_configuration_key_resource.go +++ b/internal/services/appconfiguration/app_configuration_key_resource.go @@ -11,6 +11,7 @@ import ( "time" "github.com/Azure/go-autorest/autorest" + "github.com/hashicorp/go-azure-helpers/lang/pointer" "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" "github.com/hashicorp/go-azure-sdk/resource-manager/appconfiguration/2023-03-01/configurationstores" "github.com/hashicorp/terraform-provider-azurerm/helpers/tf" @@ -194,22 +195,22 @@ func (k KeyResource) Create() sdk.ResourceFunc { } entity := appconfiguration.KeyValue{ - Key: utils.String(model.Key), - Label: utils.String(model.Label), + Key: pointer.To(model.Key), + Label: pointer.To(model.Label), Tags: tags.Expand(model.Tags), } switch model.Type { case KeyTypeKV: - entity.ContentType = utils.String(model.ContentType) - entity.Value = utils.String(model.Value) + entity.ContentType = pointer.To(model.ContentType) + entity.Value = pointer.To(model.Value) case KeyTypeVault: - entity.ContentType = utils.String(VaultKeyContentType) + entity.ContentType = pointer.To(VaultKeyContentType) ref, err := json.Marshal(VaultKeyReference{URI: model.VaultKeyReference}) if err != nil { return fmt.Errorf("while encoding vault key reference: %+v", err) } - entity.Value = utils.String(string(ref)) + entity.Value = pointer.To(string(ref)) } if _, err = client.PutKeyValue(ctx, model.Key, model.Label, &entity, "", ""); err != nil { @@ -302,19 +303,20 @@ func (k KeyResource) Read() sdk.ResourceFunc { model := KeyResourceModel{ ConfigurationStoreId: configurationStoreId.ID(), - Key: utils.NormalizeNilableString(kv.Key), - ContentType: utils.NormalizeNilableString(kv.ContentType), - Etag: utils.NormalizeNilableString(kv.Etag), - Label: utils.NormalizeNilableString(kv.Label), + Key: pointer.From(kv.Key), + ContentType: pointer.From(kv.ContentType), + Etag: pointer.From(kv.Etag), + Label: pointer.From(kv.Label), + Locked: pointer.From(kv.Locked), Tags: tags.Flatten(kv.Tags), } - if utils.NormalizeNilableString(kv.ContentType) != VaultKeyContentType { + if pointer.From(kv.ContentType) != VaultKeyContentType { model.Type = KeyTypeKV - model.Value = utils.NormalizeNilableString(kv.Value) + model.Value = pointer.From(kv.Value) } else { var ref VaultKeyReference - refBytes := []byte(utils.NormalizeNilableString(kv.Value)) + refBytes := []byte(pointer.From(kv.Value)) err := json.Unmarshal(refBytes, &ref) if err != nil { return fmt.Errorf("while unmarshalling vault reference: %+v", err) @@ -323,12 +325,9 @@ func (k KeyResource) Read() sdk.ResourceFunc { model.Type = KeyTypeVault model.VaultKeyReference = ref.URI model.ContentType = VaultKeyContentType - model.Value = utils.NormalizeNilableString(kv.Value) + model.Value = pointer.From(kv.Value) } - if kv.Locked != nil { - model.Locked = *kv.Locked - } return metadata.Encode(&model) }, Timeout: 5 * time.Minute, @@ -348,6 +347,11 @@ func (k KeyResource) Update() sdk.ResourceFunc { return err } + kv, err := client.GetKeyValue(ctx, nestedItemId.Key, nestedItemId.Label, "", "", "", []appconfiguration.KeyValueFields{}) + if err != nil { + return fmt.Errorf("while checking for key %q existence: %+v", *nestedItemId, err) + } + var model KeyResourceModel if err := metadata.Decode(&model); err != nil { return fmt.Errorf("decoding %+v", err) @@ -360,31 +364,31 @@ func (k KeyResource) Update() sdk.ResourceFunc { metadata.Client.AppConfiguration.AddToCache(*configurationStoreId, nestedItemId.ConfigurationStoreEndpoint) - if metadata.ResourceData.HasChange("value") || metadata.ResourceData.HasChange("content_type") || metadata.ResourceData.HasChange("tags") || metadata.ResourceData.HasChange("type") || metadata.ResourceData.HasChange("vault_key_reference") { - entity := appconfiguration.KeyValue{ - Key: utils.String(model.Key), - Label: utils.String(model.Label), - Tags: tags.Expand(model.Tags), - } + if metadata.ResourceData.HasChange("tag") { + kv.Tags = tags.Expand(model.Tags) + } + if metadata.ResourceData.HasChange("value") || metadata.ResourceData.HasChange("content_type") || metadata.ResourceData.HasChange("type") || metadata.ResourceData.HasChange("vault_key_reference") { switch model.Type { case KeyTypeKV: - entity.ContentType = utils.String(model.ContentType) - entity.Value = utils.String(model.Value) + kv.ContentType = pointer.To(model.ContentType) + kv.Value = pointer.To(model.Value) case KeyTypeVault: - entity.ContentType = utils.String(VaultKeyContentType) + kv.ContentType = pointer.To(VaultKeyContentType) ref, err := json.Marshal(VaultKeyReference{URI: model.VaultKeyReference}) if err != nil { return fmt.Errorf("while encoding vault key reference: %+v", err) } - entity.Value = utils.String(string(ref)) - } - if _, err = client.PutKeyValue(ctx, model.Key, model.Label, &entity, "", ""); err != nil { - return fmt.Errorf("while updating key/label pair %s/%s: %+v", model.Key, model.Label, err) + kv.Value = pointer.To(string(ref)) } } + if _, err = client.PutKeyValue(ctx, model.Key, model.Label, &kv, "", ""); err != nil { + return fmt.Errorf("while updating key/label pair %s/%s: %+v", model.Key, model.Label, err) + } + if metadata.ResourceData.HasChange("locked") { + kv.Locked = pointer.To(model.Locked) if model.Locked { if _, err = client.PutLock(ctx, model.Key, model.Label, "", ""); err != nil { return fmt.Errorf("while locking key/label pair %s/%s: %+v", model.Key, model.Label, err) @@ -395,6 +399,27 @@ func (k KeyResource) Update() sdk.ResourceFunc { } } } + + deadline, ok := ctx.Deadline() + if !ok { + return fmt.Errorf("internal-error: context had no deadline") + } + + // https://github.com/Azure/AppConfiguration/issues/763 + metadata.Logger.Infof("[DEBUG] Waiting for App Configuration Key %q to be synced", model.Key) + stateConf := &pluginsdk.StateChangeConf{ + Pending: []string{"Syncing"}, + Target: []string{"Synced"}, + Refresh: appConfigurationGetKeyRefreshFuncForUpdate(ctx, client, nestedItemId.Key, model.Label, kv), + PollInterval: 5 * time.Second, + ContinuousTargetOccurence: 2, + Timeout: time.Until(deadline), + } + + if _, err = stateConf.WaitForStateContext(ctx); err != nil { + return fmt.Errorf("waiting for App Configuration Key %q to be synced: %+v", nestedItemId.Key, err) + } + return nil }, Timeout: 30 * time.Minute, From b1e8686dc8f8f36b43f4148ac5c43a3c53debc7e Mon Sep 17 00:00:00 2001 From: teowa <104055472+teowa@users.noreply.github.com> Date: Wed, 16 Oct 2024 06:55:52 +0000 Subject: [PATCH 3/4] fix lint --- internal/services/appconfiguration/app_configuration.go | 4 ++-- .../appconfiguration/app_configuration_resource.go | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/internal/services/appconfiguration/app_configuration.go b/internal/services/appconfiguration/app_configuration.go index 5b23b5af765a..046dd2e7a684 100644 --- a/internal/services/appconfiguration/app_configuration.go +++ b/internal/services/appconfiguration/app_configuration.go @@ -125,7 +125,7 @@ func appConfigurationKeyValueEuqals(kv1, kv2 appconfiguration.KeyValue) bool { } if (kv1.Tags == nil) != (kv2.Tags == nil) || len(kv1.Tags) != len(kv2.Tags) { - log.Printf("[DEBUG] Syncing App Configuration Key `tags` field: one with value %q, another with value %q", kv1.Tags, kv2.Tags) + log.Printf("[DEBUG] Syncing App Configuration Key `tags` field: one with length %q, another with length %q", len(kv1.Tags), len(kv2.Tags)) return false } @@ -133,7 +133,7 @@ func appConfigurationKeyValueEuqals(kv1, kv2 appconfiguration.KeyValue) bool { if v != nil { v2, ok := kv2.Tags[k] if !ok || v2 == nil || *v != *v2 { - log.Printf("[DEBUG] Syncing App Configuration Key `tags` field: one with value %q, another with value %q", kv1.Tags, kv2.Tags) + log.Printf("[DEBUG] Syncing App Configuration Key `tags` field: one with value %q, another with value %q", pointer.From(v), pointer.From(v2)) return false } } diff --git a/internal/services/appconfiguration/app_configuration_resource.go b/internal/services/appconfiguration/app_configuration_resource.go index 5d62e1c0605f..b96ad1fe27d3 100644 --- a/internal/services/appconfiguration/app_configuration_resource.go +++ b/internal/services/appconfiguration/app_configuration_resource.go @@ -289,7 +289,7 @@ func resourceAppConfigurationCreate(d *pluginsdk.ResourceData, meta interface{}) deleted, err := deletedConfigurationStoresClient.ConfigurationStoresGetDeleted(ctx, deletedConfigurationStoresId) if err != nil { if response.WasStatusCode(deleted.HttpResponse, http.StatusForbidden) { - return fmt.Errorf(userIsMissingNecessaryPermission(name, location)) + return userIsMissingNecessaryPermission(name, location) } if !response.WasNotFound(deleted.HttpResponse) { return fmt.Errorf("checking for presence of deleted %s: %+v", deletedConfigurationStoresId, err) @@ -896,8 +896,8 @@ func parsePublicNetworkAccess(input string) *configurationstores.PublicNetworkAc return &out } -func userIsMissingNecessaryPermission(name, location string) string { - return fmt.Sprintf(` +func userIsMissingNecessaryPermission(name, location string) error { + return fmt.Errorf(` 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 @@ -925,7 +925,6 @@ func resourceConfigurationStoreWaitForNameAvailable(ctx context.Context, client } return nil - } func resourceConfigurationStoreNameAvailabilityRefreshFunc(ctx context.Context, client *operations.OperationsClient, configurationStoreId configurationstores.ConfigurationStoreId) pluginsdk.StateRefreshFunc { From dd86b5590d85fd0d42615f2f44c020f676e6bda1 Mon Sep 17 00:00:00 2001 From: teowa <104055472+teowa@users.noreply.github.com> Date: Thu, 31 Oct 2024 02:55:23 +0000 Subject: [PATCH 4/4] go generate --- .github/labeler-pull-request-triage.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.github/labeler-pull-request-triage.yml b/.github/labeler-pull-request-triage.yml index ed1d793fc197..18d6b28fb62d 100644 --- a/.github/labeler-pull-request-triage.yml +++ b/.github/labeler-pull-request-triage.yml @@ -574,12 +574,3 @@ service/workloads: - changed-files: - any-glob-to-any-file: - internal/services/workloads/**/* - -bug: - - '- \[ ?X ?\] Bug Fix' - -enhancement: - - '- \[ ?X ?\] Enhancement' - -breaking-change: - - '- \[ ?X ?\] Breaking Change' \ No newline at end of file