From ed3d9b82405c0da7ef84852a30014fc64d8f0fc9 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Fri, 14 Jul 2023 23:43:33 +1000 Subject: [PATCH 01/19] Use state for unknown for computed attributes which won't change during plan application These values may potentially change throughout a deployment lifecycle, but those changes will be picked up during the refresh phase, updating state prior to computing the plan These values, when unset, should be 'known' as they are provided in the API response --- .../deploymentresource/apm/v2/schema.go | 12 +++++ .../deployment/v2/deployment_read_test.go | 47 ++++++++++++++----- .../deployment/v2/schema.go | 3 ++ .../elasticsearch/v2/elasticsearch_config.go | 1 + .../v2/elasticsearch_read_test.go | 9 +++- .../elasticsearch/v2/schema.go | 15 ++++++ .../integrationsserver/v2/schema.go | 4 ++ ec/ecresource/extensionresource/schema.go | 10 ++++ 8 files changed, 88 insertions(+), 13 deletions(-) diff --git a/ec/ecresource/deploymentresource/apm/v2/schema.go b/ec/ecresource/deploymentresource/apm/v2/schema.go index 2c80d5fb5..6eabbfe11 100644 --- a/ec/ecresource/deploymentresource/apm/v2/schema.go +++ b/ec/ecresource/deploymentresource/apm/v2/schema.go @@ -83,15 +83,27 @@ func ApmSchema() schema.Attribute { }, "resource_id": schema.StringAttribute{ Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "region": schema.StringAttribute{ Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "http_endpoint": schema.StringAttribute{ Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "https_endpoint": schema.StringAttribute{ Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "instance_configuration_id": schema.StringAttribute{ Optional: true, diff --git a/ec/ecresource/deploymentresource/deployment/v2/deployment_read_test.go b/ec/ecresource/deploymentresource/deployment/v2/deployment_read_test.go index d70fe9c48..153f9de80 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/deployment_read_test.go +++ b/ec/ecresource/deploymentresource/deployment/v2/deployment_read_test.go @@ -57,6 +57,7 @@ func Test_readDeployment(t *testing.T) { ResourceId: &mock.ValidClusterID, Region: ec.String("us-east-1"), Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, UserSettingsYaml: ec.String("some.setting: value"), UserSettingsOverrideYaml: ec.String("some.setting: value2"), UserSettingsJson: ec.String("{\"some.setting\":\"value\"}"), @@ -429,6 +430,7 @@ func Test_readDeployment(t *testing.T) { ResourceId: &mock.ValidClusterID, Region: ec.String("us-east-1"), Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, UserSettingsYaml: ec.String("some.setting: value"), UserSettingsOverrideYaml: ec.String("some.setting: value2"), UserSettingsJson: ec.String("{\"some.setting\":\"value\"}"), @@ -497,7 +499,9 @@ func Test_readDeployment(t *testing.T) { Autoscaling: &elasticsearchv2.ElasticsearchTopologyAutoscaling{}, }, ), - Config: &elasticsearchv2.ElasticsearchConfig{}, + Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, + }, }, Kibana: &kibanav2.Kibana{ ElasticsearchClusterRefId: ec.String("main-elasticsearch"), @@ -544,7 +548,9 @@ func Test_readDeployment(t *testing.T) { CloudID: ec.String("up2d:someCloudID"), HttpEndpoint: ec.String("http://1239f7ee7196439ba2d105319ac5eba7.eu-central-1.aws.cloud.es.io:9200"), HttpsEndpoint: ec.String("https://1239f7ee7196439ba2d105319ac5eba7.eu-central-1.aws.cloud.es.io:9243"), - Config: &elasticsearchv2.ElasticsearchConfig{}, + Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, + }, HotTier: elasticsearchv2.CreateTierForTest( "hot_content", elasticsearchv2.ElasticsearchTopology{ @@ -607,7 +613,9 @@ func Test_readDeployment(t *testing.T) { CloudID: ec.String("up2d:someCloudID"), HttpEndpoint: ec.String("http://1239f7ee7196439ba2d105319ac5eba7.eu-central-1.aws.cloud.es.io:9200"), HttpsEndpoint: ec.String("https://1239f7ee7196439ba2d105319ac5eba7.eu-central-1.aws.cloud.es.io:9243"), - Config: &elasticsearchv2.ElasticsearchConfig{}, + Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, + }, HotTier: elasticsearchv2.CreateTierForTest( "hot_content", elasticsearchv2.ElasticsearchTopology{ @@ -753,7 +761,9 @@ func Test_readDeployment(t *testing.T) { Elasticsearch: &elasticsearchv2.Elasticsearch{ RefId: ec.String("main-elasticsearch"), Region: ec.String("aws-eu-central-1"), - Config: &elasticsearchv2.ElasticsearchConfig{}, + Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, + }, HotTier: elasticsearchv2.CreateTierForTest( "hot_content", elasticsearchv2.ElasticsearchTopology{ @@ -839,7 +849,9 @@ func Test_readDeployment(t *testing.T) { Elasticsearch: &elasticsearchv2.Elasticsearch{ RefId: ec.String("main-elasticsearch"), Region: ec.String("aws-eu-central-1"), - Config: &elasticsearchv2.ElasticsearchConfig{}, + Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, + }, HotTier: elasticsearchv2.CreateTierForTest( "hot_content", elasticsearchv2.ElasticsearchTopology{ @@ -986,6 +998,7 @@ func Test_readDeployment(t *testing.T) { RefId: ec.String("main-elasticsearch"), Region: ec.String("aws-eu-central-1"), Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, DockerImage: ec.String("docker.elastic.com/elasticsearch/cloud:7.14.1-hash"), }, HotTier: elasticsearchv2.CreateTierForTest( @@ -1064,7 +1077,9 @@ func Test_readDeployment(t *testing.T) { CloudID: ec.String("up2d:someCloudID"), HttpEndpoint: ec.String("http://1239f7ee7196439ba2d105319ac5eba7.eu-central-1.aws.cloud.es.io:9200"), HttpsEndpoint: ec.String("https://1239f7ee7196439ba2d105319ac5eba7.eu-central-1.aws.cloud.es.io:9243"), - Config: &elasticsearchv2.ElasticsearchConfig{}, + Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, + }, HotTier: elasticsearchv2.CreateTierForTest( "hot_content", elasticsearchv2.ElasticsearchTopology{ @@ -1125,7 +1140,9 @@ func Test_readDeployment(t *testing.T) { CloudID: ec.String("up2d:someCloudID"), HttpEndpoint: ec.String("http://123695e76d914005bf90b717e668ad4b.asia-east1.gcp.elastic-cloud.com:9200"), HttpsEndpoint: ec.String("https://123695e76d914005bf90b717e668ad4b.asia-east1.gcp.elastic-cloud.com:9243"), - Config: &elasticsearchv2.ElasticsearchConfig{}, + Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, + }, HotTier: elasticsearchv2.CreateTierForTest( "hot_content", elasticsearchv2.ElasticsearchTopology{ @@ -1186,7 +1203,9 @@ func Test_readDeployment(t *testing.T) { CloudID: ec.String("up2d:someCloudID"), HttpEndpoint: ec.String("http://123695e76d914005bf90b717e668ad4b.asia-east1.gcp.elastic-cloud.com:9200"), HttpsEndpoint: ec.String("https://123695e76d914005bf90b717e668ad4b.asia-east1.gcp.elastic-cloud.com:9243"), - Config: &elasticsearchv2.ElasticsearchConfig{}, + Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, + }, HotTier: elasticsearchv2.CreateTierForTest( "hot_content", elasticsearchv2.ElasticsearchTopology{ @@ -1297,7 +1316,9 @@ func Test_readDeployment(t *testing.T) { CloudID: ec.String("up2d-hot-warm:someCloudID"), HttpEndpoint: ec.String("http://123e837db6ee4391bb74887be35a7a91.us-central1.gcp.cloud.es.io:9200"), HttpsEndpoint: ec.String("https://123e837db6ee4391bb74887be35a7a91.us-central1.gcp.cloud.es.io:9243"), - Config: &elasticsearchv2.ElasticsearchConfig{}, + Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, + }, HotTier: elasticsearchv2.CreateTierForTest( "hot_content", elasticsearchv2.ElasticsearchTopology{ @@ -1385,7 +1406,9 @@ func Test_readDeployment(t *testing.T) { CloudID: ec.String("up2d-hot-warm:someCloudID"), HttpEndpoint: ec.String("http://123e837db6ee4391bb74887be35a7a91.us-central1.gcp.cloud.es.io:9200"), HttpsEndpoint: ec.String("https://123e837db6ee4391bb74887be35a7a91.us-central1.gcp.cloud.es.io:9243"), - Config: &elasticsearchv2.ElasticsearchConfig{}, + Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, + }, HotTier: elasticsearchv2.CreateTierForTest( "hot_content", elasticsearchv2.ElasticsearchTopology{ @@ -1510,7 +1533,9 @@ func Test_readDeployment(t *testing.T) { CloudID: ec.String("ccs:someCloudID"), HttpEndpoint: ec.String("http://1230b3ae633b4f51a432d50971f7f1c1.eu-west-1.aws.found.io:9200"), HttpsEndpoint: ec.String("https://1230b3ae633b4f51a432d50971f7f1c1.eu-west-1.aws.found.io:9243"), - Config: &elasticsearchv2.ElasticsearchConfig{}, + Config: &elasticsearchv2.ElasticsearchConfig{ + Plugins: []string{}, + }, RemoteCluster: elasticsearchv2.ElasticsearchRemoteClusters{ { Alias: ec.String("alias"), diff --git a/ec/ecresource/deploymentresource/deployment/v2/schema.go b/ec/ecresource/deploymentresource/deployment/v2/schema.go index cefdbb862..0dd67542b 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/schema.go +++ b/ec/ecresource/deploymentresource/deployment/v2/schema.go @@ -103,6 +103,9 @@ func DeploymentSchema() schema.Schema { "apm_secret_token": schema.StringAttribute{ Computed: true, Sensitive: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "traffic_filter": schema.SetAttribute{ ElementType: types.StringType, diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config.go b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config.go index 84e080ea1..d37e5791d 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config.go @@ -44,6 +44,7 @@ func readElasticsearchConfig(in *models.ElasticsearchConfiguration) (*Elasticsea return &ElasticsearchConfig{}, nil } + config.Plugins = []string{} if len(in.EnabledBuiltInPlugins) > 0 { config.Plugins = append(config.Plugins, in.EnabledBuiltInPlugins...) } diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_read_test.go b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_read_test.go index 74a9db454..cf2266261 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_read_test.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_read_test.go @@ -163,7 +163,9 @@ func Test_readElasticsearch(t *testing.T) { CloudID: ec.String("some CLOUD ID"), HttpEndpoint: ec.String("http://somecluster.cloud.elastic.co:9200"), HttpsEndpoint: ec.String("https://somecluster.cloud.elastic.co:9243"), - Config: &ElasticsearchConfig{}, + Config: &ElasticsearchConfig{ + Plugins: []string{}, + }, HotTier: &ElasticsearchTopology{ id: "hot_content", InstanceConfigurationId: ec.String("aws.data.highio.i3"), @@ -243,6 +245,7 @@ func Test_readElasticsearch(t *testing.T) { HttpEndpoint: ec.String("http://othercluster.cloud.elastic.co:9200"), HttpsEndpoint: ec.String("https://othercluster.cloud.elastic.co:9243"), Config: &ElasticsearchConfig{ + Plugins: []string{}, UserSettingsYaml: ec.String("some.setting: value"), UserSettingsOverrideYaml: ec.String("some.setting: value2"), UserSettingsJson: ec.String("{\"some.setting\":\"value\"}"), @@ -374,7 +377,9 @@ func Test_readElasticsearch(t *testing.T) { CloudID: ec.String("some CLOUD ID"), HttpEndpoint: ec.String("http://somecluster.cloud.elastic.co:9200"), HttpsEndpoint: ec.String("https://somecluster.cloud.elastic.co:9243"), - Config: &ElasticsearchConfig{}, + Config: &ElasticsearchConfig{ + Plugins: []string{}, + }, HotTier: &ElasticsearchTopology{ id: "hot_content", InstanceConfigurationId: ec.String("aws.data.highio.i3"), diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/schema.go b/ec/ecresource/deploymentresource/elasticsearch/v2/schema.go index e4e807a2e..fcc762fb6 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/schema.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/schema.go @@ -73,6 +73,9 @@ func ElasticsearchSchema() schema.Attribute { "resource_id": schema.StringAttribute{ Description: "The Elasticsearch resource unique identifier", Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "region": schema.StringAttribute{ Description: "The Elasticsearch resource region", @@ -84,14 +87,23 @@ func ElasticsearchSchema() schema.Attribute { "cloud_id": schema.StringAttribute{ Description: "The encoded Elasticsearch credentials to use in Beats or Logstash", Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "http_endpoint": schema.StringAttribute{ Description: "The Elasticsearch resource HTTP endpoint", Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "https_endpoint": schema.StringAttribute{ Description: "The Elasticsearch resource HTTPs endpoint", Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "hot": elasticsearchTopologySchema("'hot' topology element", true, "hot"), @@ -139,6 +151,9 @@ func elasticsearchConfigSchema() schema.Attribute { Description: "List of Elasticsearch supported plugins, which vary from version to version. Check the Stack Pack version to see which plugins are supported for each version. This is currently only available from the UI and [ecctl](https://www.elastic.co/guide/en/ecctl/master/ecctl_stack_list.html)", Optional: true, Computed: true, + PlanModifiers: []planmodifier.Set{ + setplanmodifier.UseStateForUnknown(), + }, }, "user_settings_json": schema.StringAttribute{ Description: `JSON-formatted user level "elasticsearch.yml" setting overrides`, diff --git a/ec/ecresource/deploymentresource/integrationsserver/v2/schema.go b/ec/ecresource/deploymentresource/integrationsserver/v2/schema.go index 779e46c77..061b1ef35 100644 --- a/ec/ecresource/deploymentresource/integrationsserver/v2/schema.go +++ b/ec/ecresource/deploymentresource/integrationsserver/v2/schema.go @@ -21,6 +21,7 @@ import ( "github.com/elastic/terraform-provider-ec/ec/internal/planmodifiers" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/objectplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" ) @@ -87,6 +88,9 @@ func IntegrationsServerSchema() schema.Attribute { }, }, }, + PlanModifiers: []planmodifier.Object{ + objectplanmodifier.UseStateForUnknown(), + }, }, "instance_configuration_id": schema.StringAttribute{ Optional: true, diff --git a/ec/ecresource/extensionresource/schema.go b/ec/ecresource/extensionresource/schema.go index a912f0502..10b72a68d 100644 --- a/ec/ecresource/extensionresource/schema.go +++ b/ec/ecresource/extensionresource/schema.go @@ -25,6 +25,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/types" @@ -91,14 +92,23 @@ func (r *Resource) Schema(_ context.Context, _ resource.SchemaRequest, resp *res "url": schema.StringAttribute{ Description: "The extension URL which will be used in the Elastic Cloud deployment plan.", Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "last_modified": schema.StringAttribute{ Description: "The datatime the extension was last modified.", Computed: true, + PlanModifiers: []planmodifier.String{ + stringplanmodifier.UseStateForUnknown(), + }, }, "size": schema.Int64Attribute{ Description: "The size of the extension file in bytes.", Computed: true, + PlanModifiers: []planmodifier.Int64{ + int64planmodifier.UseStateForUnknown(), + }, }, // Computed attributes "id": schema.StringAttribute{ From 3ee59143a317dd7f952a5f09c1fe9ead0cf6bc60 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Mon, 17 Jul 2023 20:40:27 +1000 Subject: [PATCH 02/19] Config with empty plugins is empty --- .../elasticsearch/v2/elasticsearch_config.go | 8 +- .../v2/elasticsearch_config_test.go | 77 +++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config_test.go diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config.go b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config.go index d37e5791d..85da508eb 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config.go @@ -21,7 +21,6 @@ import ( "bytes" "context" "encoding/json" - "reflect" "github.com/elastic/cloud-sdk-go/pkg/models" "github.com/elastic/cloud-sdk-go/pkg/util/ec" @@ -34,7 +33,12 @@ import ( type ElasticsearchConfig v1.ElasticsearchConfig func (c *ElasticsearchConfig) IsEmpty() bool { - return c == nil || reflect.ValueOf(*c).IsZero() + return c == nil || (c.DockerImage == nil && + len(c.Plugins) == 0 && + c.UserSettingsJson == nil && + c.UserSettingsOverrideJson == nil && + c.UserSettingsOverrideYaml == nil && + c.UserSettingsYaml == nil) } func readElasticsearchConfig(in *models.ElasticsearchConfiguration) (*ElasticsearchConfig, error) { diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config_test.go b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config_test.go new file mode 100644 index 000000000..5e570cdbb --- /dev/null +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config_test.go @@ -0,0 +1,77 @@ +package v2 + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestElasticsearchConfig_IsEmpty(t *testing.T) { + testString := "test" + tests := []struct { + name string + config ElasticsearchConfig + isEmpty bool + }{ + { + name: "zero valued config, is empty", + config: ElasticsearchConfig{}, + isEmpty: true, + }, + { + name: "config with empty plugins, is empty", + config: ElasticsearchConfig{ + Plugins: []string{}, + }, + isEmpty: true, + }, + { + name: "config with non-empty plugins, is non-empty", + config: ElasticsearchConfig{ + Plugins: []string{"s3"}, + }, + isEmpty: false, + }, + { + name: "config with non-empty image, is non-empty", + config: ElasticsearchConfig{ + DockerImage: &testString, + }, + isEmpty: false, + }, + { + name: "config with non-empty user settings json, is non-empty", + config: ElasticsearchConfig{ + UserSettingsJson: &testString, + }, + isEmpty: false, + }, + { + name: "config with non-empty user settings override json, is non-empty", + config: ElasticsearchConfig{ + UserSettingsOverrideJson: &testString, + }, + isEmpty: false, + }, + { + name: "config with non-empty user settings yaml, is non-empty", + config: ElasticsearchConfig{ + UserSettingsYaml: &testString, + }, + isEmpty: false, + }, + { + name: "config with non-empty user settings override yaml, is non-empty", + config: ElasticsearchConfig{ + UserSettingsOverrideYaml: &testString, + }, + isEmpty: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.isEmpty, tt.config.IsEmpty()) + }) + } +} From c06d4c8960ba4a7520aa82f7c2727f3ce69a8b61 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Wed, 19 Jul 2023 21:42:08 +1000 Subject: [PATCH 03/19] CloudID is unknown if name changes or if kibana is being enabled/disabled --- .../v2/cloud_id_plan_modifier.go | 97 +++++++++++++++++++ .../elasticsearch/v2/schema.go | 2 +- ec/ecresource/deploymentresource/read.go | 1 - 3 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 ec/ecresource/deploymentresource/elasticsearch/v2/cloud_id_plan_modifier.go diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/cloud_id_plan_modifier.go b/ec/ecresource/deploymentresource/elasticsearch/v2/cloud_id_plan_modifier.go new file mode 100644 index 000000000..1714392c3 --- /dev/null +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/cloud_id_plan_modifier.go @@ -0,0 +1,97 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package v2 + +import ( + "context" + + "github.com/elastic/terraform-provider-ec/ec/internal/planmodifiers" + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" +) + +// Use current state for instead of unknown, unless the deployment name change or Kibana is being enabled/disabled +func UseStateForUnknownUnlessNameOrKibanaStateChanges() planmodifier.String { + return useStateForUnknownUnlessNameOrKibanaStateChanges{} +} + +type useStateForUnknownUnlessNameOrKibanaStateChanges struct{} + +func (m useStateForUnknownUnlessNameOrKibanaStateChanges) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { + // Do nothing if there is no state value. + if req.StateValue.IsNull() { + return + } + + // Do nothing if there is a known planned value. + if !req.PlanValue.IsUnknown() { + return + } + + // Do nothing if there is an unknown configuration value, otherwise interpolation gets messed up. + if req.ConfigValue.IsUnknown() { + return + } + + // Do nothing if the deployment name has changed + nameChanged, diags := planmodifiers.AttributeChanged(ctx, path.Root("name"), req.Plan, req.State) + if resp.Diagnostics.Append(diags...); diags.HasError() { + return + } + if nameChanged { + return + } + + kibanaChanged, diags := kibanaStateChanging(ctx, req.Plan, req.State) + if resp.Diagnostics.Append(diags...); diags.HasError() { + return + } + if kibanaChanged { + return + } + + resp.PlanValue = req.StateValue +} + +func (r useStateForUnknownUnlessNameOrKibanaStateChanges) Description(ctx context.Context) string { + return "Use current state for instead of unknown, unless the deployment name change or Kibana is being enabled/disabled." +} + +func (r useStateForUnknownUnlessNameOrKibanaStateChanges) MarkdownDescription(ctx context.Context) string { + return "Use current state for instead of unknown, unless the deployment name change or Kibana is being enabled/disabled." +} + +func kibanaStateChanging(ctx context.Context, plan tfsdk.Plan, state tfsdk.State) (bool, diag.Diagnostics) { + var planValue attr.Value + p := path.Root("kibana") + + if diags := plan.GetAttribute(ctx, p, &planValue); diags.HasError() { + return false, diags + } + + var stateValue attr.Value + + if diags := state.GetAttribute(ctx, p, &stateValue); diags.HasError() { + return false, diags + } + + return planValue.IsNull() != stateValue.IsNull(), nil +} diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/schema.go b/ec/ecresource/deploymentresource/elasticsearch/v2/schema.go index fcc762fb6..eebfc3680 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/schema.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/schema.go @@ -88,7 +88,7 @@ func ElasticsearchSchema() schema.Attribute { Description: "The encoded Elasticsearch credentials to use in Beats or Logstash", Computed: true, PlanModifiers: []planmodifier.String{ - stringplanmodifier.UseStateForUnknown(), + UseStateForUnknownUnlessNameOrKibanaStateChanges(), }, }, "http_endpoint": schema.StringAttribute{ diff --git a/ec/ecresource/deploymentresource/read.go b/ec/ecresource/deploymentresource/read.go index e8dc57159..b3cf1e135 100644 --- a/ec/ecresource/deploymentresource/read.go +++ b/ec/ecresource/deploymentresource/read.go @@ -180,7 +180,6 @@ func (r *Resource) read(ctx context.Context, id string, state *deploymentv2.Depl deployment.Elasticsearch != nil && deployment.Elasticsearch.Config != nil && deployment.Elasticsearch.Config.IsEmpty() { - deployment.Elasticsearch.Config = nil } From f37e3d5d0dfe0ed80702a5a6033b6f04628ab53c Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Thu, 20 Jul 2023 15:12:35 +1000 Subject: [PATCH 04/19] Remove 1.3.9 control from CI acceptance tests --- build/Makefile.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Makefile.test b/build/Makefile.test index 7021dd8aa..d5d130fbb 100644 --- a/build/Makefile.test +++ b/build/Makefile.test @@ -42,7 +42,7 @@ endif .PHONY: testacc-ci testacc-ci: - @ TF_ACC_TERRAFORM_VERSION=1.3.9 EC_API_KEY=$(shell cat .ci/.apikey) $(MAKE) testacc + @ EC_API_KEY=$(shell cat .ci/.apikey) $(MAKE) testacc .PHONY: sweep-ci sweep-ci: From 850c6d2d5a1a1f64c1dfbd7735e1551a284b4a6e Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Tue, 25 Jul 2023 12:03:39 +1000 Subject: [PATCH 05/19] Set APM secret token to null if there is no apm/integrations_server resource --- .../deployment/v2/schema.go | 43 +++++++++++++++++++ .../v2/elasticsearch_config_test.go | 17 ++++++++ ec/internal/planmodifiers/has_attribute.go | 37 ++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 ec/internal/planmodifiers/has_attribute.go diff --git a/ec/ecresource/deploymentresource/deployment/v2/schema.go b/ec/ecresource/deploymentresource/deployment/v2/schema.go index 0dd67542b..589e87d23 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/schema.go +++ b/ec/ecresource/deploymentresource/deployment/v2/schema.go @@ -33,6 +33,7 @@ import ( integrationsserverv2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/integrationsserver/v2" kibanav2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/kibana/v2" observabilityv2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/observability/v2" + "github.com/elastic/terraform-provider-ec/ec/internal/planmodifiers" ) func DeploymentSchema() schema.Schema { @@ -105,6 +106,7 @@ func DeploymentSchema() schema.Schema { Sensitive: true, PlanModifiers: []planmodifier.String{ stringplanmodifier.UseStateForUnknown(), + useNullIfNotAPM{}, }, }, "traffic_filter": schema.SetAttribute{ @@ -132,6 +134,47 @@ func DeploymentSchema() schema.Schema { } } +type useNullIfNotAPM struct{} + +var _ planmodifier.String = useNullIfNotAPM{} + +func (m useNullIfNotAPM) Description(ctx context.Context) string { + return m.MarkdownDescription(ctx) +} + +func (m useNullIfNotAPM) MarkdownDescription(ctx context.Context) string { + return "Sets the plan value to null if there is no apm or integrations_server resource" +} + +func (m useNullIfNotAPM) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { + // if the config is the unknown value, use the unknown value otherwise, interpolation gets messed up + if req.ConfigValue.IsUnknown() { + return + } + + hasAPM, diags := planmodifiers.HasAttribute(ctx, path.Root("apm"), req.Plan) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + + if hasAPM { + return + } + + hasIntegrationsServer, diags := planmodifiers.HasAttribute(ctx, path.Root("integrations_server"), req.Plan) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + + if hasIntegrationsServer { + return + } + + resp.PlanValue = types.StringNull() +} + type setUnknownIfResetPasswordIsTrue struct{} var _ planmodifier.String = setUnknownIfResetPasswordIsTrue{} diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config_test.go b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config_test.go index 5e570cdbb..3aed4220d 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config_test.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/elasticsearch_config_test.go @@ -1,3 +1,20 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + package v2 import ( diff --git a/ec/internal/planmodifiers/has_attribute.go b/ec/internal/planmodifiers/has_attribute.go new file mode 100644 index 000000000..7e42b49e3 --- /dev/null +++ b/ec/internal/planmodifiers/has_attribute.go @@ -0,0 +1,37 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package planmodifiers + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" +) + +func HasAttribute(ctx context.Context, p path.Path, plan tfsdk.Plan) (bool, diag.Diagnostics) { + var value attr.Value + + if diags := plan.GetAttribute(ctx, p, &value); diags.HasError() { + return false, diags + } + + return !value.IsNull(), nil +} From ae29a40be0cd52f1876de3617e60a14087bec791 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Mon, 31 Jul 2023 10:38:56 +1000 Subject: [PATCH 06/19] Traffic filter WIP --- .../deployment/v2/deployment_read.go | 29 +++++---- .../deployment/v2/schema.go | 4 ++ .../planmodifiers/set_default_value.go | 59 +++++++++++++++++++ 3 files changed, 80 insertions(+), 12 deletions(-) create mode 100644 ec/internal/planmodifiers/set_default_value.go diff --git a/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go b/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go index 459c17de1..4bd277fb2 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go +++ b/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go @@ -26,6 +26,7 @@ import ( "github.com/blang/semver" "github.com/elastic/cloud-sdk-go/pkg/models" "github.com/elastic/cloud-sdk-go/pkg/util/ec" + "golang.org/x/exp/slices" apmv2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/apm/v2" elasticsearchv2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/elasticsearch/v2" @@ -230,22 +231,26 @@ func (dep *Deployment) ProcessSelfInObservability() { } func (dep *Deployment) HandleEmptyTrafficFilters(ctx context.Context, base DeploymentTF) diag.Diagnostics { - var diags diag.Diagnostics - // Ensure consistency between null, and empty configured traffic filter values. - // The Cloud API represents an empty set of traffic filters as a null/missing value. Terraform does distinguish between those two cases. - // If the Cloud response does not include traffic filters, then set the read value as the planned value, but only if the planned value is empty. - if dep.TrafficFilter == nil { - var baseFilters []string - diags := base.TrafficFilter.ElementsAs(ctx, &baseFilters, true) - if diags.HasError() { - return diags - } + var baseFilters []string + diags := base.TrafficFilter.ElementsAs(ctx, &baseFilters, true) + if diags.HasError() { + return diags + } - if len(baseFilters) == 0 { - dep.TrafficFilter = baseFilters + // Only include traffic filters which are part of the TF plan. + if len(baseFilters) == 0 { + dep.TrafficFilter = baseFilters + } + + intersectionFilters := []string{} + for _, filter := range dep.TrafficFilter { + if slices.Contains(baseFilters, filter) { + intersectionFilters = append(intersectionFilters, filter) } } + dep.TrafficFilter = intersectionFilters + return diags } diff --git a/ec/ecresource/deploymentresource/deployment/v2/schema.go b/ec/ecresource/deploymentresource/deployment/v2/schema.go index 589e87d23..f816f82e3 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/schema.go +++ b/ec/ecresource/deploymentresource/deployment/v2/schema.go @@ -20,6 +20,7 @@ package v2 import ( "context" + "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/types" @@ -114,6 +115,9 @@ func DeploymentSchema() schema.Schema { Optional: true, Computed: true, Description: "List of traffic filters rule identifiers that will be applied to the deployment. Removing this attribute entirely *will not* remove managed traffic filters, instead first set it to an empty list (e.g `traffic_filter = []`) to remove the managed traffic filters.", + PlanModifiers: []planmodifier.Set{ + planmodifiers.SetDefaultValue(types.StringType, []attr.Value{}), + }, }, "tags": schema.MapAttribute{ Description: "Optional map of deployment tags", diff --git a/ec/internal/planmodifiers/set_default_value.go b/ec/internal/planmodifiers/set_default_value.go new file mode 100644 index 000000000..e7f2a8033 --- /dev/null +++ b/ec/internal/planmodifiers/set_default_value.go @@ -0,0 +1,59 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// NOTE! copied from terraform-provider-tls +package planmodifiers + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +type setDefaultValue struct { + value []attr.Value + elemType attr.Type +} + +func SetDefaultValue(elemType attr.Type, value []attr.Value) planmodifier.Set { + return &setDefaultValue{value: value, elemType: elemType} +} + +var _ planmodifier.Set = (*setDefaultValue)(nil) + +func (m *setDefaultValue) Description(ctx context.Context) string { + return m.MarkdownDescription(ctx) +} + +func (m *setDefaultValue) MarkdownDescription(ctx context.Context) string { + return fmt.Sprintf("Sets the default value %v if the attribute is not set", m.value) +} + +func (m *setDefaultValue) PlanModifySet(ctx context.Context, req planmodifier.SetRequest, resp *planmodifier.SetResponse) { + if !req.ConfigValue.IsNull() { + return + } + + if req.ConfigValue.IsUnknown() { + return + } + + resp.PlanValue, resp.Diagnostics = types.SetValue(m.elemType, m.value) +} From ca3c447994116974cb62d08022429a611347721f Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Mon, 31 Jul 2023 12:09:23 +1000 Subject: [PATCH 07/19] Use private state to track traffic filters previously managed by the deployment resource --- ec/ecresource/deploymentresource/create.go | 8 +- .../deployment/v2/deployment_read.go | 24 +++++- .../deploymentresource/private_state.go | 73 +++++++++++++++++++ ec/ecresource/deploymentresource/read.go | 12 ++- ec/ecresource/deploymentresource/update.go | 30 ++++---- 5 files changed, 127 insertions(+), 20 deletions(-) create mode 100644 ec/ecresource/deploymentresource/private_state.go diff --git a/ec/ecresource/deploymentresource/create.go b/ec/ecresource/deploymentresource/create.go index 138bc36fa..ef2e5f304 100644 --- a/ec/ecresource/deploymentresource/create.go +++ b/ec/ecresource/deploymentresource/create.go @@ -81,7 +81,13 @@ func (r *Resource) Create(ctx context.Context, req resource.CreateRequest, resp resp.Diagnostics.Append(v2.HandleRemoteClusters(ctx, r.client, *res.ID, plan.Elasticsearch)...) - deployment, diags := r.read(ctx, *res.ID, nil, &plan, res.Resources) + filters := []string{} + if request.Settings != nil && request.Settings.TrafficFilterSettings != nil && request.Settings.TrafficFilterSettings.Rulesets != nil { + filters = request.Settings.TrafficFilterSettings.Rulesets + } + + deployment, diags := r.read(ctx, *res.ID, nil, &plan, res.Resources, filters) + updatePrivateStateTrafficFiltersFromCreate(ctx, resp, filters) resp.Diagnostics.Append(diags...) diff --git a/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go b/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go index 4bd277fb2..e31254c8a 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go +++ b/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go @@ -230,14 +230,19 @@ func (dep *Deployment) ProcessSelfInObservability() { } } -func (dep *Deployment) HandleEmptyTrafficFilters(ctx context.Context, base DeploymentTF) diag.Diagnostics { +func (dep *Deployment) HandleEmptyTrafficFilters(ctx context.Context, base DeploymentTF, privateFilters []string) diag.Diagnostics { var baseFilters []string diags := base.TrafficFilter.ElementsAs(ctx, &baseFilters, true) if diags.HasError() { return diags } - // Only include traffic filters which are part of the TF plan. + for _, filter := range privateFilters { + if !slices.Contains[string](baseFilters, filter) { + baseFilters = append(baseFilters, filter) + } + } + if len(baseFilters) == 0 { dep.TrafficFilter = baseFilters } @@ -251,6 +256,21 @@ func (dep *Deployment) HandleEmptyTrafficFilters(ctx context.Context, base Deplo dep.TrafficFilter = intersectionFilters + // // Ensure consistency between null, and empty configured traffic filter values. + // // The Cloud API represents an empty set of traffic filters as a null/missing value. Terraform does distinguish between those two cases. + // // If the Cloud response does not include traffic filters, then set the read value as the planned value, but only if the planned value is empty. + // if dep.TrafficFilter == nil { + // var baseFilters []string + // diags := base.TrafficFilter.ElementsAs(ctx, &baseFilters, true) + // if diags.HasError() { + // return diags + // } + + // if len(baseFilters) == 0 { + // dep.TrafficFilter = baseFilters + // } + // } + return diags } diff --git a/ec/ecresource/deploymentresource/private_state.go b/ec/ecresource/deploymentresource/private_state.go new file mode 100644 index 000000000..b036e11c2 --- /dev/null +++ b/ec/ecresource/deploymentresource/private_state.go @@ -0,0 +1,73 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package deploymentresource + +import ( + "context" + "encoding/json" + + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/resource" +) + +const trafficFilterStateKey = "traffic_filters" + +func readPrivateStateTrafficFiltersFromRead(ctx context.Context, req resource.ReadRequest) ([]string, diag.Diagnostics) { + return readPrivateStateTrafficFilters(req.Private.GetKey(ctx, trafficFilterStateKey)) +} + +func readPrivateStateTrafficFiltersFromUpdate(ctx context.Context, req resource.UpdateRequest) ([]string, diag.Diagnostics) { + return readPrivateStateTrafficFilters(req.Private.GetKey(ctx, trafficFilterStateKey)) +} + +func readPrivateStateTrafficFilters(privateFilterBytes []byte, diags diag.Diagnostics) ([]string, diag.Diagnostics) { + if privateFilterBytes == nil || diags.HasError() { + return []string{}, diags + } + + var privateFilters []string + err := json.Unmarshal(privateFilterBytes, &privateFilters) + if err != nil { + diags.AddError("failed to parse private state", err.Error()) + return []string{}, diags + } + + return privateFilters, diags +} + +func updatePrivateStateTrafficFiltersFromUpdate(ctx context.Context, resp *resource.UpdateResponse, filters []string) diag.Diagnostics { + var diags diag.Diagnostics + filterBytes, err := json.Marshal(filters) + if err != nil { + diags.AddError("failed to update private state", err.Error()) + return diags + } + + return resp.Private.SetKey(ctx, trafficFilterStateKey, filterBytes) +} + +func updatePrivateStateTrafficFiltersFromCreate(ctx context.Context, resp *resource.CreateResponse, filters []string) diag.Diagnostics { + var diags diag.Diagnostics + filterBytes, err := json.Marshal(filters) + if err != nil { + diags.AddError("failed to update private state", err.Error()) + return diags + } + + return resp.Private.SetKey(ctx, trafficFilterStateKey, filterBytes) +} diff --git a/ec/ecresource/deploymentresource/read.go b/ec/ecresource/deploymentresource/read.go index b3cf1e135..7a2412164 100644 --- a/ec/ecresource/deploymentresource/read.go +++ b/ec/ecresource/deploymentresource/read.go @@ -57,8 +57,14 @@ func (r *Resource) Read(ctx context.Context, request resource.ReadRequest, respo var newState *deploymentv2.Deployment + privateFilters, d := readPrivateStateTrafficFiltersFromRead(ctx, request) + response.Diagnostics.Append(d...) + if response.Diagnostics.HasError() { + return + } + // use state for the plan (there is no plan and config during Read) - otherwise we can get unempty plan output - newState, diags = r.read(ctx, curState.Id.ValueString(), &curState, nil, nil) + newState, diags = r.read(ctx, curState.Id.ValueString(), &curState, nil, nil, privateFilters) response.Diagnostics.Append(diags...) @@ -74,7 +80,7 @@ func (r *Resource) Read(ctx context.Context, request resource.ReadRequest, respo } // at least one of state and plan should not be nil -func (r *Resource) read(ctx context.Context, id string, state *deploymentv2.DeploymentTF, plan *deploymentv2.DeploymentTF, deploymentResources []*models.DeploymentResource) (*deploymentv2.Deployment, diag.Diagnostics) { +func (r *Resource) read(ctx context.Context, id string, state *deploymentv2.DeploymentTF, plan *deploymentv2.DeploymentTF, deploymentResources []*models.DeploymentResource, privateFilters []string) (*deploymentv2.Deployment, diag.Diagnostics) { var diags diag.Diagnostics var base deploymentv2.DeploymentTF @@ -158,7 +164,7 @@ func (r *Resource) read(ctx context.Context, id string, state *deploymentv2.Depl deployment.ResetElasticsearchPassword = base.ResetElasticsearchPassword.ValueBoolPointer() } - diags.Append(deployment.HandleEmptyTrafficFilters(ctx, base)...) + diags.Append(deployment.HandleEmptyTrafficFilters(ctx, base, privateFilters)...) deployment.SetCredentialsIfEmpty(state) diff --git a/ec/ecresource/deploymentresource/update.go b/ec/ecresource/deploymentresource/update.go index b6e77a9fb..a1c23f90c 100644 --- a/ec/ecresource/deploymentresource/update.go +++ b/ec/ecresource/deploymentresource/update.go @@ -73,11 +73,17 @@ func (r *Resource) Update(ctx context.Context, req resource.UpdateRequest, resp return } - resp.Diagnostics.Append(HandleTrafficFilterChange(ctx, r.client, plan, state)...) - + privateFilters, d := readPrivateStateTrafficFiltersFromUpdate(ctx, req) + resp.Diagnostics.Append(d...) + if resp.Diagnostics.HasError() { + return + } + planRules, diags := HandleTrafficFilterChange(ctx, r.client, plan, privateFilters) + resp.Diagnostics.Append(diags...) + updatePrivateStateTrafficFiltersFromUpdate(ctx, resp, planRules) resp.Diagnostics.Append(v2.HandleRemoteClusters(ctx, r.client, plan.Id.ValueString(), plan.Elasticsearch)...) - deployment, diags := r.read(ctx, plan.Id.ValueString(), &state, &plan, res.Resources) + deployment, diags := r.read(ctx, plan.Id.ValueString(), &state, &plan, res.Resources, planRules) resp.Diagnostics.Append(diags...) @@ -116,18 +122,14 @@ func (r *Resource) ResetElasticsearchPassword(deploymentID string, refID string) return *resetResp.Password, diags } -func HandleTrafficFilterChange(ctx context.Context, client *api.API, plan, state v2.DeploymentTF) diag.Diagnostics { - if plan.TrafficFilter.Equal(state.TrafficFilter) { - return nil - } +func HandleTrafficFilterChange(ctx context.Context, client *api.API, plan v2.DeploymentTF, stateRules ruleSet) ([]string, diag.Diagnostics) { + // if plan.TrafficFilter.Equal(state.TrafficFilter) { + // return nil + // } - var planRules, stateRules ruleSet + var planRules ruleSet if diags := plan.TrafficFilter.ElementsAs(ctx, &planRules, true); diags.HasError() { - return diags - } - - if diags := state.TrafficFilter.ElementsAs(ctx, &stateRules, true); diags.HasError() { - return diags + return []string{}, diags } var rulesToAdd, rulesToDelete []string @@ -157,7 +159,7 @@ func HandleTrafficFilterChange(ctx context.Context, client *api.API, plan, state } } - return diags + return planRules, diags } type ruleSet []string From ec60157f1578313ff6c9dbd031c05dbe878517b6 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Mon, 31 Jul 2023 19:57:20 +1000 Subject: [PATCH 08/19] Acceptance tests --- ec/acc/deployment_basic_test.go | 13 ++------ ec/acc/deployment_ccs_test.go | 9 ++++- ...loyment_basic_with_empty_traffic_filter.tf | 33 ------------------- .../deployment/v2/schema.go | 2 +- .../deploymentresource/update_test.go | 16 ++------- ec/ecresource/extensionresource/schema.go | 33 +++++++++++++++++++ 6 files changed, 46 insertions(+), 60 deletions(-) delete mode 100644 ec/acc/testdata/deployment_basic_with_empty_traffic_filter.tf diff --git a/ec/acc/deployment_basic_test.go b/ec/acc/deployment_basic_test.go index f5c16cab8..8207c3193 100644 --- a/ec/acc/deployment_basic_test.go +++ b/ec/acc/deployment_basic_test.go @@ -35,12 +35,10 @@ func TestAccDeployment_basic_tf(t *testing.T) { randomAlias := "alias" + acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) trafficFilterCfg := "testdata/deployment_basic_with_traffic_filter_2.tf" trafficFilterUpdateCfg := "testdata/deployment_basic_with_traffic_filter_3.tf" - emptyTrafficFilterCfg := "testdata/deployment_basic_with_empty_traffic_filter.tf" resetPasswordCfg := "testdata/deployment_basic_reset_password.tf" cfg := fixtureAccDeploymentResourceBasicWithAppsAlias(t, startCfg, randomAlias, randomName, getRegion(), defaultTemplate) cfgWithTrafficFilter := fixtureAccDeploymentResourceBasicWithTF(t, trafficFilterCfg, randomName, getRegion(), defaultTemplate) cfgWithTrafficFilterUpdate := fixtureAccDeploymentResourceBasicWithTF(t, trafficFilterUpdateCfg, randomName, getRegion(), defaultTemplate) - cfgWithEmptyTrafficFilter := fixtureAccDeploymentResourceBasicWithAppsAlias(t, emptyTrafficFilterCfg, randomAlias, randomName, getRegion(), defaultTemplate) cfgResetPassword := fixtureAccDeploymentResourceBasicWithAppsAlias(t, resetPasswordCfg, randomAlias, randomName, getRegion(), defaultTemplate) deploymentVersion, err := latestStackVersion() if err != nil { @@ -59,7 +57,7 @@ func TestAccDeployment_basic_tf(t *testing.T) { resource.TestCheckResourceAttr(resName, "alias", randomAlias), resource.TestCheckNoResourceAttr(resName, "apm.config"), resource.TestCheckNoResourceAttr(resName, "enterprise_search.config"), - resource.TestCheckNoResourceAttr(resName, "traffic_filter"), + resource.TestCheckResourceAttr(resName, "traffic_filter.#", "0"), // Ensure at least 1 account is trusted (self). resource.TestCheckResourceAttr(resName, "elasticsearch.trust_account.#", "1"), ), @@ -78,16 +76,9 @@ func TestAccDeployment_basic_tf(t *testing.T) { resource.TestCheckResourceAttr(resName, "traffic_filter.#", "1"), ), }, - // Unset the traffic filter (this should not remove the traffic filter) + // Unset the traffic filter to remove the traffic filter { Config: cfg, - Check: checkBasicDeploymentResource(resName, randomName, deploymentVersion, - resource.TestCheckResourceAttr(resName, "traffic_filter.#", "1"), - ), - }, - // Explicitly set the traffic filter to an empty list to remove the traffic filter - { - Config: cfgWithEmptyTrafficFilter, Check: checkBasicDeploymentResource(resName, randomName, deploymentVersion, resource.TestCheckResourceAttr(resName, "traffic_filter.#", "0"), func(s *terraform.State) error { diff --git a/ec/acc/deployment_ccs_test.go b/ec/acc/deployment_ccs_test.go index d7f41418f..6583d2059 100644 --- a/ec/acc/deployment_ccs_test.go +++ b/ec/acc/deployment_ccs_test.go @@ -50,6 +50,12 @@ func TestAccDeployment_ccs(t *testing.T) { { // Create a CCS deployment with the default settings. Config: cfg, + // The legacy CCS DT does not support autoscaling, which leads to autoscaling being 'unknown'. + // Ideally we would set autoscaling to null if the deployment template does not support autoscaling, + // but that would require's refactoring our schema and this template is no longer part of the public offering. + // + // We can revisit this if there's demand for clean plans when the template does not support autoscaling. + ExpectNonEmptyPlan: true, Check: resource.ComposeAggregateTestCheckFunc( // CCS Checks @@ -94,7 +100,8 @@ func TestAccDeployment_ccs(t *testing.T) { }, { // Change the Elasticsearch topology size and node count. - Config: secondConfigCfg, + Config: secondConfigCfg, + ExpectNonEmptyPlan: true, Check: resource.ComposeAggregateTestCheckFunc( // Changes. resource.TestCheckResourceAttrSet(ccsResName, "elasticsearch.hot.instance_configuration_id"), diff --git a/ec/acc/testdata/deployment_basic_with_empty_traffic_filter.tf b/ec/acc/testdata/deployment_basic_with_empty_traffic_filter.tf deleted file mode 100644 index 1428aed94..000000000 --- a/ec/acc/testdata/deployment_basic_with_empty_traffic_filter.tf +++ /dev/null @@ -1,33 +0,0 @@ -data "ec_stack" "latest" { - version_regex = "latest" - region = "%s" -} - -resource "ec_deployment" "basic" { - alias = "%s" - name = "%s" - region = "%s" - version = data.ec_stack.latest.version - deployment_template_id = "%s" - - elasticsearch = { - hot = { - size = "1g" - autoscaling = {} - } - } - - kibana = { - instance_configuration_id = "%s" - } - - apm = { - instance_configuration_id = "%s" - } - - enterprise_search = { - instance_configuration_id = "%s" - } - - traffic_filter = [] -} diff --git a/ec/ecresource/deploymentresource/deployment/v2/schema.go b/ec/ecresource/deploymentresource/deployment/v2/schema.go index f816f82e3..d42822500 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/schema.go +++ b/ec/ecresource/deploymentresource/deployment/v2/schema.go @@ -114,7 +114,7 @@ func DeploymentSchema() schema.Schema { ElementType: types.StringType, Optional: true, Computed: true, - Description: "List of traffic filters rule identifiers that will be applied to the deployment. Removing this attribute entirely *will not* remove managed traffic filters, instead first set it to an empty list (e.g `traffic_filter = []`) to remove the managed traffic filters.", + Description: "List of traffic filters rule identifiers that will be applied to the deployment.", PlanModifiers: []planmodifier.Set{ planmodifiers.SetDefaultValue(types.StringType, []attr.Value{}), }, diff --git a/ec/ecresource/deploymentresource/update_test.go b/ec/ecresource/deploymentresource/update_test.go index a1e294c8b..46aaf6268 100644 --- a/ec/ecresource/deploymentresource/update_test.go +++ b/ec/ecresource/deploymentresource/update_test.go @@ -187,26 +187,14 @@ func Test_handleTrafficFilterChange(t *testing.T) { TrafficFilter: tt.args.plan, } - state := v2.Deployment{ - Id: deploymentID, - TrafficFilter: tt.args.state, - } - var planTF v2.DeploymentTF - diags := tfsdk.ValueFrom(context.Background(), &plan, v2.DeploymentSchema().Type(), &planTF) - - assert.Nil(t, diags) - - var stateTF v2.DeploymentTF - - diags = tfsdk.ValueFrom(context.Background(), &state, v2.DeploymentSchema().Type(), &stateTF) - assert.Nil(t, diags) - diags = deploymentresource.HandleTrafficFilterChange(context.Background(), nil, planTF, stateTF) + filters, diags := deploymentresource.HandleTrafficFilterChange(context.Background(), nil, planTF, tt.args.state) assert.Nil(t, diags) + assert.ElementsMatch(t, tt.args.plan, filters) }) } diff --git a/ec/ecresource/extensionresource/schema.go b/ec/ecresource/extensionresource/schema.go index 10b72a68d..7f771b1a1 100644 --- a/ec/ecresource/extensionresource/schema.go +++ b/ec/ecresource/extensionresource/schema.go @@ -101,6 +101,7 @@ func (r *Resource) Schema(_ context.Context, _ resource.SchemaRequest, resp *res Computed: true, PlanModifiers: []planmodifier.String{ stringplanmodifier.UseStateForUnknown(), + useUnknownIfOtherChanges{}, }, }, "size": schema.Int64Attribute{ @@ -122,6 +123,38 @@ func (r *Resource) Schema(_ context.Context, _ resource.SchemaRequest, resp *res } } +type useUnknownIfOtherChanges struct{} + +var _ planmodifier.String = useUnknownIfOtherChanges{} + +func (m useUnknownIfOtherChanges) Description(ctx context.Context) string { + return m.MarkdownDescription(ctx) +} + +func (m useUnknownIfOtherChanges) MarkdownDescription(ctx context.Context) string { + return "Sets the plan value to null if there is no apm or integrations_server resource" +} + +func (m useUnknownIfOtherChanges) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { + // if the config is the unknown value, use the unknown value otherwise, interpolation gets messed up + if req.ConfigValue.IsUnknown() { + return + } + + for attrName := range req.Config.Schema.GetAttributes() { + hasChanged, diags := planmodifiers.AttributeChanged(ctx, path.Root(attrName), req.Plan, req.State) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + + if hasChanged { + resp.PlanValue = types.StringUnknown() + return + } + } +} + func (r *Resource) ConfigValidators(ctx context.Context) []resource.ConfigValidator { return []resource.ConfigValidator{ resourcevalidator.RequiredTogether( From 0c7ca481e2dc8235deee5f5d516c9d3c9b5cd64a Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Mon, 31 Jul 2023 19:58:05 +1000 Subject: [PATCH 09/19] Regenerate docs --- docs/resources/deployment.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/resources/deployment.md b/docs/resources/deployment.md index f92e2a0e8..fd827db86 100644 --- a/docs/resources/deployment.md +++ b/docs/resources/deployment.md @@ -268,7 +268,7 @@ resource "ec_deployment" "ccs" { - `request_id` (String) Request ID to set when you create the deployment. Use it only when previous attempts return an error and `request_id` is returned as part of the error. - `reset_elasticsearch_password` (Boolean) Explicitly resets the elasticsearch_password when true - `tags` (Map of String) Optional map of deployment tags -- `traffic_filter` (Set of String) List of traffic filters rule identifiers that will be applied to the deployment. Removing this attribute entirely *will not* remove managed traffic filters, instead first set it to an empty list (e.g `traffic_filter = []`) to remove the managed traffic filters. +- `traffic_filter` (Set of String) List of traffic filters rule identifiers that will be applied to the deployment. ### Read-Only From 572adc2f2ca793be43b8f19bb3be8e91990f5202 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Mon, 31 Jul 2023 21:55:21 +1000 Subject: [PATCH 10/19] TF 1.4 in unit tests --- build/Makefile.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Makefile.test b/build/Makefile.test index d5d130fbb..bf7c0f02e 100644 --- a/build/Makefile.test +++ b/build/Makefile.test @@ -20,7 +20,7 @@ _report_path: .PHONY: unit unit: _report_path @ echo "-> Running unit tests for $(BINARY)..." - @ TF_ACC_TERRAFORM_VERSION=1.3.9 go test $(TEST) $(TESTARGS) $(TESTUNITARGS) + @ go test $(TEST) $(TESTARGS) $(TESTUNITARGS) ## Alias to "unit". tests: unit From 8a9970b47fa91e927b03390d810c7e26e4620244 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Mon, 31 Jul 2023 22:47:04 +1000 Subject: [PATCH 11/19] Acceptance tests --- ec/acc/deployment_pre_node_role_migration_test.go | 3 +++ ec/ecresource/deploymentresource/deployment/v2/schema.go | 4 ++++ .../deploymentresource/integrationsserver/v2/schema.go | 4 ---- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ec/acc/deployment_pre_node_role_migration_test.go b/ec/acc/deployment_pre_node_role_migration_test.go index 594cef2c1..3c45fb593 100644 --- a/ec/acc/deployment_pre_node_role_migration_test.go +++ b/ec/acc/deployment_pre_node_role_migration_test.go @@ -62,6 +62,9 @@ func TestAccDeployment_pre_node_roles(t *testing.T) { }, { Config: cfgF(upgradeVersionCfg), + // Expect a non-empty plan here. We explictly avoid migrating node_roles when the version changes + // however will the migrate the deployment to node_roles on the next TF application. + ExpectNonEmptyPlan: true, Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttrSet(resName, "elasticsearch.hot.instance_configuration_id"), resource.TestCheckResourceAttr(resName, "elasticsearch.hot.size", "1g"), diff --git a/ec/ecresource/deploymentresource/deployment/v2/schema.go b/ec/ecresource/deploymentresource/deployment/v2/schema.go index d42822500..583852b7d 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/schema.go +++ b/ec/ecresource/deploymentresource/deployment/v2/schema.go @@ -156,6 +156,10 @@ func (m useNullIfNotAPM) PlanModifyString(ctx context.Context, req planmodifier. return } + if !req.PlanValue.IsUnknown() { + return + } + hasAPM, diags := planmodifiers.HasAttribute(ctx, path.Root("apm"), req.Plan) resp.Diagnostics.Append(diags...) if resp.Diagnostics.HasError() { diff --git a/ec/ecresource/deploymentresource/integrationsserver/v2/schema.go b/ec/ecresource/deploymentresource/integrationsserver/v2/schema.go index 061b1ef35..779e46c77 100644 --- a/ec/ecresource/deploymentresource/integrationsserver/v2/schema.go +++ b/ec/ecresource/deploymentresource/integrationsserver/v2/schema.go @@ -21,7 +21,6 @@ import ( "github.com/elastic/terraform-provider-ec/ec/internal/planmodifiers" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier" - "github.com/hashicorp/terraform-plugin-framework/resource/schema/objectplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" ) @@ -88,9 +87,6 @@ func IntegrationsServerSchema() schema.Attribute { }, }, }, - PlanModifiers: []planmodifier.Object{ - objectplanmodifier.UseStateForUnknown(), - }, }, "instance_configuration_id": schema.StringAttribute{ Optional: true, From f0ed15b118432dce9464e73f266794fcea373c7e Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Mon, 31 Jul 2023 23:15:28 +1000 Subject: [PATCH 12/19] Fix tests --- docs/resources/deployment.md | 8 +++--- .../integrationsserver/v2/schema.go | 27 ++++++++----------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/docs/resources/deployment.md b/docs/resources/deployment.md index fd827db86..154116b9b 100644 --- a/docs/resources/deployment.md +++ b/docs/resources/deployment.md @@ -765,6 +765,7 @@ Optional: - `config` (Attributes) Optionally define the Integrations Server configuration options for the IntegrationsServer Server (see [below for nested schema](#nestedatt--integrations_server--config)) - `elasticsearch_cluster_ref_id` (String) +- `endpoints` (Object) URLs for the accessing the Fleet and APM API's within this Integrations Server resource. (see [below for nested schema](#nestedatt--integrations_server--endpoints)) - `instance_configuration_id` (String) - `ref_id` (String) - `size` (String) @@ -773,7 +774,6 @@ Optional: Read-Only: -- `endpoints` (Attributes) URLs for the accessing the Fleet and APM API's within this Integrations Server resource. (see [below for nested schema](#nestedatt--integrations_server--endpoints)) - `http_endpoint` (String) - `https_endpoint` (String) - `region` (String) @@ -795,10 +795,10 @@ Optional: ### Nested Schema for `integrations_server.endpoints` -Read-Only: +Optional: -- `apm` (String) URL to access the APM server instance for this Integrations Server resource -- `fleet` (String) URL to access the Fleet server instance for this Integrations Server resource +- `apm` (String) +- `fleet` (String) diff --git a/ec/ecresource/deploymentresource/integrationsserver/v2/schema.go b/ec/ecresource/deploymentresource/integrationsserver/v2/schema.go index 779e46c77..d77203475 100644 --- a/ec/ecresource/deploymentresource/integrationsserver/v2/schema.go +++ b/ec/ecresource/deploymentresource/integrationsserver/v2/schema.go @@ -19,10 +19,13 @@ package v2 import ( "github.com/elastic/terraform-provider-ec/ec/internal/planmodifiers" + "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/objectplanmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/types" ) func IntegrationsServerSchema() schema.Attribute { @@ -68,24 +71,16 @@ func IntegrationsServerSchema() schema.Attribute { stringplanmodifier.UseStateForUnknown(), }, }, - "endpoints": schema.SingleNestedAttribute{ + "endpoints": schema.ObjectAttribute{ + Optional: true, Computed: true, Description: "URLs for the accessing the Fleet and APM API's within this Integrations Server resource.", - Attributes: map[string]schema.Attribute{ - "apm": schema.StringAttribute{ - Description: "URL to access the APM server instance for this Integrations Server resource", - Computed: true, - PlanModifiers: []planmodifier.String{ - stringplanmodifier.UseStateForUnknown(), - }, - }, - "fleet": schema.StringAttribute{ - Description: "URL to access the Fleet server instance for this Integrations Server resource", - Computed: true, - PlanModifiers: []planmodifier.String{ - stringplanmodifier.UseStateForUnknown(), - }, - }, + AttributeTypes: map[string]attr.Type{ + "apm": types.StringType, + "fleet": types.StringType, + }, + PlanModifiers: []planmodifier.Object{ + objectplanmodifier.UseStateForUnknown(), }, }, "instance_configuration_id": schema.StringAttribute{ From 469bff35495b45be24539707a173345a5656888c Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Tue, 1 Aug 2023 08:50:55 +1000 Subject: [PATCH 13/19] Lint --- ec/acc/deployment_pre_node_role_migration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ec/acc/deployment_pre_node_role_migration_test.go b/ec/acc/deployment_pre_node_role_migration_test.go index 3c45fb593..b672c3ff4 100644 --- a/ec/acc/deployment_pre_node_role_migration_test.go +++ b/ec/acc/deployment_pre_node_role_migration_test.go @@ -62,7 +62,7 @@ func TestAccDeployment_pre_node_roles(t *testing.T) { }, { Config: cfgF(upgradeVersionCfg), - // Expect a non-empty plan here. We explictly avoid migrating node_roles when the version changes + // Expect a non-empty plan here. We explicitly avoid migrating node_roles when the version changes // however will the migrate the deployment to node_roles on the next TF application. ExpectNonEmptyPlan: true, Check: resource.ComposeAggregateTestCheckFunc( From ea801f25c35714e017a14716f8aad5cf85e48bb1 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Tue, 1 Aug 2023 15:12:12 +1000 Subject: [PATCH 14/19] Code cleanup --- ec/ecresource/deploymentresource/create.go | 2 +- .../deployment/v2/deployment_read.go | 17 +---------- .../deploymentresource/private_state.go | 30 +++++-------------- ec/ecresource/deploymentresource/read.go | 4 +-- ec/ecresource/deploymentresource/update.go | 8 ++--- 5 files changed, 14 insertions(+), 47 deletions(-) diff --git a/ec/ecresource/deploymentresource/create.go b/ec/ecresource/deploymentresource/create.go index ef2e5f304..6c14e4a9f 100644 --- a/ec/ecresource/deploymentresource/create.go +++ b/ec/ecresource/deploymentresource/create.go @@ -87,7 +87,7 @@ func (r *Resource) Create(ctx context.Context, req resource.CreateRequest, resp } deployment, diags := r.read(ctx, *res.ID, nil, &plan, res.Resources, filters) - updatePrivateStateTrafficFiltersFromCreate(ctx, resp, filters) + updatePrivateStateTrafficFilters(ctx, resp.Private, filters) resp.Diagnostics.Append(diags...) diff --git a/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go b/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go index e31254c8a..947a65be9 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go +++ b/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go @@ -230,7 +230,7 @@ func (dep *Deployment) ProcessSelfInObservability() { } } -func (dep *Deployment) HandleEmptyTrafficFilters(ctx context.Context, base DeploymentTF, privateFilters []string) diag.Diagnostics { +func (dep *Deployment) IncludePrivateStateTrafficFilters(ctx context.Context, base DeploymentTF, privateFilters []string) diag.Diagnostics { var baseFilters []string diags := base.TrafficFilter.ElementsAs(ctx, &baseFilters, true) if diags.HasError() { @@ -256,21 +256,6 @@ func (dep *Deployment) HandleEmptyTrafficFilters(ctx context.Context, base Deplo dep.TrafficFilter = intersectionFilters - // // Ensure consistency between null, and empty configured traffic filter values. - // // The Cloud API represents an empty set of traffic filters as a null/missing value. Terraform does distinguish between those two cases. - // // If the Cloud response does not include traffic filters, then set the read value as the planned value, but only if the planned value is empty. - // if dep.TrafficFilter == nil { - // var baseFilters []string - // diags := base.TrafficFilter.ElementsAs(ctx, &baseFilters, true) - // if diags.HasError() { - // return diags - // } - - // if len(baseFilters) == 0 { - // dep.TrafficFilter = baseFilters - // } - // } - return diags } diff --git a/ec/ecresource/deploymentresource/private_state.go b/ec/ecresource/deploymentresource/private_state.go index b036e11c2..e85554255 100644 --- a/ec/ecresource/deploymentresource/private_state.go +++ b/ec/ecresource/deploymentresource/private_state.go @@ -22,20 +22,17 @@ import ( "encoding/json" "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/resource" ) -const trafficFilterStateKey = "traffic_filters" - -func readPrivateStateTrafficFiltersFromRead(ctx context.Context, req resource.ReadRequest) ([]string, diag.Diagnostics) { - return readPrivateStateTrafficFilters(req.Private.GetKey(ctx, trafficFilterStateKey)) +type PrivateState interface { + GetKey(context.Context, string) ([]byte, diag.Diagnostics) + SetKey(context.Context, string, []byte) diag.Diagnostics } -func readPrivateStateTrafficFiltersFromUpdate(ctx context.Context, req resource.UpdateRequest) ([]string, diag.Diagnostics) { - return readPrivateStateTrafficFilters(req.Private.GetKey(ctx, trafficFilterStateKey)) -} +const trafficFilterStateKey = "traffic_filters" -func readPrivateStateTrafficFilters(privateFilterBytes []byte, diags diag.Diagnostics) ([]string, diag.Diagnostics) { +func readPrivateStateTrafficFilters(ctx context.Context, state PrivateState) ([]string, diag.Diagnostics) { + privateFilterBytes, diags := state.GetKey(ctx, trafficFilterStateKey) if privateFilterBytes == nil || diags.HasError() { return []string{}, diags } @@ -50,18 +47,7 @@ func readPrivateStateTrafficFilters(privateFilterBytes []byte, diags diag.Diagno return privateFilters, diags } -func updatePrivateStateTrafficFiltersFromUpdate(ctx context.Context, resp *resource.UpdateResponse, filters []string) diag.Diagnostics { - var diags diag.Diagnostics - filterBytes, err := json.Marshal(filters) - if err != nil { - diags.AddError("failed to update private state", err.Error()) - return diags - } - - return resp.Private.SetKey(ctx, trafficFilterStateKey, filterBytes) -} - -func updatePrivateStateTrafficFiltersFromCreate(ctx context.Context, resp *resource.CreateResponse, filters []string) diag.Diagnostics { +func updatePrivateStateTrafficFilters(ctx context.Context, state PrivateState, filters []string) diag.Diagnostics { var diags diag.Diagnostics filterBytes, err := json.Marshal(filters) if err != nil { @@ -69,5 +55,5 @@ func updatePrivateStateTrafficFiltersFromCreate(ctx context.Context, resp *resou return diags } - return resp.Private.SetKey(ctx, trafficFilterStateKey, filterBytes) + return state.SetKey(ctx, trafficFilterStateKey, filterBytes) } diff --git a/ec/ecresource/deploymentresource/read.go b/ec/ecresource/deploymentresource/read.go index 7a2412164..b4ff41c0a 100644 --- a/ec/ecresource/deploymentresource/read.go +++ b/ec/ecresource/deploymentresource/read.go @@ -57,7 +57,7 @@ func (r *Resource) Read(ctx context.Context, request resource.ReadRequest, respo var newState *deploymentv2.Deployment - privateFilters, d := readPrivateStateTrafficFiltersFromRead(ctx, request) + privateFilters, d := readPrivateStateTrafficFilters(ctx, request.Private) response.Diagnostics.Append(d...) if response.Diagnostics.HasError() { return @@ -164,7 +164,7 @@ func (r *Resource) read(ctx context.Context, id string, state *deploymentv2.Depl deployment.ResetElasticsearchPassword = base.ResetElasticsearchPassword.ValueBoolPointer() } - diags.Append(deployment.HandleEmptyTrafficFilters(ctx, base, privateFilters)...) + diags.Append(deployment.IncludePrivateStateTrafficFilters(ctx, base, privateFilters)...) deployment.SetCredentialsIfEmpty(state) diff --git a/ec/ecresource/deploymentresource/update.go b/ec/ecresource/deploymentresource/update.go index a1c23f90c..609b8f944 100644 --- a/ec/ecresource/deploymentresource/update.go +++ b/ec/ecresource/deploymentresource/update.go @@ -73,14 +73,14 @@ func (r *Resource) Update(ctx context.Context, req resource.UpdateRequest, resp return } - privateFilters, d := readPrivateStateTrafficFiltersFromUpdate(ctx, req) + privateFilters, d := readPrivateStateTrafficFilters(ctx, req.Private) resp.Diagnostics.Append(d...) if resp.Diagnostics.HasError() { return } planRules, diags := HandleTrafficFilterChange(ctx, r.client, plan, privateFilters) resp.Diagnostics.Append(diags...) - updatePrivateStateTrafficFiltersFromUpdate(ctx, resp, planRules) + updatePrivateStateTrafficFilters(ctx, resp.Private, planRules) resp.Diagnostics.Append(v2.HandleRemoteClusters(ctx, r.client, plan.Id.ValueString(), plan.Elasticsearch)...) deployment, diags := r.read(ctx, plan.Id.ValueString(), &state, &plan, res.Resources, planRules) @@ -123,10 +123,6 @@ func (r *Resource) ResetElasticsearchPassword(deploymentID string, refID string) } func HandleTrafficFilterChange(ctx context.Context, client *api.API, plan v2.DeploymentTF, stateRules ruleSet) ([]string, diag.Diagnostics) { - // if plan.TrafficFilter.Equal(state.TrafficFilter) { - // return nil - // } - var planRules ruleSet if diags := plan.TrafficFilter.ElementsAs(ctx, &planRules, true); diags.HasError() { return []string{}, diags From dc1ff8d3846da82e87ba192ddbfe19754b5f3307 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Sat, 5 Aug 2023 03:47:27 +1000 Subject: [PATCH 15/19] Fix build --- .../deploymentresource/deployment/v2/deployment_read.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go b/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go index 947a65be9..4d4561651 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go +++ b/ec/ecresource/deploymentresource/deployment/v2/deployment_read.go @@ -238,7 +238,7 @@ func (dep *Deployment) IncludePrivateStateTrafficFilters(ctx context.Context, ba } for _, filter := range privateFilters { - if !slices.Contains[string](baseFilters, filter) { + if !slices.Contains(baseFilters, filter) { baseFilters = append(baseFilters, filter) } } From 095fb68614ff0e51ee4035ddb9f4bd9c60a85159 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Sat, 5 Aug 2023 09:45:00 +1000 Subject: [PATCH 16/19] Fix build --- .../v2/node_roles_plan_modifier.go | 29 ++- .../v2/node_roles_plan_modifier_test.go | 203 ++++++++++++++++++ .../elasticsearch/v2/schema.go | 2 +- 3 files changed, 231 insertions(+), 3 deletions(-) diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_plan_modifier.go b/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_plan_modifier.go index 0196e4850..559c6eebe 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_plan_modifier.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_plan_modifier.go @@ -20,6 +20,7 @@ package v2 import ( "context" + "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/elastic/terraform-provider-ec/ec/internal/planmodifiers" @@ -75,6 +76,10 @@ func (r nodeRolesDefault) MarkdownDescription(ctx context.Context) string { return "Use current state if it's still valid." } +func SetUnknownOnTopologySizeChange() planmodifier.Set { + return setUnknownOnTopologyChanges{} +} + type setUnknownOnTopologyChanges struct{} var ( @@ -88,8 +93,28 @@ func (m setUnknownOnTopologyChanges) PlanModifySet(ctx context.Context, req plan } for _, tierName := range tierNames { - for _, attr := range sizingAttributes { - hasChanged, diags := planmodifiers.AttributeChanged(ctx, path.Root("elasticsearch").AtName(tierName).AtName(attr), req.Plan, req.State) + for _, attrName := range sizingAttributes { + attrPath := path.Root("elasticsearch").AtName(tierName).AtName(attrName) + var planValue attr.Value + resp.Diagnostics.Append(req.Plan.GetAttribute(ctx, attrPath, &planValue)...) + if resp.Diagnostics.HasError() { + return + } + + var stateValue attr.Value + + resp.Diagnostics.Append(req.State.GetAttribute(ctx, attrPath, &stateValue)...) + if resp.Diagnostics.HasError() { + return + } + + // If the plan value is unknown then planmodifiers haven't run for this topology element + // Eventually the plan value will be set to the state value and it will be unchanged. + if planValue.IsUnknown() && !(stateValue.IsUnknown() || stateValue.IsNull()) { + continue + } + + hasChanged, diags := planmodifiers.AttributeChanged(ctx, attrPath, req.Plan, req.State) resp.Diagnostics.Append(diags...) if resp.Diagnostics.HasError() { return diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_plan_modifier_test.go b/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_plan_modifier_test.go index 48c7ce2cd..2b1a08979 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_plan_modifier_test.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_plan_modifier_test.go @@ -28,6 +28,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func Test_nodeRolesPlanModifier(t *testing.T) { @@ -221,3 +222,205 @@ func Test_nodeRolesPlanModifier(t *testing.T) { }) } } + +func ptr[T any](t T) *T { + return &t +} + +func TestSetUnknownOnTopologySizeChange_PlanModifySet(t *testing.T) { + tests := []struct { + name string + setSizesToUnknown bool + plan *deploymentv2.Deployment + state *deploymentv2.Deployment + planValue types.Set + expectedPlanValue types.Set + }{ + { + name: "should do nothing if the plan value is unknown", + planValue: types.SetUnknown(types.StringType), + plan: &deploymentv2.Deployment{}, + state: &deploymentv2.Deployment{}, + expectedPlanValue: types.SetUnknown(types.StringType), + }, + { + name: "should do nothing if the plan value is null", + planValue: types.SetNull(types.StringType), + plan: &deploymentv2.Deployment{}, + state: &deploymentv2.Deployment{}, + expectedPlanValue: types.SetNull(types.StringType), + }, + { + name: "should do nothing if the only deployment topology size is unchanged", + planValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + plan: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + }, + }, + state: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + }, + }, + expectedPlanValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + }, + { + name: "should set the plan value to unknown if the only deployment topology size has changed", + planValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + plan: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + }, + }, + state: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("2g"), + ZoneCount: 3, + }, + }, + }, + expectedPlanValue: types.SetUnknown(types.StringType), + }, + { + name: "should set the plan value to unknown if the only deployment topology zone count has changed", + planValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + plan: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + }, + }, + state: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 2, + }, + }, + }, + expectedPlanValue: types.SetUnknown(types.StringType), + }, + { + name: "should set the plan value to unknown if the another deployment topology is added", + planValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + plan: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + }, + }, + state: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + }, + }, + expectedPlanValue: types.SetUnknown(types.StringType), + }, + { + name: "should set the plan value to unknown if the another deployment topology size has changed", + planValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + plan: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + }, + }, + state: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + Size: ptr("2g"), + ZoneCount: 3, + }, + }, + }, + expectedPlanValue: types.SetUnknown(types.StringType), + }, + { + name: "should set the plan value to unknown if the another deployment topology zone count has changed", + planValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + plan: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + }, + }, + state: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 2, + }, + }, + }, + expectedPlanValue: types.SetUnknown(types.StringType), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stateValue := tftypesValueFromGoTypeValue(t, tt.state, deploymentv2.DeploymentSchema().Type()) + planValue := tftypesValueFromGoTypeValue(t, tt.plan, deploymentv2.DeploymentSchema().Type()) + req := planmodifier.SetRequest{ + PlanValue: tt.planValue, + State: tfsdk.State{ + Raw: stateValue, + Schema: deploymentv2.DeploymentSchema(), + }, + Plan: tfsdk.Plan{ + Raw: planValue, + Schema: deploymentv2.DeploymentSchema(), + }, + } + + resp := planmodifier.SetResponse{ + PlanValue: tt.planValue, + } + modifier := v2.SetUnknownOnTopologySizeChange() + modifier.PlanModifySet(context.Background(), req, &resp) + + require.Equal(t, tt.expectedPlanValue, resp.PlanValue) + }) + } +} diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/schema.go b/ec/ecresource/deploymentresource/elasticsearch/v2/schema.go index 87146695a..50f7725f8 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/schema.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/schema.go @@ -446,7 +446,7 @@ func elasticsearchTopologySchema(options topologySchemaOptions) schema.Attribute } if options.nodeRolesImpactedBySizeChange { - nodeRolesPlanModifiers = append(nodeRolesPlanModifiers, setUnknownOnTopologyChanges{}) + nodeRolesPlanModifiers = append(nodeRolesPlanModifiers, SetUnknownOnTopologySizeChange()) } return schema.SingleNestedAttribute{ From 828cd96c8a68b206a63cebc5c87d3df7db96b132 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Sat, 5 Aug 2023 09:53:49 +1000 Subject: [PATCH 17/19] Fix build --- .../v2/node_roles_plan_modifier_test.go | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_plan_modifier_test.go b/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_plan_modifier_test.go index 2b1a08979..0ab388fdc 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_plan_modifier_test.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_plan_modifier_test.go @@ -19,11 +19,13 @@ package v2_test import ( "context" + "fmt" "testing" deploymentv2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/deployment/v2" v2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/elasticsearch/v2" "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" @@ -233,6 +235,7 @@ func TestSetUnknownOnTopologySizeChange_PlanModifySet(t *testing.T) { setSizesToUnknown bool plan *deploymentv2.Deployment state *deploymentv2.Deployment + requestModifier func(*testing.T, planmodifier.SetRequest) planmodifier.SetRequest planValue types.Set expectedPlanValue types.Set }{ @@ -396,6 +399,141 @@ func TestSetUnknownOnTopologySizeChange_PlanModifySet(t *testing.T) { }, expectedPlanValue: types.SetUnknown(types.StringType), }, + { + name: "should do nothing if the another deployment topology size is unknown in plan but known in state", + planValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + plan: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + ZoneCount: 3, + }, + }, + }, + state: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + }, + }, + expectedPlanValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + requestModifier: func(t *testing.T, sr planmodifier.SetRequest) planmodifier.SetRequest { + diags := sr.Plan.SetAttribute(context.Background(), path.Root("elasticsearch").AtName("warm").AtName("size"), types.StringUnknown()) + require.Empty(t, diags) + return sr + }, + }, + { + name: "should do nothing if the another deployment topology zone count is unknown in plan but known in state", + planValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + plan: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + }, + }, + }, + state: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 2, + }, + }, + }, + expectedPlanValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + requestModifier: func(t *testing.T, sr planmodifier.SetRequest) planmodifier.SetRequest { + diags := sr.Plan.SetAttribute(context.Background(), path.Root("elasticsearch").AtName("warm").AtName("zone_count"), types.Int64Unknown()) + require.Empty(t, diags) + return sr + }, + }, + { + name: "should set the plan value to unknown if the another deployment topology size is unknown in plan and null in state", + planValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + plan: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + ZoneCount: 2, + }, + }, + }, + state: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + ZoneCount: 2, + }, + }, + }, + expectedPlanValue: types.SetUnknown(types.StringType), + requestModifier: func(t *testing.T, sr planmodifier.SetRequest) planmodifier.SetRequest { + diags := sr.Plan.SetAttribute(context.Background(), path.Root("elasticsearch").AtName("warm").AtName("size"), types.StringUnknown()) + require.Empty(t, diags) + return sr + }, + }, + { + name: "should set the plan value to unknown if the another deployment topology zone count is unknown in plan and null in state", + planValue: types.SetValueMust(types.StringType, []attr.Value{types.StringValue("hot")}), + plan: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + }, + }, + }, + state: &deploymentv2.Deployment{ + Elasticsearch: &v2.Elasticsearch{ + HotTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + ZoneCount: 3, + }, + WarmTier: &v2.ElasticsearchTopology{ + Size: ptr("1g"), + }, + }, + }, + expectedPlanValue: types.SetUnknown(types.StringType), + requestModifier: func(t *testing.T, sr planmodifier.SetRequest) planmodifier.SetRequest { + attrPath := path.Root("elasticsearch").AtName("warm").AtName("zone_count") + diags := sr.Plan.SetAttribute(context.Background(), attrPath, types.Int64Unknown()) + require.Empty(t, diags) + + sr.State.SetAttribute(context.Background(), attrPath, types.Int64Null()) + require.Empty(t, diags) + + return sr + }, + }, } for _, tt := range tests { @@ -413,6 +551,14 @@ func TestSetUnknownOnTopologySizeChange_PlanModifySet(t *testing.T) { Schema: deploymentv2.DeploymentSchema(), }, } + if tt.requestModifier != nil { + req = tt.requestModifier(t, req) + var v attr.Value + req.Plan.GetAttribute(context.Background(), path.Root("elasticsearch").AtName("warm").AtName("zone_count"), &v) + if v.IsUnknown() { + fmt.Println("unknown!") + } + } resp := planmodifier.SetResponse{ PlanValue: tt.planValue, From 5bf8d7b9f5f41d6b4bd1b6f7d0f92a31912a7b90 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Sat, 5 Aug 2023 14:36:12 +1000 Subject: [PATCH 18/19] Changelog --- .changelog/677.txt | 3 +++ .changelog/682.txt | 3 +++ 2 files changed, 6 insertions(+) create mode 100644 .changelog/677.txt create mode 100644 .changelog/682.txt diff --git a/.changelog/677.txt b/.changelog/677.txt new file mode 100644 index 000000000..7098a6fd0 --- /dev/null +++ b/.changelog/677.txt @@ -0,0 +1,3 @@ +```release-note:bug-fix +provider: Fix incompatibilities causing infinite configuration drift when used with Terraform CLI 1.4 or higher +``` diff --git a/.changelog/682.txt b/.changelog/682.txt new file mode 100644 index 000000000..512583827 --- /dev/null +++ b/.changelog/682.txt @@ -0,0 +1,3 @@ +```release-note:bug-fix +resource/deployment: Fix bugs related to transitioning to/from deployment topologies which include dedicated master nodes +``` From 7feaf4d9ff8ca2bc05134c4cd762218b0c4d51b3 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Sat, 5 Aug 2023 14:52:51 +1000 Subject: [PATCH 19/19] Changelog --- .changelog/640.txt | 3 ++- .changelog/642.txt | 3 +++ .changelog/648.txt | 3 +++ 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 .changelog/642.txt create mode 100644 .changelog/648.txt diff --git a/.changelog/640.txt b/.changelog/640.txt index 23049d038..945aeca32 100644 --- a/.changelog/640.txt +++ b/.changelog/640.txt @@ -1,2 +1,3 @@ ```release-note:feature -resource/deployment: Adds endpoints integrations server resources. This allows consumers to explicitly capture service urls for dependent modules (e.g APM and Fleet).``` +resource/deployment: Adds endpoints integrations server resources. This allows consumers to explicitly capture service urls for dependent modules (e.g APM and Fleet). +``` diff --git a/.changelog/642.txt b/.changelog/642.txt new file mode 100644 index 000000000..fa2f1ac68 --- /dev/null +++ b/.changelog/642.txt @@ -0,0 +1,3 @@ +```release-note:feature +resource/deployment: Add `reset_elasticsearch_password` attribute to the deployment resource. When true, this will reset the system password for the target deployment, updating the `elasticsearch_password` output as a result. +``` diff --git a/.changelog/648.txt b/.changelog/648.txt new file mode 100644 index 000000000..02d7bb9c7 --- /dev/null +++ b/.changelog/648.txt @@ -0,0 +1,3 @@ +```release-note:bug-fix +resource/deployment: Fix validation and application of elasticsearch plan strategy +```