You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by oc...@apache.org on 2022/04/09 19:51:12 UTC
[trafficcontrol] branch master updated: Don't check for locks on dequeue (#6703)
This is an automated email from the ASF dual-hosted git repository.
ocket8888 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 d5ae30b8ae Don't check for locks on dequeue (#6703)
d5ae30b8ae is described below
commit d5ae30b8ae92e0ef390148a72d26d5c9123e0ae8
Author: Srijeet Chatterjee <30...@users.noreply.github.com>
AuthorDate: Sat Apr 9 13:51:07 2022 -0600
Don't check for locks on dequeue (#6703)
* Don't check for locks on dequeue
* code review fixes
* code review changes
* Fix bad merge
---
CHANGELOG.md | 1 +
traffic_ops/testing/api/v4/cdn_locks_test.go | 9 +++++++
.../traffic_ops_golang/cachegroup/queueupdate.go | 15 +++++++-----
traffic_ops/traffic_ops_golang/cdn/queue.go | 10 ++++----
.../traffic_ops_golang/server/queue_update.go | 10 ++++----
.../traffic_ops_golang/topology/queue_update.go | 28 ++++++++++++----------
.../dialog/select/lock/dialog.select.lock.tpl.html | 2 +-
7 files changed, 47 insertions(+), 28 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2b8ed45911..89f9a8ddf8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
### Fixed
- Update traffic\_portal dependencies to mitigate `npm audit` issues.
- Fixed a cdn-in-a-box build issue when using `RHEL_VERSION=7`
+- `dequeueing` server updates should not require checking for cdn locks.
- Fixed Traffic Ops ignoring the configured database port value, which was prohibiting the use of anything other than port 5432 (the PostgreSQL default)
- [#6580](https://github.com/apache/trafficcontrol/issues/6580) Fixed cache config generation remap.config targets for MID-type servers in a Topology with other caches as parents and HTTPS origins.
- Traffic Router: fixed a null pointer exception that caused snapshots to be rejected if a topology cachegroup did not have any online/reported/admin\_down caches
diff --git a/traffic_ops/testing/api/v4/cdn_locks_test.go b/traffic_ops/testing/api/v4/cdn_locks_test.go
index 3ec63b16f6..1b3e5314b0 100644
--- a/traffic_ops/testing/api/v4/cdn_locks_test.go
+++ b/traffic_ops/testing/api/v4/cdn_locks_test.go
@@ -113,6 +113,15 @@ func TestCDNLocks(t *testing.T) {
},
Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusForbidden)),
},
+ "Ok when ADMIN USER DOESNT OWN LOCK FOR DEQUEUE": {
+ ClientSession: TOSession,
+ RequestOpts: client.RequestOptions{QueryParameters: url.Values{"topology": {"top-for-ds-req"}}},
+ RequestBody: map[string]interface{}{
+ "action": "dequeue",
+ "cdnId": getCDNID(t, "cdn2"),
+ },
+ Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)),
+ },
},
"CACHE GROUP UPDATE": {
"OK when USER OWNS LOCK": {
diff --git a/traffic_ops/traffic_ops_golang/cachegroup/queueupdate.go b/traffic_ops/traffic_ops_golang/cachegroup/queueupdate.go
index f46278ef12..f889a67c2d 100644
--- a/traffic_ops/traffic_ops_golang/cachegroup/queueupdate.go
+++ b/traffic_ops/traffic_ops_golang/cachegroup/queueupdate.go
@@ -98,20 +98,23 @@ func QueueUpdates(w http.ResponseWriter, r *http.Request) {
return
}
- // Verify rights
- userErr, sysErr, statusCode := dbhelpers.CheckIfCurrentUserHasCdnLock(inf.Tx.Tx, string(*reqObj.CDN), inf.User.UserName)
- if userErr != nil || sysErr != nil {
- api.HandleErr(w, r, inf.Tx.Tx, statusCode, userErr, sysErr)
- return
+ queue := reqObj.Action == "queue"
+ if queue {
+ userErr, sysErr, statusCode := dbhelpers.CheckIfCurrentUserHasCdnLock(inf.Tx.Tx, string(*reqObj.CDN), inf.User.UserName)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, statusCode, userErr, sysErr)
+ return
+ }
}
// Queue updates
var updatedCaches []tc.CacheName
- if reqObj.Action == queue {
+ if reqObj.Action == "queue" {
updatedCaches, err = dbhelpers.QueueUpdateForServerWithCachegroupCDN(inf.Tx.Tx, cgID, cdnID)
} else {
updatedCaches, err = dbhelpers.DequeueUpdateForServerWithCachegroupCDN(inf.Tx.Tx, cgID, cdnID)
}
+
if err != nil {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("queueing updates: "+err.Error()))
return
diff --git a/traffic_ops/traffic_ops_golang/cdn/queue.go b/traffic_ops/traffic_ops_golang/cdn/queue.go
index 7c36a8ad1d..2d602a8b74 100644
--- a/traffic_ops/traffic_ops_golang/cdn/queue.go
+++ b/traffic_ops/traffic_ops_golang/cdn/queue.go
@@ -107,10 +107,12 @@ func Queue(w http.ResponseWriter, r *http.Request) {
str = fmt.Sprintf(" profileID: %d", profileID)
}
- 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
+ if reqObj.Action == "queue" {
+ 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
+ }
}
// Ignore pagination to prevent possibility of not updating the entirity the requested CDN. Likewise, ignore orderby as nonessential.
diff --git a/traffic_ops/traffic_ops_golang/server/queue_update.go b/traffic_ops/traffic_ops_golang/server/queue_update.go
index e231ba2e72..68b5af0c58 100644
--- a/traffic_ops/traffic_ops_golang/server/queue_update.go
+++ b/traffic_ops/traffic_ops_golang/server/queue_update.go
@@ -58,10 +58,12 @@ func QueueUpdateHandler(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
+ if reqObj.Action == "queue" {
+ 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
+ }
}
if reqObj.Action == "queue" {
diff --git a/traffic_ops/traffic_ops_golang/topology/queue_update.go b/traffic_ops/traffic_ops_golang/topology/queue_update.go
index c98a421369..2ea61e6641 100644
--- a/traffic_ops/traffic_ops_golang/topology/queue_update.go
+++ b/traffic_ops/traffic_ops_golang/topology/queue_update.go
@@ -77,19 +77,21 @@ func QueueUpdateHandler(w http.ResponseWriter, r *http.Request) {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, fmt.Errorf("invalid request to queue updates: %s", err), nil)
return
}
- cdnName, ok, err := dbhelpers.GetCDNNameFromID(inf.Tx.Tx, reqObj.CDNID)
- if err != nil {
- api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting CDN name from ID '"+strconv.Itoa(int(reqObj.CDNID))+"': "+err.Error()))
- return
- }
- if !ok {
- api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("cdn "+strconv.Itoa(int(reqObj.CDNID))+" does not exist"), nil)
- 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
+ if reqObj.Action == "queue" {
+ cdnName, ok, err := dbhelpers.GetCDNNameFromID(inf.Tx.Tx, reqObj.CDNID)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting CDN name from ID '"+strconv.Itoa(int(reqObj.CDNID))+"': "+err.Error()))
+ return
+ }
+ if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("cdn "+strconv.Itoa(int(reqObj.CDNID))+" does not exist"), nil)
+ 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
+ }
}
if reqObj.Action == "queue" {
diff --git a/traffic_portal/app/src/common/modules/dialog/select/lock/dialog.select.lock.tpl.html b/traffic_portal/app/src/common/modules/dialog/select/lock/dialog.select.lock.tpl.html
index d6230fd510..706a3ea9df 100644
--- a/traffic_portal/app/src/common/modules/dialog/select/lock/dialog.select.lock.tpl.html
+++ b/traffic_portal/app/src/common/modules/dialog/select/lock/dialog.select.lock.tpl.html
@@ -51,7 +51,7 @@ under the License.
</label>
<input id="soft" name="lockType" type="radio" ng-model="lock.soft" ng-value="true" required>
<label class="has-tooltip" for="hard">Hard<div class="helptooltip">
- <div class="helptext">A <dfn>hard</dfn> lock will prevent others from queueing server updates or snapshotting the locked CDN. It will also block others from making changes that would change the state of the locked CDN.
+ <div class="helptext">A <dfn>hard</dfn> lock will prevent others from queueing server updates or snapshotting the locked CDN. It will also block others from making changes (except clearing updates) that would change the state of the locked CDN.
</div>
</div>
</label>