From 28d28f17f6a914c0093e53cf9ca5bd56e0893bde Mon Sep 17 00:00:00 2001 From: Mateusz Filipowicz Date: Sun, 16 Mar 2025 23:56:32 +0100 Subject: [PATCH] feat: simplify unifi_setting_usg resource by making geo IP fitering and UPNP enabled fields only computed (#60) * feat: simplify setting_usg resource by making geo IP fitering and UPNP enabled fields only computed * rename block to mode --- .../acctest/resource_setting_usg_test.go | 42 +++--- .../provider/settings/resource_setting_usg.go | 120 ++++++++---------- 2 files changed, 68 insertions(+), 94 deletions(-) diff --git a/internal/provider/acctest/resource_setting_usg_test.go b/internal/provider/acctest/resource_setting_usg_test.go index 0397890..36d90d9 100644 --- a/internal/provider/acctest/resource_setting_usg_test.go +++ b/internal/provider/acctest/resource_setting_usg_test.go @@ -73,8 +73,8 @@ func TestAccSettingUsg_geoIpFiltering(t *testing.T) { { Config: testAccSettingUsgSite() + testAccSettingUsgConfig_geoIpFilteringBasic(), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.enabled", "true"), - resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.block", "block"), + resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering_enabled", "true"), + resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.mode", "block"), resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.traffic_direction", "both"), resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.countries.#", "3"), resource.TestCheckTypeSetElemAttr("unifi_setting_usg.test", "geo_ip_filtering.countries.*", "RU"), @@ -86,8 +86,8 @@ func TestAccSettingUsg_geoIpFiltering(t *testing.T) { { Config: testAccSettingUsgSite() + testAccSettingUsgConfig_geoIpFilteringAllow(), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.enabled", "true"), - resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.block", "allow"), + resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering_enabled", "true"), + resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.mode", "allow"), resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.traffic_direction", "both"), resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.countries.#", "3"), resource.TestCheckTypeSetElemAttr("unifi_setting_usg.test", "geo_ip_filtering.countries.*", "US"), @@ -99,8 +99,8 @@ func TestAccSettingUsg_geoIpFiltering(t *testing.T) { { Config: testAccSettingUsgSite() + testAccSettingUsgConfig_geoIpFilteringDirections(), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.enabled", "true"), - resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.block", "block"), + resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering_enabled", "true"), + resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.mode", "block"), resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.traffic_direction", "ingress"), resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.countries.#", "2"), resource.TestCheckTypeSetElemAttr("unifi_setting_usg.test", "geo_ip_filtering.countries.*", "RU"), @@ -111,15 +111,15 @@ func TestAccSettingUsg_geoIpFiltering(t *testing.T) { { Config: testAccSettingUsgSite() + testAccSettingUsgConfig_geoIpFilteringDisabled(), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.enabled", "false"), + resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering_enabled", "false"), ), }, pt.ImportStepWithSite("unifi_setting_usg.test"), { Config: testAccSettingUsgSite() + testAccSettingUsgConfig_geoIpFilteringBasic(), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.enabled", "true"), - resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.block", "block"), + resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering_enabled", "true"), + resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.mode", "block"), resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.traffic_direction", "both"), resource.TestCheckResourceAttr("unifi_setting_usg.test", "geo_ip_filtering.countries.#", "3"), resource.TestCheckTypeSetElemAttr("unifi_setting_usg.test", "geo_ip_filtering.countries.*", "RU"), @@ -139,14 +139,14 @@ func TestAccSettingUsg_upnp(t *testing.T) { { Config: testAccSettingUsgSite() + testAccSettingUsgConfig_upnpBasic(), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("unifi_setting_usg.test", "upnp.enabled", "true"), + resource.TestCheckResourceAttr("unifi_setting_usg.test", "upnp_enabled", "true"), ), }, pt.ImportStepWithSite("unifi_setting_usg.test"), { Config: testAccSettingUsgSite() + testAccSettingUsgConfig_upnpAdvanced(), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("unifi_setting_usg.test", "upnp.enabled", "true"), + resource.TestCheckResourceAttr("unifi_setting_usg.test", "upnp_enabled", "true"), resource.TestCheckResourceAttr("unifi_setting_usg.test", "upnp.nat_pmp_enabled", "true"), resource.TestCheckResourceAttr("unifi_setting_usg.test", "upnp.secure_mode", "true"), resource.TestCheckResourceAttr("unifi_setting_usg.test", "upnp.wan_interface", "WAN"), @@ -156,7 +156,7 @@ func TestAccSettingUsg_upnp(t *testing.T) { { Config: testAccSettingUsgSite() + testAccSettingUsgConfig_upnpDisabled(), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("unifi_setting_usg.test", "upnp.enabled", "false"), + resource.TestCheckResourceAttr("unifi_setting_usg.test", "upnp_enabled", "false"), ), }, pt.ImportStepWithSite("unifi_setting_usg.test"), @@ -587,7 +587,6 @@ func testAccSettingUsgConfig_geoIpFilteringBasic() string { resource "unifi_setting_usg" "test" { site = unifi_site.test.name geo_ip_filtering = { - enabled = true countries = ["RU", "CN", "KP"] } } @@ -599,8 +598,7 @@ func testAccSettingUsgConfig_geoIpFilteringAllow() string { resource "unifi_setting_usg" "test" { site = unifi_site.test.name geo_ip_filtering = { - enabled = true - block = "allow" + mode = "allow" countries = ["US", "CA", "GB"] } } @@ -612,7 +610,6 @@ func testAccSettingUsgConfig_geoIpFilteringDirections() string { resource "unifi_setting_usg" "test" { site = unifi_site.test.name geo_ip_filtering = { - enabled = true traffic_direction = "ingress" countries = ["RU", "CN"] } @@ -624,9 +621,6 @@ func testAccSettingUsgConfig_geoIpFilteringDisabled() string { return ` resource "unifi_setting_usg" "test" { site = unifi_site.test.name - geo_ip_filtering = { - enabled = false - } } ` } @@ -636,7 +630,6 @@ func testAccSettingUsgConfig_upnpBasic() string { resource "unifi_setting_usg" "test" { site = unifi_site.test.name upnp = { - enabled = true } } ` @@ -647,7 +640,6 @@ func testAccSettingUsgConfig_upnpAdvanced() string { resource "unifi_setting_usg" "test" { site = unifi_site.test.name upnp = { - enabled = true nat_pmp_enabled = true secure_mode = true wan_interface = "WAN" @@ -660,9 +652,6 @@ func testAccSettingUsgConfig_upnpDisabled() string { return ` resource "unifi_setting_usg" "test" { site = unifi_site.test.name - upnp = { - enabled = false - } } ` } @@ -933,15 +922,13 @@ resource "unifi_setting_usg" "test" { // Geo IP Filtering geo_ip_filtering = { - enabled = true - block = "block" + mode = "block" countries = ["RU", "CN"] traffic_direction = "both" } // UPNP Settings upnp = { - enabled = true nat_pmp_enabled = true secure_mode = true } @@ -1050,6 +1037,7 @@ resource "unifi_setting_usg" "test" { func testAccSettingUsgConfig_unbindWanMonitor(enabled bool) string { return fmt.Sprintf(` resource "unifi_setting_usg" "test" { + site = unifi_site.test.name unbind_wan_monitors = %t } `, enabled) diff --git a/internal/provider/settings/resource_setting_usg.go b/internal/provider/settings/resource_setting_usg.go index eb280d8..6361021 100644 --- a/internal/provider/settings/resource_setting_usg.go +++ b/internal/provider/settings/resource_setting_usg.go @@ -30,16 +30,14 @@ import ( // GeoIPFilteringModel represents the GeoIP filtering configuration type GeoIPFilteringModel struct { - Enabled types.Bool `tfsdk:"enabled"` - Block types.String `tfsdk:"block"` + Mode types.String `tfsdk:"mode"` Countries types.List `tfsdk:"countries"` TrafficDirection types.String `tfsdk:"traffic_direction"` } func (m *GeoIPFilteringModel) AttributeTypes() map[string]attr.Type { return map[string]attr.Type{ - "enabled": types.BoolType, - "block": types.StringType, + "mode": types.StringType, "countries": types.ListType{ ElemType: types.StringType, }, @@ -49,7 +47,6 @@ func (m *GeoIPFilteringModel) AttributeTypes() map[string]attr.Type { // UpnpModel represents the UPNP configuration type UpnpModel struct { - Enabled types.Bool `tfsdk:"enabled"` NatPmpEnabled types.Bool `tfsdk:"nat_pmp_enabled"` SecureMode types.Bool `tfsdk:"secure_mode"` WANInterface types.String `tfsdk:"wan_interface"` @@ -57,7 +54,6 @@ type UpnpModel struct { func (m *UpnpModel) AttributeTypes() map[string]attr.Type { return map[string]attr.Type{ - "enabled": types.BoolType, "nat_pmp_enabled": types.BoolType, "secure_mode": types.BoolType, "wan_interface": types.StringType, @@ -130,10 +126,12 @@ type usgModel struct { MulticastDnsEnabled types.Bool `tfsdk:"multicast_dns_enabled"` // Geo IP filtering - GeoIPFiltering types.Object `tfsdk:"geo_ip_filtering"` + GeoIPFilteringEnabled types.Bool `tfsdk:"geo_ip_filtering_enabled"` + GeoIPFiltering types.Object `tfsdk:"geo_ip_filtering"` // UPNP configuration - Upnp types.Object `tfsdk:"upnp"` + UpnpEnabled types.Bool `tfsdk:"upnp_enabled"` + Upnp types.Object `tfsdk:"upnp"` // ARP Cache Configuration ArpCacheBaseReachable types.Int64 `tfsdk:"arp_cache_base_reachable"` @@ -242,13 +240,13 @@ func (d *usgModel) AsUnifiModel(ctx context.Context) (interface{}, diag.Diagnost return nil, diags } - model.GeoIPFilteringEnabled = geoIPFiltering.Enabled.ValueBool() - model.GeoIPFilteringBlock = geoIPFiltering.Block.ValueString() + model.GeoIPFilteringBlock = geoIPFiltering.Mode.ValueString() model.GeoIPFilteringTrafficDirection = geoIPFiltering.TrafficDirection.ValueString() countries, diags := utils.ListElementsToString(ctx, geoIPFiltering.Countries) if diags.HasError() { return nil, diags } + model.GeoIPFilteringEnabled = true model.GeoIPFilteringCountries = countries } else { model.GeoIPFilteringEnabled = false @@ -262,7 +260,7 @@ func (d *usgModel) AsUnifiModel(ctx context.Context) (interface{}, diag.Diagnost return nil, diags } - model.UpnpEnabled = upnp.Enabled.ValueBool() + model.UpnpEnabled = true model.UpnpNATPmpEnabled = upnp.NatPmpEnabled.ValueBool() model.UpnpSecureMode = upnp.SecureMode.ValueBool() model.UpnpWANInterface = upnp.WANInterface.ValueString() @@ -360,39 +358,45 @@ func (d *usgModel) Merge(ctx context.Context, other interface{}) diag.Diagnostic d.MulticastDnsEnabled = types.BoolValue(model.MdnsEnabled) // Set Geo IP filtering attributes - geoIPFiltering := &GeoIPFilteringModel{ - Enabled: types.BoolValue(model.GeoIPFilteringEnabled), - Block: types.StringValue(model.GeoIPFilteringBlock), - TrafficDirection: types.StringValue(model.GeoIPFilteringTrafficDirection), + d.GeoIPFilteringEnabled = types.BoolValue(model.GeoIPFilteringEnabled) + if model.GeoIPFilteringEnabled { + geoIPFiltering := &GeoIPFilteringModel{ + Mode: types.StringValue(model.GeoIPFilteringBlock), + TrafficDirection: types.StringValue(model.GeoIPFilteringTrafficDirection), + } + + countries, diags := utils.StringToListElements(ctx, model.GeoIPFilteringCountries) + if diags.HasError() { + return diags + } + geoIPFiltering.Countries = countries + + geoIPObject, diags := types.ObjectValueFrom(ctx, geoIPFiltering.AttributeTypes(), geoIPFiltering) + if diags.HasError() { + return diags + } + d.GeoIPFiltering = geoIPObject + } else { + d.GeoIPFiltering = types.ObjectNull((&GeoIPFilteringModel{}).AttributeTypes()) } - countries, diags := utils.StringToListElements(ctx, model.GeoIPFilteringCountries) - if diags.HasError() { - return diags - } - geoIPFiltering.Countries = countries - - // Create object value from attributes - geoIPObject, diags := types.ObjectValueFrom(ctx, geoIPFiltering.AttributeTypes(), geoIPFiltering) - if diags.HasError() { - return diags - } - d.GeoIPFiltering = geoIPObject - + d.UpnpEnabled = types.BoolValue(model.UpnpEnabled) // Set UPNP attributes - upnp := &UpnpModel{ - Enabled: types.BoolValue(model.UpnpEnabled), - NatPmpEnabled: types.BoolValue(model.UpnpNATPmpEnabled), - SecureMode: types.BoolValue(model.UpnpSecureMode), - WANInterface: types.StringValue(model.UpnpWANInterface), - } + if model.UpnpEnabled { + upnp := &UpnpModel{ + NatPmpEnabled: types.BoolValue(model.UpnpNATPmpEnabled), + SecureMode: types.BoolValue(model.UpnpSecureMode), + WANInterface: types.StringValue(model.UpnpWANInterface), + } - // Create object value from attributes - upnpObject, diags := types.ObjectValueFrom(ctx, upnp.AttributeTypes(), upnp) - if diags.HasError() { - return diags + upnpObject, diags := types.ObjectValueFrom(ctx, upnp.AttributeTypes(), upnp) + if diags.HasError() { + return diags + } + d.Upnp = upnpObject + } else { + d.Upnp = types.ObjectNull((&UpnpModel{}).AttributeTypes()) } - d.Upnp = upnpObject // Convert DNS Verification settings dnsVerificationModel := DNSVerificationModel{ @@ -601,26 +605,17 @@ func (r *usgResource) Schema(_ context.Context, _ resource.SchemaRequest, resp * }, }, }, + "geo_ip_filtering_enabled": schema.BoolAttribute{ + MarkdownDescription: "Whether Geo IP Filtering is enabled. When enabled, the gateway will apply the specified country-based ", + Computed: true, + }, "geo_ip_filtering": schema.SingleNestedAttribute{ MarkdownDescription: "Geographic IP filtering configuration that allows blocking or allowing traffic based on country of origin. " + "This feature uses IP geolocation databases to identify the country associated with IP addresses and apply filtering rules. " + "Useful for implementing country-specific access policies or blocking traffic from high-risk regions. Requires controller version 7.0 or later.", Optional: true, - Computed: true, - PlanModifiers: []planmodifier.Object{ - objectplanmodifier.UseStateForUnknown(), - }, - Validators: []validator.Object{ - validators.RequiredTogetherIf(path.MatchRoot("enabled"), types.BoolValue(true), path.MatchRoot("countries")), - }, Attributes: map[string]schema.Attribute{ - "enabled": schema.BoolAttribute{ - MarkdownDescription: "Enable geographic IP filtering. When enabled, traffic from specified countries will be blocked or allowed " + - "according to the configured rules. When set to `true`, you must also specify the `countries` list. " + - "Setting this to `false` disables all country-based filtering regardless of other settings.", - Required: true, - }, - "block": schema.StringAttribute{ + "mode": schema.StringAttribute{ MarkdownDescription: "Specifies whether the selected countries should be blocked or allowed. Valid values are:\n" + " * `block` (default) - Traffic from the specified countries will be blocked, while traffic from all other countries will be allowed\n" + " * `allow` - Only traffic from the specified countries will be allowed, while traffic from all other countries will be blocked\n\n" + @@ -643,11 +638,8 @@ func (r *usgResource) Schema(_ context.Context, _ resource.SchemaRequest, resp * " * `['US', 'CA', 'MX']` - United States, Canada, and Mexico\n" + " * `['CN', 'RU', 'IR']` - China, Russia, and Iran\n" + " * `['GB', 'DE', 'FR']` - United Kingdom, Germany, and France", - Optional: true, + Required: true, ElementType: types.StringType, - PlanModifiers: []planmodifier.List{ - listplanmodifier.UseStateForUnknown(), - }, Validators: []validator.List{ listvalidator.SizeAtLeast(1), listvalidator.ValueStringsAre(validators.CountryCodeAlpha2()), @@ -672,22 +664,16 @@ func (r *usgResource) Schema(_ context.Context, _ resource.SchemaRequest, resp * }, }, }, + "upnp_enabled": schema.BoolAttribute{ + MarkdownDescription: "Whether UPNP is enabled. When enabled, the gateway will automatically forward ports for UPNP-compatible devices ", + Computed: true, + }, "upnp": schema.SingleNestedAttribute{ MarkdownDescription: "UPNP (Universal Plug and Play) configuration settings. UPNP allows compatible applications and devices to automatically " + "configure port forwarding rules on the gateway without manual intervention. This is commonly used by gaming consoles, " + "media servers, VoIP applications, and other network services that require incoming connections.", Optional: true, - Computed: true, - PlanModifiers: []planmodifier.Object{ - objectplanmodifier.UseStateForUnknown(), - }, Attributes: map[string]schema.Attribute{ - "enabled": schema.BoolAttribute{ - MarkdownDescription: "Enable UPNP functionality. When enabled, applications and devices on the local network can automatically " + - "request port forwarding rules from the gateway without manual configuration. This simplifies the use of applications " + - "that require inbound connections, but may present security risks if not properly configured with `secure_mode`.", - Required: true, - }, "nat_pmp_enabled": schema.BoolAttribute{ MarkdownDescription: "Enable NAT-PMP (NAT Port Mapping Protocol) support alongside UPNP. NAT-PMP is " + "Apple's alternative to UPNP, providing similar automatic port mapping capabilities. When enabled, Apple devices " +