You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by mi...@apache.org on 2019/11/20 14:54:02 UTC

[trafficcontrol] branch master updated: Improvements to Server-to-Delivery Service assignments (#3761)

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

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


View the commit online:
https://github.com/apache/trafficcontrol/commit/c961af317b39648cf1aaa28c0b4e1ab80b2cfcd1

The following commit(s) were added to refs/heads/master by this push:
     new c961af3  Improvements to Server-to-Delivery Service assignments (#3761)
c961af3 is described below

commit c961af317b39648cf1aaa28c0b4e1ab80b2cfcd1
Author: ocket8888 <oc...@gmail.com>
AuthorDate: Wed Nov 20 07:53:46 2019 -0700

    Improvements to Server-to-Delivery Service assignments (#3761)
    
    * Fixed server-ds assignments being available accross CDNs in TP
    
    * Restricted server-ds assignments to intra-CDN only, and fixed ignoring tenancy
    
    * Added client code and tests
    
    * Fixup pursuant to review feedback
    
    * Add more human-friendly information to user error message
    
    * Refactor; added support for empty DS ID lists
    
    * go fmt
    
    * Fix bad formatting of CDN ID and name
---
 traffic_ops/client/server.go                       |  46 ++++++++
 .../servers_to_deliveryservice_assignment_test.go  | 126 +++++++++++++++++++++
 traffic_ops/testing/api/v14/tc-fixtures.json       |  49 +++++++-
 traffic_ops/testing/api/v14/user_test.go           |   2 +-
 .../server/servers_assignment.go                   |  91 +++++++++++++--
 .../TableServerDeliveryServicesController.js       |   2 +-
 6 files changed, 304 insertions(+), 12 deletions(-)

diff --git a/traffic_ops/client/server.go b/traffic_ops/client/server.go
index c3ddcd5..82b7e5d 100644
--- a/traffic_ops/client/server.go
+++ b/traffic_ops/client/server.go
@@ -29,6 +29,8 @@ import (
 
 const (
 	API_v13_Servers = "/api/1.3/servers"
+	API_v14_Server_Assign_DeliveryServices = "/api/1.4/servers/%d/deliveryservices?replace=%t"
+	API_v14_Server_DeliveryServices = "/api/1.4/servers/%d/deliveryservices"
 )
 
 // Create a Server
@@ -288,3 +290,47 @@ func (to *Session) GetServersShortNameSearch(shortname string) ([]string, ReqInf
 	}
 	return serverlst, reqInf, nil
 }
+
+// AssignDeliveryServiceIDsToServerID assigns a set of Delivery Services to a single server, optionally
+// replacing any and all existing assignments. 'server' should be the requested server's ID, 'dsIDs'
+// should be a slice of the requested Delivery Services' IDs. If 'replace' is 'true', existing
+// assignments to the server will be replaced.
+func (to *Session) AssignDeliveryServiceIDsToServerID(server int, dsIDs []int, replace bool) (tc.Alerts, ReqInf, error) {
+	// datatypes here match the library tc.Server's and tc.DeliveryService's ID fields
+
+
+	var remoteAddr net.Addr
+	reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+
+	endpoint := fmt.Sprintf(API_v14_Server_Assign_DeliveryServices, server, replace)
+
+	reqBody, err := json.Marshal(dsIDs)
+	if err != nil {
+		return tc.Alerts{}, reqInf, err
+	}
+
+	resp, remoteAddr, err := to.request(http.MethodPost, endpoint, reqBody)
+	if err != nil {
+		return tc.Alerts{}, reqInf, err
+	}
+	defer resp.Body.Close()
+	reqInf.RemoteAddr = remoteAddr
+	var alerts tc.Alerts
+	err = json.NewDecoder(resp.Body).Decode(&alerts)
+	return alerts, reqInf, err
+}
+
+func (to *Session) GetServerIDDeliveryServices(server int) ([]tc.DeliveryServiceNullable, ReqInf, error) {
+	endpoint := fmt.Sprintf(API_v14_Server_DeliveryServices, server)
+
+	resp, remoteAddr, err := to.request(http.MethodGet, endpoint, nil)
+	reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
+	if err != nil {
+		return nil, reqInf, err
+	}
+	defer resp.Body.Close()
+
+	var data tc.DeliveryServicesNullableResponse
+	err = json.NewDecoder(resp.Body).Decode(&data)
+	return data.Response, reqInf, err
+}
diff --git a/traffic_ops/testing/api/v14/servers_to_deliveryservice_assignment_test.go b/traffic_ops/testing/api/v14/servers_to_deliveryservice_assignment_test.go
new file mode 100644
index 0000000..7566d2d
--- /dev/null
+++ b/traffic_ops/testing/api/v14/servers_to_deliveryservice_assignment_test.go
@@ -0,0 +1,126 @@
+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 (
+	"testing"
+
+	"github.com/apache/trafficcontrol/lib/go-log"
+	"github.com/apache/trafficcontrol/lib/go-tc"
+)
+
+func TestAssignments(t *testing.T) {
+	WithObjs(t, []TCObj{CDNs, Types, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Tenants, DeliveryServices}, func() {
+		AssignTestDeliveryService(t)
+		AssignIncorrectTestDeliveryService(t)
+	})
+}
+
+func AssignTestDeliveryService(t *testing.T) {
+	rs, _, err := TOSession.GetServerByHostName(testData.Servers[0].HostName)
+	if err != nil {
+		t.Fatalf("Failed to fetch server information: %v", err)
+	} else if len(rs) == 0 {
+		t.Fatalf("Failed to fetch server information: No results returned!")
+	}
+	firstServer := rs[0]
+
+	rd, _, err := TOSession.GetDeliveryServiceByXMLID(testData.DeliveryServices[0].XMLID)
+	if err != nil {
+		t.Fatalf("Failed to fetch DS information: %v", err)
+	} else if len(rd) == 0 {
+		t.Fatalf("Failed to fetch DS information: No results returned!")
+	}
+	firstDS := rd[0]
+
+
+	alerts, _, err := TOSession.AssignDeliveryServiceIDsToServerID(firstServer.ID, []int{firstDS.ID}, true)
+	if err != nil {
+		t.Errorf("Couldn't assign DS '%+v' to server '%+v': %v (alerts: %v)", firstDS, firstServer, err, alerts)
+	}
+	log.Debugf("alerts: %+v", alerts)
+
+	response, _, err := TOSession.GetServerIDDeliveryServices(firstServer.ID)
+	log.Debugf("response: %+v", response)
+	if err != nil {
+		t.Fatalf("Couldn't get Delivery Services assigned to Server '%+v': %v", firstServer, err)
+	}
+
+	var found bool
+	for _,ds := range response {
+		if ds.ID != nil && *ds.ID == firstDS.ID {
+			found = true
+			break
+		}
+	}
+
+	if !found {
+		t.Errorf(`Server/DS assignment not found after "successful" assignment!`)
+	}
+}
+
+func AssignIncorrectTestDeliveryService(t *testing.T) {
+	var server *tc.Server
+	for _, s := range testData.Servers {
+		if s.CDNName == "cdn2" {
+			server = &s
+			break
+		}
+	}
+	if server == nil {
+		t.Fatalf("Couldn't find a server in CDN 'cdn2'!")
+	}
+
+	rs, _, err := TOSession.GetServerByHostName(server.HostName)
+	if err != nil {
+		t.Fatalf("Failed to fetch server information: %v", err)
+	} else if len(rs) == 0 {
+		t.Fatalf("Failed to fetch server information: No results returned!")
+	}
+	server = &rs[0]
+
+	rd, _, err := TOSession.GetDeliveryServiceByXMLID(testData.DeliveryServices[0].XMLID)
+	if err != nil {
+		t.Fatalf("Failed to fetch DS information: %v", err)
+	} else if len(rd) == 0 {
+		t.Fatalf("Failed to fetch DS information: No results returned!")
+	}
+	firstDS := rd[0]
+
+	alerts, _, err := TOSession.AssignDeliveryServiceIDsToServerID(server.ID, []int{firstDS.ID}, false)
+	if err == nil {
+		t.Errorf("Expected bad assignment to fail, but it didn't! (alerts: %v)", alerts)
+	}
+
+	response, _, err := TOSession.GetServerIDDeliveryServices(server.ID)
+	log.Debugf("response: %+v", response)
+	if err != nil {
+		t.Fatalf("Couldn't get Delivery Services assigned to Server '%+v': %v", *server, err)
+	}
+
+	var found bool
+	for _,ds := range response {
+
+		if ds.ID != nil && *ds.ID == firstDS.ID {
+			found = true
+			break
+		}
+	}
+
+	if found {
+		t.Errorf(`Invalid Server/DS assignment was created!`)
+	}
+}
diff --git a/traffic_ops/testing/api/v14/tc-fixtures.json b/traffic_ops/testing/api/v14/tc-fixtures.json
index 781f9ae..58c59c7 100644
--- a/traffic_ops/testing/api/v14/tc-fixtures.json
+++ b/traffic_ops/testing/api/v14/tc-fixtures.json
@@ -621,7 +621,7 @@
             "type": "HTTP_LIVE",
             "xmlId": "msods1",
             "anonymousBlockingEnabled": true
-        }    
+        }
     ],
     "deliveryservicesRequiredCapabilities": [
         {
@@ -902,7 +902,7 @@
             "routingDisabled": false,
             "type": "ATS_PROFILE",
             "params": [
-                 {
+                {
                     "configFile": "records.config",
                     "name": "CONFIG proxy.config.proxy_name",
                     "secure": false,
@@ -1276,6 +1276,14 @@
             "type": "ATS_PROFILE"
         },
         {
+            "cdnName": "cdn2",
+            "description": "edge2 description",
+            "lastUpdated": "2018-03-02T17:27:11.818418+00:00",
+            "name": "EDGEInCDN2",
+            "routing_disabled": false,
+            "type": "ATS_PROFILE"
+        },
+        {
             "cdnName": "cdn4",
             "description": "edge2 description",
             "lastUpdated": "2018-03-02T17:27:11.818418+00:00",
@@ -1411,6 +1419,43 @@
         },
         {
             "cachegroup": "cachegroup1",
+            "cdnName": "cdn2",
+            "domainName": "ga.atlanta.kabletown.net",
+            "guid": null,
+            "hostName": "cdn2-test-edge",
+            "httpsPort": 443,
+            "iloIpAddress": "",
+            "iloIpGateway": "",
+            "iloIpNetmask": "",
+            "iloPassword": "",
+            "iloUsername": "",
+            "interfaceMtu": 1500,
+            "interfaceName": "eth0",
+            "ip6Address": "",
+            "ip6Gateway": "",
+            "ipAddress": "0.0.0.0",
+            "ipGateway": "0.0.0.0",
+            "ipNetmask": "0.0.0.0",
+            "lastUpdated": "2018-03-28T17:30:00.220351+00:00",
+            "mgmtIpAddress": "",
+            "mgmtIpGateway": "",
+            "mgmtIpNetmask": "",
+            "offlineReason": null,
+            "physLocation": "Denver",
+            "profile": "EDGEInCDN2",
+            "rack": "",
+            "revalPending": false,
+            "routerHostName": "",
+            "routerPortName": "",
+            "status": "REPORTED",
+            "tcpPort": 80,
+            "type": "EDGE",
+            "updPending": false,
+            "xmppId": "",
+            "xmppPasswd": ""
+        },
+        {
+            "cachegroup": "cachegroup1",
             "cdnName": "cdn1",
             "domainName": "kabletown.net",
             "guid": null,
diff --git a/traffic_ops/testing/api/v14/user_test.go b/traffic_ops/testing/api/v14/user_test.go
index 333054c..99ead3e 100644
--- a/traffic_ops/testing/api/v14/user_test.go
+++ b/traffic_ops/testing/api/v14/user_test.go
@@ -81,7 +81,7 @@ func RolenameCapitalizationTest(t *testing.T) {
 		"localPasswd": "better_twelve",
 		"confirmLocalPasswd": "better_twelve",
 		"role": %d,
-		"tenantId": %d 
+		"tenantId": %d
 	}`, *roles[0].ID, tenants[0].ID)
 
 	reader := strings.NewReader(blob)
diff --git a/traffic_ops/traffic_ops_golang/server/servers_assignment.go b/traffic_ops/traffic_ops_golang/server/servers_assignment.go
index 8835360..9380e24 100644
--- a/traffic_ops/traffic_ops_golang/server/servers_assignment.go
+++ b/traffic_ops/traffic_ops_golang/server/servers_assignment.go
@@ -33,10 +33,31 @@ import (
 	"github.com/apache/trafficcontrol/lib/go-tc"
 	"github.com/apache/trafficcontrol/lib/go-util"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant"
 	"github.com/lib/pq"
 )
 
+type needsCheck struct {
+	CDN     uint
+	CDNName string
+	DSID    uint
+	DSXMLID string
+	Tenant  int
+}
+
+const needsCheckInfoQuery = `
+SELECT deliveryservice.id,
+       deliveryservice.cdn_id,
+       deliveryservice.tenant_id,
+       deliveryservice.xml_id,
+       cdn.name
+FROM deliveryservice
+LEFT OUTER JOIN cdn ON cdn.id=deliveryservice.cdn_id
+WHERE deliveryservice.id = ANY($1)
+`
+
 func AssignDeliveryServicesToServerHandler(w http.ResponseWriter, r *http.Request) {
 	inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"id"}, []string{"id"})
 	if userErr != nil || sysErr != nil {
@@ -58,16 +79,11 @@ func AssignDeliveryServicesToServerHandler(w http.ResponseWriter, r *http.Reques
 		return
 	}
 
-	serverPathParameter := inf.Params["id"]
-	server, err := strconv.Atoi(serverPathParameter)
-	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, err, nil)
-		return
-	}
+	server := inf.IntParams["id"]
+
 	serverInfo, ok, err := dbhelpers.GetServerInfo(server, inf.Tx.Tx)
 	if err != nil {
 		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server name from ID: "+err.Error()))
-		return
 	} else if !ok {
 		api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("no server with that ID found"), nil)
 		return
@@ -81,16 +97,75 @@ func AssignDeliveryServicesToServerHandler(w http.ResponseWriter, r *http.Reques
 		}
 	}
 
+	// We already know the CDN exists because that's part of the serverInfo query above
+	serverCDN, _, err := dbhelpers.GetCDNNameFromID(inf.Tx.Tx, int64(serverInfo.CDNID))
+	if err != nil {
+		sysErr = fmt.Errorf("Failed to get CDN name from ID: %v", err)
+		errCode = http.StatusInternalServerError
+		api.HandleErr(w, r, inf.Tx.Tx, errCode, nil, sysErr)
+		return
+	}
+
+	if len(dsList) > 0 {
+		if errCode, userErr, sysErr = checkTenancyAndCDN(inf.Tx.Tx, string(serverCDN), server, serverInfo, dsList, inf.User); userErr != nil || sysErr != nil {
+			api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+			return
+		}
+	}
+
 	assignedDSes, err := assignDeliveryServicesToServer(server, dsList, replace, inf.Tx.Tx)
 	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server name from ID: "+err.Error()))
 		return
+	} else if !ok {
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("no server with that ID found"), nil)
 	}
 
 	api.CreateChangeLogRawTx(api.ApiChange, "SERVER: "+serverInfo.HostName+", ID: "+strconv.Itoa(server)+", ACTION: Assigned "+strconv.Itoa(len(assignedDSes))+" DSes to server", inf.User, inf.Tx.Tx)
 	api.WriteRespAlertObj(w, r, tc.SuccessLevel, "successfully assigned dses to server", tc.AssignedDsResponse{server, assignedDSes, replace})
 }
 
+func checkTenancyAndCDN(tx *sql.Tx, serverCDN string, server int, serverInfo tc.ServerInfo, dsList []int, user *auth.CurrentUser) (int, error, error) {
+	rows, err := tx.Query(needsCheckInfoQuery, pq.Array(dsList))
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return http.StatusBadRequest, errors.New("Either at least one Delivery Service ID doesn't exist, or is outside your tenancy!"), nil
+		}
+		return http.StatusInternalServerError, nil, err
+	}
+	defer rows.Close()
+
+	tenantsToCheck := make([]needsCheck, 0, len(dsList))
+	for rows.Next() {
+		var n needsCheck
+		if err = rows.Scan(&n.DSID, &n.CDN, &n.Tenant, &n.DSXMLID, &n.CDNName); err != nil {
+			return http.StatusInternalServerError, nil, fmt.Errorf("Scanning cdn_id for ds: %v", err)
+		}
+
+		tenantsToCheck = append(tenantsToCheck, n)
+	}
+
+	if len(tenantsToCheck) != len(dsList) {
+		return http.StatusNotFound, errors.New("Either no Delivery Service ids given, or at least one id doesn't exist!"), nil
+	}
+
+	for _, t := range tenantsToCheck {
+		if ok, err := tenant.IsResourceAuthorizedToUserTx(t.Tenant, user, tx); err != nil {
+			return http.StatusInternalServerError, nil, fmt.Errorf("Checking availability of ds %d (tenant_id: %d) to tenant_id %d: %v", t.DSID, t.Tenant, err, user.TenantID, err)
+		} else if !ok {
+			// In keeping with the behavior of /deliveryservices, we don't disclose the existences
+			// of Delivery Services to which the user is forbidden access
+			return http.StatusNotFound, errors.New("Either no Delivery Service ids given, or at least one id doesn't exist!"), fmt.Errorf("User %s denied access to inaccessible DS %d (owned by tenant_id %d)", user.UserName, t.DSID, t.Tenant)
+		}
+
+		if int(t.CDN) != serverInfo.CDNID {
+			return http.StatusConflict, fmt.Errorf("Delivery Service %s (#%d) is not in the same CDN as server %s (#%d) (server is in %s (#%d), DS is in %s (#%d))!", t.DSXMLID, t.DSID, serverInfo.HostName, server, serverCDN, serverInfo.CDNID, t.CDNName, t.CDN), nil
+		}
+	}
+
+	return http.StatusOK, nil, nil
+}
+
 // ValidateDSCapabilities checks that the server meets the requirements of each delivery service to be assigned.
 func ValidateDSCapabilities(dsIDs []int, serverName string, tx *sql.Tx) (error, error, int) {
 	var dsCaps []string
diff --git a/traffic_portal/app/src/common/modules/table/serverDeliveryServices/TableServerDeliveryServicesController.js b/traffic_portal/app/src/common/modules/table/serverDeliveryServices/TableServerDeliveryServicesController.js
index 9f1d309..72aa13e 100644
--- a/traffic_portal/app/src/common/modules/table/serverDeliveryServices/TableServerDeliveryServicesController.js
+++ b/traffic_portal/app/src/common/modules/table/serverDeliveryServices/TableServerDeliveryServicesController.js
@@ -94,7 +94,7 @@ var TableServerDeliveryServicesController = function(server, deliveryServices, $
 					return params;
 				},
 				collection: function(serverService) {
-					return serverService.getServers({ type: server.type, orderby: 'hostName' });
+					return serverService.getServers({ type: server.type, orderby: 'hostName', cdn: server.cdnId }).then(function(xs){return xs.filter(function(x){return x.id!=server.id})}, function(err){throw err});
 				}
 			}
 		});