You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ra...@apache.org on 2020/01/09 19:44:41 UTC

[trafficcontrol] branch 4.0.x updated: Fix atscfg meta to omit config files for nonexistent delivery services (#4256) (#4278)

This is an automated email from the ASF dual-hosted git repository.

rawlin pushed a commit to branch 4.0.x
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git


The following commit(s) were added to refs/heads/4.0.x by this push:
     new c085104  Fix atscfg meta to omit config files for nonexistent delivery services (#4256) (#4278)
c085104 is described below

commit c085104164fbc0f0adc6e049e242106fccae3159
Author: Robert Butts <ro...@users.noreply.github.com>
AuthorDate: Thu Jan 9 12:44:32 2020 -0700

    Fix atscfg meta to omit config files for nonexistent delivery services (#4256) (#4278)
    
    * Fix atscfg meta to omit nonexistent ds files
    
    This fixes atscfg to omit files with location parameters for config
    files, hdr_rw_, regex_remap_, url_sig_, and uri_signing_ for DSes
    which don't exist (typically after deleting a DS but forgetting to
    delete the corresponding location Parameters).
    
    This is important to make atstccfg work because without it, ORT will
    request url_sig_ files for DSes which don't exist, and the API
    endpoint to get the keys will fail. Because atstccfg uses the API,
    there's no reasonable way to make it work, except to fix the meta
    config to omit those nonexistent files.
    
    In addition to nonexistent files, this also omits files for DSes
    not assigned to the server. There won't even be remap lines for
    those DSes anyway, so it's better and safer to omit the ancillary
    files altogether.
    
    * Fix TO API test for meta cfg to set DSS, real DS
    
    (cherry picked from commit ea315f079c0142169069e4705a66c5a70f0bbb56)
---
 CHANGELOG.md                                       |  1 +
 lib/go-atscfg/meta.go                              | 20 +++++++++++++++
 lib/go-atscfg/meta_test.go                         | 29 +++++++++++++++++++--
 traffic_ops/ort/atstccfg/cfgfile/meta.go           | 14 +++++-----
 traffic_ops/testing/api/v14/atsconfig_meta_test.go |  6 +++--
 .../traffic_ops_golang/ats/atsserver/meta.go       |  8 +++++-
 traffic_ops/traffic_ops_golang/ats/db.go           | 30 ++++++++++++++++++++++
 7 files changed, 95 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d37e64b..7b4cecd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -64,6 +64,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Traffic Ops now supports a "sortOrder" query parameter on some endpoints to return API responses in descending order
 - Traffic Ops now uses a consistent format for audit logs across all Go endpoints
 - Added cache-side config generator, atstccfg, installed with ORT. Includes all configs. Includes a plugin system.
+- Fixed ATS config generation to omit regex remap, header rewrite, URL Sig, and URI Signing files for delivery services not assigned to that server.
 - In Traffic Portal, all tables now include a 'CSV' link to enable the export of table data in CSV format.
 - Pylint configuration now enforced (present in [a file in the Python client directory](./traffic_control/clients/python/pylint.rc))
 - Added an optional SMTP server configuration to the TO configuration file, api now has unused abilitiy to send emails
diff --git a/lib/go-atscfg/meta.go b/lib/go-atscfg/meta.go
index c8f58e6..fbcb841 100644
--- a/lib/go-atscfg/meta.go
+++ b/lib/go-atscfg/meta.go
@@ -48,6 +48,7 @@ func MakeMetaConfig(
 	locationParams map[string]ConfigProfileParams, // map[configFile]params; 'location' and 'URL' Parameters on serverHostName's Profile
 	uriSignedDSes []tc.DeliveryServiceName,
 	scopeParams map[string]string, // map[configFileName]scopeParam
+	dsNames map[tc.DeliveryServiceName]struct{},
 ) string {
 	if tmURL == "" {
 		log.Errorln("ats.GetConfigMetadata: global tm.url parameter missing or empty! Setting empty in meta config!")
@@ -83,7 +84,26 @@ func MakeMetaConfig(
 		}
 	}
 
+locationParamsFor:
 	for cfgFile, cfgParams := range locationParams {
+		if strings.HasSuffix(cfgFile, ".config") {
+			dsConfigFilePrefixes := []string{
+				"hdr_rw_",
+				"regex_remap_",
+				"url_sig_",
+				"uri_signing_",
+			}
+			for _, prefix := range dsConfigFilePrefixes {
+				if strings.HasPrefix(cfgFile, prefix) {
+					dsName := strings.TrimSuffix(strings.TrimPrefix(cfgFile, prefix), ".config")
+					if _, ok := dsNames[tc.DeliveryServiceName(dsName)]; !ok {
+						log.Warnln("Server Profile had 'location' Parameter '" + cfgFile + "', but delivery Service '" + dsName + "' is not assigned to this Server! Not including in meta config!")
+						continue locationParamsFor
+					}
+				}
+			}
+		}
+
 		atsCfg := tc.ATSConfigMetaDataConfigFile{
 			FileNameOnDisk: cfgParams.FileNameOnDisk,
 			Location:       cfgParams.Location,
diff --git a/lib/go-atscfg/meta_test.go b/lib/go-atscfg/meta_test.go
index fed1549..5317220 100644
--- a/lib/go-atscfg/meta_test.go
+++ b/lib/go-atscfg/meta_test.go
@@ -21,6 +21,7 @@ package atscfg
 
 import (
 	"encoding/json"
+	"strings"
 	"testing"
 
 	"github.com/apache/trafficcontrol/lib/go-tc"
@@ -80,6 +81,26 @@ func TestMakeMetaConfig(t *testing.T) {
 			FileNameOnDisk: "uri_signing_mydsname.config",
 			Location:       "/my/location/",
 		},
+		"uri_signing_nonexistentds.config": ConfigProfileParams{
+			FileNameOnDisk: "uri_signing_nonexistentds.config",
+			Location:       "/my/location/",
+		},
+		"regex_remap_nonexistentds.config": ConfigProfileParams{
+			FileNameOnDisk: "regex_remap_nonexistentds.config",
+			Location:       "/my/location/",
+		},
+		"url_sig_nonexistentds.config": ConfigProfileParams{
+			FileNameOnDisk: "url_sig_nonexistentds.config",
+			Location:       "/my/location/",
+		},
+		"hdr_rw_nonexistentds.config": ConfigProfileParams{
+			FileNameOnDisk: "hdr_rw_nonexistentds.config",
+			Location:       "/my/location/",
+		},
+		"hdr_rw_mid_nonexistentds.config": ConfigProfileParams{
+			FileNameOnDisk: "hdr_rw_mid_nonexistentds.config",
+			Location:       "/my/location/",
+		},
 		"unknown.config": ConfigProfileParams{
 			FileNameOnDisk: "unknown.config",
 			Location:       "/my/location/",
@@ -90,10 +111,11 @@ func TestMakeMetaConfig(t *testing.T) {
 		},
 	}
 	uriSignedDSes := []tc.DeliveryServiceName{"mydsname"}
+	dses := map[tc.DeliveryServiceName]struct{}{"mydsname": {}}
 
 	scopeParams := map[string]string{"custom.config": string(tc.ATSConfigMetaDataConfigFileScopeProfiles)}
 
-	txt := MakeMetaConfig(serverHostName, server, tmURL, tmReverseProxyURL, locationParams, uriSignedDSes, scopeParams)
+	txt := MakeMetaConfig(serverHostName, server, tmURL, tmReverseProxyURL, locationParams, uriSignedDSes, scopeParams, dses)
 
 	cfg := tc.ATSConfigMetaData{}
 	if err := json.Unmarshal([]byte(txt), &cfg); err != nil {
@@ -223,7 +245,7 @@ func TestMakeMetaConfig(t *testing.T) {
 	}
 
 	server.Type = "MID"
-	txt = MakeMetaConfig(serverHostName, server, tmURL, tmReverseProxyURL, locationParams, uriSignedDSes, scopeParams)
+	txt = MakeMetaConfig(serverHostName, server, tmURL, tmReverseProxyURL, locationParams, uriSignedDSes, scopeParams, dses)
 	cfg = tc.ATSConfigMetaData{}
 	if err := json.Unmarshal([]byte(txt), &cfg); err != nil {
 		t.Fatalf("MakeMetaConfig returned invalid JSON: " + err.Error())
@@ -237,4 +259,7 @@ func TestMakeMetaConfig(t *testing.T) {
 		}
 		break
 	}
+	if strings.Contains(txt, "nonexistentds") {
+		t.Errorf("expected location parameters for nonexistent delivery services to not be added to config, actual '%v'", txt)
+	}
 }
diff --git a/traffic_ops/ort/atstccfg/cfgfile/meta.go b/traffic_ops/ort/atstccfg/cfgfile/meta.go
index d8f3f56..18dd27e 100644
--- a/traffic_ops/ort/atstccfg/cfgfile/meta.go
+++ b/traffic_ops/ort/atstccfg/cfgfile/meta.go
@@ -185,9 +185,6 @@ func GetConfigFileMeta(cfg config.TCCfg, serverNameOrID string) (string, error)
 
 	dsIDs := []int{}
 	for _, ds := range deliveryServices {
-		if ds.SigningAlgorithm == nil || *ds.SigningAlgorithm != tc.SigningAlgorithmURISigning {
-			continue
-		}
 		if ds.ID == nil {
 			// TODO log error?
 			continue
@@ -210,6 +207,7 @@ func GetConfigFileMeta(cfg config.TCCfg, serverNameOrID string) (string, error)
 		dssMap[*dss.DeliveryService] = struct{}{}
 	}
 
+	dsNames := map[tc.DeliveryServiceName]struct{}{}
 	uriSignedDSes := []tc.DeliveryServiceName{}
 	for _, ds := range deliveryServices {
 		if ds.ID == nil {
@@ -218,14 +216,14 @@ func GetConfigFileMeta(cfg config.TCCfg, serverNameOrID string) (string, error)
 		if ds.XMLID == nil {
 			continue // TODO log?
 		}
-		if ds.SigningAlgorithm == nil || *ds.SigningAlgorithm != tc.SigningAlgorithmURISigning {
-			continue
-		}
 		if _, ok := dssMap[*ds.ID]; !ok {
 			continue
 		}
-		uriSignedDSes = append(uriSignedDSes, tc.DeliveryServiceName(*ds.XMLID))
+		dsNames[tc.DeliveryServiceName(*ds.XMLID)] = struct{}{}
+		if ds.SigningAlgorithm != nil && *ds.SigningAlgorithm == tc.SigningAlgorithmURISigning {
+			uriSignedDSes = append(uriSignedDSes, tc.DeliveryServiceName(*ds.XMLID))
+		}
 	}
 
-	return atscfg.MakeMetaConfig(tc.CacheName(serverHostName), &serverInfo, toURL, toReverseProxyURL, locationParams, uriSignedDSes, scopeParams), nil
+	return atscfg.MakeMetaConfig(tc.CacheName(serverHostName), &serverInfo, toURL, toReverseProxyURL, locationParams, uriSignedDSes, scopeParams, dsNames), nil
 }
diff --git a/traffic_ops/testing/api/v14/atsconfig_meta_test.go b/traffic_ops/testing/api/v14/atsconfig_meta_test.go
index cc8bb39..378c797 100644
--- a/traffic_ops/testing/api/v14/atsconfig_meta_test.go
+++ b/traffic_ops/testing/api/v14/atsconfig_meta_test.go
@@ -24,6 +24,8 @@ import (
 
 func TestATSConfigMeta(t *testing.T) {
 	WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, DeliveryServices}, func() {
+		defer DeleteTestDeliveryServiceServersCreated(t)
+		CreateTestDeliveryServiceServers(t)
 		GetTestATSConfigMeta(t)
 	})
 }
@@ -49,9 +51,9 @@ func GetTestATSConfigMeta(t *testing.T) {
 	}
 
 	expected := tc.ATSConfigMetaDataConfigFile{
-		FileNameOnDisk: "hdr_rw_mid_anymap-ds.config",
+		FileNameOnDisk: "hdr_rw_ds1.config",
 		Location:       "/remap/config/location/parameter",
-		APIURI:         "cdns/cdn1/configfiles/ats/hdr_rw_mid_anymap-ds.config", // expected suffix; config gen doesn't care about API version
+		APIURI:         "cdns/cdn1/configfiles/ats/hdr_rw_ds1.config", // expected suffix; config gen doesn't care about API version
 		URL:            "",
 		Scope:          "cdns",
 	}
diff --git a/traffic_ops/traffic_ops_golang/ats/atsserver/meta.go b/traffic_ops/traffic_ops_golang/ats/atsserver/meta.go
index 65af586..26e741d 100644
--- a/traffic_ops/traffic_ops_golang/ats/atsserver/meta.go
+++ b/traffic_ops/traffic_ops_golang/ats/atsserver/meta.go
@@ -76,7 +76,13 @@ func GetConfigMetaData(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	txt := atscfg.MakeMetaConfig(serverName, server, tmParams.URL, tmParams.ReverseProxyURL, locationParams, uriSignedDSes, scopeParams)
+	dsNames, err := ats.GetDSNames(inf.Tx.Tx, server.HostName, server.Port)
+	if err != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("GetConfigMetaData getting server ds names: "+err.Error()))
+		return
+	}
+
+	txt := atscfg.MakeMetaConfig(serverName, server, tmParams.URL, tmParams.ReverseProxyURL, locationParams, uriSignedDSes, scopeParams, dsNames)
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
 	w.Write([]byte(txt))
 }
diff --git a/traffic_ops/traffic_ops_golang/ats/db.go b/traffic_ops/traffic_ops_golang/ats/db.go
index baa030c..26fe44b 100644
--- a/traffic_ops/traffic_ops_golang/ats/db.go
+++ b/traffic_ops/traffic_ops_golang/ats/db.go
@@ -749,6 +749,36 @@ WHERE
 	return dses, nil
 }
 
+// GetDSNames returns a set of delivery service names which have the given server assigned, and any error.
+func GetDSNames(tx *sql.Tx, serverHostName string, serverPort int) (map[tc.DeliveryServiceName]struct{}, error) {
+	qry := `
+SELECT
+  ds.xml_id
+FROM
+  deliveryservice ds
+  JOIN deliveryservice_server dss ON ds.id = dss.deliveryservice
+  JOIN server s ON s.id = dss.server
+WHERE
+  s.host_name = $1
+  AND s.tcp_port = $2
+`
+	rows, err := tx.Query(qry, serverHostName, serverPort)
+	if err != nil {
+		return nil, errors.New("querying: " + err.Error())
+	}
+	defer rows.Close()
+
+	dses := map[tc.DeliveryServiceName]struct{}{}
+	for rows.Next() {
+		ds := tc.DeliveryServiceName("")
+		if err := rows.Scan(&ds); err != nil {
+			return nil, errors.New("scanning: " + err.Error())
+		}
+		dses[ds] = struct{}{}
+	}
+	return dses, nil
+}
+
 type TMParams struct {
 	URL             string
 	ReverseProxyURL string