Fixed TO API /servers/{id}/deliveryservices endpoint to respond with …

…all DS's on cache that are directly assigned and inherited through topology. (#7520) * Fixes Issue: #7519 * Changelog updated for Issue: #7519 * Documentation updated for Issue: #7519 * Updated Integration tests for #7519 * CHANGELOG.md Update * Updated V3 integration tests for this change. * Fixed PR review comments * Fixed PR review comments
apache · Jun 2, 2023 · fffe8a3 · fffe8a3
1 parent 8a1140d
commit fffe8a3
Show file tree

Hide file tree

Showing 8 changed files with 98 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -60,6 +60,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#7539](https://github.com/apache/trafficcontrol/pull/7539) *Traffic Monitor* Use stats_over_http timestamp to calculate bandwidth for TM's health. 
 - [#7542](https://github.com/apache/trafficcontrol/pull/7542) *Traffic Ops* Fixed `CDN Locks` documentation to reflect the correct RFC3339 timestamps.
 - [#6340](https://github.com/apache/trafficcontrol/issues/6340) *Traffic Ops* Fixed alert messages for POST and PUT invalidation job APIs.
+- [#7519] (https://github.com/apache/trafficcontrol/issues/7519) *Traffic Ops* Fixed TO API /servers/{id}/deliveryservices endpoint to responding with all DS's on cache that are directly assigned and inherited through topology.
 - [#7511](https://github.com/apache/trafficcontrol/pull/7511) *Traffic Ops* Fixed the changelog registration message to include the username instead of duplicate email entry.
 - [#7465](https://github.com/apache/trafficcontrol/issues/7465) *Traffic Ops* Fixes server_capabilities v5 apis to respond with RFC3339 date/time Format
 - [#7441](https://github.com/apache/trafficcontrol/pull/7441) *Traffic Ops* Fixed the invalidation jobs endpoint to respect CDN locks.

diff --git a/docs/source/api/v3/servers_id_deliveryservices.rst b/docs/source/api/v3/servers_id_deliveryservices.rst
@@ -21,7 +21,7 @@
 
 ``GET``
 =======
-Retrieves all :term:`Delivery Services` assigned to a specific server.
+Retrieves all :term:`Delivery Services` assigned to a specific server either directly or inherited from topology.
 
 :Auth. Required: Yes
 :Roles Required: None\ [#tenancy]_

diff --git a/docs/source/api/v4/servers_id_deliveryservices.rst b/docs/source/api/v4/servers_id_deliveryservices.rst
@@ -21,7 +21,7 @@
 
 ``GET``
 =======
-Retrieves all :term:`Delivery Services` assigned to a specific server.
+Retrieves all :term:`Delivery Services` assigned to a specific server either directly or inherited from topology.
 
 :Auth. Required: Yes
 :Roles Required: None\ [#tenancy]_

diff --git a/docs/source/api/v5/servers_id_deliveryservices.rst b/docs/source/api/v5/servers_id_deliveryservices.rst
@@ -21,7 +21,7 @@
 
 ``GET``
 =======
-Retrieves all :term:`Delivery Services` assigned to a specific server.
+Retrieves all :term:`Delivery Services` assigned to a specific server either directly or inherited from topology.
 
 :Auth. Required: Yes
 :Roles Required: None\ [#tenancy]_

diff --git a/traffic_ops/testing/api/v3/servers_id_deliveryservices_test.go b/traffic_ops/testing/api/v3/servers_id_deliveryservices_test.go
@@ -55,7 +55,13 @@ func TestServersIDDeliveryServices(t *testing.T) {
 						"replace": true,
 					},
 					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
-						validateServersDeliveryServicesPost(GetServerID(t, "atlanta-edge-01")(), GetDeliveryServiceId(t, "ds1")())),
+						validateServersDeliveryServicesPost(
+							GetServerID(t, "atlanta-edge-01")(),
+							[]int{
+								GetDeliveryServiceId(t, "ds1")(),
+								GetDeliveryServiceId(t, "ds-based-top-with-no-mids")(),
+							},
+							2)),
 				},
 				"OK when ASSIGNING EDGE to TOPOLOGY BASED DELIVERY SERVICE": {
 					EndpointID:    GetServerID(t, "atlanta-edge-03"),
@@ -65,7 +71,12 @@ func TestServersIDDeliveryServices(t *testing.T) {
 						"replace": true,
 					},
 					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
-						validateServersDeliveryServicesPost(GetServerID(t, "atlanta-edge-03")(), GetDeliveryServiceId(t, "top-ds-in-cdn1")())),
+						validateServersDeliveryServicesPost(
+							GetServerID(t, "atlanta-edge-03")(),
+							[]int{
+								GetDeliveryServiceId(t, "top-ds-in-cdn1")(),
+							},
+							1)),
 				},
 				"OK when ASSIGNING ORIGIN to TOPOLOGY BASED DELIVERY SERVICE": {
 					EndpointID:    GetServerID(t, "denver-mso-org-01"),
@@ -75,7 +86,14 @@ func TestServersIDDeliveryServices(t *testing.T) {
 						"replace": true,
 					},
 					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
-						validateServersDeliveryServicesPost(GetServerID(t, "denver-mso-org-01")(), GetDeliveryServiceId(t, "ds-top")())),
+						validateServersDeliveryServicesPost(
+							GetServerID(t, "denver-mso-org-01")(),
+							[]int{
+								GetDeliveryServiceId(t, "ds-top")(),
+								GetDeliveryServiceId(t, "ds-top-req-cap2")(),
+								GetDeliveryServiceId(t, "ds-forked-topology")(),
+							},
+							3)),
 				},
 				"CONFLICT when SERVER NOT IN SAME CDN as DELIVERY SERVICE": {
 					EndpointID:    GetServerID(t, "cdn2-test-edge"),
@@ -160,11 +178,13 @@ func validateServersDeliveryServices(expectedDSID int) utils.CkReqFunc {
 	}
 }
 
-func validateServersDeliveryServicesPost(serverID int, expectedDSID int) utils.CkReqFunc {
+func validateServersDeliveryServicesPost(serverID int, expectedDSID []int, expectedDSCount int) utils.CkReqFunc {
 	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, 1, len(serverDeliveryServices), "Expected one Delivery Service returned Got: %d", len(serverDeliveryServices))
-		validateServersDeliveryServices(expectedDSID)(t, toclientlib.ReqInf{}, serverDeliveryServices, tc.Alerts{}, nil)
+		assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices), "Expected one Delivery Service returned Got: %d", 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
@@ -57,7 +57,13 @@ func TestServersIDDeliveryServices(t *testing.T) {
 						"replace": true,
 					},
 					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
-						validateServersDeliveryServicesPost(totest.GetServerID(t, TOSession, "atlanta-edge-01")(), totest.GetDeliveryServiceId(t, TOSession, "ds1")())),
+						validateServersDeliveryServicesPost(
+							totest.GetServerID(t, TOSession, "atlanta-edge-01")(),
+							[]int{
+								totest.GetDeliveryServiceId(t, TOSession, "ds1")(),
+								totest.GetDeliveryServiceId(t, TOSession, "ds-based-top-with-no-mids")(),
+							},
+							2)),
 				},
 				"OK when ASSIGNING EDGE to TOPOLOGY BASED DELIVERY SERVICE": {
 					EndpointID:    totest.GetServerID(t, TOSession, "atlanta-edge-03"),
@@ -67,7 +73,12 @@ func TestServersIDDeliveryServices(t *testing.T) {
 						"replace": true,
 					},
 					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
-						validateServersDeliveryServicesPost(totest.GetServerID(t, TOSession, "atlanta-edge-03")(), totest.GetDeliveryServiceId(t, TOSession, "top-ds-in-cdn1")())),
+						validateServersDeliveryServicesPost(
+							totest.GetServerID(t, TOSession, "atlanta-edge-03")(),
+							[]int{
+								totest.GetDeliveryServiceId(t, TOSession, "top-ds-in-cdn1")(),
+							},
+							1)),
 				},
 				"OK when ASSIGNING ORIGIN to TOPOLOGY BASED DELIVERY SERVICE": {
 					EndpointID:    totest.GetServerID(t, TOSession, "denver-mso-org-01"),
@@ -77,7 +88,14 @@ func TestServersIDDeliveryServices(t *testing.T) {
 						"replace": true,
 					},
 					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
-						validateServersDeliveryServicesPost(totest.GetServerID(t, TOSession, "denver-mso-org-01")(), totest.GetDeliveryServiceId(t, TOSession, "ds-top")())),
+						validateServersDeliveryServicesPost(
+							totest.GetServerID(t, TOSession, "denver-mso-org-01")(),
+							[]int{
+								totest.GetDeliveryServiceId(t, TOSession, "ds-top")(),
+								totest.GetDeliveryServiceId(t, TOSession, "ds-top-req-cap2")(),
+								totest.GetDeliveryServiceId(t, TOSession, "ds-forked-topology")(),
+							},
+							3)),
 				},
 				"CONFLICT when SERVER NOT IN SAME CDN as DELIVERY SERVICE": {
 					EndpointID:    totest.GetServerID(t, TOSession, "cdn2-test-edge"),
@@ -171,11 +189,14 @@ func validateServersDeliveryServices(expectedDSID int) utils.CkReqFunc {
 	}
 }
 
