Resolve TODOs and issues with allow_list

slackhq · Jul 5, 2024 · 7ea16be · 7ea16be
1 parent 58e330f
commit 7ea16be
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 11 deletions.
diff --git a/allow_list.go b/allow_list.go
@@ -122,18 +122,19 @@ func newAllowList(k string, raw interface{}, handleKey func(key string, value in
  }
 
  ipNet, err := netip.ParsePrefix(rawCIDR)
- //TODO: better include the error
  if err != nil {
- return nil, fmt.Errorf("config `%s` has invalid CIDR: %s", k, rawCIDR)
+ return nil, fmt.Errorf("config `%s` has invalid CIDR: %s. %w", k, rawCIDR, err)
  }
 
+ ipNet = netip.PrefixFrom(ipNet.Addr().Unmap(), ipNet.Bits())
+
  // TODO: should we error on duplicate CIDRs in the config?
  tree.Insert(ipNet, value)
 
  maskBits := ipNet.Bits()
 
  var rules *allowListRules
- if ipNet.Masked().Addr().Is4() {
+ if ipNet.Addr().Is4() {
  rules = &rules4
  } else {
  rules = &rules6
@@ -156,17 +157,15 @@ func newAllowList(k string, raw interface{}, handleKey func(key string, value in
 
  if !rules4.defaultSet {
  if rules4.allValuesMatch {
- //TODO ensure this is a 0/0
- tree.Insert(netip.Prefix{}, !rules4.allValues)
+ tree.Insert(netip.PrefixFrom(netip.IPv4Unspecified(), 0), !rules4.allValues)
  } else {
  return nil, fmt.Errorf("config `%s` contains both true and false rules, but no default set for 0.0.0.0/0", k)
  }
  }
 
  if !rules6.defaultSet {
  if rules6.allValuesMatch {
- //TODO: ensure this is a ::/0
- tree.Insert(netip.Prefix{}, !rules6.allValues)
+ tree.Insert(netip.PrefixFrom(netip.IPv6Unspecified(), 0), !rules6.allValues)
  } else {
  return nil, fmt.Errorf("config `%s` contains both true and false rules, but no default set for ::/0", k)
  }
@@ -242,12 +241,11 @@ func getRemoteAllowRanges(c *config.C, k string) (*bart.Table[*AllowList], error
  }
 
  ipNet, err := netip.ParsePrefix(rawCIDR)
- //TODO: better to include err
  if err != nil {
- return nil, fmt.Errorf("config `%s` has invalid CIDR: %s", k, rawCIDR)
+ return nil, fmt.Errorf("config `%s` has invalid CIDR: %s. %w", k, rawCIDR, err)
  }
 
- remoteAllowRanges.Insert(ipNet, allowList)
+ remoteAllowRanges.Insert(netip.PrefixFrom(ipNet.Addr().Unmap(), ipNet.Bits()), allowList)
  }
 
  return remoteAllowRanges, nil

diff --git a/allow_list_test.go b/allow_list_test.go
@@ -18,7 +18,7 @@ func TestNewAllowListFromConfig(t *testing.T) {
  "192.168.0.0": true,
  }
  r, err := newAllowListFromConfig(c, "allowlist", nil)
- assert.EqualError(t, err, "config `allowlist` has invalid CIDR: 192.168.0.0")
+ assert.EqualError(t, err, "config `allowlist` has invalid CIDR: 192.168.0.0. netip.ParsePrefix(\"192.168.0.0\"): no '/'")
  assert.Nil(t, r)
 
  c.Settings["allowlist"] = map[interface{}]interface{}{