Adjust mapping of LB API response when private network only field is … (#477)

* Adjust mapping of LB API response when private network only field is null

* Improve comment

Co-authored-by: Diogo Ferrão <diogo.ferrao@freiheit.com>

---------

Co-authored-by: Diogo Ferrão <diogo.ferrao@freiheit.com>

stackit/internal/services/loadbalancer/loadbalancer/datasource.go

@@ -330,7 +330,7 @@ func (r *loadBalancerDataSource) Read(ctx context.Context, req datasource.ReadRe
 	}
 
 	// Map response body to schema
-	err = mapFields(lbResp, &model)
+	err = mapFields(ctx, lbResp, &model)
 	if err != nil {
 		core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading load balancer", fmt.Sprintf("Processing API payload: %v", err))
 		return

stackit/internal/services/loadbalancer/loadbalancer/resource.go

@@ -591,7 +591,7 @@ func (r *loadBalancerResource) Create(ctx context.Context, req resource.CreateRe
 	}
 
 	// Map response body to schema
-	err = mapFields(waitResp, &model)
+	err = mapFields(ctx, waitResp, &model)
 	if err != nil {
 		core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating load balancer", fmt.Sprintf("Processing API payload: %v", err))
 		return
@@ -632,7 +632,7 @@ func (r *loadBalancerResource) Read(ctx context.Context, req resource.ReadReques
 	}
 
 	// Map response body to schema
-	err = mapFields(lbResp, &model)
+	err = mapFields(ctx, lbResp, &model)
 	if err != nil {
 		core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading load balancer", fmt.Sprintf("Processing API payload: %v", err))
 		return
@@ -696,7 +696,7 @@ func (r *loadBalancerResource) Update(ctx context.Context, req resource.UpdateRe
 	}
 
 	// Map response body to schema
-	err = mapFields(getResp, &model)
+	err = mapFields(ctx, getResp, &model)
 	if err != nil {
 		core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating load balancer", fmt.Sprintf("Processing API payload: %v", err))
 		return
@@ -1039,7 +1039,7 @@ func toTargetsPayload(ctx context.Context, tp *targetPool) (*[]loadbalancer.Targ
 	return &payload, nil
 }
 
-func mapFields(lb *loadbalancer.LoadBalancer, m *Model) error {
+func mapFields(ctx context.Context, lb *loadbalancer.LoadBalancer, m *Model) error {
 	if lb == nil {
 		return fmt.Errorf("response input is nil")
 	}
@@ -1075,7 +1075,7 @@ func mapFields(lb *loadbalancer.LoadBalancer, m *Model) error {
 	if err != nil {
 		return fmt.Errorf("mapping network: %w", err)
 	}
-	err = mapOptions(lb, m)
+	err = mapOptions(ctx, lb, m)
 	if err != nil {
 		return fmt.Errorf("mapping options: %w", err)
 	}
@@ -1192,14 +1192,29 @@ func mapNetworks(loadBalancerResp *loadbalancer.LoadBalancer, m *Model) error {
 	return nil
 }
 
-func mapOptions(loadBalancerResp *loadbalancer.LoadBalancer, m *Model) error {
+func mapOptions(ctx context.Context, loadBalancerResp *loadbalancer.LoadBalancer, m *Model) error {
 	if loadBalancerResp.Options == nil {
 		m.Options = types.ObjectNull(optionsTypes)
 		return nil
 	}
 
+	privateNetworkOnlyTF := types.BoolPointerValue(loadBalancerResp.Options.PrivateNetworkOnly)
+
+	// If the private_network_only field is nil in the response but is explicitly set to false in the model,
+	// we set it to false in the TF state to prevent an inconsistent result after apply error
+	if !m.Options.IsNull() && !m.Options.IsUnknown() {
+		optionsModel := options{}
+		diags := m.Options.As(ctx, &optionsModel, basetypes.ObjectAsOptions{})
+		if diags.HasError() {
+			return fmt.Errorf("convert options: %w", core.DiagsToError(diags))
+		}
+		if loadBalancerResp.Options.PrivateNetworkOnly == nil && !optionsModel.PrivateNetworkOnly.IsNull() && !optionsModel.PrivateNetworkOnly.IsUnknown() && !optionsModel.PrivateNetworkOnly.ValueBool() {
+			privateNetworkOnlyTF = types.BoolValue(false)
+		}
+	}
+
 	optionsMap := map[string]attr.Value{
-		"private_network_only": types.BoolPointerValue(loadBalancerResp.Options.PrivateNetworkOnly),
+		"private_network_only": privateNetworkOnlyTF,
 	}
 
 	err := mapACL(loadBalancerResp.Options.AccessControl, optionsMap)

stackit/internal/services/loadbalancer/loadbalancer/resource_test.go

@@ -270,6 +270,7 @@ func TestMapFields(t *testing.T) {
 	tests := []struct {
 		description             string
 		input                   *loadbalancer.LoadBalancer
+		modelPrivateNetworkOnly *bool
 		expected                *Model
 		isValid                 bool
 	}{
@@ -288,6 +289,7 @@ func TestMapFields(t *testing.T) {
 				},
 				TargetPools: nil,
 			},
+			nil,
 			&Model{
 				Id:              types.StringValue("pid,name"),
 				ProjectId:       types.StringValue("pid"),
@@ -304,9 +306,127 @@ func TestMapFields(t *testing.T) {
 			},
 			true,
 		},
-
 		{
 			"simple_values_ok",
+			&loadbalancer.LoadBalancer{
+				ExternalAddress: utils.Ptr("external_address"),
+				Listeners: utils.Ptr([]loadbalancer.Listener{
+					{
+						DisplayName: utils.Ptr("display_name"),
+						Port:        utils.Ptr(int64(80)),
+						Protocol:    utils.Ptr("protocol"),
+						ServerNameIndicators: &[]loadbalancer.ServerNameIndicator{
+							{
+								Name: utils.Ptr("domain.com"),
+							},
+						},
+						TargetPool: utils.Ptr("target_pool"),
+					},
+				}),
+				Name: utils.Ptr("name"),
+				Networks: utils.Ptr([]loadbalancer.Network{
+					{
+						NetworkId: utils.Ptr("network_id"),
+						Role:      utils.Ptr("role"),
+					},
+					{
+						NetworkId: utils.Ptr("network_id_2"),
+						Role:      utils.Ptr("role_2"),
+					},
+				}),
+				Options: utils.Ptr(loadbalancer.LoadBalancerOptions{
+					PrivateNetworkOnly: utils.Ptr(true),
+				}),
+				TargetPools: utils.Ptr([]loadbalancer.TargetPool{
+					{
+						ActiveHealthCheck: utils.Ptr(loadbalancer.ActiveHealthCheck{
+							HealthyThreshold:   utils.Ptr(int64(1)),
+							Interval:           utils.Ptr("2s"),
+							IntervalJitter:     utils.Ptr("3s"),
+							Timeout:            utils.Ptr("4s"),
+							UnhealthyThreshold: utils.Ptr(int64(5)),
+						}),
+						Name:       utils.Ptr("name"),
+						TargetPort: utils.Ptr(int64(80)),
+						Targets: utils.Ptr([]loadbalancer.Target{
+							{
+								DisplayName: utils.Ptr("display_name"),
+								Ip:          utils.Ptr("ip"),
+							},
+						}),
+						SessionPersistence: utils.Ptr(loadbalancer.SessionPersistence{
+							UseSourceIpAddress: utils.Ptr(true),
+						}),
+					},
+				}),
+			},
+			nil,
+			&Model{
+				Id:              types.StringValue("pid,name"),
+				ProjectId:       types.StringValue("pid"),
+				ExternalAddress: types.StringValue("external_address"),
+				Listeners: types.ListValueMust(types.ObjectType{AttrTypes: listenerTypes}, []attr.Value{
+					types.ObjectValueMust(listenerTypes, map[string]attr.Value{
+						"display_name": types.StringValue("display_name"),
+						"port":         types.Int64Value(80),
+						"protocol":     types.StringValue("protocol"),
+						"server_name_indicators": types.ListValueMust(types.ObjectType{AttrTypes: serverNameIndicatorTypes}, []attr.Value{
+							types.ObjectValueMust(
+								serverNameIndicatorTypes,
+								map[string]attr.Value{
+									"name": types.StringValue("domain.com"),
+								},
+							),
+						},
+						),
+						"target_pool": types.StringValue("target_pool"),
+					}),
+				}),
+				Name: types.StringValue("name"),
+				Networks: types.ListValueMust(types.ObjectType{AttrTypes: networkTypes}, []attr.Value{
+					types.ObjectValueMust(networkTypes, map[string]attr.Value{
+						"network_id": types.StringValue("network_id"),
+						"role":       types.StringValue("role"),
+					}),
+					types.ObjectValueMust(networkTypes, map[string]attr.Value{
+						"network_id": types.StringValue("network_id_2"),
+						"role":       types.StringValue("role_2"),
+					}),
+				}),
+				Options: types.ObjectValueMust(
+					optionsTypes,
+					map[string]attr.Value{
+						"private_network_only": types.BoolValue(true),
+						"acl":                  types.SetNull(types.StringType),
+					},
+				),
+				TargetPools: types.ListValueMust(types.ObjectType{AttrTypes: targetPoolTypes}, []attr.Value{
+					types.ObjectValueMust(targetPoolTypes, map[string]attr.Value{
+						"active_health_check": types.ObjectValueMust(activeHealthCheckTypes, map[string]attr.Value{
+							"healthy_threshold":   types.Int64Value(1),
+							"interval":            types.StringValue("2s"),
+							"interval_jitter":     types.StringValue("3s"),
+							"timeout":             types.StringValue("4s"),
+							"unhealthy_threshold": types.Int64Value(5),
+						}),
+						"name":        types.StringValue("name"),
+						"target_port": types.Int64Value(80),
+						"targets": types.ListValueMust(types.ObjectType{AttrTypes: targetTypes}, []attr.Value{
+							types.ObjectValueMust(targetTypes, map[string]attr.Value{
+								"display_name": types.StringValue("display_name"),
+								"ip":           types.StringValue("ip"),
+							}),
+						}),
+						"session_persistence": types.ObjectValueMust(sessionPersistenceTypes, map[string]attr.Value{
+							"use_source_ip_address": types.BoolValue(true),
+						}),
+					}),
+				}),
+			},
+			true,
+		},
+		{
+			"simple_values_ok_with_null_private_network_only_response",
 			&loadbalancer.LoadBalancer{
 				ExternalAddress: utils.Ptr("external_address"),
 				Listeners: utils.Ptr([]loadbalancer.Listener{
@@ -337,7 +457,7 @@ func TestMapFields(t *testing.T) {
 					AccessControl: &loadbalancer.LoadbalancerOptionAccessControl{
 						AllowedSourceRanges: utils.Ptr([]string{"cidr"}),
 					},
-					PrivateNetworkOnly: utils.Ptr(true),
+					PrivateNetworkOnly: nil, // API sets this to nil if it's false in the request
 				}),
 				TargetPools: utils.Ptr([]loadbalancer.TargetPool{
 					{
@@ -362,6 +482,7 @@ func TestMapFields(t *testing.T) {
 					},
 				}),
 			},
+			utils.Ptr(false),
 			&Model{
 				Id:              types.StringValue("pid,name"),
 				ProjectId:       types.StringValue("pid"),
@@ -400,7 +521,7 @@ func TestMapFields(t *testing.T) {
 						"acl": types.SetValueMust(
 							types.StringType,
 							[]attr.Value{types.StringValue("cidr")}),
-						"private_network_only": types.BoolValue(true),
+						"private_network_only": types.BoolValue(false),
 					},
 				),
 				TargetPools: types.ListValueMust(types.ObjectType{AttrTypes: targetPoolTypes}, []attr.Value{
@@ -431,12 +552,14 @@ func TestMapFields(t *testing.T) {
 		{
 			"nil_response",
 			nil,
+			nil,
 			&Model{},
 			false,
 		},
 		{
 			"no_name",
 			&loadbalancer.LoadBalancer{},
+			nil,
 			&Model{},
 			false,
 		},
@@ -446,7 +569,13 @@ func TestMapFields(t *testing.T) {
 			model := &Model{
 				ProjectId: tt.expected.ProjectId,
 			}
-			err := mapFields(tt.input, model)
+			if tt.modelPrivateNetworkOnly != nil {
+				model.Options = types.ObjectValueMust(optionsTypes, map[string]attr.Value{
+					"private_network_only": types.BoolValue(*tt.modelPrivateNetworkOnly),
+					"acl":                  types.SetNull(types.StringType),
+				})
+			}
+			err := mapFields(context.Background(), tt.input, model)
 			if !tt.isValid && err == nil {
 				t.Fatalf("Should have failed")
 			}