Bugfix: Only pass snapshot settings into the update plan if there are…

… changes (#858)

- 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.

.changelog/858.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/deployment: Avoid overriding snapshot settings with every update. The snapshot settings are now only updated if they are actually set in the terraform config. This allows managing the snapshot lifecycle policy with the elasticstack provider instead of the ec provider.
+```

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 {

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)...)

ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload_test.go

@@ -2103,6 +2103,97 @@ func Test_writeElasticsearch(t *testing.T) {
 				},
 			}),
 		},
+		{
+			name: "don't put snapshot settings into payload if there is no change between plan and state",
+			args: args{
+				esPlan: Elasticsearch{
+					RefId:      ec.String("main-elasticsearch"),
+					ResourceId: ec.String(mock.ValidClusterID),
+					Region:     ec.String("some-region"),
+					Snapshot: &ElasticsearchSnapshot{
+						Enabled: true,
+						Repository: &ElasticsearchSnapshotRepositoryInfo{
+							Reference: &ElasticsearchSnapshotRepositoryReference{
+								RepositoryName: "my-snapshot-repository",
+							},
+						},
+					},
+					HotTier: &ElasticsearchTopology{
+						id:        "hot_content",
+						Size:      ec.String("4g"),
+						ZoneCount: 1,
+					},
+				},
+				esState: &Elasticsearch{
+					RefId:      ec.String("main-elasticsearch"),
+					ResourceId: ec.String(mock.ValidClusterID),
+					Region:     ec.String("some-region"),
+					Snapshot: &ElasticsearchSnapshot{
+						Enabled: true,
+						Repository: &ElasticsearchSnapshotRepositoryInfo{
+							Reference: &ElasticsearchSnapshotRepositoryReference{
+								RepositoryName: "my-snapshot-repository",
+							},
+						},
+					},
+					HotTier: &ElasticsearchTopology{
+						id:        "hot_content",
+						Size:      ec.String("2g"),
+						ZoneCount: 1,
+					},
+				},
+				updatePayloads: testutil.UpdatePayloadsFromTemplate(t, "../../testdata/template-aws-io-optimized-v2.json"),
+				templateID:     "aws-io-optimized-v2",
+				version:        "7.7.0",
+				useNodeRoles:   false,
+			},
+			want: EnrichWithEmptyTopologies(tp770(), &models.ElasticsearchPayload{
+				Region: ec.String("some-region"),
+				RefID:  ec.String("main-elasticsearch"),
+				Settings: &models.ElasticsearchClusterSettings{
+					DedicatedMastersThreshold: 6,
+					Snapshot:                  nil,
+				},
+				Plan: &models.ElasticsearchClusterPlan{
+					AutoscalingEnabled: ec.Bool(false),
+					Elasticsearch: &models.ElasticsearchConfiguration{
+						Version: "7.7.0",
+					},
+					DeploymentTemplate: &models.DeploymentTemplateReference{
+						ID: ec.String("aws-io-optimized-v2"),
+					},
+					ClusterTopology: []*models.ElasticsearchClusterTopologyElement{
+						{
+							ID:                      "hot_content",
+							ZoneCount:               1,
+							InstanceConfigurationID: "aws.data.highio.i3",
+							Size: &models.TopologySize{
+								Resource: ec.String("memory"),
+								Value:    ec.Int32(4096),
+							},
+							NodeType: &models.ElasticsearchNodeType{
+								Data:   ec.Bool(true),
+								Ingest: ec.Bool(true),
+								Master: ec.Bool(true),
+							},
+							Elasticsearch: &models.ElasticsearchConfiguration{
+								NodeAttributes: map[string]string{"data": "hot"},
+							},
+							TopologyElementControl: &models.TopologyElementControl{
+								Min: &models.TopologySize{
+									Resource: ec.String("memory"),
+									Value:    ec.Int32(1024),
+								},
+							},
+							AutoscalingMax: &models.TopologySize{
+								Value:    ec.Int32(118784),
+								Resource: ec.String("memory"),
+							},
+						},
+					},
+				},
+			}),
+		},
 		{
 			name: "parse autodetect configuration strategy",
 			args: args{

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
 	}