Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- To support reusing a single riak cluster connection, an optional parameter is added to riak.conf: "HealthCheckInterval". This options takes a 'Duration' value (ie: 10s, 5m) which affects how often the riak cluster is health checked. Default is currently set to: "HealthCheckInterval": "5s".
- Added an API 1.4 endpoint, /api/1.4/cdns/dnsseckeys/refresh, to perform necessary behavior previously served outside the API under `/internal`.
- Adds the DS Record text to the cdn dnsseckeys endpoint in 1.4.
- Added monitoring.json snapshotting. This stores the monitoring json in the same table as the crconfig snapshot. Snapshotting is now required in order to push out monitoring changes.

### Changed
- Issue 2821: Fixed "Traffic Router may choose wrong certificate when SNI names overlap"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*

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.
*/

-- +goose Up
-- SQL in section 'Up' is executed when this migration is applied

-- snapshots
ALTER TABLE snapshot ADD COLUMN monitoring json;
UPDATE snapshot SET monitoring = '{}';
ALTER TABLE snapshot ALTER COLUMN monitoring SET NOT NULL;
ALTER TABLE snapshot RENAME content TO crconfig;

-- +goose Down
-- SQL section 'Down' is executed when this migration is rolled back
ALTER TABLE snapshot DELETE COLUMN monitoring;
ALTER TABLE snapshot RENAME crconfig TO content;
44 changes: 40 additions & 4 deletions traffic_ops/traffic_ops_golang/crconfig/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"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/monitoring"
)

// Handler creates and serves the CRConfig from the raw SQL data.
Expand Down Expand Up @@ -73,6 +74,28 @@ func SnapshotGetHandler(w http.ResponseWriter, r *http.Request) {
w.Write([]byte(`{"response":` + snapshot + `}`))
}

// SnapshotGetMonitoringHandler gets and serves the CRConfig from the snapshot table.
func SnapshotGetMonitoringHandler(w http.ResponseWriter, r *http.Request) {
inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"cdn"}, nil)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
return
}
defer inf.Close()

snapshot, cdnExists, err := GetSnapshotMonitoring(inf.Tx.Tx, inf.Params["cdn"])
if err != nil {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting snapshot: "+err.Error()))
return
}
if !cdnExists {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("CDN not found"), nil)
return
}
w.Header().Set(tc.ContentType, tc.ApplicationJson)
w.Write([]byte(`{"response":` + snapshot + `}`))
}

// SnapshotOldGetHandler gets and serves the CRConfig from the snapshot table, not wrapped in response to match the old non-API CRConfig-Snapshots endpoint
func SnapshotOldGetHandler(w http.ResponseWriter, r *http.Request) {
inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"cdn"}, nil)
Expand Down Expand Up @@ -129,11 +152,18 @@ func SnapshotHandler(w http.ResponseWriter, r *http.Request) {
return
}

if err := Snapshot(inf.Tx.Tx, crConfig); err != nil {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New(r.RemoteAddr+" snaphsotting CRConfig: "+err.Error()))
monitoringJSON, err := monitoring.GetMonitoringJSON(inf.Tx.Tx, cdn)
if err != nil {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New(r.RemoteAddr+" getting monitoring.json data: "+err.Error()))
return
}

if err := Snapshot(inf.Tx.Tx, crConfig, monitoringJSON); err != nil {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New(r.RemoteAddr+" snaphsotting CRConfig and Monitoring: "+err.Error()))
return
}
api.CreateChangeLogRawTx(api.ApiChange, "Snapshot of CRConfig performed for "+cdn, inf.User, inf.Tx.Tx)

api.CreateChangeLogRawTx(api.ApiChange, "Snapshot of CRConfig and Monitor performed for "+cdn, inf.User, inf.Tx.Tx)
w.WriteHeader(http.StatusOK) // TODO change to 204 No Content in new version
}

Expand All @@ -152,7 +182,13 @@ func SnapshotOldGUIHandler(w http.ResponseWriter, r *http.Request) {
return
}

