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

appconfiguration- avoid replace in update test #27415

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions .github/labeler-pull-request-triage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
60 changes: 60 additions & 0 deletions internal/services/appconfiguration/app_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"bytes"
"context"
"fmt"
"log"

"github.com/Azure/go-autorest/autorest"
"github.com/hashicorp/go-azure-helpers/lang/pointer"
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is added to ensure the App Config Key/Feature property is fully synced after an update.

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 length %q, another with length %q", len(kv1.Tags), len(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", pointer.From(v), pointer.From(v2))
return false
}
}
}

return true
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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)
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,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)
Expand Down Expand Up @@ -905,8 +905,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
Expand Down
Loading
Loading