Fix the assignment of a delivery service to a server, to respect capa…

…bilities (#7878) * Fix the assignment of a delivery service to a server, to respect capabilities * adding changelog * fixing Traffic Portal to account for the correct field names * adding org server edge case Co-authored-by: Chatterjee, Srijeet <[email protected]>
apache · Dec 4, 2023 · 80255a0 · 80255a0
1 parent 3710add
commit 80255a0
Show file tree

Hide file tree

Showing 9 changed files with 21 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ### Fixed
 - [#7846](https://github.com/apache/trafficcontrol/pull/7846) *Traffic Portal* Increase State character limit
+- [#7878](https://github.com/apache/trafficcontrol/pull/7878) *Traffic Ops, Traffic Portal* Fixed the case where TO was failing to assign delivery services to a server, due to a bug in the way the list of preexisting delivery services was being returned.
 
 ## [8.0.0] - 2023-09-20
 ### Added

diff --git a/lib/go-tc/deliveryservice_servers.go b/lib/go-tc/deliveryservice_servers.go
@@ -430,6 +430,7 @@ func (server DSServerV4) Upgrade() DSServerV5 {
 			MgmtIPGateway:      server.MgmtIPGateway,
 			MgmtIPNetmask:      server.MgmtIPNetmask,
 			OfflineReason:      server.OfflineReason,
+			Profiles:           server.ProfileNames,
 			PhysicalLocation:   util.CoalesceToDefault(server.PhysLocation),
 			PhysicalLocationID: util.CoalesceToDefault(server.PhysLocationID),
 			Rack:               server.Rack,

diff --git a/traffic_ops/testing/api/v3/servers_id_deliveryservices_test.go b/traffic_ops/testing/api/v3/servers_id_deliveryservices_test.go
@@ -182,7 +182,7 @@ func validateServersDeliveryServicesPost(serverID int, expectedDSID []int, expec
 	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
 		serverDeliveryServices, _, err := TOSession.GetServerIDDeliveryServicesWithHdr(serverID, nil)
 		assert.RequireNoError(t, err, "Error getting Server Delivery Services: %v", err)
-		assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices), "Expected one Delivery Service returned Got: %d", len(serverDeliveryServices))
+		assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices), "Expected %d Delivery Service returned Got: %d", expectedDSCount, len(serverDeliveryServices))
 		for i := 0; i < len(expectedDSID); i++ {
 			validateServersDeliveryServices(expectedDSID[i])(t, toclientlib.ReqInf{}, serverDeliveryServices, tc.Alerts{}, nil)
 		}

diff --git a/traffic_ops/testing/api/v4/servers_id_deliveryservices_test.go b/traffic_ops/testing/api/v4/servers_id_deliveryservices_test.go
@@ -193,7 +193,7 @@ func validateServersDeliveryServicesPost(serverID int, expectedDSID []int, expec
 	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
 		serverDeliveryServices, _, err := TOSession.GetServerIDDeliveryServices(serverID, client.RequestOptions{})
 		assert.RequireNoError(t, err, "Error getting Server Delivery Services: %v - alerts: %+v", err, serverDeliveryServices.Alerts)
-		assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices.Response), "Expected Two Delivery Service returned Got: %d", len(serverDeliveryServices.Response))
+		assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices.Response), "Expected %d Delivery Service returned Got: %d", expectedDSCount, len(serverDeliveryServices.Response))
 		for i := 0; i < len(expectedDSID); i++ {
 			validateServersDeliveryServices(expectedDSID[i])(t, toclientlib.ReqInf{}, serverDeliveryServices.Response, tc.Alerts{}, nil)
 

diff --git a/traffic_ops/testing/api/v5/cachegroupsdeliveryservices_test.go b/traffic_ops/testing/api/v5/cachegroupsdeliveryservices_test.go
@@ -42,10 +42,8 @@ func TestCacheGroupsDeliveryServices(t *testing.T) {
 					EndpointID:    GetCacheGroupId(t, "cachegroup3"),
 					ClientSession: TOSession,
 					RequestBody: []int{
-						GetDeliveryServiceId(t, "ds1")(),
-						GetDeliveryServiceId(t, "ds2")(),
-						GetDeliveryServiceId(t, "ds3")(),
 						GetDeliveryServiceId(t, "ds3")(),
+						GetDeliveryServiceId(t, "ds4")(),
 						GetDeliveryServiceId(t, "DS5")(),
 					},
 					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCGDSServerAssignments()),

diff --git a/traffic_ops/testing/api/v5/servers_id_deliveryservices_test.go b/traffic_ops/testing/api/v5/servers_id_deliveryservices_test.go
@@ -192,7 +192,7 @@ func validateServersDeliveryServicesPost(serverID int, expectedDSID []int, expec
 	return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
 		serverDeliveryServices, _, err := TOSession.GetServerIDDeliveryServices(serverID, client.RequestOptions{})
 		assert.RequireNoError(t, err, "Error getting Server Delivery Services: %v - alerts: %+v", err, serverDeliveryServices.Alerts)
-		assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices.Response), "Expected Two Delivery Service returned Got: %d", len(serverDeliveryServices.Response))
+		assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices.Response), "Expected %d Delivery Service returned Got: %d", expectedDSCount, len(serverDeliveryServices.Response))
 		for i := 0; i < len(expectedDSID); i++ {
 			validateServersDeliveryServices(expectedDSID[i])(t, toclientlib.ReqInf{}, serverDeliveryServices.Response, tc.Alerts{}, nil)
 

diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go b/traffic_ops/traffic_ops_golang/deliveryservice/servers/servers.go
@@ -945,7 +945,7 @@ func (dss *TODSSDeliveryService) Read(h http.Header, useIMS bool) ([]interface{}
 	}
 
 	where += `
-ds.id in (
+(ds.id in (
 	SELECT deliveryService FROM deliveryservice_server WHERE server = :server
 ) OR ds.id in (
 	SELECT id FROM deliveryservice
@@ -955,7 +955,18 @@ ds.id in (
 			SELECT name FROM cachegroup
 			WHERE id = (
 				SELECT cachegroup FROM server WHERE id = :server
-			))))
+			))))) 
+AND
+(( 
+(SELECT (t.name = 'ORG') FROM type t JOIN server s ON s.type = t.id WHERE s.id = :server) 
+OR 
+(SELECT COALESCE(ARRAY_AGG(ssc.server_capability), '{}') 
+FROM server_server_capability ssc 
+WHERE ssc."server" = :server) 
+@> 
+(
+SELECT COALESCE(ds.required_capabilities, '{}')
+)))
 `
 
 	tenantIDs, err := tenant.GetUserTenantIDListTx(tx, user.TenantID)

diff --git a/traffic_ops/traffic_ops_golang/server/servers_assignment.go b/traffic_ops/traffic_ops_golang/server/servers_assignment.go
@@ -305,7 +305,7 @@ func ValidateDSCapabilities(dsIDs []int, serverName string, tx *sql.Tx) (error,
 	for id, caps := range dsCaps {
 		for _, dsrc := range caps {
 			if !util.ContainsStr(sCaps, dsrc) {
-				return errors.New(fmt.Sprintf("cache %s cannot assign delivery service %d without having the required delivery service capabilities: %v", serverName, id, dsCaps)), nil, http.StatusBadRequest
+				return errors.New(fmt.Sprintf("cache %s cannot assign delivery service %d without having the required delivery service capabilities: %v", serverName, id, caps)), nil, http.StatusBadRequest
 			}
 		}
 	}

diff --git a/...tal/app/src/common/modules/table/deliveryServiceServers/TableAssignDSServersController.js b/...tal/app/src/common/modules/table/deliveryServiceServers/TableAssignDSServersController.js
@@ -33,7 +33,7 @@ var TableAssignDSServersController = function(deliveryService, servers, assigned
 		},
 		{
 			headerName: "Cache Group",
-			field: "cachegroup",
+			field: "cacheGroup",
 		},
 		{
 			headerName: "Profile(s)",