From 713dcac29fb9a1c4345735601a8ed3cf47641916 Mon Sep 17 00:00:00 2001 From: Dominik Giger Date: Mon, 23 Sep 2024 20:23:27 +0200 Subject: [PATCH] Elasticsearch trust settings: Only add them to the update payload if there are changes. (#859) The trust setting should only be added to the payload when they change. If they are left out of the payload, no changes to the trust will be made. --- .changelog/859.txt | 3 + .../elasticsearch/v2/elasticsearch_payload.go | 19 +-- .../v2/elasticsearch_payload_test.go | 121 ++++++++++++++++++ 3 files changed, 135 insertions(+), 8 deletions(-) create mode 100644 .changelog/859.txt diff --git a/.changelog/859.txt b/.changelog/859.txt new file mode 100644 index 000000000..b755065f1 --- /dev/null +++ b/.changelog/859.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/deployment: Avoid sending an update for trust settings if they have not changed. +``` \ No newline at end of file diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload.go b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload.go index fc6d95229..5c8a544c9 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload.go @@ -61,6 +61,10 @@ func ElasticsearchPayload(ctx context.Context, plan types.Object, state *types.O return nil, diags } + if es == nil { + return nil, nil + } + var esState *ElasticsearchTF if state != nil { esState, diags = objectToElasticsearch(ctx, *state) @@ -69,10 +73,6 @@ func ElasticsearchPayload(ctx context.Context, plan types.Object, state *types.O } } - if es == nil { - return nil, nil - } - templatePayload := EnrichElasticsearchTemplate(payloadFromUpdate(updateResources), dtID, version, useNodeRoles) payload, diags := es.payload(ctx, templatePayload, esState) @@ -168,11 +168,14 @@ func (es *ElasticsearchTF) payload(ctx context.Context, res *models.Elasticsearc res.Plan.AutoscalingEnabled = ec.Bool(es.Autoscale.ValueBool()) } - res.Settings, ds = elasticsearchTrustAccountPayload(ctx, es.TrustAccount, res.Settings) - diags.Append(ds...) + // Only add trust settings to update payload if trust has changed + if state == nil || !es.TrustAccount.Equal(state.TrustAccount) || !es.TrustExternal.Equal(state.TrustExternal) { + res.Settings, ds = elasticsearchTrustAccountPayload(ctx, es.TrustAccount, res.Settings) + diags.Append(ds...) - res.Settings, ds = elasticsearchTrustExternalPayload(ctx, es.TrustExternal, res.Settings) - diags.Append(ds...) + res.Settings, ds = elasticsearchTrustExternalPayload(ctx, es.TrustExternal, res.Settings) + diags.Append(ds...) + } res.Settings, ds = elasticsearchKeystoreContentsPayload(ctx, es.KeystoreContents, res.Settings, state) diags.Append(ds...) diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload_test.go b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload_test.go index b843e6912..9cadc8e54 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload_test.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_payload_test.go @@ -2666,6 +2666,127 @@ func Test_writeElasticsearch(t *testing.T) { }, }), }, + { + name: "don't put trust settings into the payload if they haven't changed", + args: args{ + esPlan: Elasticsearch{ + RefId: ec.String("main-elasticsearch"), + ResourceId: ec.String(mock.ValidClusterID), + Region: ec.String("some-region"), + TrustAccount: ElasticsearchTrustAccounts{ + { + AccountId: ec.String("id1"), + TrustAllowlist: []string{"a", "b"}, + }, + { + AccountId: ec.String("id2"), + TrustAll: ec.Bool(true), + }, + }, + TrustExternal: ElasticsearchTrustExternals{ + { + RelationshipId: ec.String("id3"), + TrustAll: ec.Bool(true), + }, + { + RelationshipId: ec.String("id4"), + TrustAllowlist: []string{"c", "d"}, + }, + }, + Strategy: ec.String("rolling_all"), + HotTier: &ElasticsearchTopology{ + id: "hot_content", + Size: ec.String("2g"), + ZoneCount: 1, + }, + }, + esState: &Elasticsearch{ + RefId: ec.String("main-elasticsearch"), + ResourceId: ec.String(mock.ValidClusterID), + Region: ec.String("some-region"), + TrustAccount: ElasticsearchTrustAccounts{ + { + AccountId: ec.String("id1"), + TrustAllowlist: []string{"a", "b"}, + }, + { + AccountId: ec.String("id2"), + TrustAll: ec.Bool(true), + }, + }, + TrustExternal: ElasticsearchTrustExternals{ + { + RelationshipId: ec.String("id3"), + TrustAll: ec.Bool(true), + }, + { + RelationshipId: ec.String("id4"), + TrustAllowlist: []string{"c", "d"}, + }, + }, + Strategy: ec.String("rolling_all"), + 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, + }, + Plan: &models.ElasticsearchClusterPlan{ + AutoscalingEnabled: ec.Bool(false), + Elasticsearch: &models.ElasticsearchConfiguration{ + Version: "7.7.0", + }, + DeploymentTemplate: &models.DeploymentTemplateReference{ + ID: ec.String("aws-io-optimized-v2"), + }, + Transient: &models.TransientElasticsearchPlanConfiguration{ + Strategy: &models.PlanStrategy{ + Rolling: &models.RollingStrategyConfig{GroupBy: "__all__"}, + }, + }, + ClusterTopology: []*models.ElasticsearchClusterTopologyElement{ + { + ID: "hot_content", + ZoneCount: 1, + InstanceConfigurationID: "aws.data.highio.i3", + Size: &models.TopologySize{ + Resource: ec.String("memory"), + Value: ec.Int32(2048), + }, + 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"), + }, + }, + }, + }, + }), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {