Skip to content

Commit

Permalink
Don't leave the apm_secret_token unknown if we're not adding APM or I…
Browse files Browse the repository at this point in the history
…ntegrations Server in the current plan. (#689)

* Move plan modifiers to their own files

* Move TfTypesValueFromGoTypeValue to testutils

* Update Secret Token plan modifier to set Null unless APM or Integrations Server are being added in the current planning phase

* Changelog

* PR feedback
  • Loading branch information
tobio authored Aug 21, 2023
1 parent 45c6e3c commit e4e72ea
Show file tree
Hide file tree
Showing 11 changed files with 350 additions and 110 deletions.
3 changes: 3 additions & 0 deletions .changelog/689.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/deployment: Prevent an endless diff loop after importing deployments with APM or Integrations Server resources.
```
79 changes: 1 addition & 78 deletions ec/ecresource/deploymentresource/deployment/v2/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -111,7 +108,7 @@ func DeploymentSchema() schema.Schema {
Sensitive: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
useNullIfNotAPM{},
UseNullUnlessAddingAPMOrIntegrationsServer(),
},
},
"traffic_filter": schema.SetAttribute{
Expand Down Expand Up @@ -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()
}
}
Original file line number Diff line number Diff line change
@@ -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()
}
}
Original file line number Diff line number Diff line change
@@ -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) {
hasAttribute, diags := planmodifiers.HasAttribute(ctx, p, plan)
if diags.HasError() {
return false, diags
}

if hasAttribute {
var value attr.Value
diags.Append(state.GetAttribute(ctx, p, &value)...)
if diags.HasError() {
return false, diags
}

// Check if the attribute has been added, i.e exists in plan, but not in state
if value.IsNull() {
return true, diags
}
}

return false, diags
}
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
Loading

0 comments on commit e4e72ea

Please sign in to comment.