ezelkow1 commented on a change in pull request #4990:
URL: https://github.com/apache/trafficcontrol/pull/4990#discussion_r479525618
##########
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:
I just mean like
if !isChild && (!strings.HasPrefix(server.Type, tc.MidTypePrefix) ||
(string(childServer.Type) != tc.MonitorTypeName))
its more of just a personal preference, not a big deal but it makes it
easier to see 2 separate blocks of operations separated by the or. Up to you if
you want to do it or not
----------------------------------------------------------------
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]