You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ro...@apache.org on 2020/01/17 15:44:35 UTC

[trafficcontrol] branch master updated: Fix ip_allow config generation for mids to include rascal servers (#4296)

This is an automated email from the ASF dual-hosted git repository.

rob pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/master by this push:
     new 6b11bd9  Fix ip_allow config generation for mids to include rascal servers (#4296)
6b11bd9 is described below

commit 6b11bd9cad471c549ff8c22a0ed068e56f91f5b9
Author: Michael Hoppal <54...@users.noreply.github.com>
AuthorDate: Fri Jan 17 08:44:28 2020 -0700

    Fix ip_allow config generation for mids to include rascal servers (#4296)
    
    * Fix ip_allow config generation to include rascal servers
    
    * Fix ip_allow generation in atstccfg
    
    * Add API tests for ip_allow
    
    * Make tests more robust
---
 lib/go-util/net.go                                 |  48 ++++++++
 lib/go-util/net_test.go                            |  49 ++++++++
 .../ort/atstccfg/cfgfile/ipallowdotconfig.go       |   7 +-
 .../testing/api/v14/ip_allow_dot_config_test.go    | 125 +++++++++++++++++++++
 traffic_ops/testing/api/v14/tc-fixtures.json       |   4 +-
 .../ats/atsserver/ipallowdotconfig.go              |   4 +-
 6 files changed, 230 insertions(+), 7 deletions(-)

diff --git a/lib/go-util/net.go b/lib/go-util/net.go
index 50af321..d240eb0 100644
--- a/lib/go-util/net.go
+++ b/lib/go-util/net.go
@@ -21,7 +21,10 @@ package util
 
 import (
 	"bytes"
+	"errors"
 	"net"
+	"strconv"
+	"strings"
 
 	"github.com/apache/trafficcontrol/lib/go-log"
 )
@@ -200,3 +203,48 @@ func IPToCIDR(ip net.IP) *net.IPNet {
 	}
 	return &net.IPNet{IP: ip, Mask: fullMask}
 }
+
+func IP4ToNum(ip string) (uint32, error) {
+	parts := strings.Split(ip, `.`)
+	if len(parts) != 4 {
+		return 0, errors.New("malformed IPv4")
+	}
+	intParts := []uint32{}
+	for _, part := range parts {
+		i, err := strconv.ParseUint(part, 10, 32)
+		if err != nil {
+			return 0, errors.New("malformed IPv4")
+		}
+		intParts = append(intParts, uint32(i))
+	}
+
+	num := intParts[3]
+	num += intParts[2] << 8
+	num += intParts[1] << 16
+	num += intParts[0] << 24
+
+	return num, nil
+}
+
+func IP4InRange(ip, ipRange string) (bool, error) {
+	ab := strings.Split(ipRange, `-`)
+	if len(ab) != 2 {
+		if len(ab) == 1 { // no range check for equality
+			return ip == ipRange, nil
+		}
+		return false, errors.New("malformed range")
+	}
+	ipNum, err := IP4ToNum(ip)
+	if err != nil {
+		return false, errors.New("malformed ip")
+	}
+	aNum, err := IP4ToNum(ab[0])
+	if err != nil {
+		return false, errors.New("malformed range (first part)")
+	}
+	bNum, err := IP4ToNum(ab[1])
+	if err != nil {
+		return false, errors.New("malformed range (second part)")
+	}
+	return ipNum >= aNum && ipNum <= bNum, nil
+}
diff --git a/lib/go-util/net_test.go b/lib/go-util/net_test.go
index ce0db4e..0ffaf59 100644
--- a/lib/go-util/net_test.go
+++ b/lib/go-util/net_test.go
@@ -17,6 +17,7 @@ package util
 // When adding symbols, document the RFC and section they correspond to.
 
 import (
+	"fmt"
 	"net"
 	"testing"
 )
@@ -190,3 +191,51 @@ func TestLastIP(t *testing.T) {
 		}
 	}
 }
