You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by sr...@apache.org on 2022/09/15 19:09:05 UTC
[trafficcontrol] branch master updated: Allow update time params on Servers when CDN is locked. (#7065)
This is an automated email from the ASF dual-hosted git repository.
srijeet0406 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 4631c3ae5d Allow update time params on Servers when CDN is locked. (#7065)
4631c3ae5d is described below
commit 4631c3ae5dd3b9ed04ef9adae40a1de993b4306c
Author: Steve Hamrick <sh...@users.noreply.github.com>
AuthorDate: Thu Sep 15 13:08:57 2022 -0600
Allow update time params on Servers when CDN is locked. (#7065)
* Allow lock updates on some server params
* Add CHANGELOG and dont update when non-allowed fields are present.
* Sort changelog
* Fix issues
* Code review and add to v5 tests
---
CHANGELOG.md | 25 +++--
.../testing/api/v4/serverupdatestatus_test.go | 113 +++++++++++++++++++++
.../testing/api/v5/serverupdatestatus_test.go | 113 +++++++++++++++++++++
.../traffic_ops_golang/dbhelpers/db_helpers.go | 7 +-
traffic_ops/traffic_ops_golang/server/update.go | 17 +++-
5 files changed, 257 insertions(+), 18 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index df848da75e..a2749bba2e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,23 +5,22 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
## [unreleased]
### Added
-- [#2101](https://github.com/apache/trafficcontrol/issues/2101) Added the ability to tell if a Delivery Service is the target of another steering DS.
-- [#6033](https://github.com/apache/trafficcontrol/issues/6033) Added ability to assign multiple server capabilities to a server.
-- [#7032](https://github.com/apache/trafficcontrol/issues/7032) Add t3c-apply flag to use local ATS version for config generation rather than Server package Parameter, to allow managing the ATS OS package via external tools. See 'man t3c-apply' and 'man t3c-generate' for details.
-- Traffic Ops API version 5.0
-
-- [Traffic Monitor] Added logging for `ipv4Availability` and `ipv6Availability` in TM.
-- [Traffic Ops] Added the `ASN` field in TO Server struct, which provides the ability to query servers by `ASN`.
+- *Traffic Monitor* Added logging for `ipv4Availability` and `ipv6Availability` in TM.
+- *Traffic Ops* Added API version 5.0
+- *Traffic Ops* Added the `ASN` field in TO Server struct, which provides the ability to query servers by `ASN`.
+- [#2101](https://github.com/apache/trafficcontrol/issues/2101) *Traffic Portal* Added the ability to tell if a Delivery Service is the target of another steering DS.
+- [#6033](https://github.com/apache/trafficcontrol/issues/6033) *Traffic Ops, Traffic Portal* Added ability to assign multiple server capabilities to a server.
+- [#7032](https://github.com/apache/trafficcontrol/issues/7032) *Cache Config* Add t3c-apply flag to use local ATS version for config generation rather than Server package Parameter, to allow managing the ATS OS package via external tools. See 'man t3c-apply' and 'man t3c-generate' for details.
### Changed
-- Traffic Portal now obscures sensitive text in Delivery Service "Raw Remap" fields, private SSL keys, "Header Rewrite" rules, and ILO interface passwords by default.
-- Traffic Router now uses Traffic Ops API 4.0 by default
-- The Traffic Ops Python client now uses Traffic Ops API 4.1 by default.
+- *Traffic Ops* Python client now uses Traffic Ops API 4.1 by default.
+- *Traffic Portal* Obscures sensitive text in Delivery Service "Raw Remap" fields, private SSL keys, "Header Rewrite" rules, and ILO interface passwords by default.
+- *Traffic Router* Uses Traffic Ops API 4.0 by default
### Fixed
-- Traffic Stats: Reuse InfluxDB client handle to prevent potential connection leaks
-- [#7021](https://github.com/apache/trafficcontrol/issues/7021) Fixed cache config for Delivery Services with IP Origins
-
+- *Traffic Stats* Reuse InfluxDB client handle to prevent potential connection leaks.
+- [#7021](https://github.com/apache/trafficcontrol/issues/7021) *Cache Config* Fixed cache config for Delivery Services with IP Origins.
+- [#7047](https://github.com/apache/trafficcontrol/issues/7047) *Traffic Ops* allow `apply_time` query parameters on the `servers/{id-name}/update` when the CDN is locked.
## [7.0.0] - 2022-07-19
### Added
diff --git a/traffic_ops/testing/api/v4/serverupdatestatus_test.go b/traffic_ops/testing/api/v4/serverupdatestatus_test.go
index efb83c63a2..49eeaa8c98 100644
--- a/traffic_ops/testing/api/v4/serverupdatestatus_test.go
+++ b/traffic_ops/testing/api/v4/serverupdatestatus_test.go
@@ -29,6 +29,119 @@ import (
client "github.com/apache/trafficcontrol/traffic_ops/v4-client"
)
+func TestServerUpdateApplyTimeLocked(t *testing.T) {
+ WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, ServiceCategories, Topologies, DeliveryServices}, func() {
+ opts := client.NewRequestOptions()
+ opts.QueryParameters.Set("hostName", "atlanta-edge-01")
+ resp, _, err := TOSession.GetServers(opts)
+ if err != nil {
+ t.Fatalf("cannot get server by hostname: %v", err)
+ }
+ if len(resp.Response) != 1 {
+ t.Fatalf("Expected a server named 'atlanta-edge-01' to exist")
+ }
+ testServer := resp.Response[0]
+
+ testVals := func(configApply, revalApply *time.Time) {
+ resp, _, err := TOSession.GetServers(opts)
+ if err != nil {
+ t.Errorf("cannot get Server by name '%s': %v - alerts: %+v", *testServer.HostName, err, resp.Alerts)
+ } else if len(resp.Response) != 1 {
+ t.Fatalf("GET Server expected 1, actual %v", len(resp.Response))
+ }
+
+ beforeServer := resp.Response[0]
+
+ // Ensure baseline
+ if beforeServer.UpdPending == nil {
+ t.Fatalf("server '%s' had nil UpdPending before update status change", *testServer.HostName)
+ }
+ if beforeServer.RevalPending == nil {
+ t.Fatalf("server '%s' had nil RevalPending before update status change", *testServer.HostName)
+ }
+ if beforeServer.ConfigUpdateTime == nil {
+ t.Fatalf("server '%s' had nil ConfigUpdateTime before update status change", *testServer.HostName)
+ }
+ if beforeServer.ConfigApplyTime == nil {
+ t.Fatalf("server '%s' had nil ConfigApplyTime before update status change", *testServer.HostName)
+ }
+ if beforeServer.RevalUpdateTime == nil {
+ t.Fatalf("server '%s' had nil RevalUpdateTime before update status change", *testServer.HostName)
+ }
+ if beforeServer.RevalApplyTime == nil {
+ t.Fatalf("server '%s' had nil RevalApplyTime before update status change", *testServer.HostName)
+ }
+
+ _, _, err = TOSession.CreateCDNLock(tc.CDNLock{CDN: *testServer.CDNName, Soft: util.BoolPtr(false)}, client.NewRequestOptions())
+ if err != nil {
+ t.Fatalf("cannont acquire lock on the Server '%s' CDN '%s: %v", *testServer.HostName, *testServer.CDNName, err)
+ }
+
+ opsSession, _, err := client.LoginWithAgent(Config.TrafficOps.URL, Config.TrafficOps.Users.Operations,
+ Config.TrafficOps.UserPassword, true, "to-api-v4-locks-ops",
+ true, time.Second*time.Duration(Config.Default.Session.TimeoutInSecs))
+ if err != nil {
+ t.Fatalf("cannot login as '%s' user: %v", Config.TrafficOps.Users.Operations, err)
+ }
+
+ badQueryOpts := client.NewRequestOptions()
+ badQueryOpts.QueryParameters.Set("updated", "true")
+ if _, _, err := opsSession.SetUpdateServerStatusTimes(*testServer.HostName, configApply, revalApply, badQueryOpts); err == nil {
+ t.Fatalf("expected SetUpdateServerStatusTimes to not allow updates due to `updated` field")
+ }
+
+ badQueryOpts = client.NewRequestOptions()
+ badQueryOpts.QueryParameters.Set("reval_updated", "true")
+ if _, _, err := opsSession.SetUpdateServerStatusTimes(*testServer.HostName, configApply, revalApply, badQueryOpts); err == nil {
+ t.Fatalf("expected SetUpdateServerStatusTimes to not allow updates due to `reval_updated` field")
+ }
+
+ if alerts, _, err := opsSession.SetUpdateServerStatusTimes(*testServer.HostName, configApply, revalApply, client.RequestOptions{}); err != nil {
+ t.Fatalf("SetUpdateServerStatusTimes error. expected: nil, actual: %v - alerts: %+v", err, alerts.Alerts)
+ }
+
+ resp, _, err = opsSession.GetServers(opts)
+ if err != nil {
+ t.Fatalf("cannot GET Server by name '%s': %v - alerts: %+v", *testServer.HostName, err, resp.Alerts)
+ } else if len(resp.Response) != 1 {
+ t.Fatalf("GET Server expected 1, actual %v", len(resp.Response))
+ }
+ afterServer := resp.Response[0]
+
+ opts := client.NewRequestOptions()
+ opts.QueryParameters.Set("cdn", *testServer.CDNName)
+ _, _, err = TOSession.DeleteCDNLocks(opts)
+ if err != nil {
+ t.Fatalf("cannont delete acquired lock on the Server '%s' CDN '%s: %v", *testServer.HostName, *testServer.CDNName, err)
+ }
+
+ if afterServer.UpdPending == nil {
+ t.Fatalf("server '%s' had nil UpdPending after update status change", *testServer.HostName)
+ }
+ if afterServer.RevalPending == nil {
+ t.Fatalf("Server '%s' had nil RevalPending after update status change", *testServer.HostName)
+ }
+
+ // Ensure values were actually set
+ if configApply != nil {
+ if afterServer.ConfigApplyTime == nil || !afterServer.ConfigApplyTime.Equal(*configApply) {
+ t.Fatalf("failed to set server's ConfigApplyTime. expected: %v actual: %v", *configApply, afterServer.ConfigApplyTime)
+ }
+ }
+ if revalApply != nil {
+ if afterServer.RevalApplyTime == nil || !afterServer.RevalApplyTime.Equal(*revalApply) {
+ t.Fatalf("failed to set server's RevalApplyTime. expected: %v actual: %v", *revalApply, afterServer.RevalApplyTime)
+ }
+ }
+
+ }
+
+ now := time.Now().Round(time.Microsecond)
+ testVals(util.TimePtr(now), nil)
+ testVals(nil, util.TimePtr(now))
+ })
+}
+
func TestServerUpdateStatusLastAssigned(t *testing.T) {
WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, ServiceCategories, Topologies, DeliveryServices}, func() {
opts := client.NewRequestOptions()
diff --git a/traffic_ops/testing/api/v5/serverupdatestatus_test.go b/traffic_ops/testing/api/v5/serverupdatestatus_test.go
index 64dfd367cc..54ac8520b9 100644
--- a/traffic_ops/testing/api/v5/serverupdatestatus_test.go
+++ b/traffic_ops/testing/api/v5/serverupdatestatus_test.go
@@ -29,6 +29,119 @@ import (
client "github.com/apache/trafficcontrol/traffic_ops/v5-client"
)
+func TestServerUpdateApplyTimeLocked(t *testing.T) {
+ WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, ServiceCategories, Topologies, DeliveryServices}, func() {
+ opts := client.NewRequestOptions()
+ opts.QueryParameters.Set("hostName", "atlanta-edge-01")
+ resp, _, err := TOSession.GetServers(opts)
+ if err != nil {
+ t.Fatalf("cannot get server by hostname: %v", err)
+ }
+ if len(resp.Response) != 1 {
+ t.Fatalf("Expected a server named 'atlanta-edge-01' to exist")
+ }
+ testServer := resp.Response[0]
+
+ testVals := func(configApply, revalApply *time.Time) {
+ resp, _, err := TOSession.GetServers(opts)
+ if err != nil {
+ t.Errorf("cannot get Server by name '%s': %v - alerts: %+v", *testServer.HostName, err, resp.Alerts)
+ } else if len(resp.Response) != 1 {
+ t.Fatalf("GET Server expected 1, actual %v", len(resp.Response))
+ }
+
+ beforeServer := resp.Response[0]
+
+ // Ensure baseline
+ if beforeServer.UpdPending == nil {
+ t.Fatalf("server '%s' had nil UpdPending before update status change", *testServer.HostName)
+ }
+ if beforeServer.RevalPending == nil {
+ t.Fatalf("server '%s' had nil RevalPending before update status change", *testServer.HostName)
+ }
+ if beforeServer.ConfigUpdateTime == nil {
+ t.Fatalf("server '%s' had nil ConfigUpdateTime before update status change", *testServer.HostName)
+ }
+ if beforeServer.ConfigApplyTime == nil {
+ t.Fatalf("server '%s' had nil ConfigApplyTime before update status change", *testServer.HostName)
+ }
+ if beforeServer.RevalUpdateTime == nil {
+ t.Fatalf("server '%s' had nil RevalUpdateTime before update status change", *testServer.HostName)
+ }
+ if beforeServer.RevalApplyTime == nil {
+ t.Fatalf("server '%s' had nil RevalApplyTime before update status change", *testServer.HostName)
+ }
+
+ _, _, err = TOSession.CreateCDNLock(tc.CDNLock{CDN: *testServer.CDNName, Soft: util.BoolPtr(false)}, client.NewRequestOptions())
+ if err != nil {
+ t.Fatalf("cannont acquire lock on the Server '%s' CDN '%s: %v", *testServer.HostName, *testServer.CDNName, err)
+ }
+
+ opsSession, _, err := client.LoginWithAgent(Config.TrafficOps.URL, Config.TrafficOps.Users.Operations,
+ Config.TrafficOps.UserPassword, true, "to-api-v4-locks-ops",
+ true, time.Second*time.Duration(Config.Default.Session.TimeoutInSecs))
+ if err != nil {
+ t.Fatalf("cannot login as '%s' user: %v", Config.TrafficOps.Users.Operations, err)
+ }
+
+ badQueryOpts := client.NewRequestOptions()
+ badQueryOpts.QueryParameters.Set("updated", "true")
+ if _, _, err := opsSession.SetUpdateServerStatusTimes(*testServer.HostName, configApply, revalApply, badQueryOpts); err == nil {
+ t.Fatalf("expected SetUpdateServerStatusTimes to not allow updates due to `updated` field")
+ }
+
+ badQueryOpts = client.NewRequestOptions()
+ badQueryOpts.QueryParameters.Set("reval_updated", "true")
+ if _, _, err := opsSession.SetUpdateServerStatusTimes(*testServer.HostName, configApply, revalApply, badQueryOpts); err == nil {
+ t.Fatalf("expected SetUpdateServerStatusTimes to not allow updates due to `reval_updated` field")
+ }
+
+ if alerts, _, err := opsSession.SetUpdateServerStatusTimes(*testServer.HostName, configApply, revalApply, client.RequestOptions{}); err != nil {
+ t.Fatalf("SetUpdateServerStatusTimes error. expected: nil, actual: %v - alerts: %+v", err, alerts.Alerts)
+ }
+
+ resp, _, err = opsSession.GetServers(opts)
+ if err != nil {
+ t.Fatalf("cannot GET Server by name '%s': %v - alerts: %+v", *testServer.HostName, err, resp.Alerts)
+ } else if len(resp.Response) != 1 {
+ t.Fatalf("GET Server expected 1, actual %v", len(resp.Response))
+ }
+ afterServer := resp.Response[0]
+
+ opts := client.NewRequestOptions()
+ opts.QueryParameters.Set("cdn", *testServer.CDNName)
+ _, _, err = TOSession.DeleteCDNLocks(opts)
+ if err != nil {
+ t.Fatalf("cannont delete acquired lock on the Server '%s' CDN '%s: %v", *testServer.HostName, *testServer.CDNName, err)
+ }
+
+ if afterServer.UpdPending == nil {
+ t.Fatalf("server '%s' had nil UpdPending after update status change", *testServer.HostName)
+ }
+ if afterServer.RevalPending == nil {
+ t.Fatalf("Server '%s' had nil RevalPending after update status change", *testServer.HostName)
+ }
+
+ // Ensure values were actually set
+ if configApply != nil {
+ if afterServer.ConfigApplyTime == nil || !afterServer.ConfigApplyTime.Equal(*configApply) {
+ t.Fatalf("failed to set server's ConfigApplyTime. expected: %v actual: %v", *configApply, afterServer.ConfigApplyTime)
+ }
+ }
+ if revalApply != nil {
+ if afterServer.RevalApplyTime == nil || !afterServer.RevalApplyTime.Equal(*revalApply) {
+ t.Fatalf("failed to set server's RevalApplyTime. expected: %v actual: %v", *revalApply, afterServer.RevalApplyTime)
+ }
+ }
+
+ }
+
+ now := time.Now().Round(time.Microsecond)
+ testVals(util.TimePtr(now), nil)
+ testVals(nil, util.TimePtr(now))
+ })
+}
+
func TestServerUpdateStatusLastAssigned(t *testing.T) {
WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, ServiceCategories, Topologies, DeliveryServices}, func() {
opts := client.NewRequestOptions()
diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
index ed078e6ba2..10c5ff7fb0 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
@@ -105,7 +105,12 @@ WHERE tm_user.email = $1
// CheckIfCurrentUserHasCdnLock checks if the current user has the lock on the cdn that the requested operation is to be performed on.
// This will succeed if the either there is no lock by any user on the CDN, or if the current user has the lock on the CDN.
func CheckIfCurrentUserHasCdnLock(tx *sql.Tx, cdn, user string) (error, error, int) {
- query := `SELECT c.username, ARRAY_REMOVE(ARRAY_AGG(u.username), NULL) AS shared_usernames FROM cdn_lock c LEFT JOIN cdn_lock_user u ON c.username = u.owner AND c.cdn = u.cdn WHERE c.cdn=$1 GROUP BY c.username`
+ query := `
+SELECT c.username, ARRAY_REMOVE(ARRAY_AGG(u.username), NULL) AS shared_usernames
+FROM cdn_lock c
+ LEFT JOIN cdn_lock_user u ON c.username = u.owner AND c.cdn = u.cdn
+WHERE c.cdn=$1
+GROUP BY c.username`
var userName string
var sharedUserNames []string
rows, err := tx.Query(query, cdn)
diff --git a/traffic_ops/traffic_ops_golang/server/update.go b/traffic_ops/traffic_ops_golang/server/update.go
index dc300c35ab..741498397c 100644
--- a/traffic_ops/traffic_ops_golang/server/update.go
+++ b/traffic_ops/traffic_ops_golang/server/update.go
@@ -296,10 +296,19 @@ func UpdateHandlerV4(w http.ResponseWriter, r *http.Request) {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err)
return
}
- userErr, sysErr, statusCode := dbhelpers.CheckIfCurrentUserHasCdnLock(inf.Tx.Tx, string(cdnName), inf.User.UserName)
- if userErr != nil || sysErr != nil {
- api.HandleErr(w, r, inf.Tx.Tx, statusCode, userErr, sysErr)
- return
+
+ _, hasConfigUpdatedBoolParam := inf.Params["updated"]
+ _, hasRevalUpdatedBoolParam := inf.Params["reval_updated"]
+ _, hasConfigApplyTimeParam := inf.Params["config_apply_time"]
+ _, hasRevalidateApplyTimeParam := inf.Params["revalidate_apply_time"]
+ // Allow `apply_time` changes when the CDN is locked, but not `updated`
+ canIgnoreLock := (hasConfigApplyTimeParam || hasRevalidateApplyTimeParam) && !hasConfigUpdatedBoolParam && !hasRevalUpdatedBoolParam
+ if !canIgnoreLock {
+ userDoesntHaveLockErr, sysErr, statusCode := dbhelpers.CheckIfCurrentUserHasCdnLock(inf.Tx.Tx, string(cdnName), inf.User.UserName)
+ if sysErr != nil || userDoesntHaveLockErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, statusCode, userDoesntHaveLockErr, sysErr)
+ return
+ }
}
// TODO parse JSON body to trump Query Params?