This is an automated email from the ASF dual-hosted git repository.
ocket8888 pushed a commit to branch 5.0.x
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/5.0.x by this push:
new 38c86ba Validate ORG server for topology based DS (#5307)
38c86ba is described below
commit 38c86ba87d45ca09cafd648384023e046a2efbfb
Author: rimashah25 <[email protected]>
AuthorDate: Wed Dec 2 15:20:10 2020 -0700
Validate ORG server for topology based DS (#5307)
* Edited updateV30() to check if ORG server exists and is part of topology
cachegroup
* Added helper function call to db_helpers.go
* Updated code based on review feedback
* Removed fmt statements
* Added test case. Formatted code.
* Updated CHANGELOG.md
* Updated based on feedback.
* Updated testcase
(cherry picked from commit 38fc19e5af2cb4a2dca168dd2ccbbc81654dbe47)
---
CHANGELOG.md | 1 +
.../testing/api/v3/deliveryservices_test.go | 48 ++++++++++++++++++++
.../traffic_ops_golang/dbhelpers/db_helpers.go | 53 +++++++++++++++++++++-
.../deliveryservice/deliveryservices.go | 5 ++
.../topology_validation/topology_validation.go | 5 +-
5 files changed, 109 insertions(+), 3 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5245584..d3fd9f1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -73,6 +73,7 @@ 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)
diff --git a/traffic_ops/testing/api/v3/deliveryservices_test.go
b/traffic_ops/testing/api/v3/deliveryservices_test.go
index 5842990..c51ccf8 100644
--- a/traffic_ops/testing/api/v3/deliveryservices_test.go
+++ b/traffic_ops/testing/api/v3/deliveryservices_test.go
@@ -48,6 +48,7 @@ func TestDeliveryServices(t *testing.T) {
GetTestDeliveryServicesIMS(t)
GetAccessibleToTest(t)
UpdateTestDeliveryServices(t)
+ UpdateValidateORGServerCacheGroup(t)
UpdateTestDeliveryServicesWithHeaders(t, header)
UpdateNullableTestDeliveryServices(t)
UpdateDeliveryServiceWithInvalidRemapText(t)
@@ -880,6 +881,53 @@ func UpdateDeliveryServiceWithInvalidSliceRangeRequest(t
*testing.T) {
}
+// UpdateValidateORGServerCacheGroup validates ORG server's cachegroup are
part of topology's cachegroup
+func UpdateValidateORGServerCacheGroup(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)
+ }
+
+ //Update DS's Topology to a non-ORG server's cachegroup
+ origTopo := *remoteDS[0].Topology
+ *remoteDS[0].Topology = "4-tiers"
+ ds, reqInf, err :=
TOSession.UpdateDeliveryServiceV30WithHdr(*remoteDS[0].ID, remoteDS[0], nil)
+ if err == nil {
+ t.Errorf("shouldnot UPDATE DeliveryService by ID: %v, but
update was successful", ds.XMLID)
+ }
+ if reqInf.StatusCode != http.StatusBadRequest {
+ t.Fatalf("expected to fail since ORG server's topology not part
of DS. Expected:%v, Got: :%v", http.StatusBadRequest, reqInf.StatusCode)
+ }
+
+ // Retrieve the DS to check if topology was updated with missing ORG
server
+ params.Set("id", strconv.Itoa(*remoteDS[0].ID))
+ apiResp, _, err := TOSession.GetDeliveryServicesV30WithHdr(nil, params)
+ if err != nil {
+ t.Fatalf("cannot GET Delivery Service by ID: %v - %v",
*remoteDS[0].XMLID, err)
+ }
+ if len(apiResp) < 1 {
+ t.Fatalf("cannot GET Delivery Service by ID: %v - nil",
*remoteDS[0].XMLID)
+ }
+
+ //Set topology back to as it was for further testing
+ remoteDS[0].Topology = &origTopo
+ _, _, err = TOSession.UpdateDeliveryServiceV30WithHdr(*remoteDS[0].ID,
remoteDS[0], nil)
+ if err != nil {
+ t.Fatalf("couldn't update topology:%v, %v",
*remoteDS[0].Topology, err)
+ }
+}
+
func GetAccessibleToTest(t *testing.T) {
//Every delivery service is associated with the root tenant
err := getByTenants(1, len(testData.DeliveryServices))
diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
index 53edd6d..09a9e06 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
@@ -23,7 +23,6 @@ import (
"database/sql"
"errors"
"fmt"
- "github.com/jmoiron/sqlx"
"net/http"
"strconv"
"strings"
@@ -32,6 +31,7 @@ import (
"github.com/apache/trafficcontrol/lib/go-tc"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/topology/topology_validation"
+ "github.com/jmoiron/sqlx"
"github.com/lib/pq"
)
@@ -1090,3 +1090,54 @@ 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) {
+ // get servers and respective cachegroup name that have ORG type in a
delivery service
+ q := `
+ SELECT s.host_name, c.name
+ FROM server s
+ INNER JOIN deliveryservice_server ds ON ds.server = s.id
+ INNER JOIN type t ON t.id = s.type
+ INNER JOIN cachegroup c ON c.id = s.cachegroup
+ WHERE ds.deliveryservice=$1 AND t.name=$2
+ `
+
+ serverName := ""
+ cacheGroupName := ""
+ servers := make(map[string]string)
+ var offendingSCG []string
+ rows, err := tx.Query(q, dsID, tc.OriginTypeName)
+ 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(&serverName, &cacheGroupName); err != nil {
+ return nil, fmt.Errorf("querying deliveryservice origin
server: %s", err), http.StatusInternalServerError
+ }
+ servers[cacheGroupName] = serverName
+ }
+
+ if len(servers) > 0 {
+ _, 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
+ topoCachegroups := make(map[string]string)
+ for _, cg := range cachegroups {
+ topoCachegroups[cg] = ""
+ }
+ for cg, s := range servers {
+ _, ok := topoCachegroups[cg]
+ if !ok {
+ offendingSCG = append(offendingSCG,
fmt.Sprintf("%s (%s)", cg, s))
+ }
+ }
+ }
+ if len(offendingSCG) > 0 {
+ return errors.New("the following ORG server cachegroups are not
in the delivery service's topology (" + dsTopology + "): " +
strings.Join(offendingSCG, ", ")), 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 199058a..eb1bad3 100644
--- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
+++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
@@ -796,6 +796,11 @@ func updateV30(w http.ResponseWriter, r *http.Request, inf
*api.APIInfo, ds *tc.
return nil, status, userErr, sysErr
}
}
+
+ userErr, sysErr, status :=
dbhelpers.CheckOriginServerInCacheGroupTopology(tx, *ds.ID, *ds.Topology)
+ if userErr != nil || sysErr != nil {
+ return nil, status, userErr, sysErr
+ }
}
resultRows, err := tx.Query(updateDSQuery(),
diff --git
a/traffic_ops/traffic_ops_golang/topology/topology_validation/topology_validation.go
b/traffic_ops/traffic_ops_golang/topology/topology_validation/topology_validation.go
index e91f9e7..de19198 100644
---
a/traffic_ops/traffic_ops_golang/topology/topology_validation/topology_validation.go
+++
b/traffic_ops/traffic_ops_golang/topology/topology_validation/topology_validation.go
@@ -25,11 +25,12 @@ package topology_validation
import (
"errors"
"fmt"
- "github.com/jmoiron/sqlx"
- "github.com/lib/pq"
"strings"
"github.com/apache/trafficcontrol/lib/go-log"
+
+ "github.com/jmoiron/sqlx"
+ "github.com/lib/pq"
)
// CheckForEmptyCacheGroups checks if the cachegroups are empty (altogether)
or empty in any of the given CDN IDs.
