Skip to content

Commit

Permalink
Prevent unassigning all ONLINE ORG servers from active MSO-enabled DS
Browse files Browse the repository at this point in the history 
Also, fix some cases where it was possible to delete/unassign all
`EDGE`-type servers from a delivery service if there were also
`ORG`-type servers assigned.

Closes: apache#6069
Closes: apache#5212
  • Loading branch information
@rawlinp
rawlinp committed Jul 30, 2021
1 parent c5542ee commit a05fe85
Show file tree
Hide file tree
Showing 10 changed files with 379 additions and 228 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Traffic Ops: Adds API endpoints to fetch (GET), create (POST) or delete (DELETE) a cdn notification. Create and delete are limited to users with operations or admin role.
- Added ACME certificate renewals and ACME account registration using external account binding
- Added functionality to automatically renew ACME certificates.
- Traffic Ops: [#6069](https://github.com/apache/trafficcontrol/issues/6069) - prevent unassigning all ONLINE ORG servers from an MSO-enabled delivery service
- Added ORT flag to set local.dns bind address from server service addresses
- Added an endpoint for statuses on asynchronous jobs and applied it to the ACME renewal endpoint.
- Added two new cdn.conf options to make Traffic Vault configuration more backend-agnostic: `traffic_vault_backend` and `traffic_vault_config`
Expand Down
116 changes: 73 additions & 43 deletions traffic_ops/testing/api/v4/deliveryserviceservers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,69 +86,92 @@ func TryToRemoveLastServerInDeliveryService(t *testing.T) {
t.Fatalf("Need at least one status with a name other than '%s' or '%s'", tc.CacheStatusOnline, tc.CacheStatusReported)
}

// TODO: this isn't sufficient to define a single server, so there might be
// a better way to do this.
opts.QueryParameters = url.Values{}
opts.QueryParameters.Set("hostName", dssaTestingXMLID)
opts.QueryParameters.Set("domainName", dssaTestingXMLID)
servers, _, err := TOSession.GetServers(opts)
server := getServer(dssaTestingXMLID, t)
orgServer := getServer("test-mso-org-01", t)

resp, _, err := TOSession.CreateDeliveryServiceServers(*ds.ID, []int{*server.ID, *orgServer.ID}, true, client.RequestOptions{})
if err != nil {
t.Fatalf("Unexpected error fetching server '%s.%s': %v - alerts: %+v", dssaTestingXMLID, dssaTestingXMLID, err, servers.Alerts)
}
if len(servers.Response) != 1 {
t.Fatalf("Expected exactly one server with FQDN '%s.%s', got: %d", dssaTestingXMLID, dssaTestingXMLID, len(servers.Response))
}
server := servers.Response[0]
if server.ID == nil {
t.Fatal("Server had null/undefined ID after creation")
t.Fatalf("Failed to assign servers to Delivery Service: %v - alerts: %+v", err, resp.Alerts)
}

resp, _, err := TOSession.CreateDeliveryServiceServers(*ds.ID, []int{*server.ID}, true, client.RequestOptions{})
if err != nil {
t.Fatalf("Failed to assign server to Delivery Service: %v - alerts: %+v", err, resp.Alerts)
_, _, err = TOSession.CreateDeliveryServiceServers(*ds.ID, []int{*orgServer.ID}, true, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to remove the only EDGE server assigned to a Delivery Service")
}

_, _, err = TOSession.CreateDeliveryServiceServers(*ds.ID, []int{}, true, client.RequestOptions{})
_, _, err = TOSession.CreateDeliveryServiceServers(*ds.ID, []int{*server.ID}, true, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to remove the only server assigned to a Delivery Service")
t.Error("Didn't get expected error trying to remove the only ORG server assigned to an MSO-enabled Delivery Service")
}

_, _, err = TOSession.DeleteDeliveryServiceServer(*ds.ID, *server.ID, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to remove the only server assigned to a Delivery Service")
t.Error("Didn't get expected error trying to remove the only EDGE server assigned to a Delivery Service")
}

_, _, err = TOSession.DeleteDeliveryServiceServer(*ds.ID, *orgServer.ID, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to remove the only ORG server assigned to an MSO-enabled Delivery Service")
}

alerts, _, err := TOSession.DeleteServer(*server.ID, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to delete the only server assigned to a Delivery Service")
t.Error("Didn't get expected error trying to delete the only EDGE server assigned to a Delivery Service")
} else {
t.Logf("Got expected error trying to delete the only EDGE server assigned to a Delivery Service: %v - alerts: %+v", err, alerts.Alerts)
}

alerts, _, err = TOSession.DeleteServer(*orgServer.ID, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to delete the only ORG server assigned to an MSO-enabled Delivery Service")
} else {
t.Logf("Got expected error trying to delete the only server assigned to a Delivery Service: %v - alerts: %+v", err, alerts.Alerts)
t.Logf("Got expected error trying to delete the only ORG server assigned to an MSO-enabled Delivery Service: %v - alerts: %+v", err, alerts.Alerts)
}

alerts, _, err = TOSession.AssignDeliveryServiceIDsToServerID(*server.ID, []int{}, true, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to remove a Delivery Service from the only server to which it is assigned")
t.Error("Didn't get expected error trying to remove a Delivery Service from the only EDGE server to which it is assigned")
} else {
t.Logf("Got expected error trying to remove a Delivery Service from the only EDGE server to which it is assigned: %v - alerts: %+v", err, alerts.Alerts)
}

alerts, _, err = TOSession.AssignDeliveryServiceIDsToServerID(*orgServer.ID, []int{}, true, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to remove a Delivery Service from the only EDGE server to which it is assigned")
} else {
t.Logf("Got expected error trying to remove a Delivery Service from the only server to which it is assigned: %v - alerts: %+v", err, alerts.Alerts)
t.Logf("Got expected error trying to remove a Delivery Service from the only EDGE server to which it is assigned: %v - alerts: %+v", err, alerts.Alerts)
}

server.StatusID = &badStatusID
orgServer.StatusID = &badStatusID
putRequest := tc.ServerPutStatus{
Status: util.JSONNameOrIDStr{ID: &badStatusID},
OfflineReason: util.StrPtr("test"),
}
alerts, _, err = TOSession.UpdateServerStatus(*server.ID, putRequest, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to put server into a bad state when it's the only one assigned to a Delivery Service")
t.Error("Didn't get expected error trying to put server into a bad state when it's the only EDGE assigned to a Delivery Service")
} else {
t.Logf("Got expected error trying to put server into a bad state when it's the only EDGE assigned to a Delivery Service: %v - alerts: %+v", err, alerts.Alerts)
}
alerts, _, err = TOSession.UpdateServerStatus(*orgServer.ID, putRequest, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to put server into a bad state when it's the only ORG assigned to an MSO Delivery Service")
} else {
t.Logf("Got expected error trying to put server into a bad state when it's the only one assigned to a Delivery Service: %v - alerts: %+v", err, alerts.Alerts)
t.Logf("Got expected error trying to put server into a bad state when it's the only ORG assigned to an MSO Delivery Service: %v - alerts: %+v", err, alerts.Alerts)
}

alerts, _, err = TOSession.UpdateServer(*server.ID, server, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to put server into a bad state when it's the only one assigned to a Delivery Service")
t.Error("Didn't get expected error trying to put server into a bad state when it's the only EDGE assigned to a Delivery Service")
} else {
t.Logf("Got expected error trying to put server into a bad state when it's the only one assigned to a Delivery Service: %v - alerts: %+v", err, alerts.Alerts)
t.Logf("Got expected error trying to put server into a bad state when it's the only EDGE assigned to a Delivery Service: %v - alerts: %+v", err, alerts.Alerts)
}
alerts, _, err = TOSession.UpdateServer(*orgServer.ID, orgServer, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to put server into a bad state when it's the only ORG assigned to an MSO Delivery Service")
} else {
t.Logf("Got expected error trying to put server into a bad state when it's the only ORG assigned to an MSO Delivery Service: %v - alerts: %+v", err, alerts.Alerts)
}

server.HostName = util.StrPtr(dssaTestingXMLID + "-quest")
Expand All @@ -170,22 +193,11 @@ func TryToRemoveLastServerInDeliveryService(t *testing.T) {
if err != nil {
t.Fatalf("Failed to create server: %v - alerts: %+v", err, alerts.Alerts)
}
opts.QueryParameters.Set("hostName", *server.HostName)
servers, _, err = TOSession.GetServers(opts)
if err != nil {
t.Fatalf("Could not fetch server after creation: %v - alerts: %+v", err, servers.Alerts)
}
if len(servers.Response) != 1 {
t.Fatalf("Expected exactly 1 server with hostname '%s'; got: %d", *server.HostName, len(servers.Response))
}
server = servers.Response[0]
if server.ID == nil {
t.Fatal("Server had null/undefined ID after creation")
}
server = getServer(*server.HostName, t)

_, _, err = TOSession.CreateDeliveryServiceServers(*ds.ID, []int{*server.ID}, true, client.RequestOptions{})
if err == nil {
t.Error("Didn't get expected error trying to replace the last server assigned to a Delivery Service with a server in a bad state")
t.Error("Didn't get expected error trying to replace the last EDGE server assigned to a Delivery Service with a server in a bad state")
}

// Cleanup
Expand All @@ -196,6 +208,24 @@ func TryToRemoveLastServerInDeliveryService(t *testing.T) {
}
}

func getServer(hostname string, t *testing.T) tc.ServerV4 {
opts := client.NewRequestOptions()
opts.QueryParameters = url.Values{}
opts.QueryParameters.Set("hostName", hostname)
servers, _, err := TOSession.GetServers(opts)
if err != nil {
t.Fatalf("Unexpected error fetching server '%s': %v - alerts: %+v", hostname, err, servers.Alerts)
}
if len(servers.Response) != 1 {
t.Fatalf("Expected exactly one server with FQDN '%s', got: %d", hostname, len(servers.Response))
}
server := servers.Response[0]
if server.ID == nil {
t.Fatal("Server had null/undefined ID after creation")
}
return server
}

func GetTestDSSIMS(t *testing.T) {
const noLimit = 999999

Expand Down Expand Up @@ -409,7 +439,7 @@ func CreateTestDeliveryServiceServersWithRequiredCapabilities(t *testing.T) {
{
serverName: "atlanta-edge-01",
description: "missing requirements for server -> DS assignment",
err: errors.New(`Caching server cannot be assigned to this delivery service without having the required delivery service capabilities`),
err: errors.New("cannot be assigned to this delivery service"),
ssc: sscs[0],
capability: tc.DeliveryServicesRequiredCapability{
DeliveryServiceID: helperGetDeliveryServiceID(t, util.StrPtr("ds-test-minor-versions")),
Expand Down Expand Up @@ -471,7 +501,7 @@ func CreateTestDeliveryServiceServersWithRequiredCapabilities(t *testing.T) {
}
}
if !found {
t.Fatalf("Expected to find an error-level alert relating to '%v', but it wasn't found", ctc.err)
t.Fatalf("Expected to find an error-level alert relating to '%v', but it wasn't found. Actual alerts: %v", ctc.err, assignResp.Alerts.Alerts)
}
}

Expand Down
5 changes: 3 additions & 2 deletions traffic_ops/testing/api/v4/servers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,7 @@ func GetTestServersQueryParameters(t *testing.T) {
"midInParentCachegroup": false,
"midInSecondaryCachegroup": false,
"midInSecondaryCachegroupInCDN1": false,
"test-mso-org-01": false,
}
response, _, err = TOSession.GetServers(opts)
if err != nil {
Expand Down Expand Up @@ -1212,7 +1213,7 @@ func UpdateTestServers(t *testing.T) {

typeOpts := client.NewRequestOptions()
typeOpts.QueryParameters.Set("useInTable", "server")
serverTypes, _, err := TOSession.GetTypes(opts)
serverTypes, _, err := TOSession.GetTypes(typeOpts)
if err != nil {
t.Fatalf("cannot get Server Types: %v - alerts: %+v", err, serverTypes.Alerts)
}
Expand All @@ -1237,7 +1238,7 @@ func UpdateTestServers(t *testing.T) {
if err == nil {
t.Errorf("expected error when updating Server Type of a server assigned to DSes")
} else {
t.Logf("type change update alerts: %+v", alerts)
t.Logf("got expected error when updating Server Type of a server assigned to DSes - type change update alerts: %+v, err: %v", alerts, err)
}
}

Expand Down
51 changes: 50 additions & 1 deletion traffic_ops/testing/api/v4/tc-fixtures.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1446,7 +1446,7 @@
"logsEnabled": false,
"missLat": 0,
"missLong": 0,
"multiSiteOrigin": false,
"multiSiteOrigin": true,
"orgServerFqdn": "https://example.com",
"protocol": 0,
"qstringIgnore": 0,
Expand Down Expand Up @@ -3132,6 +3132,55 @@
"xmppId": "denver-mso-org-01\\\\@ocdn.kabletown.net",
"xmppPasswd": "X"
},
{
"cachegroup": "multiOriginCachegroup",
"cdnName": "cdn1",
"domainName": "ga.denver.kabletown.net",
"guid": null,
"hostName": "test-mso-org-01",
"httpsPort": 443,
"iloIpAddress": "",
"iloIpGateway": "",
"iloIpNetmask": "",
"iloPassword": "",
"iloUsername": "",
"interfaces": [
{
"ipAddresses": [
{
"address": "127.0.4.1/30",
"gateway": "127.0.4.1",
"serviceAddress": true
},
{
"address": "2345:1244:12:8::20/64",
"gateway": "2345:1244:12:8::20",
"serviceAddress": false
}
],
"maxBandwidth": null,
"monitor": true,
"mtu": 9000,
"name": "bond0",
"routerHostName": "router16",
"routerPort": "9015"
}
],
"mgmtIpAddress": "",
"mgmtIpGateway": "",
"mgmtIpNetmask": "",
"offlineReason": null,
"physLocation": "Denver",
"profile": "MSO",
"rack": "RR 119.02",
"revalPending": false,
"status": "ONLINE",
"tcpPort": 80,
"type": "ORG",
"updPending": false,
"xmppId": "test-mso-org-01\\\\@ocdn.kabletown.net",
"xmppPasswd": "X"
},
{
"cachegroup": "multiOriginCachegroup",
"cdnName": "cdn2",
Expand Down
64 changes: 64 additions & 0 deletions traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,70 @@ func GetServerCapabilitiesFromName(name string, tx *sql.Tx) ([]string, error) {
return caps, nil
}

// GetServerCapabilitiesOfServers gets all of the server capabilities of the given server hostnames.
func GetServerCapabilitiesOfServers(names []string, tx *sql.Tx) (map[string][]string, error) {
serverCaps := make(map[string][]string, len(names))
q := `
SELECT
s.host_name,
ARRAY_REMOVE(ARRAY_AGG(ssc.server_capability ORDER BY ssc.server_capability), NULL) AS capabilities
FROM server s
LEFT JOIN server_server_capability ssc ON s.id = ssc.server
WHERE
s.host_name = ANY($1)
GROUP BY s.host_name
`
rows, err := tx.Query(q, pq.Array(&names))
if err != nil {
return nil, errors.New("querying server capabilities by host names: " + err.Error())
}
defer log.Close(rows, "closing rows in GetServerCapabilitiesOfServers")

for rows.Next() {
hostname := ""
caps := []string{}
if err := rows.Scan(&hostname, pq.Array(&caps)); err != nil {
return nil, errors.New("scanning server capabilities: " + err.Error())
}
serverCaps[hostname] = caps
}
return serverCaps, nil
}

// GetRequiredCapabilitiesOfDeliveryServices gets all of the required capabilities of the given delivery service IDs.
func GetRequiredCapabilitiesOfDeliveryServices(ids []int, tx *sql.Tx) (map[int][]string, error) {
queryIDs := make([]int64, 0, len(ids))
for _, id := range ids {
queryIDs = append(queryIDs, int64(id))
}
dsCaps := make(map[int][]string, len(ids))
q := `
SELECT
d.id,
ARRAY_REMOVE(ARRAY_AGG(dsrc.required_capability ORDER BY dsrc.required_capability), NULL) AS required_capabilities
FROM deliveryservice d
LEFT JOIN deliveryservices_required_capability dsrc on d.id = dsrc.deliveryservice_id
WHERE
d.id = ANY($1)
GROUP BY d.id
`
rows, err := tx.Query(q, pq.Array(&queryIDs))
if err != nil {
return nil, errors.New("querying delivery service required capabilities by IDs: " + err.Error())
}
defer log.Close(rows, "closing rows in GetRequiredCapabilitiesOfDeliveryServices")

for rows.Next() {
id := 0
caps := []string{}
if err := rows.Scan(&id, pq.Array(&caps)); err != nil {
return nil, errors.New("scanning required capabilities: " + err.Error())
}
dsCaps[id] = caps
}
return dsCaps, nil
}

const dsrExistsQuery = `
SELECT EXISTS(
SELECT id
Expand Down
Loading

0 comments on commit a05fe85

Please sign in to comment.