From af7d7899455de0348679c91d05cc95b18f0e788b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Palet?= Date: Tue, 23 Jul 2024 11:37:57 +0100 Subject: [PATCH] =?UTF-8?q?Adjust=20mapping=20of=20LB=20API=20response=20w?= =?UTF-8?q?hen=20private=20network=20only=20field=20is=20=E2=80=A6=20(#477?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Adjust mapping of LB API response when private network only field is null * Improve comment Co-authored-by: Diogo Ferrão --------- Co-authored-by: Diogo Ferrão --- .../loadbalancer/loadbalancer/datasource.go | 2 +- .../loadbalancer/loadbalancer/resource.go | 29 +++- .../loadbalancer/resource_test.go | 145 +++++++++++++++++- 3 files changed, 160 insertions(+), 16 deletions(-) diff --git a/stackit/internal/services/loadbalancer/loadbalancer/datasource.go b/stackit/internal/services/loadbalancer/loadbalancer/datasource.go index 89e52e47..114561fc 100644 --- a/stackit/internal/services/loadbalancer/loadbalancer/datasource.go +++ b/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 diff --git a/stackit/internal/services/loadbalancer/loadbalancer/resource.go b/stackit/internal/services/loadbalancer/loadbalancer/resource.go index 312f2e62..35bb7c85 100644 --- a/stackit/internal/services/loadbalancer/loadbalancer/resource.go +++ b/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) diff --git a/stackit/internal/services/loadbalancer/loadbalancer/resource_test.go b/stackit/internal/services/loadbalancer/loadbalancer/resource_test.go index f6cd3608..500429cc 100644 --- a/stackit/internal/services/loadbalancer/loadbalancer/resource_test.go +++ b/stackit/internal/services/loadbalancer/loadbalancer/resource_test.go @@ -268,10 +268,11 @@ func TestToTargetPoolUpdatePayload(t *testing.T) { func TestMapFields(t *testing.T) { tests := []struct { - description string - input *loadbalancer.LoadBalancer - expected *Model - isValid bool + description string + input *loadbalancer.LoadBalancer + modelPrivateNetworkOnly *bool + expected *Model + isValid bool }{ { "default_values_ok", @@ -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") }