Remove tm_path from CDN Snapshots

docs/source/api/v1/cdns_name_snapshot.rst

@@ -321,6 +321,11 @@ Response Structure
 	:date:       The UNIX epoch timestamp date in the Traffic Ops server's own timezone
 	:tm_host:    The :abbr:`FQDN (Fully Qualified Domain Name)` of the Traffic Ops server
 	:tm_path:    A path relative to the root of the Traffic Ops server where a request may be replaced to have this :term:`Snapshot` overwritten by the current *configured state* of the CDN
+
+		.. deprecated:: ATCv6
+
+			This information should never be used; instead all tools and (especially) components **must** use the documented API. This field was removed in APIv4
+
 	:tm_user:    The username of the currently logged-in user
 	:tm_version: The full version number of the Traffic Ops server, including release number, git commit hash, and supported Enterprise Linux version
 

docs/source/api/v1/cdns_name_snapshot_new.rst

@@ -320,6 +320,11 @@ Response Structure
 	:date:       The UNIX epoch timestamp date in the Traffic Ops server's own timezone
 	:tm_host:    The :abbr:`FQDN (Fully Qualified Domain Name)` of the Traffic Ops server
 	:tm_path:    A path relative to the root of the Traffic Ops server where a request may be replaced to have this :term:`Snapshot` overwritten by the current *configured state* of the CDN
+
+		.. deprecated:: ATCv6
+
+			This information should never be used; instead all tools and (especially) components **must** use the documented API. This field was removed in APIv4
+
 	:tm_user:    The username of the currently logged-in user
 	:tm_version: The full version number of the Traffic Ops server, including release number, git commit hash, and supported Enterprise Linux version
 

docs/source/api/v2/cdns_name_snapshot.rst

@@ -315,6 +315,10 @@ Response Structure
 	:date:       The UNIX epoch timestamp date in the Traffic Ops server's own timezone
 	:tm_host:    The :abbr:`FQDN (Fully Qualified Domain Name)` of the Traffic Ops server
 	:tm_path:    A path relative to the root of the Traffic Ops server where a request may be replaced to have this :term:`Snapshot` overwritten by the current *configured state* of the CDN
+		.. deprecated:: ATCv6
+
+			This information should never be used; instead all tools and (especially) components **must** use the documented API. This field was removed in APIv4
+
 	:tm_user:    The username of the currently logged-in user
 	:tm_version: The full version number of the Traffic Ops server, including release number, git commit hash, and supported Enterprise Linux version
 

docs/source/api/v2/cdns_name_snapshot_new.rst

@@ -314,6 +314,11 @@ Response Structure
 	:date:       The UNIX epoch timestamp date in the Traffic Ops server's own timezone
 	:tm_host:    The :abbr:`FQDN (Fully Qualified Domain Name)` of the Traffic Ops server
 	:tm_path:    A path relative to the root of the Traffic Ops server where a request may be replaced to have this :term:`Snapshot` overwritten by the current *configured state* of the CDN
+
+		.. deprecated:: ATCv6
+
+			This information should never be used; instead all tools and (especially) components **must** use the documented API. This field was removed in APIv4
+
 	:tm_user:    The username of the currently logged-in user
 	:tm_version: The full version number of the Traffic Ops server, including release number, git commit hash, and supported Enterprise Linux version
 

docs/source/api/v3/cdns_name_snapshot.rst

@@ -317,6 +317,11 @@ Response Structure
 	:CDN_name:   The name of this CDN
 	:date:       The UNIX epoch timestamp date in the Traffic Ops server's own timezone
 	:tm_host:    The :abbr:`FQDN (Fully Qualified Domain Name)` of the Traffic Ops server
+
+		.. deprecated:: ATCv6
+
+			This information should never be used; instead all tools and (especially) components **must** use the documented API. This field was removed in APIv4
+
 	:tm_path:    A path relative to the root of the Traffic Ops server where a request may be replaced to have this :term:`Snapshot` overwritten by the current *configured state* of the CDN
 	:tm_user:    The username of the currently logged-in user
 	:tm_version: The full version number of the Traffic Ops server, including release number, git commit hash, and supported Enterprise Linux version

docs/source/api/v3/cdns_name_snapshot_new.rst

@@ -318,6 +318,11 @@ Response Structure
 	:date:       The UNIX epoch timestamp date in the Traffic Ops server's own timezone
 	:tm_host:    The :abbr:`FQDN (Fully Qualified Domain Name)` of the Traffic Ops server
 	:tm_path:    A path relative to the root of the Traffic Ops server where a request may be replaced to have this :term:`Snapshot` overwritten by the current *configured state* of the CDN