+
+func TestIP4ToNum(t *testing.T) {
+	var tests = []struct {
+		ip     string
+		number uint32
+	}{
+		{"127.0.0.1", uint32(2130706433)},
+		{"127.0.0.4", uint32(2130706436)},
+		{"127.255.255.255", uint32(2147483647)},
+	}
+	for _, tt := range tests {
+		t.Run(tt.ip, func(t *testing.T) {
+			n, err := IP4ToNum(tt.ip)
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+
+			}
+			if n != tt.number {
+				t.Errorf("got %v, want %v", n, tt.number)
+			}
+		})
+	}
+}
+
+func TestIP4InRange(t *testing.T) {
+	var tests = []struct {
+		ip      string
+		ipRange string
+		inRange bool
+	}{
+		{"111.0.0.1", "127.0.0.0-127.255.255.255", false},
+		{"128.0.0.1", "127.0.0.0-127.255.255.255", false},
+		{"127.0.0.1", "127.0.0.0-127.255.255.255", true},
+		{"127.0.0.1", "127.0.0.1", true},
+	}
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf("%v in range %v", tt.ip, tt.inRange), func(t *testing.T) {
+			exists, err := IP4InRange(tt.ip, tt.ipRange)
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+
+			}
+			if exists != tt.inRange {
+				t.Errorf("got %v, want %v", exists, tt.inRange)
+			}
+		})
+	}
+}
diff --git a/traffic_ops/ort/atstccfg/cfgfile/ipallowdotconfig.go b/traffic_ops/ort/atstccfg/cfgfile/ipallowdotconfig.go
index dd09618..119d06b 100644
--- a/traffic_ops/ort/atstccfg/cfgfile/ipallowdotconfig.go
+++ b/traffic_ops/ort/atstccfg/cfgfile/ipallowdotconfig.go
@@ -22,6 +22,7 @@ package cfgfile
 import (
 	"errors"
 	"strconv"
+	"strings"
 
 	"github.com/apache/trafficcontrol/lib/go-atscfg"
 	"github.com/apache/trafficcontrol/lib/go-tc"
@@ -110,10 +111,10 @@ func GetConfigFileServerIPAllowDotConfig(cfg config.TCCfg, serverNameOrID string
 
 	childServers := map[tc.CacheName]atscfg.IPAllowServer{}
 	for _, sv := range servers {
-		if _, ok := childCGs[sv.Cachegroup]; !ok {
-			continue
+		_, ok := childCGs[sv.Cachegroup]
+		if ok || (strings.HasPrefix(string(serverType), tc.MidTypePrefix) && string(sv.Type) == tc.MonitorTypeName) {
+			childServers[tc.CacheName(sv.HostName)] = atscfg.IPAllowServer{IPAddress: sv.IPAddress, IP6Address: sv.IP6Address}
 		}
-		childServers[tc.CacheName(sv.HostName)] = atscfg.IPAllowServer{IPAddress: sv.IPAddress, IP6Address: sv.IP6Address}
 	}
 
 	txt := atscfg.MakeIPAllowDotConfig(serverName, serverType, toToolName, toURL, fileParams, childServers)
diff --git a/traffic_ops/testing/api/v14/ip_allow_dot_config_test.go b/traffic_ops/testing/api/v14/ip_allow_dot_config_test.go
new file mode 100644
index 0000000..1f8f981
--- /dev/null
+++ b/traffic_ops/testing/api/v14/ip_allow_dot_config_test.go
@@ -0,0 +1,125 @@
+package v14
+
+/*
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+import (
+	"fmt"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/apache/trafficcontrol/lib/go-util"
+
+	"github.com/apache/trafficcontrol/lib/go-tc"
+)
+
+const ipAllow = "ip_allow.config"
+
+var (
+	expectedRules = []string{
+		"src_ip=127.0.0.1 action=ip_allow method=ALL\n",
+		"src_ip=::1 action=ip_allow method=ALL\n",
+	}
+	midExpectedRules = []string{
+		"src_ip=10.0.0.0-10.255.255.255 action=ip_allow method=ALL\n",
+		"src_ip=172.16.0.0-172.31.255.255 action=ip_allow method=ALL\n",
+		"src_ip=192.168.0.0-192.168.255.255 action=ip_allow method=ALL\n",
+		"src_ip=::-ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff action=ip_deny method=ALL\n",
+		"src_ip=::-ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff action=ip_deny method=ALL\n",
+	}
+	edgeExpectedRules = []string{
+		"src_ip=0.0.0.0-255.255.255.255 action=ip_deny method=PUSH|PURGE|DELETE\n",
+		"src_ip=::-ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff action=ip_deny method=PUSH|PURGE|DELETE\n",
+	}
+	rascalServerIP = ""
+	rascalRule     = "src_ip=%v action=ip_allow method=ALL"
+)
+
+func TestIPAllowDotConfig(t *testing.T) {
+	WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, DeliveryServices}, func() {
+		rascalServerIP = getServer(t, "RASCAL").IPAddress
+		// rascalRule = fmt.Sprintf("src_ip=%v action=ip_allow method=ALL", rascalServer.IPAddress)
+		GetTestIPAllowDotConfig(t)
+		GetTestIPAllowMidDotConfig(t)
+	})
+}
+
+func GetTestIPAllowDotConfig(t *testing.T) {
+	// Get edge server
+	s := getServer(t, "EDGE")
+	output, _, err := TOSession.GetATSServerConfig(s.ID, ipAllow)
+	if err != nil {
+		t.Fatalf("cannot GET server %v config %v: %v", s.HostName, ipAllow, err)
+	}
+	for _, r := range append(expectedRules, edgeExpectedRules...) {
+		if !strings.Contains(output, r) {
+			t.Errorf("expected rule %v not found in ip_allow config", r)
+		}
+	}
+	// Make sure edge does not contain rule for rascal server
+	exists, ipRange := getIPRule(output, rascalServerIP)
+	rascalRule := fmt.Sprintf(rascalRule, ipRange)
+	if exists && strings.Contains(output, rascalRule) {
+		t.Errorf("rascal IP was not supposed to be in an allowed rule: %v", rascalRule)
+	}
+}
+
+func GetTestIPAllowMidDotConfig(t *testing.T) {
+	// Get mid server
+	s := getServer(t, "MID")
+	output, _, err := TOSession.GetATSServerConfig(s.ID, ipAllow)
+	if err != nil {
+		t.Errorf("cannot GET server %v config %v: %v", s.HostName, ipAllow, err)
+	}
+	for _, r := range append(expectedRules, midExpectedRules...) {
+		if !strings.Contains(output, r) {
+			t.Errorf("expected rule %v not found in ip_allow config", r)
+		}
+	}
+
+	// Make sure mid contains an allowed rule that includes the rascal server
+	exists, ipRange := getIPRule(output, rascalServerIP)
+	rascalRule := fmt.Sprintf(rascalRule, ipRange)
+	if !(exists && strings.Contains(output, rascalRule)) {
+		t.Errorf("expected rascal to be include as allowed in mid ip allow config")
+	}
+}
+
+func getServer(t *testing.T, serverType string) tc.Server {
+	v := url.Values{}
+	v.Add("type", serverType)
+	servers, _, err := TOSession.GetServersByType(v)
+	if err != nil {
+		t.Fatalf("cannot GET Server by type %v: %v", serverType, err)
+	}
+	if len(servers) == 0 {
+		t.Fatalf("cannot find any Servers by type %v", serverType)
+	}
+	return servers[0]
+}
+
+// getIPRuleRange returns if the given IP is included in the set of rules and which ip range it is included in
+func getIPRule(rules, ip string) (bool, string) {
+	for _, r := range strings.Split(rules, "\n")[1:] {
+		if !strings.Contains(r, "src_ip") {
+			continue
+		}
+		ipRange := r[7:strings.IndexAny(r, " ")]
+		if exists, _ := util.IP4InRange(ip, ipRange); exists {
+			return true, ipRange
+		}
+	}
+	return false, ""
+}
diff --git a/traffic_ops/testing/api/v14/tc-fixtures.json b/traffic_ops/testing/api/v14/tc-fixtures.json
index a1d6ce8..f2631e4 100644
--- a/traffic_ops/testing/api/v14/tc-fixtures.json
+++ b/traffic_ops/testing/api/v14/tc-fixtures.json
@@ -1895,7 +1895,7 @@
             "routerPortName": "",
             "status": "REPORTED",
             "tcpPort": 81,
-            "type": "TRAFFIC_MONITOR",
+            "type": "RASCAL",
             "updPending": false,
             "xmppId": "",
             "xmppPasswd": "X"
@@ -2368,7 +2368,7 @@
         {
             "description": "Traffic Monitor (Rascal)",
             "lastUpdated": "2018-03-02T19:13:46.832327+00:00",
-            "name": "TRAFFIC_MONITOR",
+            "name": "RASCAL",
             "useInTable": "server"
         }
     ],
diff --git a/traffic_ops/traffic_ops_golang/ats/atsserver/ipallowdotconfig.go b/traffic_ops/traffic_ops_golang/ats/atsserver/ipallowdotconfig.go
index cdcd3d0..520200b 100644
--- a/traffic_ops/traffic_ops_golang/ats/atsserver/ipallowdotconfig.go
+++ b/traffic_ops/traffic_ops_golang/ats/atsserver/ipallowdotconfig.go
@@ -86,7 +86,7 @@ FROM
   JOIN type tp on tp.id = s.type
   JOIN cachegroup cg on cg.id = s.cachegroup
 WHERE
-  (tp.name = '` + tc.MonitorTypeName + `' OR tp.name LIKE '` + tc.EdgeTypePrefix + `%')
+  (tp.name = '` + tc.MonitorTypeName + `' OR ( tp.name LIKE '` + tc.EdgeTypePrefix + `%')
   AND cg.id IN (
     SELECT
       cg2.id
@@ -94,7 +94,7 @@ WHERE
      server s2
      JOIN cachegroup cg2 ON (cg2.parent_cachegroup_id = s2.cachegroup OR cg2.secondary_parent_cachegroup_id = s2.cachegroup)
     WHERE
-      s2.host_name = $1
+      s2.host_name = $1 )
   )
 `
 	rows, err := tx.Query(qry, serverName)