From eb0982ca4ca6c31b1bf02f411b22b3bc6e92c768 Mon Sep 17 00:00:00 2001 From: shamrickus Date: Wed, 7 Sep 2022 11:02:55 -0600 Subject: [PATCH 1/5] Allow lock updates on some server params --- .../testing/api/v4/serverupdatestatus_test.go | 102 ++++++++++++++++++ .../dbhelpers/db_helpers.go | 7 +- .../traffic_ops_golang/server/update.go | 5 +- 3 files changed, 112 insertions(+), 2 deletions(-) diff --git a/traffic_ops/testing/api/v4/serverupdatestatus_test.go b/traffic_ops/testing/api/v4/serverupdatestatus_test.go index efb83c63a2..8a3520045d 100644 --- a/traffic_ops/testing/api/v4/serverupdatestatus_test.go +++ b/traffic_ops/testing/api/v4/serverupdatestatus_test.go @@ -29,6 +29,108 @@ 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.Errorf("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.Errorf("cannot login as '%s' user: %v", Config.TrafficOps.Users.Operations, err) + } + + // Make change + 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.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)) + } + afterServer := resp.Response[0] + + opts := client.NewRequestOptions() + opts.QueryParameters.Set("cdn", *testServer.CDNName) + _, _, err = TOSession.DeleteCDNLocks(opts) + if err != nil { + t.Errorf("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.Errorf("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.Errorf("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..cede65a1b1 100644 --- a/traffic_ops/traffic_ops_golang/server/update.go +++ b/traffic_ops/traffic_ops_golang/server/update.go @@ -296,8 +296,11 @@ func UpdateHandlerV4(w http.ResponseWriter, r *http.Request) { api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err) return } + + _, hasConfigApplyTimeParam := inf.Params["config_apply_time"] + _, hasRevalidateApplyTimeParam := inf.Params["revalidate_apply_time"] userErr, sysErr, statusCode := dbhelpers.CheckIfCurrentUserHasCdnLock(inf.Tx.Tx, string(cdnName), inf.User.UserName) - if userErr != nil || sysErr != nil { + if sysErr != nil || (userErr != nil && !hasConfigApplyTimeParam && !hasRevalidateApplyTimeParam) { api.HandleErr(w, r, inf.Tx.Tx, statusCode, userErr, sysErr) return } From d460ade32e4e477e1a0f45c12558418b2c834298 Mon Sep 17 00:00:00 2001 From: shamrickus Date: Wed, 7 Sep 2022 16:07:09 -0600 Subject: [PATCH 2/5] Add CHANGELOG and dont update when non-allowed fields are present. --- CHANGELOG.md | 23 +++++++++---------- .../testing/api/v4/serverupdatestatus_test.go | 13 ++++++++++- .../traffic_ops_golang/server/update.go | 10 +++++--- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c74a5b183a..119cd0f90a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,21 +5,20 @@ 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 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`. - -### 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 +- [#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. +- *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`. ### 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 +- [#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.. +- *Traffic Stats* Reuse InfluxDB client handle to prevent potential connection leaks. +### Changed +- *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 ## [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 8a3520045d..4ecaf3b226 100644 --- a/traffic_ops/testing/api/v4/serverupdatestatus_test.go +++ b/traffic_ops/testing/api/v4/serverupdatestatus_test.go @@ -84,7 +84,18 @@ func TestServerUpdateApplyTimeLocked(t *testing.T) { t.Errorf("cannot login as '%s' user: %v", Config.TrafficOps.Users.Operations, err) } - // Make change + 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) } diff --git a/traffic_ops/traffic_ops_golang/server/update.go b/traffic_ops/traffic_ops_golang/server/update.go index cede65a1b1..a605467351 100644 --- a/traffic_ops/traffic_ops_golang/server/update.go +++ b/traffic_ops/traffic_ops_golang/server/update.go @@ -297,11 +297,15 @@ func UpdateHandlerV4(w http.ResponseWriter, r *http.Request) { return } + _, hasConfigUpdatedBoolParam := inf.Params["updated"] + _, hasRevalUpdatedBoolParam := inf.Params["reval_updated"] _, hasConfigApplyTimeParam := inf.Params["config_apply_time"] _, hasRevalidateApplyTimeParam := inf.Params["revalidate_apply_time"] - userErr, sysErr, statusCode := dbhelpers.CheckIfCurrentUserHasCdnLock(inf.Tx.Tx, string(cdnName), inf.User.UserName) - if sysErr != nil || (userErr != nil && !hasConfigApplyTimeParam && !hasRevalidateApplyTimeParam) { - api.HandleErr(w, r, inf.Tx.Tx, statusCode, userErr, sysErr) + // Allow `apply_time` changes when the CDN is locked, but not `updated` + canIgnoreLock := (hasConfigApplyTimeParam || hasRevalidateApplyTimeParam) && !hasConfigUpdatedBoolParam && !hasRevalUpdatedBoolParam + userDoesntHaveLockErr, sysErr, statusCode := dbhelpers.CheckIfCurrentUserHasCdnLock(inf.Tx.Tx, string(cdnName), inf.User.UserName) + if sysErr != nil || (userDoesntHaveLockErr != nil && !canIgnoreLock) { + api.HandleErr(w, r, inf.Tx.Tx, statusCode, userDoesntHaveLockErr, sysErr) return } From dd4bec00671c64a2f2fdc8fdeb7c8e868c298252 Mon Sep 17 00:00:00 2001 From: shamrickus Date: Wed, 7 Sep 2022 16:10:38 -0600 Subject: [PATCH 3/5] Sort changelog --- CHANGELOG.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 119cd0f90a..9d7ac55c6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,21 +5,21 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ## [unreleased] ### Added +- *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`. - [#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. -- *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`. - -### Fixed -- [#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.. -- *Traffic Stats* Reuse InfluxDB client handle to prevent potential connection leaks. ### Changed - *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) *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 - [Traffic Portal] Added Layered Profile feature to /servers/ From 930599df5364474eb59f9e94d1dedad507004cb0 Mon Sep 17 00:00:00 2001 From: shamrickus Date: Thu, 15 Sep 2022 10:05:25 -0600 Subject: [PATCH 4/5] Fix issues --- CHANGELOG.md | 2 +- traffic_ops/traffic_ops_golang/server/update.go | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d7ac55c6a..fe3dee53cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ### Fixed - *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.. +- [#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/traffic_ops_golang/server/update.go b/traffic_ops/traffic_ops_golang/server/update.go index a605467351..4b792ca798 100644 --- a/traffic_ops/traffic_ops_golang/server/update.go +++ b/traffic_ops/traffic_ops_golang/server/update.go @@ -303,10 +303,12 @@ func UpdateHandlerV4(w http.ResponseWriter, r *http.Request) { _, hasRevalidateApplyTimeParam := inf.Params["revalidate_apply_time"] // Allow `apply_time` changes when the CDN is locked, but not `updated` canIgnoreLock := (hasConfigApplyTimeParam || hasRevalidateApplyTimeParam) && !hasConfigUpdatedBoolParam && !hasRevalUpdatedBoolParam - userDoesntHaveLockErr, sysErr, statusCode := dbhelpers.CheckIfCurrentUserHasCdnLock(inf.Tx.Tx, string(cdnName), inf.User.UserName) - if sysErr != nil || (userDoesntHaveLockErr != nil && !canIgnoreLock) { - api.HandleErr(w, r, inf.Tx.Tx, statusCode, userDoesntHaveLockErr, sysErr) - return + if !canIgnoreLock { + userDoesntHaveLockErr, sysErr, statusCode := dbhelpers.CheckIfCurrentUserHasCdnLock(inf.Tx.Tx, string(cdnName), inf.User.UserName) + if sysErr != nil || (userDoesntHaveLockErr != nil && !canIgnoreLock) { + api.HandleErr(w, r, inf.Tx.Tx, statusCode, userDoesntHaveLockErr, sysErr) + return + } } // TODO parse JSON body to trump Query Params? From cb06665c8cf639c11bd6df1ea41b1292d9f386ce Mon Sep 17 00:00:00 2001 From: shamrickus Date: Thu, 15 Sep 2022 10:30:51 -0600 Subject: [PATCH 5/5] Code review and add to v5 tests --- .../testing/api/v4/serverupdatestatus_test.go | 26 ++-- .../testing/api/v5/serverupdatestatus_test.go | 113 ++++++++++++++++++ .../traffic_ops_golang/server/update.go | 2 +- 3 files changed, 127 insertions(+), 14 deletions(-) diff --git a/traffic_ops/testing/api/v4/serverupdatestatus_test.go b/traffic_ops/testing/api/v4/serverupdatestatus_test.go index 4ecaf3b226..49eeaa8c98 100644 --- a/traffic_ops/testing/api/v4/serverupdatestatus_test.go +++ b/traffic_ops/testing/api/v4/serverupdatestatus_test.go @@ -54,34 +54,34 @@ func TestServerUpdateApplyTimeLocked(t *testing.T) { // Ensure baseline if beforeServer.UpdPending == nil { - t.Fatalf("Server '%s' had nil UpdPending before update status change", *testServer.HostName) + 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) + 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) + 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) + 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) + 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) + 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.Errorf("cannont acquire lock on the Server '%s' CDN '%s: %v", *testServer.HostName, *testServer.CDNName, err) + 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.Errorf("cannot login as '%s' user: %v", Config.TrafficOps.Users.Operations, err) + t.Fatalf("cannot login as '%s' user: %v", Config.TrafficOps.Users.Operations, err) } badQueryOpts := client.NewRequestOptions() @@ -102,7 +102,7 @@ func TestServerUpdateApplyTimeLocked(t *testing.T) { resp, _, err = opsSession.GetServers(opts) if err != nil { - t.Errorf("cannot GET Server by name '%s': %v - alerts: %+v", *testServer.HostName, err, resp.Alerts) + 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)) } @@ -112,11 +112,11 @@ func TestServerUpdateApplyTimeLocked(t *testing.T) { opts.QueryParameters.Set("cdn", *testServer.CDNName) _, _, err = TOSession.DeleteCDNLocks(opts) if err != nil { - t.Errorf("cannont delete acquired lock on the Server '%s' CDN '%s: %v", *testServer.HostName, *testServer.CDNName, err) + 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) + 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) @@ -125,12 +125,12 @@ func TestServerUpdateApplyTimeLocked(t *testing.T) { // Ensure values were actually set if configApply != nil { if afterServer.ConfigApplyTime == nil || !afterServer.ConfigApplyTime.Equal(*configApply) { - t.Errorf("Failed to set server's ConfigApplyTime. expected: %v actual: %v", *configApply, afterServer.ConfigApplyTime) + 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.Errorf("Failed to set server's RevalApplyTime. expected: %v actual: %v", *revalApply, afterServer.RevalApplyTime) + t.Fatalf("failed to set server's RevalApplyTime. expected: %v actual: %v", *revalApply, afterServer.RevalApplyTime) } } 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/server/update.go b/traffic_ops/traffic_ops_golang/server/update.go index 4b792ca798..741498397c 100644 --- a/traffic_ops/traffic_ops_golang/server/update.go +++ b/traffic_ops/traffic_ops_golang/server/update.go @@ -305,7 +305,7 @@ func UpdateHandlerV4(w http.ResponseWriter, r *http.Request) { 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 && !canIgnoreLock) { + if sysErr != nil || userDoesntHaveLockErr != nil { api.HandleErr(w, r, inf.Tx.Tx, statusCode, userDoesntHaveLockErr, sysErr) return }