+
+		.. deprecated:: ATCv6
+
+			This information should never be used; instead all tools and (especially) components **must** use the documented API. This field was removed in APIv4
+
 	:tm_user:    The username of the currently logged-in user
 	:tm_version: The full version number of the Traffic Ops server, including release number, git commit hash, and supported Enterprise Linux version
 

docs/source/api/v4/cdns_name_snapshot.rst

@@ -317,7 +317,6 @@ Response Structure
 	:CDN_name:   The name of this CDN
 	:date:       The UNIX epoch timestamp date in the Traffic Ops server's own timezone
 	:tm_host:    The :abbr:`FQDN (Fully Qualified Domain Name)` of the Traffic Ops server
-	:tm_path:    A path relative to the root of the Traffic Ops server where a request may be replaced to have this :term:`Snapshot` overwritten by the current *configured state* of the CDN
 	:tm_user:    The username of the currently logged-in user
 	:tm_version: The full version number of the Traffic Ops server, including release number, git commit hash, and supported Enterprise Linux version
 
@@ -541,7 +540,6 @@ Response Structure
 			"CDN_name": "CDN-in-a-Box",
 			"date": 1590600715,
 			"tm_host": "trafficops.infra.ciab.test:443",
-			"tm_path": "/api/4.0/snapshot",
 			"tm_user": "admin",
 			"tm_version": "development"
 		    },

docs/source/api/v4/cdns_name_snapshot_new.rst

@@ -317,7 +317,6 @@ Response Structure
 	:CDN_name:   The name of this CDN
 	:date:       The UNIX epoch timestamp date in the Traffic Ops server's own timezone
 	:tm_host:    The :abbr:`FQDN (Fully Qualified Domain Name)` of the Traffic Ops server
-	:tm_path:    A path relative to the root of the Traffic Ops server where a request may be replaced to have this :term:`Snapshot` overwritten by the current *configured state* of the CDN
 	:tm_user:    The username of the currently logged-in user
 	:tm_version: The full version number of the Traffic Ops server, including release number, git commit hash, and supported Enterprise Linux version
 
@@ -555,7 +554,6 @@ Response Structure
 				"CDN_name": "CDN-in-a-Box",
 				"date": 1590607873,
 				"tm_host": "trafficops.infra.ciab.test:443",
-				"tm_path": "/api/4.0/cdns/CDN-in-a-Box/snapshot/new",
 				"tm_user": "admin",
 				"tm_version": "development"
 			},

lib/go-tc/crconfig.go

@@ -199,11 +199,13 @@ type CRConfigMonitor struct {
 	ServerStatus *CRConfigServerStatus `json:"status,omitempty"`
 }
 
+// CRConfigStats is the type of the 'stats' property of a CDN Snapshot.
 type CRConfigStats struct {
 	CDNName         *string `json:"CDN_name,omitempty"`
 	DateUnixSeconds *int64  `json:"date,omitempty"`
 	TMHost          *string `json:"tm_host,omitempty"`
-	TMPath          *string `json:"tm_path,omitempty"`
-	TMUser          *string `json:"tm_user,omitempty"`
-	TMVersion       *string `json:"tm_version,omitempty"`
+	// Deprecated: Don't ever use this for anything. It's been removed from APIv4 responses.
+	TMPath    *string `json:"tm_path,omitempty"`
+	TMUser    *string `json:"tm_user,omitempty"`
+	TMVersion *string `json:"tm_version,omitempty"`
 }

traffic_ops/app/db/migrations/2021012800000000_remove_tm_path.sql

@@ -0,0 +1,35 @@
+/*
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+        http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+*/
+
+/*
+This migration removes the 'tm_path' property from the 'stats' property of the
+'crconfig' column of stored CDN Snapshots, if it exists.
+
+When reverted, it will insert (if possible) a 'tm_path' value of
+/api/4.0/cdns/{{CDN Name}}/snapshot
+where 'CDN Name' is the name of the CDN snapshotted as determined by the
+Snapshot data - NOT the linked CDN object. This is so it does not self-confilct
+afterward regarding which CDN is named, even if the one to which it is linked
+is wrong, somehow.
+*/
+
+-- +goose Up
+UPDATE snapshot
+SET crconfig = crconfig::jsonb #- '{stats,tm_path}'
+WHERE crconfig::jsonb ? 'stats' AND (crconfig::jsonb -> 'stats') ? 'tm_path';
+
+-- +goose Down
+UPDATE snapshot SET crconfig = jsonb_set(crconfig::jsonb, '{stats,tm_path}', ('"/api/4.0/cdns/' || (crconfig::jsonb -> 'stats' ->> 'CDN_name') || '/snapshot"')::jsonb)
+WHERE crconfig::jsonb ? 'stats' AND (crconfig::jsonb -> 'stats') ? 'CDN_name';

