diff --git a/CHANGELOG.md b/CHANGELOG.md index 25a8fe8a21..b7ab9ab4df 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/traffic_ops/testing/api/v3/topologies_test.go b/traffic_ops/testing/api/v3/topologies_test.go index 4350c8ed85..e2dff86d05 100644 --- a/traffic_ops/testing/api/v3/topologies_test.go +++ b/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(¶ms, 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) diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go index 1b8c2ca9f1..8908e6e742 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go +++ b/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 +} diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go index 95def1e92e..1d9a89d388 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go +++ b/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 } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/servers/delete.go b/traffic_ops/traffic_ops_golang/deliveryservice/servers/delete.go index e54eaddd0a..421d776e57 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/servers/delete.go +++ b/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 diff --git a/traffic_ops/traffic_ops_golang/topology/topologies.go b/traffic_ops/traffic_ops_golang/topology/topologies.go index 10f6edf318..4e0f9f532f 100644 --- a/traffic_ops/traffic_ops_golang/topology/topologies.go +++ b/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