-func validateServersDeliveryServicesPost(serverID int, expectedDSID int) utils.CkReqFunc {
+func validateServersDeliveryServicesPost(serverID int, expectedDSID []int, expectedDSCount int) utils.CkReqFunc {
 	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, 1, len(serverDeliveryServices.Response), "Expected one Delivery Service returned Got: %d", len(serverDeliveryServices.Response))
-		validateServersDeliveryServices(expectedDSID)(t, toclientlib.ReqInf{}, serverDeliveryServices.Response, tc.Alerts{}, nil)
+		assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices.Response), "Expected Two Delivery Service returned Got: %d", 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/servers_id_deliveryservices_test.go b/traffic_ops/testing/api/v5/servers_id_deliveryservices_test.go
@@ -56,7 +56,13 @@ func TestServersIDDeliveryServices(t *testing.T) {
 						"replace": true,
 					},
 					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
-						validateServersDeliveryServicesPost(GetServerID(t, "atlanta-edge-01")(), GetDeliveryServiceId(t, "ds1")())),
+						validateServersDeliveryServicesPost(
+							GetServerID(t, "atlanta-edge-01")(),
+							[]int{
+								GetDeliveryServiceId(t, "ds1")(),
+								GetDeliveryServiceId(t, "ds-based-top-with-no-mids")(),
+							},
+							2)),
 				},
 				"OK when ASSIGNING EDGE to TOPOLOGY BASED DELIVERY SERVICE": {
 					EndpointID:    GetServerID(t, "atlanta-edge-03"),
@@ -66,7 +72,12 @@ func TestServersIDDeliveryServices(t *testing.T) {
 						"replace": true,
 					},
 					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
-						validateServersDeliveryServicesPost(GetServerID(t, "atlanta-edge-03")(), GetDeliveryServiceId(t, "top-ds-in-cdn1")())),
+						validateServersDeliveryServicesPost(
+							GetServerID(t, "atlanta-edge-03")(),
+							[]int{
+								GetDeliveryServiceId(t, "top-ds-in-cdn1")(),
+							},
+							1)),
 				},
 				"OK when ASSIGNING ORIGIN to TOPOLOGY BASED DELIVERY SERVICE": {
 					EndpointID:    GetServerID(t, "denver-mso-org-01"),
@@ -76,7 +87,14 @@ func TestServersIDDeliveryServices(t *testing.T) {
 						"replace": true,
 					},
 					Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK),
