From f6dda14546766c4c18c0f130f85d83dc90cfbbae Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Sun, 13 Aug 2023 21:46:55 +1000 Subject: [PATCH 1/5] Move plan modifiers to their own files --- .../deployment/v2/schema.go | 77 ------------------- .../set_unknown_if_reset_password_is_true.go | 55 +++++++++++++ .../deployment/v2/use_null_if_no_apm.go | 55 +++++++++++++ 3 files changed, 110 insertions(+), 77 deletions(-) create mode 100644 ec/ecresource/deploymentresource/deployment/v2/set_unknown_if_reset_password_is_true.go create mode 100644 ec/ecresource/deploymentresource/deployment/v2/use_null_if_no_apm.go diff --git a/ec/ecresource/deploymentresource/deployment/v2/schema.go b/ec/ecresource/deploymentresource/deployment/v2/schema.go index 4c92fc86d..8f5e0305e 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/schema.go +++ b/ec/ecresource/deploymentresource/deployment/v2/schema.go @@ -18,10 +18,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/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" @@ -141,77 +138,3 @@ 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 - } - - if !req.PlanValue.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{} - -func (m setUnknownIfResetPasswordIsTrue) Description(ctx context.Context) string { - return m.MarkdownDescription(ctx) -} - -func (m setUnknownIfResetPasswordIsTrue) MarkdownDescription(ctx context.Context) string { - return "Sets the planned value to unknown if the reset_elasticsearch_password config value is true" -} - -func (m setUnknownIfResetPasswordIsTrue) 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 - } - - var isResetting *bool - resp.Diagnostics.Append(req.Plan.GetAttribute(ctx, path.Root("reset_elasticsearch_password"), &isResetting)...) - if resp.Diagnostics.HasError() { - return - } - - if isResetting != nil && *isResetting { - resp.PlanValue = types.StringUnknown() - } -} diff --git a/ec/ecresource/deploymentresource/deployment/v2/set_unknown_if_reset_password_is_true.go b/ec/ecresource/deploymentresource/deployment/v2/set_unknown_if_reset_password_is_true.go new file mode 100644 index 000000000..07ac3bfde --- /dev/null +++ b/ec/ecresource/deploymentresource/deployment/v2/set_unknown_if_reset_password_is_true.go @@ -0,0 +1,55 @@ +// 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/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +type setUnknownIfResetPasswordIsTrue struct{} + +var _ planmodifier.String = setUnknownIfResetPasswordIsTrue{} + +func (m setUnknownIfResetPasswordIsTrue) Description(ctx context.Context) string { + return m.MarkdownDescription(ctx) +} + +func (m setUnknownIfResetPasswordIsTrue) MarkdownDescription(ctx context.Context) string { + return "Sets the planned value to unknown if the reset_elasticsearch_password config value is true" +} + +func (m setUnknownIfResetPasswordIsTrue) 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 + } + + var isResetting *bool + resp.Diagnostics.Append(req.Plan.GetAttribute(ctx, path.Root("reset_elasticsearch_password"), &isResetting)...) + if resp.Diagnostics.HasError() { + return + } + + if isResetting != nil && *isResetting { + resp.PlanValue = types.StringUnknown() + } +} diff --git a/ec/ecresource/deploymentresource/deployment/v2/use_null_if_no_apm.go b/ec/ecresource/deploymentresource/deployment/v2/use_null_if_no_apm.go new file mode 100644 index 000000000..4f4a8a6b4 --- /dev/null +++ b/ec/ecresource/deploymentresource/deployment/v2/use_null_if_no_apm.go @@ -0,0 +1,55 @@ +package v2 + +import ( + "context" + + "github.com/elastic/terraform-provider-ec/ec/internal/planmodifiers" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +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 + } + + if !req.PlanValue.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() +} From 879dd7f9e2e4fd7a7c2bbd3fd029eb166256a04e Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Sun, 13 Aug 2023 21:47:46 +1000 Subject: [PATCH 2/5] Move TfTypesValueFromGoTypeValue to testutils --- .../v2/node_roles_plan_modifier_test.go | 15 +++---- .../v2/node_roles_validator_test.go | 3 +- .../v2/node_types_plan_modifier_test.go | 5 ++- .../v2/node_types_validator_test.go | 3 +- .../v2/topology_plan_modifier_test.go | 21 ++-------- .../deploymentresource/testutil/tftypes.go | 42 +++++++++++++++++++ 6 files changed, 57 insertions(+), 32 deletions(-) create mode 100644 ec/ecresource/deploymentresource/testutil/tftypes.go 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 0ab388fdc..afae31db5 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,11 @@ 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/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/testutil" "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" @@ -174,9 +174,9 @@ func Test_nodeRolesPlanModifier(t *testing.T) { stateValue, diags := types.SetValueFrom(context.Background(), types.StringType, tt.args.attributeState) assert.Nil(t, diags) - deploymentStateValue := tftypesValueFromGoTypeValue(t, tt.args.deploymentState, deploymentv2.DeploymentSchema().Type()) + deploymentStateValue := testutil.TfTypesValueFromGoTypeValue(t, tt.args.deploymentState, deploymentv2.DeploymentSchema().Type()) - deploymentPlanValue := tftypesValueFromGoTypeValue(t, tt.args.deploymentPlan, deploymentv2.DeploymentSchema().Type()) + deploymentPlanValue := testutil.TfTypesValueFromGoTypeValue(t, tt.args.deploymentPlan, deploymentv2.DeploymentSchema().Type()) req := planmodifier.SetRequest{ // AttributeState: attributeStateValue, @@ -538,8 +538,8 @@ func TestSetUnknownOnTopologySizeChange_PlanModifySet(t *testing.T) { 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()) + stateValue := testutil.TfTypesValueFromGoTypeValue(t, tt.state, deploymentv2.DeploymentSchema().Type()) + planValue := testutil.TfTypesValueFromGoTypeValue(t, tt.plan, deploymentv2.DeploymentSchema().Type()) req := planmodifier.SetRequest{ PlanValue: tt.planValue, State: tfsdk.State{ @@ -553,11 +553,6 @@ func TestSetUnknownOnTopologySizeChange_PlanModifySet(t *testing.T) { } 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{ diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_validator_test.go b/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_validator_test.go index 25a663f8f..3c2faa952 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_validator_test.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/node_roles_validator_test.go @@ -23,6 +23,7 @@ import ( 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/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/testutil" "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/utils" "github.com/hashicorp/terraform-plugin-framework/attr" "github.com/hashicorp/terraform-plugin-framework/schema/validator" @@ -67,7 +68,7 @@ func TestNodeRolesValidator_ValidateSet(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { v := v2.VersionSupportsNodeRoles() - config := tftypesValueFromGoTypeValue(t, &deploymentv2.Deployment{ + config := testutil.TfTypesValueFromGoTypeValue(t, &deploymentv2.Deployment{ Version: tt.version, }, deploymentv2.DeploymentSchema().Type()) resp := validator.SetResponse{} diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/node_types_plan_modifier_test.go b/ec/ecresource/deploymentresource/elasticsearch/v2/node_types_plan_modifier_test.go index fb7691e44..19c169a32 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/node_types_plan_modifier_test.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/node_types_plan_modifier_test.go @@ -23,6 +23,7 @@ import ( 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/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/testutil" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" @@ -143,9 +144,9 @@ func Test_nodeTypesPlanModifier(t *testing.T) { t.Run(tt.name, func(t *testing.T) { modifier := v2.UseNodeTypesDefault() - deploymentStateValue := tftypesValueFromGoTypeValue(t, tt.args.deploymentState, deploymentv2.DeploymentSchema().Type()) + deploymentStateValue := testutil.TfTypesValueFromGoTypeValue(t, tt.args.deploymentState, deploymentv2.DeploymentSchema().Type()) - deploymentPlanValue := tftypesValueFromGoTypeValue(t, tt.args.deploymentPlan, deploymentv2.DeploymentSchema().Type()) + deploymentPlanValue := testutil.TfTypesValueFromGoTypeValue(t, tt.args.deploymentPlan, deploymentv2.DeploymentSchema().Type()) req := planmodifier.StringRequest{ // ConfigValue value is not used in the plan modifer, diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/node_types_validator_test.go b/ec/ecresource/deploymentresource/elasticsearch/v2/node_types_validator_test.go index fef8f0330..bac3a1a20 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/node_types_validator_test.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/node_types_validator_test.go @@ -23,6 +23,7 @@ import ( 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/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/testutil" "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/utils" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/tfsdk" @@ -66,7 +67,7 @@ func TestNodeTypesValidator_ValidateString(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { v := v2.VersionSupportsNodeTypes() - config := tftypesValueFromGoTypeValue(t, &deploymentv2.Deployment{ + config := testutil.TfTypesValueFromGoTypeValue(t, &deploymentv2.Deployment{ Version: tt.version, }, deploymentv2.DeploymentSchema().Type()) resp := validator.StringResponse{} diff --git a/ec/ecresource/deploymentresource/elasticsearch/v2/topology_plan_modifier_test.go b/ec/ecresource/deploymentresource/elasticsearch/v2/topology_plan_modifier_test.go index 1880d1f88..ce79adbc2 100644 --- a/ec/ecresource/deploymentresource/elasticsearch/v2/topology_plan_modifier_test.go +++ b/ec/ecresource/deploymentresource/elasticsearch/v2/topology_plan_modifier_test.go @@ -24,10 +24,9 @@ import ( "github.com/elastic/cloud-sdk-go/pkg/util/ec" 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/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/testutil" "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" - "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/stretchr/testify/assert" ) @@ -119,9 +118,9 @@ func Test_topologyPlanModifier(t *testing.T) { t.Run(tt.name, func(t *testing.T) { modifier := v2.UseTopologyStateForUnknown("hot") - deploymentStateValue := tftypesValueFromGoTypeValue(t, tt.args.deploymentState, deploymentv2.DeploymentSchema().Type()) + deploymentStateValue := testutil.TfTypesValueFromGoTypeValue(t, tt.args.deploymentState, deploymentv2.DeploymentSchema().Type()) - deploymentPlanValue := tftypesValueFromGoTypeValue(t, tt.args.deploymentPlan, deploymentv2.DeploymentSchema().Type()) + deploymentPlanValue := testutil.TfTypesValueFromGoTypeValue(t, tt.args.deploymentPlan, deploymentv2.DeploymentSchema().Type()) plan := tfsdk.Plan{ Raw: deploymentPlanValue, @@ -146,17 +145,3 @@ func Test_topologyPlanModifier(t *testing.T) { }) } } - -func attrValueFromGoTypeValue(t *testing.T, goValue any, attributeType attr.Type) attr.Value { - var attrValue attr.Value - diags := tfsdk.ValueFrom(context.Background(), goValue, attributeType, &attrValue) - assert.Nil(t, diags) - return attrValue -} - -func tftypesValueFromGoTypeValue(t *testing.T, goValue any, attributeType attr.Type) tftypes.Value { - attrValue := attrValueFromGoTypeValue(t, goValue, attributeType) - tftypesValue, err := attrValue.ToTerraformValue(context.Background()) - assert.Nil(t, err) - return tftypesValue -} diff --git a/ec/ecresource/deploymentresource/testutil/tftypes.go b/ec/ecresource/deploymentresource/testutil/tftypes.go new file mode 100644 index 000000000..e11450d50 --- /dev/null +++ b/ec/ecresource/deploymentresource/testutil/tftypes.go @@ -0,0 +1,42 @@ +// 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 testutil + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/stretchr/testify/assert" +) + +func attrValueFromGoTypeValue(t *testing.T, goValue any, attributeType attr.Type) attr.Value { + var attrValue attr.Value + diags := tfsdk.ValueFrom(context.Background(), goValue, attributeType, &attrValue) + assert.Nil(t, diags) + return attrValue +} + +func TfTypesValueFromGoTypeValue(t *testing.T, goValue any, attributeType attr.Type) tftypes.Value { + attrValue := attrValueFromGoTypeValue(t, goValue, attributeType) + tftypesValue, err := attrValue.ToTerraformValue(context.Background()) + assert.Nil(t, err) + return tftypesValue +} From e5b1d780ee89cc284e342384aed795e5f5890700 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Sun, 13 Aug 2023 21:49:18 +1000 Subject: [PATCH 3/5] Update Secret Token plan modifier to set Null unless APM or Integrations Server are being added in the current planning phase --- .../deployment/v2/schema.go | 2 +- .../deployment/v2/use_null_if_no_apm.go | 55 ------- ...se_null_unless_adding_apm_plan_modifier.go | 99 +++++++++++++ ...ll_unless_adding_apm_plan_modifier_test.go | 135 ++++++++++++++++++ 4 files changed, 235 insertions(+), 56 deletions(-) delete mode 100644 ec/ecresource/deploymentresource/deployment/v2/use_null_if_no_apm.go create mode 100644 ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier.go create mode 100644 ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier_test.go diff --git a/ec/ecresource/deploymentresource/deployment/v2/schema.go b/ec/ecresource/deploymentresource/deployment/v2/schema.go index 8f5e0305e..c13dbf6e5 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/schema.go +++ b/ec/ecresource/deploymentresource/deployment/v2/schema.go @@ -108,7 +108,7 @@ func DeploymentSchema() schema.Schema { Sensitive: true, PlanModifiers: []planmodifier.String{ stringplanmodifier.UseStateForUnknown(), - useNullIfNotAPM{}, + UseNullUnlessAddingAPMOrIntegrationsServer(), }, }, "traffic_filter": schema.SetAttribute{ diff --git a/ec/ecresource/deploymentresource/deployment/v2/use_null_if_no_apm.go b/ec/ecresource/deploymentresource/deployment/v2/use_null_if_no_apm.go deleted file mode 100644 index 4f4a8a6b4..000000000 --- a/ec/ecresource/deploymentresource/deployment/v2/use_null_if_no_apm.go +++ /dev/null @@ -1,55 +0,0 @@ -package v2 - -import ( - "context" - - "github.com/elastic/terraform-provider-ec/ec/internal/planmodifiers" - "github.com/hashicorp/terraform-plugin-framework/path" - "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" - "github.com/hashicorp/terraform-plugin-framework/types" -) - -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 - } - - if !req.PlanValue.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() -} diff --git a/ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier.go b/ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier.go new file mode 100644 index 000000000..b9d895271 --- /dev/null +++ b/ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier.go @@ -0,0 +1,99 @@ +// 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" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +func UseNullUnlessAddingAPMOrIntegrationsServer() planmodifier.String { + return useNullUnlessAddingAPMOrIntegrationsServer{} +} + +type useNullUnlessAddingAPMOrIntegrationsServer struct{} + +var _ planmodifier.String = useNullUnlessAddingAPMOrIntegrationsServer{} + +func (m useNullUnlessAddingAPMOrIntegrationsServer) Description(ctx context.Context) string { + return m.MarkdownDescription(ctx) +} + +func (m useNullUnlessAddingAPMOrIntegrationsServer) MarkdownDescription(ctx context.Context) string { + return "Sets the plan value to null if there is no apm or integrations_server resource" +} + +func (m useNullUnlessAddingAPMOrIntegrationsServer) 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 + } + + // Critically, we'll return here if this value has been set from state. + // The rest of this function only applies if there is no value already in state. + if !req.PlanValue.IsUnknown() { + return + } + + addedAPM, diags := wasAttributeAdded(ctx, path.Root("apm"), req.Plan, req.State) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + + addedIntegrationsServer, diags := wasAttributeAdded(ctx, path.Root("integrations_server"), req.Plan, req.State) + resp.Diagnostics.Append(diags...) + if resp.Diagnostics.HasError() { + return + } + + if addedAPM || addedIntegrationsServer { + return + } + + resp.PlanValue = types.StringNull() +} + +func wasAttributeAdded(ctx context.Context, p path.Path, plan tfsdk.Plan, state tfsdk.State) (bool, diag.Diagnostics) { + hasIntegrationsServer, diags := planmodifiers.HasAttribute(ctx, p, plan) + if diags.HasError() { + return false, diags + } + + if hasIntegrationsServer { + var value attr.Value + diags.Append(state.GetAttribute(ctx, p, &value)...) + if diags.HasError() { + return false, diags + } + + // Check if Integrations Server has been enabled, i.e exists in plan, but not in state + if value.IsNull() { + return true, diags + } + } + + return false, diags +} diff --git a/ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier_test.go b/ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier_test.go new file mode 100644 index 000000000..58bfd7f52 --- /dev/null +++ b/ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier_test.go @@ -0,0 +1,135 @@ +// 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_test + +import ( + "context" + "testing" + + apmv2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/apm/v2" + v2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/deployment/v2" + integrationsserverv2 "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/integrationsserver/v2" + "github.com/elastic/terraform-provider-ec/ec/ecresource/deploymentresource/testutil" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/stretchr/testify/require" +) + +func TestUseNullUnlessAddingAPMOrIntegrationsServer_PlanModifyString(t *testing.T) { + tests := []struct { + name string + planValue types.String + state *v2.Deployment + plan v2.Deployment + expectedPlanValue types.String + }{ + { + name: "should update the plan value to null if neither the plan nor state define either apm or integrations server", + state: &v2.Deployment{}, + plan: v2.Deployment{}, + expectedPlanValue: types.StringNull(), + planValue: types.StringUnknown(), + }, + { + name: "should update the plan value to null if apm exists in both the plan and state", + state: &v2.Deployment{ + Apm: &apmv2.Apm{}, + }, + plan: v2.Deployment{ + Apm: &apmv2.Apm{}, + }, + expectedPlanValue: types.StringNull(), + planValue: types.StringUnknown(), + }, + { + name: "should update the plan value to null if integrations server exists in both the plan and state", + state: &v2.Deployment{ + IntegrationsServer: &integrationsserverv2.IntegrationsServer{}, + }, + plan: v2.Deployment{ + IntegrationsServer: &integrationsserverv2.IntegrationsServer{}, + }, + expectedPlanValue: types.StringNull(), + planValue: types.StringUnknown(), + }, + { + name: "should do nothing if the plan value is known", + state: &v2.Deployment{}, + plan: v2.Deployment{ + IntegrationsServer: &integrationsserverv2.IntegrationsServer{}, + }, + expectedPlanValue: types.StringValue("sekret"), + planValue: types.StringValue("sekret"), + }, + { + name: "should do nothing if the plan value is null", + state: &v2.Deployment{}, + plan: v2.Deployment{ + IntegrationsServer: &integrationsserverv2.IntegrationsServer{}, + }, + expectedPlanValue: types.StringNull(), + planValue: types.StringNull(), + }, + { + name: "should do nothing if the plan value is unknown and the plan adds an apm resource", + state: &v2.Deployment{}, + plan: v2.Deployment{ + Apm: &apmv2.Apm{}, + }, + expectedPlanValue: types.StringUnknown(), + planValue: types.StringUnknown(), + }, + { + name: "should do nothing if the plan value is unknown and the plan adds an integrations server resource", + state: &v2.Deployment{}, + plan: v2.Deployment{ + IntegrationsServer: &integrationsserverv2.IntegrationsServer{}, + }, + expectedPlanValue: types.StringUnknown(), + planValue: types.StringUnknown(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stateValue := testutil.TfTypesValueFromGoTypeValue(t, tt.state, v2.DeploymentSchema().Type()) + planValue := testutil.TfTypesValueFromGoTypeValue(t, tt.plan, v2.DeploymentSchema().Type()) + req := planmodifier.StringRequest{ + PlanValue: tt.planValue, + State: tfsdk.State{ + Raw: stateValue, + Schema: v2.DeploymentSchema(), + }, + Plan: tfsdk.Plan{ + Raw: planValue, + Schema: v2.DeploymentSchema(), + }, + } + + resp := planmodifier.StringResponse{ + PlanValue: tt.planValue, + } + modifier := v2.UseNullUnlessAddingAPMOrIntegrationsServer() + + modifier.PlanModifyString(context.Background(), req, &resp) + + require.Equal(t, tt.expectedPlanValue, resp.PlanValue) + }) + } +} From 738b29e91678c14ff404a8f227cd261d6b932b17 Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Mon, 14 Aug 2023 08:38:03 +1000 Subject: [PATCH 4/5] Changelog --- .changelog/689.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/689.txt diff --git a/.changelog/689.txt b/.changelog/689.txt new file mode 100644 index 000000000..475eaf262 --- /dev/null +++ b/.changelog/689.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/deployment: Prevent an endless diff loop after importing deployments with APM or Integrations Server resources. +``` From df1c111c4584fe385d9bf129871bc12f10b089be Mon Sep 17 00:00:00 2001 From: Toby Brain Date: Mon, 21 Aug 2023 08:21:21 +1000 Subject: [PATCH 5/5] PR feedback --- ...=> use_null_unless_adding_apm_or_integrations_server.go} | 6 +++--- ...e_null_unless_adding_apm_or_integrations_server_test.go} | 0 2 files changed, 3 insertions(+), 3 deletions(-) rename ec/ecresource/deploymentresource/deployment/v2/{use_null_unless_adding_apm_plan_modifier.go => use_null_unless_adding_apm_or_integrations_server.go} (94%) rename ec/ecresource/deploymentresource/deployment/v2/{use_null_unless_adding_apm_plan_modifier_test.go => use_null_unless_adding_apm_or_integrations_server_test.go} (100%) diff --git a/ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier.go b/ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_or_integrations_server.go similarity index 94% rename from ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier.go rename to ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_or_integrations_server.go index b9d895271..faa5d2450 100644 --- a/ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier.go +++ b/ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_or_integrations_server.go @@ -77,19 +77,19 @@ func (m useNullUnlessAddingAPMOrIntegrationsServer) PlanModifyString(ctx context } func wasAttributeAdded(ctx context.Context, p path.Path, plan tfsdk.Plan, state tfsdk.State) (bool, diag.Diagnostics) { - hasIntegrationsServer, diags := planmodifiers.HasAttribute(ctx, p, plan) + hasAttribute, diags := planmodifiers.HasAttribute(ctx, p, plan) if diags.HasError() { return false, diags } - if hasIntegrationsServer { + if hasAttribute { var value attr.Value diags.Append(state.GetAttribute(ctx, p, &value)...) if diags.HasError() { return false, diags } - // Check if Integrations Server has been enabled, i.e exists in plan, but not in state + // Check if the attribute has been added, i.e exists in plan, but not in state if value.IsNull() { return true, diags } diff --git a/ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier_test.go b/ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_or_integrations_server_test.go similarity index 100% rename from ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_plan_modifier_test.go rename to ec/ecresource/deploymentresource/deployment/v2/use_null_unless_adding_apm_or_integrations_server_test.go