fix(v6provider/refresh): refreshing missing resources now clears them…

… from state (#133) The new resources implemented with the Plugin Framework had a bug where they would not clear our state of resources which no longer exist in the API. This has now been fixed, so resources deleted outside of Terraform are correctly detected as missing, yielding a plan to recreate them. Explicitly testing this behavior seems to be fraught with annoyance and flickering results, so for now I've left that out.
krystal · Jun 20, 2024 · 0ce024b · 0ce024b
1 parent b4cb8bf
commit 0ce024b
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 55 deletions.
diff --git a/internal/v6provider/resource_ip.go b/internal/v6provider/resource_ip.go
@@ -17,7 +17,6 @@ import (
 	"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"
 	"github.com/krystal/go-katapult"
 	"github.com/krystal/go-katapult/core"
@@ -215,7 +214,7 @@ func (r *IPResource) Create(
 	diags = resp.State.Set(ctx, plan)
 	resp.Diagnostics.Append(diags...)
 
-	if err := r.IPRead(ctx, &plan, &resp.State); err != nil {
+	if err := r.IPRead(ctx, ip.ID, &plan); err != nil {
 		resp.Diagnostics.AddError("IP Address Read Error", err.Error())
 		return
 	}
@@ -236,7 +235,16 @@ func (r *IPResource) Read(
 		return
 	}
 
-	if err := r.IPRead(ctx, state, &resp.State); err != nil {
+	if err := r.IPRead(ctx, state.ID.ValueString(), state); err != nil {
+		if errors.Is(err, katapult.ErrNotFound) {
+			r.M.Logger.Info(
+				"IP Address not found, removing from state",
+				"id", state.ID.ValueString(),
+			)
+			resp.State.RemoveResource(ctx)
+			return
+		}
+
 		resp.Diagnostics.AddError("IP Address Read Error", err.Error())
 		return
 	}
@@ -266,7 +274,8 @@ func (r *IPResource) Update(
 		return
 	}
 
-	ipRef := core.IPAddressRef{ID: state.ID.ValueString()}
+	id := state.ID.ValueString()
+	ipRef := core.IPAddressRef{ID: id}
 	args := &core.IPAddressUpdateArguments{}
 
 	if !plan.VIP.Equal(state.VIP) {
@@ -284,7 +293,7 @@ func (r *IPResource) Update(
 		return
 	}
 
-	if err := r.IPRead(ctx, &plan, &resp.State); err != nil {
+	if err := r.IPRead(ctx, id, &plan); err != nil {
 		resp.Diagnostics.AddError("IP Address Read Error", err.Error())
 		return
 	}
@@ -314,16 +323,11 @@ func (r *IPResource) Delete(
 
 func (r *IPResource) IPRead(
 	ctx context.Context,
+	id string,
 	model *IPResourceModel,
-	state *tfsdk.State,
 ) error {
-	ip, _, err := r.M.Core.IPAddresses.GetByID(ctx, model.ID.ValueString())
+	ip, _, err := r.M.Core.IPAddresses.GetByID(ctx, id)
 	if err != nil {
-		if errors.Is(err, katapult.ErrNotFound) {
-			state.RemoveResource(ctx)
-			return nil
-		}
-
 		return err
 	}
 

diff --git a/internal/v6provider/resource_load_balancer.go b/internal/v6provider/resource_load_balancer.go
@@ -14,7 +14,6 @@ import (
 	"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"
 	"github.com/krystal/go-katapult"
 	"github.com/krystal/go-katapult/core"
@@ -199,7 +198,7 @@ func (r *LoadBalancerResource) Create(
 		return
 	}
 
-	if err := r.LoadBalancerRead(ctx, lb.ID, &plan, &resp.State); err != nil {
+	if err := r.LoadBalancerRead(ctx, lb.ID, &plan); err != nil {
 		resp.Diagnostics.AddError("Load Balancer Read Error", err.Error())
 		return
 	}
@@ -219,12 +218,18 @@ func (r *LoadBalancerResource) Read(
 		return
 	}
 
-	if err := r.LoadBalancerRead(
-		ctx,
-		state.ID.ValueString(),
-		state,
-		&resp.State,
-	); err != nil {
+	err := r.LoadBalancerRead(ctx, state.ID.ValueString(), state)
+	if err != nil {
+		if errors.Is(err, katapult.ErrNotFound) {
+			r.M.Logger.Info(
+				"Load Balancer not found, removing from state",
+				"id", state.ID.ValueString(),
+			)
+			resp.State.RemoveResource(ctx)
+
+			return
+		}
+
 		resp.Diagnostics.AddError("Load Balancer Read Error", err.Error())
 		return
 	}
@@ -281,7 +286,7 @@ func (r *LoadBalancerResource) Update(
 		return
 	}
 
-	if err := r.LoadBalancerRead(ctx, id, &plan, &resp.State); err != nil {
+	if err := r.LoadBalancerRead(ctx, id, &plan); err != nil {
 		resp.Diagnostics.AddError("Load Balancer Read Error", err.Error())
 		return
 	}
@@ -323,16 +328,9 @@ func (r *LoadBalancerResource) LoadBalancerRead(
 	ctx context.Context,
 	id string,
 	model *LoadBalancerResourceModel,
-	state *tfsdk.State,
 ) error {
 	lb, _, err := r.M.Core.LoadBalancers.GetByID(ctx, id)
 	if err != nil {
-		if errors.Is(err, katapult.ErrNotFound) {
-			state.RemoveResource(ctx)
-
-			return nil
-		}
-
 		return err
 	}
 

diff --git a/internal/v6provider/resource_load_balancer_rule.go b/internal/v6provider/resource_load_balancer_rule.go
@@ -21,7 +21,6 @@ import (
 	"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"
-	"github.com/hashicorp/terraform-plugin-framework/tfsdk"
 	"github.com/hashicorp/terraform-plugin-framework/types"
 	"github.com/hashicorp/terraform-plugin-framework/types/basetypes"
 	"github.com/krystal/go-katapult"
@@ -391,11 +390,7 @@ func (r *LoadBalancerRuleResource) Create(
 		return
 	}
 
-	if err := r.LoadBalancerRuleRead(ctx,
-		lbr.ID,
-		&plan,
-		&resp.State,
-	); err != nil {
+	if err := r.LoadBalancerRuleRead(ctx, lbr.ID, &plan); err != nil {
 		resp.Diagnostics.AddError(
 			"LoadBalancerRule Read Error",
 			"Error reading LoadBalancerRule: "+err.Error(),
@@ -417,11 +412,17 @@ func (r *LoadBalancerRuleResource) Read(
 		return
 	}
 
-	if err := r.LoadBalancerRuleRead(ctx,
-		state.ID.ValueString(),
-		state,
-		&resp.State,
-	); err != nil {
+	err := r.LoadBalancerRuleRead(ctx, state.ID.ValueString(), state)
+	if err != nil {
+		if errors.Is(err, katapult.ErrNotFound) {
+			r.M.Logger.Info(
+				"LoadBalancerRule not found, removing from state",
+				"id", state.ID.ValueString(),
+			)
+			resp.State.RemoveResource(ctx)
+			return
+		}
+
 		resp.Diagnostics.AddError(
 			"LoadBalancerRule Read Error",
 			"Error reading LoadBalancerRule: "+err.Error(),
@@ -454,7 +455,6 @@ func (r *LoadBalancerRuleResource) Update(
 	}
 
 	id := state.ID.ValueString()
-
 	lbrRef := core.LoadBalancerRuleRef{ID: id}
 	args, diags := buildLoadBalancerRuleUpdateArgs(ctx, &plan, &state)
 	resp.Diagnostics.Append(diags...)
@@ -475,12 +475,7 @@ func (r *LoadBalancerRuleResource) Update(
 		plan.LoadBalancerID = state.LoadBalancerID
 	}
 
-	if err := r.LoadBalancerRuleRead(
-		ctx,
-		id,
-		&plan,
-		&resp.State,
-	); err != nil {
+	if err = r.LoadBalancerRuleRead(ctx, id, &plan); err != nil {
 		resp.Diagnostics.AddError(
 			"LoadBalancerRule Read Error",
 			"Error reading LoadBalancerRule: "+err.Error(),
@@ -526,17 +521,9 @@ func (r *LoadBalancerRuleResource) LoadBalancerRuleRead(
 	ctx context.Context,
 	id string,
 	model *LoadBalancerRuleResourceModel,
-	state *tfsdk.State,
 ) error {
-	lbr, _, err := r.M.Core.LoadBalancerRules.GetByID(ctx,
-		id)
+	lbr, _, err := r.M.Core.LoadBalancerRules.GetByID(ctx, id)
 	if err != nil {
-		if errors.Is(err, katapult.ErrNotFound) {
-			state.RemoveResource(ctx)
-
-			return nil
-		}
-
 		return err
 	}