traffic_ops/testing/api/v1/crconfig_test.go

@@ -17,7 +17,6 @@ package v1
 
 import (
 	"encoding/json"
-	"strings"
 	"testing"
 
 	"github.com/apache/trafficcontrol/lib/go-tc"
@@ -113,9 +112,7 @@ func UpdateTestCRConfigSnapshot(t *testing.T) {
 	}
 
 	if crc.Stats.TMPath == nil {
-		t.Errorf("GetCRConfig crc.Stats.Path expected: 'snapshot/"+cdn+"', actual: %+v", crc.Stats.TMPath)
-	} else if !strings.HasSuffix(*crc.Stats.TMPath, "snapshot/"+cdn) {
-		t.Errorf("GetCRConfig crc.Stats.Path expected: '/snapshot"+cdn+"', actual: %+v", *crc.Stats.TMPath)
+		t.Errorf("GetCRConfig crc.Stats.Path expected: some string (but deprecated so we don't check the contents), actual: null")
 	}
 
 	if crc.Stats.TMHost == nil {

traffic_ops/testing/api/v2/crconfig_test.go

@@ -17,7 +17,6 @@ package v2
 
 import (
 	"encoding/json"
-	"strings"
 	"testing"
 
 	"github.com/apache/trafficcontrol/lib/go-tc"
@@ -113,9 +112,7 @@ func UpdateTestCRConfigSnapshot(t *testing.T) {
 	}
 
 	if crc.Stats.TMPath == nil {
-		t.Errorf("GetCRConfig crc.Stats.Path expected: '/snapshot', actual: %+v", crc.Stats.TMPath)
-	} else if !strings.HasSuffix(*crc.Stats.TMPath, "snapshot") {
-		t.Errorf("GetCRConfig crc.Stats.Path expected: '/snapshot', actual: %+v", *crc.Stats.TMPath)
+		t.Error("GetCRConfig crc.Stats.Path expected: some non-null string (but we don't check contents because it's deprecated), actual: null")
 	}
 
 	if crc.Stats.TMHost == nil {

traffic_ops/testing/api/v3/crconfig_test.go

@@ -17,7 +17,6 @@ package v3
 
 import (
 	"encoding/json"
-	"strings"
 	"testing"
 
 	"github.com/apache/trafficcontrol/lib/go-tc"
@@ -114,9 +113,7 @@ func UpdateTestCRConfigSnapshot(t *testing.T) {
 	}
 
 	if crc.Stats.TMPath == nil {
-		t.Errorf("GetCRConfig crc.Stats.Path expected: '/snapshot', actual: %+v", crc.Stats.TMPath)
-	} else if !strings.HasSuffix(*crc.Stats.TMPath, "snapshot") {
-		t.Errorf("GetCRConfig crc.Stats.Path expected: '/snapshot', actual: %+v", *crc.Stats.TMPath)
+		t.Error("GetCRConfig crc.Stats.Path expected: some non-null string (but we don't check contents because it's deprecated), actual: null")
 	}
 
 	if crc.Stats.TMHost == nil {

traffic_ops/testing/api/v4/crconfig_test.go

@@ -17,7 +17,6 @@ package v4
 
 import (
 	"encoding/json"
-	"strings"
 	"testing"
 
 	"github.com/apache/trafficcontrol/lib/go-tc"
@@ -113,10 +112,8 @@ func UpdateTestCRConfigSnapshot(t *testing.T) {
 		}
 	}
 
-	if crc.Stats.TMPath == nil {
-		t.Errorf("GetCRConfig crc.Stats.Path expected: '/snapshot', actual: %+v", crc.Stats.TMPath)
-	} else if !strings.HasSuffix(*crc.Stats.TMPath, "snapshot") {
-		t.Errorf("GetCRConfig crc.Stats.Path expected: '/snapshot', actual: %+v", *crc.Stats.TMPath)
+	if crc.Stats.TMPath != nil {
+		t.Errorf("Expected no TMPath in APIv4, but it was: %v", *crc.Stats.TMPath)
 	}
 
 	if crc.Stats.TMHost == nil {

traffic_ops/traffic_ops_golang/crconfig/crconfig.go

@@ -30,7 +30,7 @@ import (
 )
 
 // Make creates and returns the CRConfig from the database.
-func Make(tx *sql.Tx, cdn, user, toHost, reqPath, toVersion string, useClientReqHost bool, emulateOldPath bool) (*tc.CRConfig, error) {
+func Make(tx *sql.Tx, cdn, user, toHost, toVersion string, useClientReqHost bool, emulateOldPath bool) (*tc.CRConfig, error) {
 	crc := tc.CRConfig{}
 	err := error(nil)
 
@@ -67,11 +67,11 @@ func Make(tx *sql.Tx, cdn, user, toHost, reqPath, toVersion string, useClientReq
 		toHost = getTMURLHost(paramTMURL)
 	}
 
+	crc.Stats = makeStats(cdn, user, toHost, toVersion)
 	if emulateOldPath {
-		reqPath = "/tools/write_crconfig/" + cdn
+		crc.Stats.TMPath = new(string)
+		*crc.Stats.TMPath = "/tools/write_crconfig/" + cdn
 	}
-
-	crc.Stats = makeStats(cdn, user, toHost, reqPath, toVersion)
 	return &crc, nil
 }
 

traffic_ops/traffic_ops_golang/crconfig/handler.go

@@ -23,6 +23,7 @@ import (
 	"database/sql"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -48,7 +49,8 @@ func Handler(w http.ResponseWriter, r *http.Request) {
 	defer inf.Close()
 
 	start := time.Now()
-	crConfig, err := Make(inf.Tx.Tx, inf.Params["cdn"], inf.User.UserName, r.Host, r.URL.Path, inf.Config.Version, inf.Config.CRConfigUseRequestHost, inf.Config.CRConfigEmulateOldPath)
+	emulate := inf.Config.CRConfigEmulateOldPath || inf.Version.Major < 4
+	crConfig, err := Make(inf.Tx.Tx, inf.Params["cdn"], inf.User.UserName, r.Host, inf.Config.Version, inf.Config.CRConfigUseRequestHost, emulate)
 	if err != nil {
 		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
 		return
@@ -75,8 +77,17 @@ func SnapshotGetHandler(w http.ResponseWriter, r *http.Request) {
 		api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("CDN not found"), nil)
 		return
 	}
-	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-	w.Write([]byte(`{"response":` + snapshot + `}`))
+
+	var decoded tc.CRConfig
+	if err = json.Unmarshal([]byte(snapshot), &decoded); err != nil {
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("failed to unmarshal stored snapshot for cdn '%s': %v", inf.Params["cdn"], err))
+	}
+
+	if inf.Version.Major < 4 || (inf.Config != nil && inf.Config.CRConfigEmulateOldPath) {
+		decoded.Stats.TMPath = new(string)
+		*decoded.Stats.TMPath = fmt.Sprintf("/api/4.0/cdns/%s/snapshot", inf.Params["cdn"])
+	}
+	api.WriteResp(w, r, decoded)
 }
 
 func SnapshotGetMonitoringLegacyHandler(w http.ResponseWriter, r *http.Request) {
@@ -214,7 +225,8 @@ func snapshotHandler(w http.ResponseWriter, r *http.Request, deprecated bool) {
 		}
 	}
 
-	crConfig, err := Make(inf.Tx.Tx, cdn, inf.User.UserName, r.Host, r.URL.Path, inf.Config.Version, inf.Config.CRConfigUseRequestHost, inf.Config.CRConfigEmulateOldPath)
+	// We never store tm_path, even though low API versions show it in responses.
+	crConfig, err := Make(inf.Tx.Tx, cdn, inf.User.UserName, r.Host, inf.Config.Version, inf.Config.CRConfigUseRequestHost, false)
 	if err != nil {
 		api.HandleErrOptionalDeprecation(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err, deprecated, &alt)
 		return
@@ -243,7 +255,7 @@ func snapshotHandler(w http.ResponseWriter, r *http.Request, deprecated bool) {
 	api.WriteResp(w, r, "SUCCESS")
 }
 
-// SnapshotGUIHandler creates the CRConfig JSON and writes it to the snapshot table in the database. The response emulates the old Perl UI function. This should go away when the old Perl UI ceases to exist.
+// SnapshotOldGUIHandler creates the CRConfig JSON and writes it to the snapshot table in the database. The response emulates the old Perl UI function. This should go away when the old Perl UI ceases to exist.
 func SnapshotOldGUIHandler(w http.ResponseWriter, r *http.Request) {
 	inf, userErr, sysErr, _ := api.NewInfo(r, []string{"cdn"}, nil)
 	if userErr != nil || sysErr != nil {
@@ -260,7 +272,8 @@ func SnapshotOldGUIHandler(w http.ResponseWriter, r *http.Request) {
 
 	cdn := inf.Params["cdn"]
 
-	crConfig, err := Make(inf.Tx.Tx, cdn, inf.User.UserName, r.Host, r.URL.Path, inf.Config.Version, inf.Config.CRConfigUseRequestHost, inf.Config.CRConfigEmulateOldPath)
+	// We never store tm_path, even though low API versions show it in responses.
+	crConfig, err := Make(inf.Tx.Tx, cdn, inf.User.UserName, r.Host, inf.Config.Version, inf.Config.CRConfigUseRequestHost, false)
 	if err != nil {
 		writePerlHTMLErr(w, r, inf.Tx.Tx, errors.New(r.RemoteAddr+" making CRConfig: "+err.Error()), err)
 		return

traffic_ops/traffic_ops_golang/crconfig/stats.go

@@ -25,13 +25,12 @@ import (
 	"github.com/apache/trafficcontrol/lib/go-tc"
 )
 
-func makeStats(cdn string, user string, host string, path string, version string) tc.CRConfigStats {
+func makeStats(cdn string, user string, host string, version string) tc.CRConfigStats {
 	epoch := time.Now().Unix()
 	return tc.CRConfigStats{
 		CDNName:         &cdn,
 		DateUnixSeconds: &epoch,
 		TMHost:          &host,
-		TMPath:          &path,
 		TMUser:          &user,
 		TMVersion:       &version,
 	}

traffic_ops/traffic_ops_golang/crconfig/stats_test.go

@@ -31,7 +31,6 @@ func ExpectedMakeStats() tc.CRConfigStats {
 	return tc.CRConfigStats{
 		CDNName:   randStr(),
 		TMHost:    randStr(),
-		TMPath:    randStr(),
 		TMUser:    randStr(),
 		TMVersion: randStr(),
 	}
@@ -40,7 +39,7 @@ func ExpectedMakeStats() tc.CRConfigStats {
 func TestMakeStats(t *testing.T) {
 	expected := ExpectedMakeStats()
 	start := time.Now()
-	actual := makeStats(*expected.CDNName, *expected.TMUser, *expected.TMHost, *expected.TMPath, *expected.TMVersion)
+	actual := makeStats(*expected.CDNName, *expected.TMUser, *expected.TMHost, *expected.TMVersion)
 	end := time.Now()
 	expected.DateUnixSeconds = actual.DateUnixSeconds
 	if !reflect.DeepEqual(expected, actual) {

traffic_router/core/src/test/resources/publish/CrConfig.json

@@ -1855,7 +1855,6 @@
     "tm_version": "1.5.0-3027.8d600a8e",
     "CDN_name": "thecdn",
     "tm_user": "admin1",
-    "tm_path": "/tools/write_crconfig/thecdn",
     "date": 1465084800,
     "tm_host": "localhost:8889"
   },
@@ -1961,4 +1960,3 @@
     "edge.http.routing": "true"
   }
 }
-

traffic_router/core/src/test/resources/publish/CrConfig2.json

@@ -1162,7 +1162,6 @@
     "tm_version": "1.5.0-3027.8d600a8e",
     "CDN_name": "thecdn",
     "tm_user": "admin2",
-    "tm_path": "/tools/write_crconfig/thecdn",
     "date": 1465171200,
     "tm_host": "localhost:8889"
   },
@@ -1251,4 +1250,3 @@
     "dnssec.enabled": "false"
   }
 }
-

traffic_router/core/src/test/resources/publish/CrConfig3.json

@@ -1213,7 +1213,6 @@
     "tm_version": "1.5.0-3027.8d600a8e",
     "CDN_name": "thecdn",
     "tm_user": "admin3",
-    "tm_path": "/tools/write_crconfig/thecdn",
     "date": 1465257600,
     "tm_host": "localhost:8889"
   },
@@ -1302,4 +1301,3 @@
     "dnssec.enabled": "false"
   }
 }
-