From 2a9cc0b468bbd358a7f8cea77eeb4956c06abf90 Mon Sep 17 00:00:00 2001 From: Dominik Giger Date: Fri, 20 Sep 2024 14:45:38 +0200 Subject: [PATCH] Bugfix: Only pass snapshot settings into the update plan if there are changes. - This ensures the settings are only put into the update request if there are actual changes in the config - Otherwise the settings are left out of the payload - This solves a bug where every apply would reset the policy back to the default (because putting the default settings in the plan will cause it to reset the policy). - When using the elasticstack provider to manage the policy, we want the ec provider to not touch it. --- .../v2/elasticsearch_keystore_contents.go | 12 +++--------- .../elasticsearch/v2/elasticsearch_payload.go | 14 +++++++++++--- .../elasticsearch/v2/elasticsearch_snapshot.go | 8 +++++++- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_keystore_contents.go b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_keystore_contents.go index b8a03f33c..9dc275e2d 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_keystore_contents.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_keystore_contents.go @@ -37,10 +37,10 @@ type ElasticsearchKeystoreContents struct { AsFile *bool `tfsdk:"as_file"` } -func elasticsearchKeystoreContentsPayload(ctx context.Context, keystoreContentsTF types.Map, model *models.ElasticsearchClusterSettings, esStateObj *types.Object) (*models.ElasticsearchClusterSettings, diag.Diagnostics) { +func elasticsearchKeystoreContentsPayload(ctx context.Context, keystoreContentsTF types.Map, model *models.ElasticsearchClusterSettings, esState *ElasticsearchTF) (*models.ElasticsearchClusterSettings, diag.Diagnostics) { var diags diag.Diagnostics - if (keystoreContentsTF.IsNull() || len(keystoreContentsTF.Elements()) == 0) && esStateObj == nil { + if (keystoreContentsTF.IsNull() || len(keystoreContentsTF.Elements()) == 0) && esState == nil { return model, nil } @@ -69,13 +69,7 @@ func elasticsearchKeystoreContentsPayload(ctx context.Context, keystoreContentsT } // remove secrets that were in state but are removed from plan - if esStateObj != nil && !esStateObj.IsNull() { - var esState ElasticsearchTF - - if diags := tfsdk.ValueAs(ctx, esStateObj, &esState); diags.HasError() { - return nil, diags - } - + if esState != nil { if !esState.KeystoreContents.IsNull() { for k := range esState.KeystoreContents.Elements() { if _, ok := secrets[k]; !ok { diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload.go b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload.go index 9c569b9d2..fc6d95229 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload.go @@ -61,13 +61,21 @@ func ElasticsearchPayload(ctx context.Context, plan types.Object, state *types.O return nil, diags } + var esState *ElasticsearchTF + if state != nil { + esState, diags = objectToElasticsearch(ctx, *state) + if diags.HasError() { + return nil, diags + } + } + if es == nil { return nil, nil } templatePayload := EnrichElasticsearchTemplate(payloadFromUpdate(updateResources), dtID, version, useNodeRoles) - payload, diags := es.payload(ctx, templatePayload, state) + payload, diags := es.payload(ctx, templatePayload, esState) if diags.HasError() { return nil, diags } @@ -123,7 +131,7 @@ func CheckAvailableMigration(ctx context.Context, plan types.Object, state types return false, nil } -func (es *ElasticsearchTF) payload(ctx context.Context, res *models.ElasticsearchPayload, state *types.Object) (*models.ElasticsearchPayload, diag.Diagnostics) { +func (es *ElasticsearchTF) payload(ctx context.Context, res *models.ElasticsearchPayload, state *ElasticsearchTF) (*models.ElasticsearchPayload, diag.Diagnostics) { var diags diag.Diagnostics if !es.RefId.IsNull() { @@ -149,7 +157,7 @@ func (es *ElasticsearchTF) payload(ctx context.Context, res *models.Elasticsearc res.Plan.Elasticsearch, ds = elasticsearchConfigPayload(ctx, es.Config, res.Plan.Elasticsearch) diags.Append(ds...) - res.Settings, ds = elasticsearchSnapshotPayload(ctx, es.Snapshot, res.Settings) + res.Settings, ds = elasticsearchSnapshotPayload(ctx, es.Snapshot, res.Settings, state) diags.Append(ds...) diags.Append(elasticsearchSnapshotSourcePayload(ctx, es.SnapshotSource, res.Plan)...) diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_snapshot.go b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_snapshot.go index 8b068d31d..d40583afb 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_snapshot.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_snapshot.go @@ -78,12 +78,18 @@ func readElasticsearchSnapshot(in *models.ElasticsearchClusterSettings) (*Elasti return &snapshot, nil } -func elasticsearchSnapshotPayload(ctx context.Context, srcObj attr.Value, model *models.ElasticsearchClusterSettings) (*models.ElasticsearchClusterSettings, diag.Diagnostics) { +func elasticsearchSnapshotPayload(ctx context.Context, srcObj attr.Value, model *models.ElasticsearchClusterSettings, state *ElasticsearchTF) (*models.ElasticsearchClusterSettings, diag.Diagnostics) { var snapshot ElasticsearchSnapshotTF if srcObj.IsNull() || srcObj.IsUnknown() { return model, nil } + // Only put snapshot updates into the payload, if the plan is making changes to the snapshot settings + // (To avoid overwriting changes made outside, i.e. with the elasticstack provider) + if state != nil && state.Snapshot.Equal(srcObj) { + return model, nil + } + if diags := tfsdk.ValueAs(ctx, srcObj, &snapshot); diags.HasError() { return model, diags }