From b5f9c7ce768e663149ddd9439d31e5bcc98b9bf5 Mon Sep 17 00:00:00 2001 From: Freddie Rice Date: Tue, 26 Jan 2021 10:56:10 -0600 Subject: [PATCH 1/4] cloudflare_access_rule: add extra validation to break bad subnets during plan vs apply --- cloudflare/resource_cloudflare_access_rule.go | 45 +++++++++++++++++++ .../resource_cloudflare_access_rule_test.go | 26 +++++++++++ 2 files changed, 71 insertions(+) diff --git a/cloudflare/resource_cloudflare_access_rule.go b/cloudflare/resource_cloudflare_access_rule.go index 83eaccd405..9a3faf9683 100644 --- a/cloudflare/resource_cloudflare_access_rule.go +++ b/cloudflare/resource_cloudflare_access_rule.go @@ -3,6 +3,7 @@ package cloudflare import ( "fmt" "log" + "net" "strings" cloudflare "github.com/cloudflare/cloudflare-go" @@ -258,3 +259,47 @@ func configurationDiffSuppress(k, old, new string, d *schema.ResourceData) bool return false } + +func validateAccessRuleConfiguration(v interface{}, k string) (warnings []string, errors []error) { + config := v.(map[string]interface{}) + + target := config["target"].(string) + value := config["value"].(string) + + switch target { + case "ip_range": + return validateAccessRuleConfigurationIPRange(value) + default: + } + + return warnings, errors +} + +func validateAccessRuleConfigurationIPRange(v string) (warnings []string, errors []error) { + ip, ipNet, err := net.ParseCIDR(v) + if err != nil { + errors = append(errors, fmt.Errorf("failed to parse value as CIDR: %v", err)) + return warnings, errors + } + + if ipNet == nil { + errors = append(errors, fmt.Errorf("ip_range must hold a range")) + return warnings, errors + } + + if ip.To4() != nil { + ones, _ := ipNet.Mask.Size() + if ones != 24 && ones != 32 { + errors = append(errors, fmt.Errorf("ip_range with ipv4 address must be a /24 or /32, got a /%d", ones)) + return warnings, errors + } + } else { + ones, _ := ipNet.Mask.Size() + if ones != 32 && ones != 48 && ones != 64 { + errors = append(errors, fmt.Errorf("ip_range with ipv4 address must be in (/32, /48, /64), instead got a /%d", ones)) + return warnings, errors + } + } + + return warnings, errors +} diff --git a/cloudflare/resource_cloudflare_access_rule_test.go b/cloudflare/resource_cloudflare_access_rule_test.go index b8b952114e..02b2ad4af9 100644 --- a/cloudflare/resource_cloudflare_access_rule_test.go +++ b/cloudflare/resource_cloudflare_access_rule_test.go @@ -38,3 +38,29 @@ resource "cloudflare_access_rule" "test" { } }`, mode, notes, target, value) } + +func TestValidateAccessRuleConfigurationIPRange(t *testing.T) { + ipRangeValid := map[string]bool{ + "192.168.0.1/32": true, + "192.168.0.1/24": true, + "192.168.0.1/64": false, + "192.168.0.1/31": false, + "192.168.0.1/16": false, + "fd82:0f75:cf0d:d7b3::/64": true, + "fd82:0f75:cf0d:d7b3::/48": true, + "fd82:0f75:cf0d:d7b3::/32": true, + "fd82:0f75:cf0d:d7b3::/63": false, + "fd82:0f75:cf0d:d7b3::/16": false, + } + + for ipRange, valid := range ipRangeValid { + warnings, errors := validateAccessRuleConfigurationIPRange(ipRange) + isValid := len(errors) == 0 + if len(warnings) != 0 { + t.Fatalf("ipRange is either invalid or valid, no room for warnings") + } + if isValid != valid { + t.Fatalf("%s resulted in %v, expected %v", ipRange, isValid, valid) + } + } +} From edb518c375d14e473fea79025f6e8cd60d4aaf33 Mon Sep 17 00:00:00 2001 From: Freddie Rice Date: Tue, 26 Jan 2021 10:57:32 -0600 Subject: [PATCH 2/4] cloudflare_access_rule: add extra validation to break bad subnets during plan vs apply --- cloudflare/resource_cloudflare_access_rule.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cloudflare/resource_cloudflare_access_rule.go b/cloudflare/resource_cloudflare_access_rule.go index 9a3faf9683..910c84d7ff 100644 --- a/cloudflare/resource_cloudflare_access_rule.go +++ b/cloudflare/resource_cloudflare_access_rule.go @@ -42,6 +42,7 @@ func resourceCloudflareAccessRule() *schema.Resource { Required: true, ForceNew: true, DiffSuppressFunc: configurationDiffSuppress, + ValidateFunc: validateAccessRuleConfiguration, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "target": { From 01d330ef7f196edccc9b1c97da6e411a4ea89d1f Mon Sep 17 00:00:00 2001 From: Freddie Rice Date: Tue, 26 Jan 2021 20:47:00 -0600 Subject: [PATCH 3/4] cloudflare_access_rule: fix ipv4 rules --- cloudflare/resource_cloudflare_access_rule.go | 2 +- cloudflare/resource_cloudflare_access_rule_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cloudflare/resource_cloudflare_access_rule.go b/cloudflare/resource_cloudflare_access_rule.go index 910c84d7ff..a25da386b4 100644 --- a/cloudflare/resource_cloudflare_access_rule.go +++ b/cloudflare/resource_cloudflare_access_rule.go @@ -290,7 +290,7 @@ func validateAccessRuleConfigurationIPRange(v string) (warnings []string, errors if ip.To4() != nil { ones, _ := ipNet.Mask.Size() - if ones != 24 && ones != 32 { + if ones != 16 && ones != 24 { errors = append(errors, fmt.Errorf("ip_range with ipv4 address must be a /24 or /32, got a /%d", ones)) return warnings, errors } diff --git a/cloudflare/resource_cloudflare_access_rule_test.go b/cloudflare/resource_cloudflare_access_rule_test.go index 02b2ad4af9..0be2b166c2 100644 --- a/cloudflare/resource_cloudflare_access_rule_test.go +++ b/cloudflare/resource_cloudflare_access_rule_test.go @@ -41,11 +41,11 @@ resource "cloudflare_access_rule" "test" { func TestValidateAccessRuleConfigurationIPRange(t *testing.T) { ipRangeValid := map[string]bool{ - "192.168.0.1/32": true, + "192.168.0.1/32": false, "192.168.0.1/24": true, "192.168.0.1/64": false, "192.168.0.1/31": false, - "192.168.0.1/16": false, + "192.168.0.1/16": true, "fd82:0f75:cf0d:d7b3::/64": true, "fd82:0f75:cf0d:d7b3::/48": true, "fd82:0f75:cf0d:d7b3::/32": true, From 8614a7a12566a00bd9472aeb6d7133d5c85245c7 Mon Sep 17 00:00:00 2001 From: Freddie Rice Date: Tue, 26 Jan 2021 22:15:09 -0600 Subject: [PATCH 4/4] cloudflare_access_rule: add acc test and fix ipv6 error name --- cloudflare/resource_cloudflare_access_rule.go | 4 ++-- .../resource_cloudflare_access_rule_test.go | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/cloudflare/resource_cloudflare_access_rule.go b/cloudflare/resource_cloudflare_access_rule.go index a25da386b4..813fcd15e3 100644 --- a/cloudflare/resource_cloudflare_access_rule.go +++ b/cloudflare/resource_cloudflare_access_rule.go @@ -291,13 +291,13 @@ func validateAccessRuleConfigurationIPRange(v string) (warnings []string, errors if ip.To4() != nil { ones, _ := ipNet.Mask.Size() if ones != 16 && ones != 24 { - errors = append(errors, fmt.Errorf("ip_range with ipv4 address must be a /24 or /32, got a /%d", ones)) + errors = append(errors, fmt.Errorf("ip_range with ipv4 address must be a /16 or /24, got a /%d", ones)) return warnings, errors } } else { ones, _ := ipNet.Mask.Size() if ones != 32 && ones != 48 && ones != 64 { - errors = append(errors, fmt.Errorf("ip_range with ipv4 address must be in (/32, /48, /64), instead got a /%d", ones)) + errors = append(errors, fmt.Errorf("ip_range with ipv6 address must be in (/32, /48, /64), instead got a /%d", ones)) return warnings, errors } } diff --git a/cloudflare/resource_cloudflare_access_rule_test.go b/cloudflare/resource_cloudflare_access_rule_test.go index 0be2b166c2..d41b10c385 100644 --- a/cloudflare/resource_cloudflare_access_rule_test.go +++ b/cloudflare/resource_cloudflare_access_rule_test.go @@ -2,6 +2,7 @@ package cloudflare import ( "fmt" + "regexp" "testing" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" @@ -27,6 +28,29 @@ func TestAccAccessRuleASN(t *testing.T) { }) } +func TestAccAccessRuleIPRange(t *testing.T) { + name := "cloudflare_access_rule.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccessRuleAccountConfig("challenge", "this is notes", "ip_range", "104.16.0.0/20"), + ExpectError: regexp.MustCompile("ip_range with ipv4 address must be a /16 or /24, got a /20"), + }, { + Config: testAccessRuleAccountConfig("challenge", "this is notes", "ip_range", "104.16.0.0/24"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(name, "notes", "this is notes"), + resource.TestCheckResourceAttr(name, "mode", "challenge"), + resource.TestCheckResourceAttr(name, "configuration.target", "ip_range"), + resource.TestCheckResourceAttr(name, "configuration.value", "104.16.0.0/24"), + ), + }, + }, + }) +} + func testAccessRuleAccountConfig(mode, notes, target, value string) string { return fmt.Sprintf(` resource "cloudflare_access_rule" "test" {