Skip to content

Commit 4bedbd6

Browse files
authored
Merge pull request #176 from OpenVPN/fix/settings-import-incomplete-state
Fix/settings import incomplete state
2 parents 6ba7098 + 21ae30b commit 4bedbd6

File tree

3 files changed

+161
-98
lines changed

3 files changed

+161
-98
lines changed

cloudconnexa/data_source_settings.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func dataSourceSettings() *schema.Resource {
7878
"domain_routing_subnet": {
7979
Type: schema.TypeList,
8080
Computed: true,
81-
Elem: domainRoutingSubnet(),
81+
Elem: domainRoutingSubnetSchema(),
8282
},
8383
"snat": {
8484
Type: schema.TypeBool,
@@ -93,6 +93,16 @@ func dataSourceSettings() *schema.Resource {
9393
Type: schema.TypeString,
9494
Computed: true,
9595
},
96+
"dns_log_enabled": {
97+
Type: schema.TypeBool,
98+
Computed: true,
99+
Description: "Whether DNS logging is enabled.",
100+
},
101+
"access_visibility_enabled": {
102+
Type: schema.TypeBool,
103+
Computed: true,
104+
Description: "Whether access visibility is enabled.",
105+
},
96106
},
97107
}
98108
}

cloudconnexa/resource_settings.go

Lines changed: 74 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func resourceSettings() *schema.Resource {
9696
Type: schema.TypeList,
9797
MaxItems: 1,
9898
Optional: true,
99-
Elem: domainRoutingSubnet(),
99+
Elem: domainRoutingSubnetSchema(),
100100
},
101101
"snat": {
102102
Type: schema.TypeBool,
@@ -163,8 +163,8 @@ func dnsZoneSchema() *schema.Resource {
163163
}
164164
}
165165

