Don't leave the apm_secret_token unknown if we're not adding APM or Integrations Server in the current plan.

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

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"
 
@@ -111,7 +108,7 @@ func DeploymentSchema() schema.Schema {
 				Sensitive: true,
 				PlanModifiers: []planmodifier.String{
 					stringplanmodifier.UseStateForUnknown(),
-					useNullIfNotAPM{},
+					UseNullUnlessAddingAPMOrIntegrationsServer(),
 				},
 			},
 			"traffic_filter": schema.SetAttribute{
@@ -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()
-	}
-}

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()
+	}
+}

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
+}

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)
+		})
+	}
+}