Skip to content

Commit

Permalink
Bugfix: Only pass snapshot settings into the update plan if there are…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
gigerdo committed Sep 20, 2024
1 parent c155361 commit 2a9cc0b
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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() {
Expand All @@ -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)...)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 2a9cc0b

Please sign in to comment.