You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ma...@apache.org on 2021/10/19 19:46:15 UTC
[trafficcontrol] branch master updated: POST request to
/api/4.0/phys_locations accepts mismatch values for regionName (#6269)
This is an automated email from the ASF dual-hosted git repository.
mattjackson 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 f2ee630 POST request to /api/4.0/phys_locations accepts mismatch values for regionName (#6269)
f2ee630 is described below
commit f2ee630cd3e6cdd67dcc44cc2de810e419d7c573
Author: Srijeet Chatterjee <30...@users.noreply.github.com>
AuthorDate: Tue Oct 19 14:46:05 2021 -0500
POST request to /api/4.0/phys_locations accepts mismatch values for regionName (#6269)
* check for mismatch values for regionName in POST and PUT /phys_locations
* modify update prerequisites
---
CHANGELOG.md | 1 +
traffic_ops/testing/api/v4/phys_locations_test.go | 50 ++++++++++++++++++++++
.../traffic_ops_golang/dbhelpers/db_helpers.go | 12 ++++++
traffic_ops/traffic_ops_golang/login/register.go | 1 -
.../physlocation/phys_locations.go | 27 +++++++++++-
traffic_ops/v4-client/phys_location.go | 2 +-
6 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 47a9fa0..6784a37 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- [#6125](https://github.com/apache/trafficcontrol/issues/6125) - Fix `/cdns/{name}/federations?id=#` to search for CDN.
- [#6255](https://github.com/apache/trafficcontrol/issues/6255) - Unreadable Prod Mode CDN Notifications in Traffic Portal
- [#6259](https://github.com/apache/trafficcontrol/issues/6259) - Traffic Portal No Longer Allows Spaces in Server Object "Router Port Name"
+- [#6175](https://github.com/apache/trafficcontrol/issues/6175) - POST request to /api/4.0/phys_locations accepts mismatch values for regionName.
### Changed
- Updated `t3c` to request less unnecessary deliveryservice-server assignment and invalidation jobs data via new query params supported by Traffic Ops
diff --git a/traffic_ops/testing/api/v4/phys_locations_test.go b/traffic_ops/testing/api/v4/phys_locations_test.go
index c787689..727aca8 100644
--- a/traffic_ops/testing/api/v4/phys_locations_test.go
+++ b/traffic_ops/testing/api/v4/phys_locations_test.go
@@ -26,6 +26,7 @@ import (
"time"
"github.com/apache/trafficcontrol/lib/go-rfc"
+ "github.com/apache/trafficcontrol/lib/go-tc"
client "github.com/apache/trafficcontrol/traffic_ops/v4-client"
)
@@ -50,6 +51,7 @@ func TestPhysLocations(t *testing.T) {
header.Set(rfc.IfMatch, etag)
UpdateTestPhysLocationsWithHeaders(t, header)
GetTestPaginationSupportPhysLocation(t)
+ CreatePhysLocationWithMismatchedRegionNameAndID(t)
})
}
@@ -276,6 +278,54 @@ func DeleteTestPhysLocations(t *testing.T) {
}
}
+func CreatePhysLocationWithMismatchedRegionNameAndID(t *testing.T) {
+ resp, _, err := TOSession.GetRegions(client.NewRequestOptions())
+ if err != nil {
+ t.Errorf("Unexpected error getting regions: %v - alerts: %+v", err, resp.Alerts)
+ }
+ if len(resp.Response) < 2 {
+ t.Fatalf("expected at least two regions to be returned, but got none")
+ }
+ physLocation := tc.PhysLocation{
+ Address: "100 blah lane",
+ City: "foo",
+ Comments: "comment",
+ Email: "bar@foobar.com",
+ Name: "testPhysicalLocation",
+ Phone: "111-222-3333",
+ RegionID: resp.Response[0].ID,
+ RegionName: resp.Response[1].Name,
+ ShortName: "testLocation1",
+ State: "CO",
+ Zip: "80602",
+ }
+ _, _, err = TOSession.CreatePhysLocation(physLocation, client.NewRequestOptions())
+ if err == nil {
+ t.Fatalf("expected an error about mismatched region name and ID, but got nothing")
+ }
+
+ physLocation.RegionName = resp.Response[0].Name
+ _, _, err = TOSession.CreatePhysLocation(physLocation, client.NewRequestOptions())
+ if err != nil {
+ t.Fatalf("expected no error while creating phys location, but got %v", err)
+ }
+
+ opts := client.NewRequestOptions()
+ opts.QueryParameters.Set("name", "testPhysicalLocation")
+ response, _, err := TOSession.GetPhysLocations(opts)
+ if err != nil {
+ t.Fatalf("cannot get Physical Location by name 'testPhysicalLocation': %v - alerts: %+v", err, resp.Alerts)
+ }
+ if len(response.Response) != 1 {
+ t.Fatalf("Expected exactly one Physical Location to exist with name 'testPhysicalLocation', found: %d", len(resp.Response))
+ }
+
+ _, _, err = TOSession.DeletePhysLocation(response.Response[0].ID, client.NewRequestOptions())
+ if err != nil {
+ t.Errorf("error deleteing physical location 'testPhysicalLocation': %v", err)
+ }
+}
+
func GetTestPaginationSupportPhysLocation(t *testing.T) {
opts := client.NewRequestOptions()
opts.QueryParameters.Set("orderby", "id")
diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
index 2e7128b..aef6e3a 100644
--- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
+++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
@@ -1727,3 +1727,15 @@ func GetCDNNameDomain(cdnID int, tx *sql.Tx) (string, string, error) {
}
return cdnName, cdnDomain, nil
}
+
+// GetRegionNameFromID returns the name of the region associated with the supplied ID.
+func GetRegionNameFromID(tx *sql.Tx, regionID int) (string, bool, error) {
+ var regionName string
+ if err := tx.QueryRow(`SELECT name FROM region WHERE id = $1`, regionID).Scan(®ionName); err != nil {
+ if errors.Is(err, sql.ErrNoRows) {
+ return regionName, false, nil
+ }
+ return regionName, false, fmt.Errorf("querying region name from ID: %w", err)
+ }
+ return regionName, true, nil
+}
diff --git a/traffic_ops/traffic_ops_golang/login/register.go b/traffic_ops/traffic_ops_golang/login/register.go
index a10d7e4..8f664a3 100644
--- a/traffic_ops/traffic_ops_golang/login/register.go
+++ b/traffic_ops/traffic_ops_golang/login/register.go
@@ -225,7 +225,6 @@ func RegisterUser(w http.ResponseWriter, r *http.Request) {
} else {
req.Email = reqV4.Email
req.TenantID = reqV4.TenantID
- dbhelpers.GetRoleIDFromName(tx, reqV4.Role)
roleID, ok, err := dbhelpers.GetRoleIDFromName(tx, reqV4.Role)
if err != nil {
api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("error fetching ID from role name: %w", err))
diff --git a/traffic_ops/traffic_ops_golang/physlocation/phys_locations.go b/traffic_ops/traffic_ops_golang/physlocation/phys_locations.go
index da8922d..d6fbd85 100644
--- a/traffic_ops/traffic_ops_golang/physlocation/phys_locations.go
+++ b/traffic_ops/traffic_ops_golang/physlocation/phys_locations.go
@@ -20,6 +20,8 @@ package physlocation
*/
import (
+ "errors"
+ "fmt"
"net/http"
"strconv"
"time"
@@ -116,9 +118,30 @@ JOIN region r ON pl.region = r.id ` + where + orderBy + pagination +
select max(last_updated) as t from last_deleted l where l.table_name='phys_location') as res`
}
+// MatchRegionNameAndID checks to see if the supplied region name and ID in the phys_location body correspond to each other.
+func (pl *TOPhysLocation) MatchRegionNameAndID() (error, error, int) {
+ if pl.RegionName != nil {
+ regionName, ok, err := dbhelpers.GetRegionNameFromID(pl.APIInfo().Tx.Tx, *pl.RegionID)
+ if err != nil {
+ return nil, fmt.Errorf("error fetching name from region ID: %w", err), http.StatusInternalServerError
+ } else if !ok {
+ return errors.New("no such region"), nil, http.StatusNotFound
+ }
+ if regionName != *pl.RegionName {
+ return errors.New("region name and ID do not match"), nil, http.StatusBadRequest
+ }
+ }
+ return nil, nil, http.StatusOK
+}
+
func (pl *TOPhysLocation) Update(h http.Header) (error, error, int) { return api.GenericUpdate(h, pl) }
-func (pl *TOPhysLocation) Create() (error, error, int) { return api.GenericCreate(pl) }
-func (pl *TOPhysLocation) Delete() (error, error, int) { return api.GenericDelete(pl) }
+func (pl *TOPhysLocation) Create() (error, error, int) {
+ if userErr, sysErr, statusCode := pl.MatchRegionNameAndID(); userErr != nil || sysErr != nil {
+ return userErr, sysErr, statusCode
+ }
+ return api.GenericCreate(pl)
+}
+func (pl *TOPhysLocation) Delete() (error, error, int) { return api.GenericDelete(pl) }
func selectQuery() string {
return `
diff --git a/traffic_ops/v4-client/phys_location.go b/traffic_ops/v4-client/phys_location.go
index 5f43dea..c9c94f3 100644
--- a/traffic_ops/v4-client/phys_location.go
+++ b/traffic_ops/v4-client/phys_location.go
@@ -30,7 +30,7 @@ func (to *Session) CreatePhysLocation(pl tc.PhysLocation, opts RequestOptions) (
if pl.RegionID == 0 && pl.RegionName != "" {
regionOpts := NewRequestOptions()
regionOpts.QueryParameters.Set("name", pl.RegionName)
- regions, reqInf, err := to.GetRegions(opts)
+ regions, reqInf, err := to.GetRegions(regionOpts)
if err != nil {
err = fmt.Errorf("resolving Region name '%s' to an ID", pl.RegionName)
return regions.Alerts, reqInf, err