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 2022/05/23 15:53:19 UTC

[trafficcontrol] branch master updated: Remove Traffic Router Profile name restrictions (#6213)

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 215bb7670a Remove Traffic Router Profile name restrictions (#6213)
215bb7670a is described below

commit 215bb7670aa5f39aa0c8560a4a63c9f051637692
Author: ocket8888 <oc...@apache.org>
AuthorDate: Mon May 23 09:53:11 2022 -0600

    Remove Traffic Router Profile name restrictions (#6213)
    
    * Remove restriction on TR Profiles forcing them to be named like 'CCR%'
    
    * Remove incorrect warning from the docs
    
    * Rename CIAB Profile to not use legacy TR Acronym
    
    * Update CHANGELOG
    
    * Remove unnecessary, unused test function from t3c tests
    
    * Add string constants for Profile Types to Go library
    
    * Switch usages in Go source of raw string for ATS_PROFILE to use constant
    
    * Switch usages in Go source of raw string for GROVE_PROFILE to use constant
    
    * Switch usages in Go source of raw string for TR_PROFILE to use constant
---
 CHANGELOG.md                                       |  1 +
 cache-config/testing/ort-tests/tcdata/profiles.go  | 21 ----------------
 docs/source/overview/profiles_and_parameters.rst   |  2 --
 grove/grovetccfg/grovetccfg.go                     |  5 ++--
 .../cdn-in-a-box/traffic_ops/to-access.sh          |  2 +-
 .../cdn-in-a-box/traffic_ops/trafficops-init.sh    |  2 +-
 .../{040-CCR_CIAB.json => 040-TRAFFIC_ROUTER.json} |  2 +-
 lib/go-tc/profiles.go                              | 28 +++++++++++++++++++++-
 traffic_ops/testing/api/v2/profiles_test.go        | 10 ++++----
 traffic_ops/testing/api/v3/profiles_test.go        | 10 ++++----
 traffic_ops/testing/api/v4/profiles_test.go        | 13 +++++-----
 .../traffic_ops_golang/cdn/dnssecrefresh.go        |  2 +-
 traffic_ops/traffic_ops_golang/cdn/domains.go      | 10 ++++----
 traffic_ops/traffic_ops_golang/cdn/genksk.go       |  2 +-
 .../traffic_ops_golang/profile/profiles_test.go    |  3 +--
 15 files changed, 56 insertions(+), 57 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b8e85c1d01..0ad4e78efe 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -76,6 +76,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - Adds updates to the trafficcontrol-health-client to, use new ATS Host status formats, detect and use proper
   traffic_ctl commands, and adds new markup-poll-threshold config.
 - Traffic Monitor now defaults to 100 historical "CRConfig" Snapshots stored internally if not specified in configuration (previous default was 20,000)
+- `TRAFFIC_ROUTER`-type Profiles no longer need to have names that match any kind of pattern (e.g. `CCR_.*`)
 
 ## [6.1.0] - 2022-01-18
 ### Added
diff --git a/cache-config/testing/ort-tests/tcdata/profiles.go b/cache-config/testing/ort-tests/tcdata/profiles.go
index 6bd4817785..942c3f6548 100644
--- a/cache-config/testing/ort-tests/tcdata/profiles.go
+++ b/cache-config/testing/ort-tests/tcdata/profiles.go
@@ -22,27 +22,6 @@ import (
 	"github.com/apache/trafficcontrol/lib/go-tc"
 )
 
-// CreateBadProfiles ensures that profiles can't be created with bad values
-func (r *TCData) CreateBadProfiles(t *testing.T) {
-
-	// blank profile
-	prs := []tc.Profile{
-		tc.Profile{Type: "", Name: "", Description: "", CDNID: 0},
-		tc.Profile{Type: "ATS_PROFILE", Name: "badprofile", Description: "description", CDNID: 0},
-		tc.Profile{Type: "ATS_PROFILE", Name: "badprofile", Description: "", CDNID: 1},
-		tc.Profile{Type: "ATS_PROFILE", Name: "", Description: "description", CDNID: 1},
-		tc.Profile{Type: "", Name: "badprofile", Description: "description", CDNID: 1},
-	}
-
-	for _, pr := range prs {
-		resp, _, err := TOSession.CreateProfile(pr)
-
-		if err == nil {
-			t.Errorf("Creating bad profile %+v succeeded, response: %+v", pr, resp)
-		}
-	}
-}
-
 func (r *TCData) CreateTestProfiles(t *testing.T) {
 
 	for _, pr := range r.TestData.Profiles {
diff --git a/docs/source/overview/profiles_and_parameters.rst b/docs/source/overview/profiles_and_parameters.rst
index c75085f470..18cae3bb93 100644
--- a/docs/source/overview/profiles_and_parameters.rst
+++ b/docs/source/overview/profiles_and_parameters.rst
@@ -120,8 +120,6 @@ TP_PROFILE
 TR_PROFILE
 	A Traffic Router Profile.
 
-	.. warning:: For legacy reasons, the names of Profiles of this type *must* begin with ``CCR_`` or ``TR_``. This is **not** enforced by the :ref:`to-api` or Traffic Portal, but certain Traffic Control operations/components expect this and will fail to work otherwise!
-
 	.. seealso:: :ref:`tr-profile`
 
 TS_PROFILE
diff --git a/grove/grovetccfg/grovetccfg.go b/grove/grovetccfg/grovetccfg.go
index fe7337c295..0b6c50dc68 100644
--- a/grove/grovetccfg/grovetccfg.go
+++ b/grove/grovetccfg/grovetccfg.go
@@ -51,7 +51,6 @@ const GroveConfigFile = "grove.cfg"
 const GroveConfigPath = "/etc/grove/" + GroveConfigFile
 const ConfigHistory = "cfg_history/"
 const RemapHistory = "remap_history/"
-const GroveProfileType = "GROVE_PROFILE"
 
 // Exit codes are defined in the documentation, DO NOT change to iota, to avoid ambiguity.
 const (
@@ -263,7 +262,7 @@ func main() {
 	}
 	// end of API 1.2 stuff
 
-	if hostProfile.Type == GroveProfileType {
+	if hostProfile.Type == tc.GroveProfileType {
 		updateRequired, cfg, err := createGroveCfg(toc, hostServer)
 		if err != nil {
 			fmt.Println(time.Now().Format(time.RFC3339Nano) + " Error getting config rules for '" + GroveConfigPath + "' :" + err.Error())
@@ -286,7 +285,7 @@ func main() {
 			}
 		}
 	} else {
-		fmt.Println(time.Now().Format(time.RFC3339Nano) + " Warning: the profile '" + hostServer.Profile + "' is not a '" + GroveProfileType + "', will not build a config from it.")
+		fmt.Println(time.Now().Format(time.RFC3339Nano) + " Warning: the profile '" + hostServer.Profile + "' is not a '" + tc.GroveProfileType + "', will not build a config from it.")
 	}
 
 	rules := remap.RemapRules{}
diff --git a/infrastructure/cdn-in-a-box/traffic_ops/to-access.sh b/infrastructure/cdn-in-a-box/traffic_ops/to-access.sh
index 1863b2d58e..333f16560e 100755
--- a/infrastructure/cdn-in-a-box/traffic_ops/to-access.sh
+++ b/infrastructure/cdn-in-a-box/traffic_ops/to-access.sh
@@ -236,7 +236,7 @@ to-enroll() {
 			;;
 		"tr" )
 			export MY_TYPE="CCR"
-			export MY_PROFILE="CCR_CIAB"
+			export MY_PROFILE="TRAFFIC_ROUTER"
 			export MY_STATUS="ONLINE"
 			;;
 		"tp" )
diff --git a/infrastructure/cdn-in-a-box/traffic_ops/trafficops-init.sh b/infrastructure/cdn-in-a-box/traffic_ops/trafficops-init.sh
index 851be011c5..77193cb890 100755
--- a/infrastructure/cdn-in-a-box/traffic_ops/trafficops-init.sh
+++ b/infrastructure/cdn-in-a-box/traffic_ops/trafficops-init.sh
@@ -142,7 +142,7 @@ traffic_router_zonemanager_timeout() {
   fi;
 
   local modified_crconfig crconfig_path zonemanager_timeout;
-  crconfig_path=/traffic_ops_data/profiles/040-CCR_CIAB.json;
+  crconfig_path=/traffic_ops_data/profiles/040-TRAFFIC_ROUTER.json;
   modified_crconfig="$(mktemp)";
   # 5 minutes, which is the default zonemanager.cache.maintenance.interval value
   zonemanager_timeout="$(( 60 * 5 ))";
diff --git a/infrastructure/cdn-in-a-box/traffic_ops_data/profiles/040-CCR_CIAB.json b/infrastructure/cdn-in-a-box/traffic_ops_data/profiles/040-TRAFFIC_ROUTER.json
similarity index 99%
rename from infrastructure/cdn-in-a-box/traffic_ops_data/profiles/040-CCR_CIAB.json
rename to infrastructure/cdn-in-a-box/traffic_ops_data/profiles/040-TRAFFIC_ROUTER.json
index 06eeefa711..4dac341a5f 100644
--- a/infrastructure/cdn-in-a-box/traffic_ops_data/profiles/040-CCR_CIAB.json
+++ b/infrastructure/cdn-in-a-box/traffic_ops_data/profiles/040-TRAFFIC_ROUTER.json
@@ -1,7 +1,7 @@
 {
   "cdnName": "$CDN_NAME",
   "description": "Traffic Router for CDN-In-A-Box",
-  "name": "CCR_CIAB",
+  "name": "TRAFFIC_ROUTER",
   "routingDisabled": false,
   "type": "TR_PROFILE",
   "params": [
diff --git a/lib/go-tc/profiles.go b/lib/go-tc/profiles.go
index 2df4c95276..e2bc60942f 100644
--- a/lib/go-tc/profiles.go
+++ b/lib/go-tc/profiles.go
@@ -34,6 +34,32 @@ import (
 	"github.com/lib/pq"
 )
 
+// These are the valid values for the Type property of a Profile. No other
+// values will be accepted, and these are not configurable.
+const (
+	CacheServerProfileType     = "ATS_PROFILE"
+	DeliveryServiceProfileType = "DS_PROFILE"
+	ElasticSearchProfileType   = "ES_PROFILE"
+	GroveProfileType           = "GROVE_PROFILE"
+	InfluxdbProfileType        = "INFLUXDB_PROFILE"
+	KafkaProfileType           = "KAFKA_PROFILE"
+	LogstashProfileType        = "LOGSTASH_PROFILE"
+	OriginProfileType          = "ORG_PROFILE"
+	// RiakProfileType is the type of a Profile used on the legacy RiakKV system
+	// which used to be used as a back-end for Traffic Vault.
+	//
+	// Deprecated: Support for Riak as a Traffic Vault back-end is being dropped
+	// in the near future. Profiles of type UnknownProfileType should be used on
+	// PostgreSQL database servers instead.
+	RiakProfileType           = "RIAK_PROFILE"
+	SplunkProfileType         = "SPLUNK_PROFILE"
+	TrafficMonitorProfileType = "TM_PROFILE"
+	TrafficPortalProfileType  = "TP_PROFILE"
+	TrafficRouterProfileType  = "TR_PROFILE"
+	TrafficStatsProfileType   = "TS_PROFILE"
+	UnkownProfileType         = "UNK_PROFILE"
+)
+
 // ProfilesResponse is a list of profiles returned by GET requests.
 type ProfilesResponse struct {
 	Response []Profile `json:"response"`
@@ -190,7 +216,7 @@ func (profileImport *ProfileImportRequest) Validate(tx *sql.Tx) error {
 			log.Errorf("%v: %v", errString, err.Error())
 			errs = append(errs, errors.New(errString))
 		} else if ok {
-			errs = append(errs, fmt.Errorf("A profile with the name \"%v\" already exists", *profile.Name))
+			errs = append(errs, fmt.Errorf("a profile with the name \"%s\" already exists", *profile.Name))
 		}
 	}
 
diff --git a/traffic_ops/testing/api/v2/profiles_test.go b/traffic_ops/testing/api/v2/profiles_test.go
index 92af1e1e90..5305c018ff 100644
--- a/traffic_ops/testing/api/v2/profiles_test.go
+++ b/traffic_ops/testing/api/v2/profiles_test.go
@@ -39,11 +39,11 @@ func CreateBadProfiles(t *testing.T) {
 
 	// blank profile
 	prs := []tc.Profile{
-		tc.Profile{Type: "", Name: "", Description: "", CDNID: 0},
-		tc.Profile{Type: "ATS_PROFILE", Name: "badprofile", Description: "description", CDNID: 0},
-		tc.Profile{Type: "ATS_PROFILE", Name: "badprofile", Description: "", CDNID: 1},
-		tc.Profile{Type: "ATS_PROFILE", Name: "", Description: "description", CDNID: 1},
-		tc.Profile{Type: "", Name: "badprofile", Description: "description", CDNID: 1},
+		{Type: "", Name: "", Description: "", CDNID: 0},
+		{Type: tc.CacheServerProfileType, Name: "badprofile", Description: "description", CDNID: 0},
+		{Type: tc.CacheServerProfileType, Name: "badprofile", Description: "", CDNID: 1},
+		{Type: tc.CacheServerProfileType, Name: "", Description: "description", CDNID: 1},
+		{Type: "", Name: "badprofile", Description: "description", CDNID: 1},
 	}
 
 	for _, pr := range prs {
diff --git a/traffic_ops/testing/api/v3/profiles_test.go b/traffic_ops/testing/api/v3/profiles_test.go
index 1cd84acbd4..a5c7667889 100644
--- a/traffic_ops/testing/api/v3/profiles_test.go
+++ b/traffic_ops/testing/api/v3/profiles_test.go
@@ -117,11 +117,11 @@ func CreateBadProfiles(t *testing.T) {
 
 	// blank profile
 	prs := []tc.Profile{
-		tc.Profile{Type: "", Name: "", Description: "", CDNID: 0},
-		tc.Profile{Type: "ATS_PROFILE", Name: "badprofile", Description: "description", CDNID: 0},
-		tc.Profile{Type: "ATS_PROFILE", Name: "badprofile", Description: "", CDNID: 1},
-		tc.Profile{Type: "ATS_PROFILE", Name: "", Description: "description", CDNID: 1},
-		tc.Profile{Type: "", Name: "badprofile", Description: "description", CDNID: 1},
+		{Type: "", Name: "", Description: "", CDNID: 0},
+		{Type: tc.CacheServerProfileType, Name: "badprofile", Description: "description", CDNID: 0},
+		{Type: tc.CacheServerProfileType, Name: "badprofile", Description: "", CDNID: 1},
+		{Type: tc.CacheServerProfileType, Name: "", Description: "description", CDNID: 1},
+		{Type: "", Name: "badprofile", Description: "description", CDNID: 1},
 	}
 
 	for _, pr := range prs {
diff --git a/traffic_ops/testing/api/v4/profiles_test.go b/traffic_ops/testing/api/v4/profiles_test.go
index 3ef9382ee6..091c1bc240 100644
--- a/traffic_ops/testing/api/v4/profiles_test.go
+++ b/traffic_ops/testing/api/v4/profiles_test.go
@@ -235,9 +235,9 @@ func CreateBadProfiles(t *testing.T) {
 	// blank profile
 	prs := []tc.Profile{
 		{Type: "", Name: "", Description: "", CDNID: 0},
-		{Type: "ATS_PROFILE", Name: "badprofile", Description: "description", CDNID: 0},
-		{Type: "ATS_PROFILE", Name: "badprofile", Description: "", CDNID: 1},
-		{Type: "ATS_PROFILE", Name: "", Description: "description", CDNID: 1},
+		{Type: tc.CacheServerProfileType, Name: "badprofile", Description: "description", CDNID: 0},
+		{Type: tc.CacheServerProfileType, Name: "badprofile", Description: "", CDNID: 1},
+		{Type: tc.CacheServerProfileType, Name: "", Description: "description", CDNID: 1},
 		{Type: "", Name: "badprofile", Description: "description", CDNID: 1},
 	}
 
@@ -453,10 +453,9 @@ func UpdateTestProfiles(t *testing.T) {
 	expectedCDNId := cdns.Response[0].ID
 	expectedName := "testing"
 	expectedRoutingDisabled := true
-	expectedType := "TR_PROFILE"
 
 	remoteProfile.Description = expectedProfileDesc
-	remoteProfile.Type = expectedType
+	remoteProfile.Type = tc.TrafficRouterProfileType
 	remoteProfile.CDNID = expectedCDNId
 	remoteProfile.Name = expectedName
 	remoteProfile.RoutingDisabled = expectedRoutingDisabled
@@ -480,8 +479,8 @@ func UpdateTestProfiles(t *testing.T) {
 	if respProfile.Description != expectedProfileDesc {
 		t.Errorf("results do not match actual: %s, expected: %s", respProfile.Description, expectedProfileDesc)
 	}
-	if respProfile.Type != expectedType {
-		t.Errorf("results do not match actual: %s, expected: %s", respProfile.Type, expectedType)
+	if respProfile.Type != tc.TrafficRouterProfileType {
+		t.Errorf("results do not match actual: %s, expected: %s", respProfile.Type, tc.TrafficRouterProfileType)
 	}
 	if respProfile.CDNID != expectedCDNId {
 		t.Errorf("results do not match actual: %d, expected: %d", respProfile.CDNID, expectedCDNId)
diff --git a/traffic_ops/traffic_ops_golang/cdn/dnssecrefresh.go b/traffic_ops/traffic_ops_golang/cdn/dnssecrefresh.go
index 25c10ffaa9..99e65b102d 100644
--- a/traffic_ops/traffic_ops_golang/cdn/dnssecrefresh.go
+++ b/traffic_ops/traffic_ops_golang/cdn/dnssecrefresh.go
@@ -375,7 +375,7 @@ WITH cdn_profile_ids AS (
     MAX(p.id) as profile_id -- We only want 1 profile, so get the probably-newest if there's more than one.
   FROM
     cdn c
-    LEFT JOIN profile p ON c.id = p.cdn AND (p.name like 'CCR%' OR p.name like 'TR%')
+    LEFT JOIN profile p ON c.id = p.cdn AND (p.type = '` + tc.TrafficRouterProfileType + `')
     GROUP BY c.name, c.dnssec_enabled, c.domain_name
 )
 SELECT
diff --git a/traffic_ops/traffic_ops_golang/cdn/domains.go b/traffic_ops/traffic_ops_golang/cdn/domains.go
index b0d4fef183..21df9d6820 100644
--- a/traffic_ops/traffic_ops_golang/cdn/domains.go
+++ b/traffic_ops/traffic_ops_golang/cdn/domains.go
@@ -21,23 +21,21 @@ package cdn
 
 import (
 	"fmt"
-	"github.com/apache/trafficcontrol/lib/go-log"
-	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
 	"net/http"
 	"time"
 
+	"github.com/apache/trafficcontrol/lib/go-log"
 	"github.com/apache/trafficcontrol/lib/go-tc"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
 
 	"github.com/jmoiron/sqlx"
 )
 
-const RouterProfilePrefix = "CCR"
-
 func selectMaxLastUpdatedQuery() string {
 	return `SELECT max(t) from (
 		SELECT max(profile.last_updated) as t FROM profile
-JOIN cdn ON profile.cdn = cdn.id WHERE profile.name LIKE '` + RouterProfilePrefix + `%'
+JOIN cdn ON profile.cdn = cdn.id WHERE profile.type = '` + tc.TrafficRouterProfileType + `'
 UNION ALL
 	select max(last_updated) as t from last_deleted l where l.table_name='profile') as res`
 }
@@ -48,7 +46,7 @@ func getDomainsList(useIMS bool, header http.Header, tx *sqlx.Tx) ([]tc.Domain,
 	domains := []tc.Domain{}
 
 	q := `SELECT p.id, p.name, p.description, domain_name FROM profile AS p
-	JOIN cdn ON p.cdn = cdn.id WHERE p.name LIKE '` + RouterProfilePrefix + `%'`
+	JOIN cdn ON p.cdn = cdn.id WHERE p.type = '` + tc.TrafficRouterProfileType + `'`
 
 	if useIMS {
 		runSecond, maxTime = ims.TryIfModifiedSinceQuery(tx, header, nil, selectMaxLastUpdatedQuery())
diff --git a/traffic_ops/traffic_ops_golang/cdn/genksk.go b/traffic_ops/traffic_ops_golang/cdn/genksk.go
index 81b21e7021..72c157a08b 100644
--- a/traffic_ops/traffic_ops_golang/cdn/genksk.go
+++ b/traffic_ops/traffic_ops_golang/cdn/genksk.go
@@ -135,7 +135,7 @@ WITH cdn_profile_id AS (
     JOIN cdn c ON c.id = p.cdn
   WHERE
     c.name = $1
-    AND (p.name like 'CCR%' OR p.name like 'TR%')
+    AND (p.type = '` + tc.TrafficRouterProfileType + `')
   FETCH FIRST 1 ROWS ONLY
 )
 SELECT
diff --git a/traffic_ops/traffic_ops_golang/profile/profiles_test.go b/traffic_ops/traffic_ops_golang/profile/profiles_test.go
index ebd6e86944..443ec66397 100644
--- a/traffic_ops/traffic_ops_golang/profile/profiles_test.go
+++ b/traffic_ops/traffic_ops_golang/profile/profiles_test.go
@@ -42,7 +42,6 @@ func getTestProfiles() []tc.ProfileNullable {
 	ID := 1
 	name := "profile1"
 	description := "desc1"
-	pt := "TR_PROFILE"
 	cdnID := 1
 	cdnName := "cdn1"
 	rd := true
@@ -55,7 +54,7 @@ func getTestProfiles() []tc.ProfileNullable {
 		CDNName:         &cdnName,
 		CDNID:           &cdnID,
 		RoutingDisabled: &rd,
-		Type:            &pt,
+		Type:            util.StrPtr(tc.TrafficRouterProfileType),
 	}
 	profiles = append(profiles, testCase)