-						validateServersDeliveryServicesPost(GetServerID(t, "denver-mso-org-01")(), GetDeliveryServiceId(t, "ds-top")())),
+						validateServersDeliveryServicesPost(
+							GetServerID(t, "denver-mso-org-01")(),
+							[]int{
+								GetDeliveryServiceId(t, "ds-top")(),
+								GetDeliveryServiceId(t, "ds-top-req-cap2")(),
+								GetDeliveryServiceId(t, "ds-forked-topology")(),
+							},
+							3)),
 				},
 				"CONFLICT when SERVER NOT IN SAME CDN as DELIVERY SERVICE": {
 					EndpointID:    GetServerID(t, "cdn2-test-edge"),
@@ -170,11 +188,14 @@ func validateServersDeliveryServices(expectedDSID int) utils.CkReqFunc {
 	}
 }
 
-func validateServersDeliveryServicesPost(serverID int, expectedDSID int) utils.CkReqFunc {
+func validateServersDeliveryServicesPost(serverID int, expectedDSID []int, expectedDSCount int) utils.CkReqFunc {
 	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, 1, len(serverDeliveryServices.Response), "Expected one Delivery Service returned Got: %d", len(serverDeliveryServices.Response))
-		validateServersDeliveryServices(expectedDSID)(t, toclientlib.ReqInf{}, serverDeliveryServices.Response, tc.Alerts{}, nil)
+		assert.RequireEqual(t, expectedDSCount, len(serverDeliveryServices.Response), "Expected Two Delivery Service returned Got: %d", 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
@@ -895,7 +895,20 @@ func (dss *TODSSDeliveryService) Read(h http.Header, useIMS bool) ([]interface{}
 	} else {
 		where = "WHERE "
 	}
-	where += "ds.id in (SELECT deliveryService FROM deliveryservice_server where server = :server)"
+
+	where += `
+ds.id in (
+	SELECT deliveryService FROM deliveryservice_server WHERE server = :server
+) OR ds.id in (
+	SELECT id FROM deliveryservice 
+	WHERE topology in ( 
+		SELECT topology FROM topology_cachegroup 
+		WHERE cachegroup = ( 
+			SELECT name FROM cachegroup 
+			WHERE id = ( 
+				SELECT cachegroup FROM server WHERE id = :server 
+			))))
+`
 
 	tenantIDs, err := tenant.GetUserTenantIDListTx(tx, user.TenantID)
 	if err != nil {