Skip to content

Commit

Permalink
fix(v6provider): add plan modifier rules to reduce excessive known af…
Browse files Browse the repository at this point in the history 
…ter apply issues (#131)

Fixes load balancer rules being marked as requiring re-creation when any
attribute is modified on the associate load balancer. This was happening
cause the `id` attribute on the load balancer got marked as "known after
apply" whenever there were any changes to the load balancer. This in
turn caused any load balancer rules which uses the load balancer ID to
be marked as re-create, since any change to the `load_balancer_id`
attribute requires the rule to be re-created.

We're essentially changing the planning phase so that is copies the
value from the current state if not known in the plan. Additionally, the
load balancer resource gets a custom plan modifier that correctly
handles changes between `virtual_machine_ids`,
`virtual_machine_group_ids` and `tag_ids`.
  • Loading branch information
@jimehk
jimehk authored Jun 19, 2024
1 parent 987b814 commit 0fcb3a0
Show file tree
Hide file tree
Showing 41 changed files with 4,601 additions and 3,822 deletions.
25 changes: 24 additions & 1 deletion internal/v6provider/resource_ip.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
Expand Down Expand Up @@ -79,11 +80,15 @@ func (r IPResource) Schema(
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"network_id": schema.StringAttribute{
Optional: true,
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
stringplanmodifier.RequiresReplace(),
},
},
Expand All @@ -103,19 +108,31 @@ func (r IPResource) Schema(
},
"address": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"address_with_mask": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"reverse_dns": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"vip": schema.BoolAttribute{
Description: "Default is `false`.",
MarkdownDescription: "Default is `false`.",
Optional: true,
Computed: true,
Default: booldefault.StaticBool(false),
PlanModifiers: []planmodifier.Bool{
boolplanmodifier.UseStateForUnknown(),
},
},
"label": schema.StringAttribute{
Description: "VIP label. Required when vip is true.",
Expand All @@ -131,9 +148,15 @@ func (r IPResource) Schema(
},
"allocation_type": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"allocation_id": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
},
}
Expand Down Expand Up @@ -243,7 +266,7 @@ func (r *IPResource) Update(
return
}

ipRef := core.IPAddressRef{ID: plan.ID.String()}
ipRef := core.IPAddressRef{ID: state.ID.ValueString()}
args := &core.IPAddressUpdateArguments{}

