Validate DS's ORG server's cachegroup with topology's cachegroup

CHANGELOG.md

@@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - #5344 - Add a page that addresses migrating from Traffic Ops API v1 for each endpoint
 - [#5296](https://github.com/apache/trafficcontrol/issues/5296) - Fixed a bug where users couldn't update any regex in Traffic Ops/ Traffic Portal
 - Added API endpoints for ACME accounts
+- Traffic Ops: Added validation to ensure that the cachegroups of a delivery services' assigned ORG servers are present in the topology
 
 ### Fixed
 - [#5195](https://github.com/apache/trafficcontrol/issues/5195) - Correctly show CDN ID in Changelog during Snap
@@ -57,6 +58,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
     - Traffic Router: Added support for topology-based delivery services
     - Traffic Monitor: Added the ability to mark topology-based delivery services as available
     - CDN-in-a-Box: Add a second mid to CDN-in-a-Box, add topology `demo1-top`, and make the `demo1` delivery service topology-based
+    - Traffic Ops: Added validation to ensure assigned ORG server cachegroups are in the topology when updating a delivery service
 - Updated /servers/details to use multiple interfaces in API v3
 - Added [Edge Traffic Routing](https://traffic-control-cdn.readthedocs.io/en/latest/admin/traffic_router.html#edge-traffic-routing) feature which allows Traffic Router to localize more DNS record types than just the routing name for DNS delivery services
 - Added the ability to speedily build development RPMs from any OS without needing Docker
@@ -93,7 +95,6 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Traffic Portal: upgraded change log UI table to use more powerful/performant ag-grid component
 - Traffic Portal: change log days are now configurable in traffic_portal_properties.json (default is 7 days) and can be overridden by the user in TP
 - [#5319](https://github.com/apache/trafficcontrol/issues/5319) - Added support for building RPMs that target CentOS 8
-- Traffic Ops: Added validation to ensure assigned ORG server cachegroups are in the topology when updating a delivery service
 
 ### Fixed
 - Fixed #5188 - DSR (delivery service request) incorrectly marked as complete and error message not displaying when DSR fulfilled and DS update fails in Traffic Portal. [Related Github issue](https://github.com/apache/trafficcontrol/issues/5188)
@@ -158,7 +159,6 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - #5191 - Error from IMS requests to /federations/all
 - Fixed Astats csv issue where it could crash if caches dont return proc data
 
-
 ### Changed
 - Changed some Traffic Ops Go Client methods to use `DeliveryServiceNullable` inputs and outputs.
 - When creating invalidation jobs through TO/TP, if an identical regex is detected that overlaps its time, then warnings

traffic_ops/testing/api/v3/topologies_test.go

@@ -39,6 +39,7 @@ func TestTopologies(t *testing.T) {
 	WithObjs(t, []TCObj{Types, CacheGroups, CDNs, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, Servers, ServerCapabilities, ServerServerCapabilitiesForTopologies, Topologies, Tenants, DeliveryServices, TopologyBasedDeliveryServiceRequiredCapabilities}, func() {
 		UpdateTestTopologies(t)
 		ValidationTestTopologies(t)
+		UpdateValidateTopologyORGServerCacheGroup(t)
 		EdgeParentOfEdgeSucceedsWithWarning(t)
 	})
 }
@@ -240,6 +241,55 @@ func UpdateTestTopologies(t *testing.T) {
 	}
 }
 
+func UpdateValidateTopologyORGServerCacheGroup(t *testing.T) {
+	params := url.Values{}
+	params.Set("xmlId", "ds-top")
+
+	//Get the correct DS
+	remoteDS, _, err := TOSession.GetDeliveryServicesV30WithHdr(nil, params)
+	if err != nil {
+		t.Errorf("cannot GET Delivery Services: %v", err)
+	}
+
+	//Assign ORG server to DS
+	assignServer := []string{"denver-mso-org-01"}
+	_, _, err = TOSession.AssignServersToDeliveryService(assignServer, *remoteDS[0].XMLID)
+	if err != nil {
+		t.Errorf("cannot assign server to Delivery Services: %v", err)
+	}
+
+	//Get Topology node to update and remove ORG server nodes
+	origTopo := *remoteDS[0].Topology
+	resp, _, err := TOSession.GetTopologyWithHdr(origTopo, nil)
+	if err != nil {
+		t.Fatalf("couldn't find any topologies: %v", err)
+	}
+
+	// remove org server cachegroup
+	var p []int
+	newNodes := []tc.TopologyNode{{Id: 0, Cachegroup: "topology-edge-cg-01", Parents: p, LastUpdated: nil}}
+	if *remoteDS[0].Topology == resp.Name {
+		resp.Nodes = newNodes
+	}
+	_, _, err = TOSession.UpdateTopology(*remoteDS[0].Topology, *resp)
+	if err == nil {
+		t.Fatalf("shouldnot UPDATE topology:%v to %v, but update was a success", *remoteDS[0].Topology, newNodes[0].Cachegroup)
+	} else if !strings.Contains(err.Error(), "ORG servers are assigned to delivery services that use this topology, and their cachegroups cannot be removed:") {
+		t.Errorf("expected error messsage containing: \"ORG servers are assigned to delivery services that use this topology, and their cachegroups cannot be removed\", got:%s", err.Error())
+
+	}
+
+	//TODO: Need to fix the query in deliveryservice/servers/delete.go for DeleteDeliveryServiceServer() to work correctly.
+
+	// Remove org server assignment and reset DS back to as it was for further testing
+	//params.Set("hostName", "denver-mso-org-01")
+	//serverResp, _, err := TOSession.GetServersWithHdr(&params, nil)
+	//_, _, err = TOSession.DeleteDeliveryServiceServer(*remoteDS[0].ID, *serverResp.Response[0].ID)
+	//if err != nil {
+	//	t.Errorf("cannot delete assigned server from Delivery Services: %v", err)
+	//}
+}
+
 func DeleteTestTopologies(t *testing.T) {
 	for _, top := range testData.Topologies {
 		delResp, _, err := TOSession.DeleteTopology(top.Name)

traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go

@@ -869,7 +869,7 @@ func CheckTopology(tx *sqlx.Tx, ds tc.DeliveryServiceNullableV30) (int, error, e
 	}
 
 	if err = topology_validation.CheckForEmptyCacheGroups(tx, cacheGroupIDs, []int{*ds.CDNID}, true, []int{}); err != nil {
-		return http.StatusBadRequest, fmt.Errorf("empty cachegroups in Topology %s found for CDN %d: %s", *ds.Topology, ds.CDNID, err.Error()), nil
+		return http.StatusBadRequest, fmt.Errorf("empty cachegroups in Topology %s found for CDN %d: %s", *ds.Topology, *ds.CDNID, err.Error()), nil
 	}
 
 	return statusCode, userErr, sysErr
@@ -1100,8 +1100,8 @@ GROUP BY t.name, ds.topology
 	return dsType, reqCap, topology, true, nil
 }
 
-// CheckOriginServerInCacheGroupTopology checks if a DS has ORG server and if it does, to make sure the cachegroup is part of DS
-func CheckOriginServerInCacheGroupTopology(tx *sql.Tx, dsID int, dsTopology string) (error, error, int) {
+// CheckOriginServerInDSCG checks if a DS has ORG server and if it does, to make sure the cachegroup is part of DS
+func CheckOriginServerInDSCG(tx *sql.Tx, dsID int, dsTopology string) (error, error, int) {
 	// get servers and respective cachegroup name that have ORG type in a delivery service
 	q := `
 		SELECT s.host_name, c.name 
@@ -1129,11 +1129,12 @@ func CheckOriginServerInCacheGroupTopology(tx *sql.Tx, dsID int, dsTopology stri
 	}
 
 	if len(servers) > 0 {
+		//Validation for DS
 		_, cachegroups, sysErr := GetTopologyCachegroups(tx, dsTopology)
 		if sysErr != nil {
 			return nil, fmt.Errorf("validating %s servers in topology: %v", tc.OriginTypeName, sysErr), http.StatusInternalServerError
 		}
-		// put slice values into map
+		// put slice values into map for DS's validation
 		topoCachegroups := make(map[string]string)
 		for _, cg := range cachegroups {
 			topoCachegroups[cg] = ""
@@ -1150,3 +1151,52 @@ func CheckOriginServerInCacheGroupTopology(tx *sql.Tx, dsID int, dsTopology stri
 	}
 	return nil, nil, http.StatusOK
 }
+
+// CheckTopologyOrgServerCGInDSCG checks if ORG server are part of DS. IF they are then the user is not allowed to remove the ORG servers from the associated DS's topology
+func CheckTopologyOrgServerCGInDSCG(tx *sql.Tx, cdnIds []int, dsTopology string, topologyCGNames []string) (error, error, int) {
+	// get servers and respective cachegroup name that have ORG type for evert delivery service
+	q := `
+		SELECT ARRAY_AGG(d.xml_id), s.id, c.name
+		FROM server s
+			INNER JOIN deliveryservice_server ds ON ds.server = s.id
+			INNER JOIN deliveryservice d ON d.id = ds.deliveryservice
+			INNER JOIN type t ON t.id = s.type
+			INNER JOIN cachegroup c ON c.id = s.cachegroup
+		WHERE d.cdn_id =ANY($1) AND t.name=$2 AND d.topology=$3
+		GROUP BY s.id, c.name
+	`
+	serverId := ""
+	cacheGroupName := ""
+	dsNames := []string{}
+	serversCG := make(map[string]string)
+	serversDS := make(map[string][]string)
+	rows, err := tx.Query(q, pq.Array(cdnIds), tc.OriginTypeName, dsTopology)
+	if err != nil {
+		return nil, fmt.Errorf("querying deliveryservice origin server: %s", err), http.StatusInternalServerError
+	}
+	defer log.Close(rows, "error closing rows")
+	for rows.Next() {
+		if err := rows.Scan(pq.Array(&dsNames), &serverId, &cacheGroupName); err != nil {
+			return nil, fmt.Errorf("querying deliveryservice origin server: %s", err), http.StatusInternalServerError
+		}
+		serversCG[cacheGroupName] = serverId
+		serversDS[serverId] = dsNames
+	}
+
+	var offendingDSSerCG []string
+	// put slice values into map for Topology's validation
+	topoCacheGroupNames := make(map[string]string)
+	for _, currentCG := range topologyCGNames {
+		topoCacheGroupNames[currentCG] = ""
+	}
+	for cg, s := range serversCG {
+		_, currentTopoCGOk := topoCacheGroupNames[cg]
+		if !currentTopoCGOk {
+			offendingDSSerCG = append(offendingDSSerCG, fmt.Sprintf("cachegroup=%s (serverID=%s, delivery_services=%s)", cg, s, serversDS[s]))
+		}
+	}
+	if len(offendingDSSerCG) > 0 {
+		return errors.New("ORG servers are assigned to delivery services that use this topology, and their cachegroups cannot be removed: " + strings.Join(offendingDSSerCG, ", ")), nil, http.StatusBadRequest
+	}
+	return nil, nil, http.StatusOK
+}

traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go

@@ -881,7 +881,7 @@ func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 *
 			}
 		}
 
-		userErr, sysErr, status := dbhelpers.CheckOriginServerInCacheGroupTopology(tx, *ds.ID, *ds.Topology)
+		userErr, sysErr, status := dbhelpers.CheckOriginServerInDSCG(tx, *ds.ID, *ds.Topology)
 		if userErr != nil || sysErr != nil {
 			return nil, status, userErr, sysErr
 		}

traffic_ops/traffic_ops_golang/deliveryservice/servers/delete.go

@@ -44,6 +44,7 @@ func DeleteDeprecated(w http.ResponseWriter, r *http.Request) {
 	delete(w, r, true)
 }
 
+//TODO: Check if the query works correctly when last assigned server is a ORG server
 const lastServerQuery = `
 SELECT COUNT(*) = 0
 FROM deliveryservice_server

traffic_ops/traffic_ops_golang/topology/topologies.go

@@ -150,6 +150,17 @@ func (topology *TOTopology) Validate() error {
 	rules["empty cachegroups"] = topology_validation.CheckForEmptyCacheGroups(topology.ReqInfo.Tx, cacheGroupIds, dsCDNs, false, nil)
 	rules["required capabilities"] = topology.validateDSRequiredCapabilities()
 
+	//Get current Topology-CG for the requested change.
+	topoCachegroupNames := topology.getCachegroupNames()
+	userErr, sysErr, _ = dbhelpers.CheckTopologyOrgServerCGInDSCG(topology.ReqInfo.Tx.Tx, dsCDNs, topology.Name, topoCachegroupNames)
+	if userErr != nil {
+		return userErr
+	}
+	if sysErr != nil {
+		log.Errorf("error while validate topology: %s", sysErr.Error())
+		return errors.New("unable to validate topology")
+	}
+
 	/* Only perform further checks if everything so far is valid */
 	if err = util.JoinErrs(tovalidate.ToErrors(rules)); err != nil {
 		return err