ezelkow1 commented on a change in pull request #4990:
URL: https://github.com/apache/trafficcontrol/pull/4990#discussion_r479476416
##########
File path: lib/go-atscfg/ipallowdotconfig.go
##########
@@ -166,48 +170,79 @@ func MakeIPAllowDotConfig(
ips := []*net.IPNet{}
ip6s := []*net.IPNet{}
- for serverName, server := range childServers {
- if ip := net.ParseIP(server.IPAddress).To4(); ip != nil
{
- // got an IP - convert it to a CIDR and add it
to the list
- ips = append(ips, util.IPToCIDR(ip))
- } else {
- // not an IP, try a CIDR
- if ip, cidr, err :=
net.ParseCIDR(server.IPAddress); err != nil {
- // not a CIDR or IP - error out
- log.Errorln("MakeIPAllowDotConfig
server '" + string(serverName) + "' IP '" + server.IPAddress + " is not an IPv4
address or CIDR - skipping!")
- } else {
- // got a valid CIDR - now make sure
it's v4
- ip = ip.To4()
- if ip == nil {
- // valid CIDR, but not v4
-
log.Errorln("MakeIPAllowDotConfig server '" + string(serverName) + "' IP '" +
server.IPAddress + " is a CIDR, but not v4 - skipping!")
- } else {
- // got a valid IPv4 CIDR - add
it to the list
- ips = append(ips, cidr)
- }
- }
+ cgMap := map[string]tc.CacheGroupNullable{}
+ for _, cg := range cacheGroups {
+ if cg.Name == nil {
+ return "ERROR: got cachegroup with nil name!'"
}
+ cgMap[*cg.Name] = cg
+ }
- if server.IP6Address != "" {
- ip6 := net.ParseIP(server.IP6Address)
- if ip6 != nil && ip6.To4() == nil {
- // got a valid IPv6 - add it to the list
- ip6s = append(ip6s, util.IPToCIDR(ip6))
- } else {
- // not a v6 IP, try a CIDR
- if ip, cidr, err :=
net.ParseCIDR(server.IP6Address); err != nil {
- // not a CIDR or IP - error out
-
log.Errorln("MakeIPAllowDotConfig server '" + string(serverName) + "' IP6 '" +
server.IP6Address + " is not an IPv6 address or CIDR - skipping!")
+ if server.Cachegroup == nil {
+ return "ERROR: server had nil Cachegroup!"
+ }
+
+ serverCG, ok := cgMap[*server.Cachegroup]
+ if !ok {
+ return "ERROR: Server cachegroup not in cachegroups!"
+ }
+
+ childCGs := map[string]tc.CacheGroupNullable{}
+ for cgName, cg := range cgMap {
+ if (cg.ParentName != nil && *cg.ParentName ==
*serverCG.Name) || (cg.SecondaryParentName != nil && *cg.SecondaryParentName ==
*serverCG.Name) {
+ childCGs[cgName] = cg
+ }
+ }
+
+ for _, childServer := range servers {
+ if childServer.Cachegroup == nil {
+ log.Errorln("Servers had server with nil
Cachegroup, skipping!")
+ continue
+ } else if childServer.HostName == nil {
+ log.Errorln("Servers had server with nil
HostName, skipping!")
+ continue
+ }
+
+ // We need to add IPs to the allow of
+ // - all children of this server
+ // - all monitors, if this server is a Mid
+ //
+ // TODO: handle Topologies. Mids currently block
everything but Edges
+ // We should decide how to handle that in a
post-edge-mid world.
+ // That probably means adding all child and
monitor IPs, and blocking everything else,
+ // for all non-first-tier caches.
+ //
+ _, isChild := childCGs[*childServer.Cachegroup]
+ if !isChild && (!strings.HasPrefix(server.Type,
tc.MidTypePrefix) || string(childServer.Type) != tc.MonitorTypeName) {
Review comment:
minor thing just from my C background, this is correct from operator
preference but might be a bit confusing. Might just be nice to have parenthesis
around that last group, up to you if you want to do it or not
##########
File path: lib/go-atscfg/ipallowdotconfig.go
##########
@@ -166,48 +170,79 @@ func MakeIPAllowDotConfig(
ips := []*net.IPNet{}
ip6s := []*net.IPNet{}
- for serverName, server := range childServers {
- if ip := net.ParseIP(server.IPAddress).To4(); ip != nil
{
- // got an IP - convert it to a CIDR and add it
to the list
- ips = append(ips, util.IPToCIDR(ip))
- } else {
- // not an IP, try a CIDR
- if ip, cidr, err :=
net.ParseCIDR(server.IPAddress); err != nil {
- // not a CIDR or IP - error out
- log.Errorln("MakeIPAllowDotConfig
server '" + string(serverName) + "' IP '" + server.IPAddress + " is not an IPv4
address or CIDR - skipping!")
- } else {
- // got a valid CIDR - now make sure
it's v4
- ip = ip.To4()
- if ip == nil {
- // valid CIDR, but not v4
-
log.Errorln("MakeIPAllowDotConfig server '" + string(serverName) + "' IP '" +
server.IPAddress + " is a CIDR, but not v4 - skipping!")
- } else {
- // got a valid IPv4 CIDR - add
it to the list
- ips = append(ips, cidr)
- }
- }
+ cgMap := map[string]tc.CacheGroupNullable{}
+ for _, cg := range cacheGroups {
+ if cg.Name == nil {
+ return "ERROR: got cachegroup with nil name!'"
}
+ cgMap[*cg.Name] = cg
+ }
- if server.IP6Address != "" {
- ip6 := net.ParseIP(server.IP6Address)
- if ip6 != nil && ip6.To4() == nil {
- // got a valid IPv6 - add it to the list
- ip6s = append(ip6s, util.IPToCIDR(ip6))
- } else {
- // not a v6 IP, try a CIDR
- if ip, cidr, err :=
net.ParseCIDR(server.IP6Address); err != nil {
- // not a CIDR or IP - error out
-
log.Errorln("MakeIPAllowDotConfig server '" + string(serverName) + "' IP6 '" +
server.IP6Address + " is not an IPv6 address or CIDR - skipping!")
+ if server.Cachegroup == nil {
+ return "ERROR: server had nil Cachegroup!"
+ }
+
+ serverCG, ok := cgMap[*server.Cachegroup]
+ if !ok {
+ return "ERROR: Server cachegroup not in cachegroups!"
+ }
+
+ childCGs := map[string]tc.CacheGroupNullable{}
+ for cgName, cg := range cgMap {
+ if (cg.ParentName != nil && *cg.ParentName ==
*serverCG.Name) || (cg.SecondaryParentName != nil && *cg.SecondaryParentName ==
*serverCG.Name) {
+ childCGs[cgName] = cg
+ }
+ }
+
+ for _, childServer := range servers {
+ if childServer.Cachegroup == nil {
+ log.Errorln("Servers had server with nil
Cachegroup, skipping!")
+ continue
+ } else if childServer.HostName == nil {
+ log.Errorln("Servers had server with nil
HostName, skipping!")
+ continue
+ }
+
+ // We need to add IPs to the allow of
+ // - all children of this server
+ // - all monitors, if this server is a Mid
+ //
+ // TODO: handle Topologies. Mids currently block
everything but Edges
+ // We should decide how to handle that in a
post-edge-mid world.
+ // That probably means adding all child and
monitor IPs, and blocking everything else,
+ // for all non-first-tier caches.
+ //
+ _, isChild := childCGs[*childServer.Cachegroup]
+ if !isChild && (!strings.HasPrefix(server.Type,
tc.MidTypePrefix) || string(childServer.Type) != tc.MonitorTypeName) {
+ continue
+ }
+ // childServers[tc.CacheName(*sv.HostName)] =
atscfg.IPAllowServer{IPAddress: sv.IPAddress, IP6Address: sv.IP6Address}
Review comment:
remove unless you want it there for reference for something
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