if !plan.VIP.Equal(state.VIP) {
Expand Down
116 changes: 113 additions & 3 deletions internal/v6provider/resource_load_balancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/tfsdk"
"github.com/hashicorp/terraform-plugin-framework/types"
Expand Down Expand Up @@ -91,6 +94,9 @@ func (r LoadBalancerResource) Schema(
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"name": schema.StringAttribute{
Required: true,
Expand All @@ -105,6 +111,9 @@ func (r LoadBalancerResource) Schema(
),
},
ElementType: types.StringType,
PlanModifiers: []planmodifier.Set{
loadBalancerResourceIDsPlanModifier(),
},
},
"virtual_machine_group_ids": schema.SetAttribute{
Optional: true,
Expand All @@ -116,6 +125,9 @@ func (r LoadBalancerResource) Schema(
),
},
ElementType: types.StringType,
PlanModifiers: []planmodifier.Set{
loadBalancerResourceIDsPlanModifier(),
},
},
"tag_ids": schema.SetAttribute{
Optional: true,
Expand All @@ -127,14 +139,23 @@ func (r LoadBalancerResource) Schema(
),
},
ElementType: types.StringType,
PlanModifiers: []planmodifier.Set{
loadBalancerResourceIDsPlanModifier(),
},
},
"ip_address": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"https_redirect": schema.BoolAttribute{
Optional: true,
Computed: true,
Default: booldefault.StaticBool(false),
PlanModifiers: []planmodifier.Bool{
boolplanmodifier.UseStateForUnknown(),
},
},
},
}
Expand Down Expand Up @@ -333,9 +354,13 @@ func populateLoadBalancerTargets(
ids []string,
) {
list := flattenLoadBalancerResourceIDs(ids)
model.VirtualMachineIDs = types.SetNull(types.StringType)
model.TagIDs = types.SetNull(types.StringType)
model.VirtualMachineGroupIDs = types.SetNull(types.StringType)
model.VirtualMachineIDs = types.SetValueMust(
types.StringType, []attr.Value{},
)
model.VirtualMachineGroupIDs = types.SetValueMust(
types.StringType, []attr.Value{},
)
model.TagIDs = types.SetValueMust(types.StringType, []attr.Value{})

if len(ids) == 0 {
return
Expand Down Expand Up @@ -389,3 +414,88 @@ func extractLoadBalancerResourceTypeAndIDs(

return t, ids
}

// loadBalancerResourceIDsPlanModifier handles the planning of the resource IDs
// attributes for the load balancer resource. This is needed to ensure correct
// planning when between one of the three attributes used to specify resource
// IDs of different types.
//
// It is based on setplanmanager.UseStateForUnknown(), and behaves identically
// to it when the attribute being planned is configured with one or more values
// in the config.
//
// When the attribute being planned however is not the with values, it will
// forcibly set the planned value to a empty list. Without this, Terraform does
// not realize that the old attribute needs to be cleared out when switching
// between VM IDs, VM Group IDs, and Tag IDs.
func loadBalancerResourceIDsPlanModifier() planmodifier.Set {
return &loadBalancerResourceIDsModifier{}
}

type loadBalancerResourceIDsModifier struct{}

var _ planmodifier.Set = &loadBalancerResourceIDsModifier{}

func (m *loadBalancerResourceIDsModifier) Description(
_ context.Context,
) string {
return "Handles load balancer resource ID change planning."
}

func (m *loadBalancerResourceIDsModifier) MarkdownDescription(
_ context.Context,
) string {
return "Handles load balancer resource ID change planning."
}

func (m *loadBalancerResourceIDsModifier) PlanModifySet(
ctx context.Context,
req planmodifier.SetRequest,
resp *planmodifier.SetResponse,
) {
// 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
}

model := LoadBalancerResourceModel{}
resp.Diagnostics.Append(req.Plan.Get(ctx, &model)...)

// Determine the resource type based on which attribute has one or more
// elements, and extract the path based on the `tfsdk` struct tag.
var resourceTypeAttr string
switch {
case len(model.VirtualMachineIDs.Elements()) > 0:
resourceTypeAttr = getTagValue(model, "VirtualMachineIDs", "tfsdk")
case len(model.VirtualMachineGroupIDs.Elements()) > 0:
resourceTypeAttr = getTagValue(model, "VirtualMachineGroupIDs", "tfsdk")
case len(model.TagIDs.Elements()) > 0:
resourceTypeAttr = getTagValue(model, "TagIDs", "tfsdk")
}
if resourceTypeAttr == "" {
return
}

// Set the plan value to a empty set if the resource type is not that of the
// current path. This is required for the plan to include removal of
// existing values when switching between VM IDs, VM Group IDs and Tag IDs.
if !req.Path.Equal(path.Root(resourceTypeAttr)) {
resp.PlanValue = types.SetValueMust(types.StringType, []attr.Value{})
return
}

// Set the plan value to the state value if the resource type is that of the
// current path.
resp.PlanValue = req.StateValue
}
45 changes: 45 additions & 0 deletions internal/v6provider/resource_load_balancer_rule.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default"
"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/setplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
Expand Down Expand Up @@ -111,6 +114,9 @@ func LoadBalancerRuleSchemaAttributes() map[string]schema.Attribute {
return map[string]schema.Attribute{
"id": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"load_balancer_id": schema.StringAttribute{
Required: true,
Expand All @@ -131,6 +137,9 @@ func LoadBalancerRuleSchemaAttributes() map[string]schema.Attribute {
string(core.StickyRuleAlgorithm),
),
},
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"destination_port": schema.Int64Attribute{
Required: true,
Expand All @@ -152,26 +161,41 @@ func LoadBalancerRuleSchemaAttributes() map[string]schema.Attribute {
Optional: true,
Computed: true,
Default: booldefault.StaticBool(false),
PlanModifiers: []planmodifier.Bool{
boolplanmodifier.UseStateForUnknown(),
},
},
"certificate_ids": schema.SetAttribute{
Optional: true,
Computed: true,
ElementType: types.StringType,
PlanModifiers: []planmodifier.Set{
setplanmodifier.UseStateForUnknown(),
},
},
"backend_ssl": schema.BoolAttribute{
Optional: true,
Computed: true,
Default: booldefault.StaticBool(false),
PlanModifiers: []planmodifier.Bool{
boolplanmodifier.UseStateForUnknown(),
},
},
"passthrough_ssl": schema.BoolAttribute{
Optional: true,
Computed: true,
Default: booldefault.StaticBool(false),
PlanModifiers: []planmodifier.Bool{
boolplanmodifier.UseStateForUnknown(),
},
},
"check_enabled": schema.BoolAttribute{
Optional: true,
Computed: true,
Default: booldefault.StaticBool(false),
PlanModifiers: []planmodifier.Bool{
boolplanmodifier.UseStateForUnknown(),
},
},
"check_fall": schema.Int64Attribute{
Optional: true,
Expand All @@ -183,6 +207,9 @@ func LoadBalancerRuleSchemaAttributes() map[string]schema.Attribute {
),
int64validator.AtLeast(1),
},
PlanModifiers: []planmodifier.Int64{
int64planmodifier.UseStateForUnknown(),
},
},
"check_interval": schema.Int64Attribute{
Optional: true,
Expand All @@ -194,6 +221,9 @@ func LoadBalancerRuleSchemaAttributes() map[string]schema.Attribute {
),
int64validator.AtLeast(1),
},
PlanModifiers: []planmodifier.Int64{
int64planmodifier.UseStateForUnknown(),
},
},
"check_http_statuses": schema.StringAttribute{
Optional: true,
Expand All @@ -204,6 +234,9 @@ func LoadBalancerRuleSchemaAttributes() map[string]schema.Attribute {
path.MatchRoot("check_enabled"),
),
},
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"check_path": schema.StringAttribute{
Optional: true,
Expand All @@ -215,6 +248,9 @@ func LoadBalancerRuleSchemaAttributes() map[string]schema.Attribute {
),
stringvalidator.LengthAtLeast(1),
},
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"check_protocol": schema.StringAttribute{
Optional: true,
Expand All @@ -230,6 +266,9 @@ func LoadBalancerRuleSchemaAttributes() map[string]schema.Attribute {
string(core.TCPProtocol),
),
},
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"check_rise": schema.Int64Attribute{
Optional: true,
Expand All @@ -240,6 +279,9 @@ func LoadBalancerRuleSchemaAttributes() map[string]schema.Attribute {
path.MatchRoot("check_enabled"),
),
},
PlanModifiers: []planmodifier.Int64{
int64planmodifier.UseStateForUnknown(),
},
},
"check_timeout": schema.Int64Attribute{
Optional: true,
Expand All @@ -250,6 +292,9 @@ func LoadBalancerRuleSchemaAttributes() map[string]schema.Attribute {
path.MatchRoot("check_enabled"),
),
},
PlanModifiers: []planmodifier.Int64{
int64planmodifier.UseStateForUnknown(),
},
},
}
}
Expand Down
Loading

0 comments on commit 0fcb3a0

Please sign in to comment.