if err := Snapshot(inf.Tx.Tx, crConfig); err != nil {
tm, err := monitoring.GetMonitoringJSON(inf.Tx.Tx, inf.Params["cdn"])
if err != nil {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New(r.RemoteAddr+" getting monitoring.json data: "+err.Error()))
return
}

if err := Snapshot(inf.Tx.Tx, crConfig, tm); err != nil {
writePerlHTMLErr(w, r, inf.Tx.Tx, errors.New(r.RemoteAddr+" making CRConfig: "+err.Error()), err)
return
}
Expand Down
57 changes: 48 additions & 9 deletions traffic_ops/traffic_ops_golang/crconfig/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ import (

"github.com/apache/trafficcontrol/lib/go-log"
"github.com/apache/trafficcontrol/lib/go-tc"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/monitoring"
)

// Snapshot takes the CRConfig JSON-serializable object (which may be generated via crconfig.Make), and writes it to the snapshot table.
func Snapshot(tx *sql.Tx, crc *tc.CRConfig) error {
// It also takes the monitoring config JSON and writes it to the snapshot table.
func Snapshot(tx *sql.Tx, crc *tc.CRConfig, monitoringJSON *monitoring.Monitoring) error {
log.Debugln("calling Snapshot")
bts, err := json.Marshal(crc)
if err != nil {
Expand All @@ -40,10 +42,16 @@ func Snapshot(tx *sql.Tx, crc *tc.CRConfig) error {
if crc.Stats.DateUnixSeconds != nil {
date = time.Unix(*crc.Stats.DateUnixSeconds, 0)
}

btstm, err := json.Marshal(monitoringJSON)
if err != nil {
return errors.New("marshalling JSON: " + err.Error())
}

log.Debugf("calling Snapshot, writing %+v\n", date)
q := `insert into snapshot (cdn, content, last_updated) values ($1, $2, $3) on conflict(cdn) do update set content=$2, last_updated=$3`
if _, err := tx.Exec(q, crc.Stats.CDNName, bts, date); err != nil {
return errors.New("Error inserting the snapshot into database: " + err.Error())
q := `insert into snapshot (cdn, crconfig, last_updated, monitoring) values ($1, $2, $3, $4) on conflict(cdn) do update set crconfig=$2, last_updated=$3, monitoring=$4`
if _, err := tx.Exec(q, crc.Stats.CDNName, bts, date, btstm); err != nil {
return errors.New("Error inserting the crconfig and monitoring snapshot into database: " + err.Error())
}
return nil
}
Expand All @@ -58,21 +66,52 @@ func GetSnapshot(tx *sql.Tx, cdn string) (string, bool, error) {
snapshot := sql.NullString{}
// cdn left join snapshot, so we get a row with null if the CDN exists but the snapshot doesn't, and no rows if the CDN doesn't exist.
q := `
select s.content as snapshot
from cdn as c
left join snapshot as s on s.cdn = c.name
where c.name = $1
SELECT s.crconfig AS snapshot
FROM cdn AS c
LEFT JOIN snapshot AS s ON s.cdn = c.name
WHERE c.name = $1
`
if err := tx.QueryRow(q, cdn).Scan(&snapshot); err != nil {
if err == sql.ErrNoRows {
// CDN doesn't exist
return "", false, nil
}
return "", false, errors.New("Error querying snapshot: " + err.Error())
return "", false, errors.New("Error querying crconfig snapshot: " + err.Error())
}
if !snapshot.Valid {
// CDN exists, but snapshot doesn't
return `{}`, true, nil
}
return snapshot.String, true, nil
}

// GetSnapshotMonitoring gets the monitor snapshot for the given CDN.
// If the CDN does not exist, false is returned.
// If the CDN exists, but the snapshot does not, the string for an empty JSON object "{}" is returned.
// An error is only returned on database error, never if the CDN or snapshot does not exist.
// Because all snapshotting is handled by the crconfig endpoints we have to also do the monitoring one
// here as well
func GetSnapshotMonitoring(tx *sql.Tx, cdn string) (string, bool, error) {
log.Debugln("calling GetSnapshotMonitoring")

monitorSnapshot := sql.NullString{}
// cdn left join snapshot, so we get a row with null if the CDN exists but the snapshot doesn't, and no rows if the CDN doesn't exist.
q := `
SELECT s.monitoring AS snapshot
FROM cdn AS c
LEFT JOIN snapshot AS s ON s.cdn = c.name
WHERE c.name = $1
`
if err := tx.QueryRow(q, cdn).Scan(&monitorSnapshot); err != nil {
if err == sql.ErrNoRows {
// CDN doesn't exist
return "", false, nil
}
return "", false, errors.New("Error querying monitor snapshot: " + err.Error())
}
if !monitorSnapshot.Valid {
// CDN exists, but snapshot doesn't
return `{}`, true, nil
}
return monitorSnapshot.String, true, nil
}
37 changes: 26 additions & 11 deletions traffic_ops/traffic_ops_golang/crconfig/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,32 @@ package crconfig

import (
"context"
"database/sql"
"database/sql/driver"
"encoding/json"
"reflect"
"testing"
"time"

"github.com/apache/trafficcontrol/lib/go-tc"

"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/monitoring"
"gopkg.in/DATA-DOG/go-sqlmock.v1"
)

func ExpectedGetSnapshot(crc *tc.CRConfig) ([]byte, error) {
return json.Marshal(crc)
}

func ExpectedGetMontioringSnapshot(crc *tc.CRConfig, tx *sql.Tx) ([]byte, error) {
tm, _ := monitoring.GetMonitoringJSON(tx, *crc.Stats.CDNName)
return json.Marshal(tm)
}

func MockGetSnapshot(mock sqlmock.Sqlmock, expected []byte, cdn string) {
rows := sqlmock.NewRows([]string{"snapshot"})
rows = rows.AddRow(expected)
mock.ExpectQuery("select").WithArgs(cdn).WillReturnRows(rows)
rows = rows.AddRow(expected)
mock.ExpectQuery("SELECT").WithArgs(cdn).WillReturnRows(rows)
}

func TestGetSnapshot(t *testing.T) {
Expand Down Expand Up @@ -97,8 +104,8 @@ func (a Any) Match(v driver.Value) bool {
return true
}

func MockSnapshot(mock sqlmock.Sqlmock, expected []byte, cdn string) {
mock.ExpectExec("insert").WithArgs(cdn, expected, AnyTime{}).WillReturnResult(sqlmock.NewResult(1, 1))
func MockSnapshot(mock sqlmock.Sqlmock, expected []byte, expectedtm []byte, cdn string) {
mock.ExpectExec("insert").WithArgs(cdn, expected, AnyTime{}, expectedtm).WillReturnResult(sqlmock.NewResult(1, 1))
}

func TestSnapshot(t *testing.T) {
Expand All @@ -112,23 +119,31 @@ func TestSnapshot(t *testing.T) {

crc := &tc.CRConfig{}
crc.Stats.CDNName = &cdn

mock.ExpectBegin()

dbCtx, _ := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
tx, err := db.BeginTx(dbCtx, nil)
if err != nil {
t.Fatalf("creating transaction: %v", err)
}

expected, err := ExpectedGetSnapshot(crc)
if err != nil {
t.Fatalf("GetSnapshot creating expected err expected: nil, actual: %v", err)
}
MockSnapshot(mock, expected, cdn)
mock.ExpectCommit()

dbCtx, _ := context.WithTimeout(context.TODO(), time.Duration(10)*time.Second)
tx, err := db.BeginTx(dbCtx, nil)
expectedtm, err := ExpectedGetMontioringSnapshot(crc, tx)
if err != nil {
t.Fatalf("creating transaction: %v", err)
t.Fatalf("GetSnapshotMonitor creating expected err expected: nil, actual: %v", err)
}

tm, _ := monitoring.GetMonitoringJSON(tx, *crc.Stats.CDNName)
MockSnapshot(mock, expected, expectedtm, cdn)
mock.ExpectCommit()

defer tx.Commit()

if err := Snapshot(tx, crc); err != nil {
if err := Snapshot(tx, crc, tm); err != nil {
t.Fatalf("GetSnapshot err expected: nil, actual: %v", err)
}
}
4 changes: 2 additions & 2 deletions traffic_ops/traffic_ops_golang/monitoring/monitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,10 @@ func Get(w http.ResponseWriter, r *http.Request) {
return
}
defer inf.Close()
api.RespWriter(w, r, inf.Tx.Tx)(getMonitoringJSON(inf.Tx.Tx, inf.Params["cdn"]))
api.RespWriter(w, r, inf.Tx.Tx)(GetMonitoringJSON(inf.Tx.Tx, inf.Params["cdn"]))
}

func getMonitoringJSON(tx *sql.Tx, cdnName string) (*Monitoring, error) {
func GetMonitoringJSON(tx *sql.Tx, cdnName string) (*Monitoring, error) {
monitors, caches, routers, err := getMonitoringServers(tx, cdnName)
if err != nil {
return nil, fmt.Errorf("error getting servers: %v", err)
Expand Down
16 changes: 8 additions & 8 deletions traffic_ops/traffic_ops_golang/monitoring/monitoring_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -659,9 +659,9 @@ func TestGetMonitoringJSON(t *testing.T) {
t.Fatalf("creating transaction: %v", err)
}

sqlResp, err := getMonitoringJSON(tx, cdn)
sqlResp, err := GetMonitoringJSON(tx, cdn)
if err != nil {
t.Errorf("getMonitoringJSON expected: nil error, actual: %v", err)
t.Errorf("GetMonitoringJSON expected: nil error, actual: %v", err)
}

resp.Response.TrafficServers = sortCaches(resp.Response.TrafficServers)
Expand All @@ -676,22 +676,22 @@ func TestGetMonitoringJSON(t *testing.T) {
sqlResp.DeliveryServices = sortDeliveryServices(sqlResp.DeliveryServices)

if !reflect.DeepEqual(sqlResp.TrafficServers, resp.Response.TrafficServers) {
t.Errorf("getMonitoringJSON expected TrafficServers: %+v actual: %+v", resp.Response.TrafficServers, sqlResp.TrafficServers)
t.Errorf("GetMonitoringJSON expected TrafficServers: %+v actual: %+v", resp.Response.TrafficServers, sqlResp.TrafficServers)
}
if !reflect.DeepEqual(sqlResp.TrafficMonitors, resp.Response.TrafficMonitors) {
t.Errorf("getMonitoringJSON expected TrafficMonitors: %+v actual: %+v", resp.Response.TrafficMonitors, sqlResp.TrafficMonitors)
t.Errorf("GetMonitoringJSON expected TrafficMonitors: %+v actual: %+v", resp.Response.TrafficMonitors, sqlResp.TrafficMonitors)
}
if !reflect.DeepEqual(sqlResp.Cachegroups, resp.Response.Cachegroups) {
t.Errorf("getMonitoringJSON expected Cachegroups: %+v actual: %+v", resp.Response.Cachegroups, sqlResp.Cachegroups)
t.Errorf("GetMonitoringJSON expected Cachegroups: %+v actual: %+v", resp.Response.Cachegroups, sqlResp.Cachegroups)
}
if !reflect.DeepEqual(sqlResp.Profiles, resp.Response.Profiles) {
t.Errorf("getMonitoringJSON expected Profiles: %+v actual: %+v", resp.Response.Profiles, sqlResp.Profiles)
t.Errorf("GetMonitoringJSON expected Profiles: %+v actual: %+v", resp.Response.Profiles, sqlResp.Profiles)
}
if !reflect.DeepEqual(sqlResp.DeliveryServices, resp.Response.DeliveryServices) {
t.Errorf("getMonitoringJSON expected DeliveryServices: %+v actual: %+v", resp.Response.DeliveryServices, sqlResp.DeliveryServices)
t.Errorf("GetMonitoringJSON expected DeliveryServices: %+v actual: %+v", resp.Response.DeliveryServices, sqlResp.DeliveryServices)
}
if !reflect.DeepEqual(sqlResp.Config, resp.Response.Config) {
t.Errorf("getMonitoringJSON expected Config: %+v actual: %+v", resp.Response.Config, sqlResp.Config)
t.Errorf("GetMonitoringJSON expected Config: %+v actual: %+v", resp.Response.Config, sqlResp.Config)
}

}
6 changes: 1 addition & 5 deletions traffic_ops/traffic_ops_golang/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ import (
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/federations"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/hwinfo"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/login"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/monitoring"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/parameter"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/physlocation"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/ping"
Expand Down Expand Up @@ -142,7 +141,7 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
{1.4, http.MethodGet, `cdns/dnsseckeys/refresh/?(\.json)?$`, cdn.RefreshDNSSECKeys, auth.PrivLevelOperations, Authenticated, nil},

//CDN: Monitoring: Traffic Monitor
{1.1, http.MethodGet, `cdns/{cdn}/configs/monitoring(\.json)?$`, monitoring.Get, auth.PrivLevelReadOnly, Authenticated, nil},
{1.1, http.MethodGet, `cdns/{cdn}/configs/monitoring(\.json)?$`, crconfig.SnapshotGetMonitoringHandler, auth.PrivLevelReadOnly, Authenticated, nil},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we are just changing the behavior of the end point that TM uses, do you feel that there is value in knowing what the non-snapped Monitor config would look like? Seems to me like we should have two seperate end points, one for the monitoring snapshot and one for the raw monitoring.config file.

Copy link
Copy Markdown
Member

@rob05c rob05c Dec 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does change the behavior, because the existing behavior is fundamentally a bug, and it's never correct to use non-snapshotted data for monitoring.

do you feel that there is value in knowing what the non-snapped Monitor config would look like? Seems to me like we should have two seperate end points, one for the monitoring snapshot and one for the raw monitoring.config file.

I think that sounds good in theory, but in practice, I'm very concerned that someone might use the data, and it's always wrong to use it for anything practical. The monitoring data must always be synchronized with the CRConfig snapshot, or the race conditions in #1738 will happen.

But I can see the use case, e.g. for GUIs like the Portal to be able to present a diff. The CRConfig has a "raw" endpoint at /cdns/{cdn}/snapshot/new. Following that convention, what if we add a new un-snapshotted endpoint at cdns/{cdn}/configs/monitoring/new, and extensively document in the API Documentation, above the route in the routes.go file, and a GoDoc comment over the function itself, that it MUST NOT ever be used for any logic decisions, but only for presenting users with a comparison. How does that sound?

I understand the use case, but it really worries me if anyone were ever to use it for anything, they'd reintroduce the subtle and dangerous bug.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think the idea is that at some point we will want to be able to diff what is snapshotted vs what is raw. There could also be some future use cases where the raw data really is desired, I just want to make sure that we keep it available like it is today.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dneuman64 would you be good with us opening up a separate issue for that? We were throwing around some ideas like a diff end point instead of a full raw one and things like that, but its a bit out of the scope of this PR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think a new issue is fine.


//Division: CRUD
{1.1, http.MethodGet, `divisions/?(\.json)?$`, api.ReadHandler(division.GetTypeSingleton()), auth.PrivLevelReadOnly, Authenticated, nil},
Expand Down Expand Up @@ -276,9 +275,6 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
{1.2, http.MethodGet, `servers/status$`, handlerToFunc(proxyHandler), 0, NoAuth, []Middleware{}},
{1.2, http.MethodGet, `servers/totals$`, handlerToFunc(proxyHandler), 0, NoAuth, []Middleware{}},

//Monitoring
{1.2, http.MethodGet, `cdns/{name}/configs/monitoring(\.json)?$`, monitoring.Get, auth.PrivLevelReadOnly, Authenticated, nil},

//ASNs
{1.3, http.MethodGet, `asns/?(\.json)?$`, api.ReadHandler(asn.GetTypeSingleton()), auth.PrivLevelReadOnly, Authenticated, nil},
{1.3, http.MethodPut, `asns/?$`, api.UpdateHandler(asn.GetTypeSingleton()), auth.PrivLevelOperations, Authenticated, nil},
Expand Down