You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by zr...@apache.org on 2021/01/06 20:38:29 UTC
[trafficcontrol] branch master updated: Return the correct error
while trying to update the root tenant (#5397)
This is an automated email from the ASF dual-hosted git repository.
zrhoffman 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 e6e469c Return the correct error while trying to update the root tenant (#5397)
e6e469c is described below
commit e6e469ca1bf77385553b0bc13879877359618359
Author: Srijeet Chatterjee <30...@users.noreply.github.com>
AuthorDate: Wed Jan 6 13:37:09 2021 -0700
Return the correct error while trying to update the root tenant (#5397)
* Return the correct error while trying to update the root tenant
* use const
* fix test
* code review
* code review fixes
---
CHANGELOG.md | 1 +
traffic_ops/testing/api/v3/servers_test.go | 2 +-
traffic_ops/testing/api/v3/tenants_test.go | 26 +++++++++++++++++++---
traffic_ops/traffic_ops_golang/apitenant/tenant.go | 26 +++++++++++++++-------
traffic_ops/v3-client/tenant.go | 13 ++++++-----
5 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ca7e3be..52bd9f3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Traffic Ops: Added validation to ensure that the cachegroups of a delivery services' assigned ORG servers are present in the topology
### Fixed
+- [#5396](https://github.com/apache/trafficcontrol/issues/5396) - Return the correct error type if user tries to update the root tenant
- [#5378](https://github.com/apache/trafficcontrol/issues/5378) - Updating a non existent DS should return a 404, instead of a 500
- [#5380](https://github.com/apache/trafficcontrol/issues/5380) - Show the correct servers (including ORGs) when a topology based DS with required capabilities + ORG servers is queried for the assigned servers
- [#5195](https://github.com/apache/trafficcontrol/issues/5195) - Correctly show CDN ID in Changelog during Snap
diff --git a/traffic_ops/testing/api/v3/servers_test.go b/traffic_ops/testing/api/v3/servers_test.go
index fdc3ff5..67374a0 100644
--- a/traffic_ops/testing/api/v3/servers_test.go
+++ b/traffic_ops/testing/api/v3/servers_test.go
@@ -204,7 +204,7 @@ func UpdateTestServerStatus(t *testing.T) {
respServer := resp.Response[0]
- if *remoteServer.StatusLastUpdated != *respServer.StatusLastUpdated {
+ if !remoteServer.StatusLastUpdated.Equal(*respServer.StatusLastUpdated) {
t.Errorf("since status didnt change, no change in 'StatusLastUpdated' time was expected. Difference observer: old value: %v, new value: %v",
remoteServer.StatusLastUpdated.String(), respServer.StatusLastUpdated.String())
}
diff --git a/traffic_ops/testing/api/v3/tenants_test.go b/traffic_ops/testing/api/v3/tenants_test.go
index 55461d1..33b8aab 100644
--- a/traffic_ops/testing/api/v3/tenants_test.go
+++ b/traffic_ops/testing/api/v3/tenants_test.go
@@ -33,6 +33,7 @@ func TestTenants(t *testing.T) {
SortTestTenants(t)
GetTestTenants(t)
UpdateTestTenants(t)
+ UpdateTestRootTenant(t)
currentTime := time.Now().UTC().Add(-5 * time.Second)
time := currentTime.Format(time.RFC1123)
var header http.Header
@@ -62,12 +63,12 @@ func UpdateTestTenantsWithHeaders(t *testing.T, header http.Header) {
if newParent != nil {
modTenant.ParentID = newParent.ID
- _, err = TOSession.UpdateTenantWithHdr(strconv.Itoa(modTenant.ID), modTenant, header)
+ _, reqInf, err := TOSession.UpdateTenantWithHdr(strconv.Itoa(modTenant.ID), modTenant, header)
if err == nil {
t.Fatalf("expected a precondition failed error, got none")
}
- if !strings.Contains(err.Error(), "412 Precondition Failed[412]") {
- t.Errorf("expected a precondition failed error, got %v instead", err.Error())
+ if reqInf.StatusCode != http.StatusPreconditionFailed {
+ t.Errorf("expected a status 412 Precondition Failed, but got %d", reqInf.StatusCode)
}
}
}
@@ -170,6 +171,25 @@ func UpdateTestTenants(t *testing.T) {
}
+func UpdateTestRootTenant(t *testing.T) {
+ // Retrieve the Tenant by name so we can get the id for the Update
+ name := "root"
+ modTenant, _, err := TOSession.TenantByNameWithHdr(name, nil)
+ if err != nil {
+ t.Errorf("cannot GET Tenant by name: %s - %v", name, err)
+ }
+
+ modTenant.Active = false
+ modTenant.ParentID = modTenant.ID
+ _, reqInf, err := TOSession.UpdateTenantWithHdr(strconv.Itoa(modTenant.ID), modTenant, nil)
+ if err == nil {
+ t.Fatalf("expected an error when trying to update the 'root' tenant, but got nothing")
+ }
+ if reqInf.StatusCode != http.StatusBadRequest {
+ t.Errorf("expected a status 400 Bad Request, but got %d", reqInf.StatusCode)
+ }
+}
+
func DeleteTestTenants(t *testing.T) {
t1 := "tenant1"
diff --git a/traffic_ops/traffic_ops_golang/apitenant/tenant.go b/traffic_ops/traffic_ops_golang/apitenant/tenant.go
index 8861439..bd2b980 100644
--- a/traffic_ops/traffic_ops_golang/apitenant/tenant.go
+++ b/traffic_ops/traffic_ops_golang/apitenant/tenant.go
@@ -39,6 +39,8 @@ import (
"time"
)
+const rootName = `root`
+
// TOTenant provides a local type against which to define methods
type TOTenant struct {
api.APIInfoImpl `json:"-"`
@@ -184,13 +186,16 @@ func (ten *TOTenant) IsTenantAuthorized(user *auth.CurrentUser) (bool, error) {
// get current parentID to check if it's being changed
var parentID int
tx := ten.APIInfo().Tx.Tx
- err = tx.QueryRow(`SELECT parent_id FROM tenant WHERE id = ` + strconv.Itoa(*ten.ID)).Scan(&parentID)
- if err != nil {
- return false, err
- }
- if parentID == *ten.ParentID {
- // parent not being changed
- return ok, err
+ // If it's the root tenant, don't check for parent
+ if ten.Name != nil && *ten.Name != rootName {
+ err = tx.QueryRow(`SELECT parent_id FROM tenant WHERE id = ` + strconv.Itoa(*ten.ID)).Scan(&parentID)
+ if err != nil {
+ return false, err
+ }
+ if parentID == *ten.ParentID {
+ // parent not being changed
+ return ok, err
+ }
}
}
@@ -202,7 +207,12 @@ func (ten *TOTenant) IsTenantAuthorized(user *auth.CurrentUser) (bool, error) {
return tenant.IsResourceAuthorizedToUserTx(*ten.ParentID, user, ten.APIInfo().Tx.Tx)
}
-func (tn *TOTenant) Update(h http.Header) (error, error, int) { return api.GenericUpdate(h, tn) }
+func (tn *TOTenant) Update(h http.Header) (error, error, int) {
+ if tn.Name != nil && *tn.Name == rootName {
+ return errors.New("trying to change the root tenant, which is immutable"), nil, http.StatusBadRequest
+ }
+ return api.GenericUpdate(h, tn)
+}
func (ten *TOTenant) Delete() (error, error, int) {
result, err := ten.APIInfo().Tx.NamedExec(deleteQuery(), ten)
diff --git a/traffic_ops/v3-client/tenant.go b/traffic_ops/v3-client/tenant.go
index a7df17d..49e021c 100644
--- a/traffic_ops/v3-client/tenant.go
+++ b/traffic_ops/v3-client/tenant.go
@@ -119,25 +119,26 @@ func (to *Session) CreateTenant(t *tc.Tenant) (*tc.TenantResponse, error) {
return &data, nil
}
-func (to *Session) UpdateTenantWithHdr(id string, t *tc.Tenant, header http.Header) (*tc.TenantResponse, error) {
+func (to *Session) UpdateTenantWithHdr(id string, t *tc.Tenant, header http.Header) (*tc.TenantResponse, ReqInf, error) {
var data tc.TenantResponse
jsonReq, err := json.Marshal(t)
if err != nil {
- return nil, err
+ return nil, ReqInf{}, err
}
- _, err = put(to, fmt.Sprintf(API_TENANT_ID, id), jsonReq, &data, header)
+ reqInf, err := put(to, fmt.Sprintf(API_TENANT_ID, id), jsonReq, &data, header)
if err != nil {
- return nil, err
+ return nil, reqInf, err
}
- return &data, nil
+ return &data, reqInf, nil
}
// UpdateTenant updates the Tenant matching the ID it's passed with
// the Tenant it is passed.
// Deprecated: UpdateTenant will be removed in 6.0. Use UpdateTenantWithHdr.
func (to *Session) UpdateTenant(id string, t *tc.Tenant) (*tc.TenantResponse, error) {
- return to.UpdateTenantWithHdr(id, t, nil)
+ data, _, err := to.UpdateTenantWithHdr(id, t, nil)
+ return data, err
}
// DeleteTenant deletes the Tenant matching the ID it's passed.