166-
// domainRoutingSubnet returns a Terraform schema for domain routing subnet configuration
167-
func domainRoutingSubnet() *schema.Resource {
166+
// domainRoutingSubnetSchema returns a Terraform schema for domain routing subnet configuration
167+
func domainRoutingSubnetSchema() *schema.Resource {
168168
return &schema.Resource{
169169
Schema: map[string]*schema.Schema{
170170
"ip_v4_address": {
@@ -383,6 +383,7 @@ func resourceSettingsUpdate(ctx context.Context, d *schema.ResourceData, m inter
383383

384384
// resourceSettingsRead reads the settings resource from the CloudConnexa API
385385
// and updates the Terraform state with the current values.
386+
// All API calls are unconditional to ensure proper import functionality.
386387
func resourceSettingsRead(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
387388
c := m.(*cloudconnexa.Client)
388389
var diags diag.Diagnostics
@@ -393,38 +394,40 @@ func resourceSettingsRead(ctx context.Context, d *schema.ResourceData, m interfa
393394
}
394395
d.Set("allow_trusted_devices", allowTrustedDevices)
395396

396-
if len(d.Get("dns_servers").([]interface{})) > 0 {
397-
dnsServers, err := c.Settings.GetDNSServers()
398-
if err != nil {
399-
return append(diags, diag.FromErr(err)...)
400-
}
401-
if dnsServers != nil && (dnsServers.PrimaryIPV4 != "" || dnsServers.SecondaryIPV4 != "") {
402-
value := make(map[string]interface{})
403-
value["primary_ip_v4"] = dnsServers.PrimaryIPV4
404-
value["secondary_ip_v4"] = dnsServers.SecondaryIPV4
405-
d.Set("dns_servers", []interface{}{value})
406-
}
397+
twoFactorAuth, err := c.Settings.GetTwoFactorAuthEnabled()
398+
if err != nil {
399+
return append(diags, diag.FromErr(err)...)
407400
}
401+
d.Set("two_factor_auth", twoFactorAuth)
408402

409-
if d.Get("default_dns_suffix") != "" {
410-
defaultDNSSuffix, err := c.Settings.GetDefaultDNSSuffix()
411-
if err != nil {
412-
return append(diags, diag.FromErr(err)...)
413-
}
414-
d.Set("default_dns_suffix", defaultDNSSuffix)
403+
dnsServers, err := c.Settings.GetDNSServers()
404+
if err != nil {
405+
return append(diags, diag.FromErr(err)...)
406+
}
407+
if dnsServers != nil && (dnsServers.PrimaryIPV4 != "" || dnsServers.SecondaryIPV4 != "") {
408+
value := make(map[string]interface{})
409+
value["primary_ip_v4"] = dnsServers.PrimaryIPV4
410+
value["secondary_ip_v4"] = dnsServers.SecondaryIPV4
411+
d.Set("dns_servers", []interface{}{value})
412+
}
413+
414+
defaultDNSSuffix, err := c.Settings.GetDefaultDNSSuffix()
415+
if err != nil {
416+
return append(diags, diag.FromErr(err)...)
415417
}
418+
d.Set("default_dns_suffix", defaultDNSSuffix)
416419

417420
dnsProxyEnabled, err := c.Settings.GetDNSProxyEnabled()
418421
if err != nil {
419422
return append(diags, diag.FromErr(err)...)
420423
}
421424
d.Set("dns_proxy_enabled", dnsProxyEnabled)
422425

423-
if len(d.Get("dns_zones").([]interface{})) > 0 {
424-
dnsZones, err := c.Settings.GetDNSZones()
425-
if err != nil {
426-
return append(diags, diag.FromErr(err)...)
427-
}
426+
dnsZones, err := c.Settings.GetDNSZones()
427+
if err != nil {
428+
return append(diags, diag.FromErr(err)...)
429+
}
430+
if len(dnsZones) > 0 {
428431
var zoneValues []interface{}
429432
for _, zone := range dnsZones {
430433
value := make(map[string]interface{})
@@ -435,120 +438,94 @@ func resourceSettingsRead(ctx context.Context, d *schema.ResourceData, m interfa
435438
d.Set("dns_zones", zoneValues)
436439
}
437440

438-
if d.Get("connect_auth") != "" {
439-
connectAuth, err := c.Settings.GetDefaultConnectAuth()
440-
if err != nil {
441-
return append(diags, diag.FromErr(err)...)
442-
}
443-
d.Set("connect_auth", connectAuth)
441+
connectAuth, err := c.Settings.GetDefaultConnectAuth()
442+
if err != nil {
443+
return append(diags, diag.FromErr(err)...)
444444
}
445+
d.Set("connect_auth", connectAuth)
445446

446-
if d.Get("device_allowance_per_user") != 0 {
447-
deviceAllowance, err := c.Settings.GetDefaultDeviceAllowancePerUser()
448-
if err != nil {
449-
return append(diags, diag.FromErr(err)...)
450-
}
451-
d.Set("device_allowance_per_user", deviceAllowance)
447+
deviceAllowance, err := c.Settings.GetDefaultDeviceAllowancePerUser()
448+
if err != nil {
449+
return append(diags, diag.FromErr(err)...)
452450
}
451+
d.Set("device_allowance_per_user", deviceAllowance)
453452

454453
deviceAllowanceForceUpdate, err := c.Settings.GetForceUpdateDeviceAllowanceEnabled()
455454
if err != nil {
456455
return append(diags, diag.FromErr(err)...)
457456
}
458457
d.Set("device_allowance_force_update", deviceAllowanceForceUpdate)
459458

460-
if d.Get("device_enforcement") != "" {
461-
deviceEnforcement, err := c.Settings.GetDeviceEnforcement()
462-
if err != nil {
463-
return append(diags, diag.FromErr(err)...)
464-
}
465-
d.Set("device_enforcement", deviceEnforcement)
459+
deviceEnforcement, err := c.Settings.GetDeviceEnforcement()
460+
if err != nil {
461+
return append(diags, diag.FromErr(err)...)
466462
}
463+
d.Set("device_enforcement", deviceEnforcement)
467464

468-
if d.Get("profile_distribution") != "" {
469-
profileDistribution, err := c.Settings.GetProfileDistribution()
470-
if err != nil {
471-
return append(diags, diag.FromErr(err)...)
472-
}
473-
d.Set("profile_distribution", profileDistribution)
465+
profileDistribution, err := c.Settings.GetProfileDistribution()
466+
if err != nil {
467+
return append(diags, diag.FromErr(err)...)
474468
}
469+
d.Set("profile_distribution", profileDistribution)
475470

476-
if d.Get("connection_timeout") != 0 {
477-
connectionTimeout, err := c.Settings.GetConnectionTimeout()
478-
if err != nil {
479-
return append(diags, diag.FromErr(err)...)
480-
}
481-
d.Set("connection_timeout", connectionTimeout)
471+
connectionTimeout, err := c.Settings.GetConnectionTimeout()
472+
if err != nil {
473+
return append(diags, diag.FromErr(err)...)
482474
}
475+
d.Set("connection_timeout", connectionTimeout)
483476

484-
// Get and set client options if they exist
485-
if len(d.Get("client_options").([]interface{})) > 0 {
486-
clientOptions, err := c.Settings.GetClientOptions()
487-
if err != nil {
488-
return append(diags, diag.FromErr(err)...)
489-
}
490-
d.Set("client_options", clientOptions)
477+
clientOptions, err := c.Settings.GetClientOptions()
478+
if err != nil {
479+
return append(diags, diag.FromErr(err)...)
491480
}
481+
d.Set("client_options", clientOptions)
492482

493-
// Get and set default region if specified
494-
if d.Get("default_region") != "" {
495-
defaultRegion, err := c.Settings.GetDefaultRegion()
496-
if err != nil {
497-
return append(diags, diag.FromErr(err)...)
498-
}
499-
d.Set("default_region", defaultRegion)
483+
defaultRegion, err := c.Settings.GetDefaultRegion()
484+
if err != nil {
485+
return append(diags, diag.FromErr(err)...)
500486
}
487+
d.Set("default_region", defaultRegion)
501488

502-
// Get and set domain routing subnet if specified
503-
if len(d.Get("domain_routing_subnet").([]interface{})) > 0 {
504-
domainRoutingSubnet, err := c.Settings.GetDomainRoutingSubnet()
505-
if err != nil {
506-
return append(diags, diag.FromErr(err)...)
507-
}
508-
if domainRoutingSubnet.IPV4Address != "" || domainRoutingSubnet.IPV6Address != "" {
509-
value := make(map[string]interface{})
510-
value["ip_v4_address"] = domainRoutingSubnet.IPV4Address
511-
value["ip_v6_address"] = domainRoutingSubnet.IPV6Address
512-
d.Set("domain_routing_subnet", []interface{}{value})
513-
}
489+
domainRoutingSubnet, err := c.Settings.GetDomainRoutingSubnet()
490+
if err != nil {
491+
return append(diags, diag.FromErr(err)...)
492+
}
493+
if domainRoutingSubnet != nil && (domainRoutingSubnet.IPV4Address != "" || domainRoutingSubnet.IPV6Address != "") {
494+
value := make(map[string]interface{})
495+
value["ip_v4_address"] = domainRoutingSubnet.IPV4Address
496+
value["ip_v6_address"] = domainRoutingSubnet.IPV6Address
497+
d.Set("domain_routing_subnet", []interface{}{value})
514498
}
515499

516-
// Get and set SNAT enabled status
517500
snat, err := c.Settings.GetSnatEnabled()
518501
if err != nil {
519502
return append(diags, diag.FromErr(err)...)
520503
}
521504
d.Set("snat", snat)
522505

523-
// Get and set subnet configuration if specified
524-
if len(d.Get("subnet").([]interface{})) > 0 {
525-
subnet, err := c.Settings.GetSubnet()
526-
if err != nil {
527-
return append(diags, diag.FromErr(err)...)
528-
}
506+
subnet, err := c.Settings.GetSubnet()
507+
if err != nil {
508+
return append(diags, diag.FromErr(err)...)
509+
}
510+
if subnet != nil && (len(subnet.IPV4Address) > 0 || len(subnet.IPV6Address) > 0) {
529511
subnetValue := make(map[string]interface{})
530512
subnetValue["ip_v4_address"] = subnet.IPV4Address
531513
subnetValue["ip_v6_address"] = subnet.IPV6Address
532514
d.Set("subnet", []interface{}{subnetValue})
533515
}
534516

535-
// Get and set topology if specified
536-
if d.Get("topology") != "" {
537-
topology, err := c.Settings.GetTopology()
538-
if err != nil {
539-
return append(diags, diag.FromErr(err)...)
540-
}
541-
d.Set("topology", topology)
517+
topology, err := c.Settings.GetTopology()
518+
if err != nil {
519+
return append(diags, diag.FromErr(err)...)
542520
}
521+
d.Set("topology", topology)
543522

544-
// Get and set DNS Log enabled status
545523
dnsLog, err := c.Settings.GetDNSLogEnabled()
546524
if err != nil {
547525
return append(diags, diag.FromErr(err)...)
548526
}
549527
d.Set("dns_log_enabled", dnsLog)
550528

551-
// Get and set Access Visibility enabled status
552529
accessVisibility, err := c.Settings.GetAccessVisibilityEnabled()
553530
if err != nil {
554531
return append(diags, diag.FromErr(err)...)
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package cloudconnexa
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
8+
)
9+
10+
// TestAccCloudConnexaSettings_import tests that importing the settings resource
11+
// properly populates all attributes from the API.
12+
func TestAccCloudConnexaSettings_import(t *testing.T) {
13+
rn := "cloudconnexa_settings.test"
14+
15+
resource.Test(t, resource.TestCase{
16+
PreCheck: func() { testAccPreCheck(t) },
17+
ProviderFactories: testAccProviderFactories,
18+
Steps: []resource.TestStep{
19+
{
20+
Config: testAccCloudConnexaSettingsConfig(),
21+
},
22+
{
23+
ResourceName: rn,
24+
ImportState: true,
25+
ImportStateId: "settings",
26+
ImportStateVerify: true,
27+
},
28+
},
29+
})
30+
}
31+
32+
// TestAccCloudConnexaSettings_basic tests that the settings resource can be created
33+
// and read back with the correct values.
34+
func TestAccCloudConnexaSettings_basic(t *testing.T) {
35+
rn := "cloudconnexa_settings.test"
36+
37+
resource.Test(t, resource.TestCase{
38+
PreCheck: func() { testAccPreCheck(t) },
39+
ProviderFactories: testAccProviderFactories,
40+
Steps: []resource.TestStep{
41+
{
42+
Config: testAccCloudConnexaSettingsConfig(),
43+
Check: resource.ComposeTestCheckFunc(
44+
resource.TestCheckResourceAttr(rn, "id", "settings"),
45+
resource.TestCheckResourceAttrSet(rn, "allow_trusted_devices"),
46+
resource.TestCheckResourceAttrSet(rn, "two_factor_auth"),
47+
resource.TestCheckResourceAttrSet(rn, "dns_proxy_enabled"),
48+
resource.TestCheckResourceAttrSet(rn, "device_allowance_force_update"),
49+
resource.TestCheckResourceAttrSet(rn, "snat"),
50+
resource.TestCheckResourceAttrSet(rn, "dns_log_enabled"),
51+
resource.TestCheckResourceAttrSet(rn, "access_visibility_enabled"),
52+
resource.TestCheckResourceAttrSet(rn, "connect_auth"),
53+
resource.TestCheckResourceAttrSet(rn, "device_allowance_per_user"),
54+
resource.TestCheckResourceAttrSet(rn, "device_enforcement"),
55+
resource.TestCheckResourceAttrSet(rn, "profile_distribution"),
56+
resource.TestCheckResourceAttrSet(rn, "connection_timeout"),
57+
resource.TestCheckResourceAttrSet(rn, "default_region"),
58+
resource.TestCheckResourceAttrSet(rn, "topology"),
59+
),
60+
},
61+
},
62+
})
63+
}
64+
65+
// testAccCloudConnexaSettingsConfig generates a minimal Terraform configuration
66+
// for the settings resource that relies on reading existing settings from the API.
67+
func testAccCloudConnexaSettingsConfig() string {
68+
return fmt.Sprintf(`
69+
provider "cloudconnexa" {
70+
base_url = "https://%s.api.openvpn.com"
71+
}
72+
73+
resource "cloudconnexa_settings" "test" {
74+
}
75+
`, testCloudID)
76+
}

0 commit comments

Comments
 (0)