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 2020/03/24 22:27:06 UTC
[trafficcontrol] branch master updated: Deprecate DELETE/GET
regions/:id (#4508)
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 68b7516 Deprecate DELETE/GET regions/:id (#4508)
68b7516 is described below
commit 68b75168d4ab31787648d1e566f3540b477f7196
Author: Michael Hoppal <54...@users.noreply.github.com>
AuthorDate: Tue Mar 24 16:26:58 2020 -0600
Deprecate DELETE/GET regions/:id (#4508)
---
CHANGELOG.md | 1 +
docs/source/api/v1/regions_id.rst | 9 +++
docs/source/api/v2/regions_id.rst | 68 -----------------
.../clients/python/trafficops/tosession.py | 11 ---
traffic_ops/client/region.go | 4 +-
traffic_ops/traffic_ops_golang/api/api.go | 1 -
.../traffic_ops_golang/api/shared_handlers.go | 86 ++++++++++++++++++++++
traffic_ops/traffic_ops_golang/routing/routes.go | 6 +-
8 files changed, 100 insertions(+), 86 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index eeb6741..596efe8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -85,6 +85,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- /profile/:id (GET)
- /profile/:id/unassigned_parameters
- /profile/trimmed
+ - /regions/:id (GET, DELETE)
- /regions/:region_name/phys_locations
- /regions/name/:region_name
- /riak/bucket/:bucket/key/:key/vault
diff --git a/docs/source/api/v1/regions_id.rst b/docs/source/api/v1/regions_id.rst
index e094652..2bff774 100644
--- a/docs/source/api/v1/regions_id.rst
+++ b/docs/source/api/v1/regions_id.rst
@@ -21,6 +21,9 @@
``GET``
=======
+.. deprecated:: ATCv4
+ Use the ``GET`` method of :ref:`to-api-v1-regions` with the query parameter ``id`` instead.
+
Retrieves a specific :term:`Region`.
@@ -84,6 +87,12 @@ Response Structure
"lastUpdated": "2018-12-05 17:50:58+00",
"name": "Montreal"
}
+ ],
+ "alerts": [
+ {
+ "text": "This endpoint is deprecated, please use GET /regions with query parameter id instead",
+ "level": "warning"
+ }
]}
diff --git a/docs/source/api/v2/regions_id.rst b/docs/source/api/v2/regions_id.rst
index 3e5fbd6..47fbe16 100644
--- a/docs/source/api/v2/regions_id.rst
+++ b/docs/source/api/v2/regions_id.rst
@@ -19,74 +19,6 @@
``regions/{{ID}}``
******************
-``GET``
-=======
-Retrieves a specific :term:`Region`.
-
-
-:Auth. Required: Yes
-:Roles Required: None
-:Response Type: Array
-
-Request Structure
------------------
-.. table:: Request Path Parameters
-
- +------+----------------------------------------------------------+
- | Name | Description |
- +======+==========================================================+
- | ID | The integral, unique identifier of the region to inspect |
- +------+----------------------------------------------------------+
-
-.. code-block:: http
- :caption: Request Example
-
- GET /api/2.0/regions/2 HTTP/1.1
- Host: trafficops.infra.ciab.test
- User-Agent: curl/7.47.0
- Accept: */*
- Cookie: mojolicious=...
-
-Response Structure
-------------------
- +----------------------+--------+------------------------------------------------+
- | Parameter | Type | Description |
- +======================+========+================================================+
- |``id`` | string | Region ID. |
- +----------------------+--------+------------------------------------------------+
- |``name`` | string | Region name. |
- +----------------------+--------+------------------------------------------------+
- |``division`` | string | Division ID. |
- +----------------------+--------+------------------------------------------------+
- |``divisionName`` | string | Division name. |
- +----------------------+--------+------------------------------------------------+
-
-.. code-block:: http
- :caption: Response Example
-
- HTTP/1.1 200 OK
- Access-Control-Allow-Credentials: true
- Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept, Set-Cookie, Cookie
- Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
- Access-Control-Allow-Origin: *
- Content-Type: application/json
- Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly
- Whole-Content-Sha512: nSYbR+fRXaxhYl7dWgf0Udo2AsiXEnwvED1CPbk7ZNWK03I3TOhtmCQx9ABnJJ6xKYnlt6EKMeopVTK0nKU+SQ==
- X-Server-Name: traffic_ops_golang/
- Date: Thu, 06 Dec 2018 02:07:17 GMT
- Content-Length: 117
-
- { "response": [
- {
- "divisionName": "Quebec",
- "division": 1,
- "id": 2,
- "lastUpdated": "2018-12-05 17:50:58+00",
- "name": "Montreal"
- }
- ]}
-
-
``PUT``
=======
Updates a :term:`Region`.
diff --git a/traffic_control/clients/python/trafficops/tosession.py b/traffic_control/clients/python/trafficops/tosession.py
index 19b5f46..7f19470 100644
--- a/traffic_control/clients/python/trafficops/tosession.py
+++ b/traffic_control/clients/python/trafficops/tosession.py
@@ -1551,17 +1551,6 @@ class TOSession(RestApiSession):
:raises: Union[LoginError, OperationError]
"""
- @api_request('get', 'regions/{region_id:d}', ('2.0',))
- def get_region_by_id(self, region_id=None):
- """
- Get Region by ID
- :ref:`to-api-regions-id`
- :param region_id: The region id of the region to retrieve
- :type region_id: int
- :rtype: Tuple[Union[Dict[str, Any], List[Dict[str, Any]]], requests.Response]
- :raises: Union[LoginError, OperationError]
- """
-
@api_request('put', 'regions/{region_id:d}', ('2.0',))
def update_region(self, region_id=None):
"""
diff --git a/traffic_ops/client/region.go b/traffic_ops/client/region.go
index 1f80e41..616c5b0 100644
--- a/traffic_ops/client/region.go
+++ b/traffic_ops/client/region.go
@@ -96,7 +96,7 @@ func (to *Session) GetRegions() ([]tc.Region, ReqInf, error) {
// GetRegionByID GETs a Region by the Region ID.
func (to *Session) GetRegionByID(id int) ([]tc.Region, ReqInf, error) {
- route := fmt.Sprintf("%s/%d", API_REGIONS, id)
+ route := fmt.Sprintf("%s?id=%d", API_REGIONS, id)
resp, remoteAddr, err := to.request(http.MethodGet, route, nil)
reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
if err != nil {
@@ -132,7 +132,7 @@ func (to *Session) GetRegionByName(name string) ([]tc.Region, ReqInf, error) {
// DeleteRegionByID DELETEs a Region by ID.
func (to *Session) DeleteRegionByID(id int) (tc.Alerts, ReqInf, error) {
- route := fmt.Sprintf("%s/%d", API_REGIONS, id)
+ route := fmt.Sprintf("%s?id=%d", API_REGIONS, id)
resp, remoteAddr, err := to.request(http.MethodDelete, route, nil)
reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
if err != nil {
diff --git a/traffic_ops/traffic_ops_golang/api/api.go b/traffic_ops/traffic_ops_golang/api/api.go
index a66707a..af138f1 100644
--- a/traffic_ops/traffic_ops_golang/api/api.go
+++ b/traffic_ops/traffic_ops_golang/api/api.go
@@ -158,7 +158,6 @@ func HandleDeprecatedErr(w http.ResponseWriter, r *http.Request, tx *sql.Tx, sta
log.Errorf("HandleDeprecatedErr called after a write already occurred! Attempting to write the error anyway! Path %s", r.URL.Path)
// Don't return, attempt to rollback and write the error anyway
}
- setRespWritten(r)
if tx != nil {
if err := tx.Rollback(); err != nil && err != sql.ErrTxDone {
diff --git a/traffic_ops/traffic_ops_golang/api/shared_handlers.go b/traffic_ops/traffic_ops_golang/api/shared_handlers.go
index 513fdcd..faceffe 100644
--- a/traffic_ops/traffic_ops_golang/api/shared_handlers.go
+++ b/traffic_ops/traffic_ops_golang/api/shared_handlers.go
@@ -368,6 +368,92 @@ func DeleteHandler(deleter Deleter) http.HandlerFunc {
}
}
+// DeprecatedDeleteHandler creates a handler function from the pointer to a struct implementing the Deleter interface with a optional deprecation notice
+// this generic handler encapsulates the logic for handling:
+// *fetching the id from the path parameter
+// *current user
+// *change log entry
+// *forming and writing the body over the wire
+func DeprecatedDeleteHandler(deleter Deleter, alternative *string) http.HandlerFunc {
+ return func(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := NewInfo(r, nil, nil)
+ if userErr != nil || sysErr != nil {
+ HandleDeprecatedErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr, alternative)
+ return
+ }
+ defer inf.Close()
+
+ interfacePtr := reflect.ValueOf(deleter)
+ if interfacePtr.Kind() != reflect.Ptr {
+ HandleDeprecatedErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("reflect: can only indirect from a pointer"), alternative)
+ return
+ }
+ objectType := reflect.Indirect(interfacePtr).Type()
+ obj := reflect.New(objectType).Interface().(Deleter)
+ obj.SetInfo(inf)
+
+ deleteKeyOptionExists, userErr, sysErr, errCode := hasDeleteKeyOption(obj, inf.Params)
+ if userErr != nil || sysErr != nil {
+ HandleDeprecatedErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr, alternative)
+ return
+ }
+ if !deleteKeyOptionExists {
+ keyFields := obj.GetKeyFieldsInfo() // expecting a slice of the key fields info which is a struct with the field name and a function to convert a string into a interface{} of the right type. in most that will be [{Field:"id",Func: func(s string)(interface{},error){return strconv.Atoi(s)}}]
+ keys := make(map[string]interface{})
+ for _, kf := range keyFields {
+ paramKey := inf.Params[kf.Field]
+ if paramKey == "" {
+ HandleDeprecatedErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("missing key: "+kf.Field), nil, alternative)
+ return
+ }
+
+ paramValue, err := kf.Func(paramKey)
+ if err != nil {
+ HandleDeprecatedErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("failed to parse key: "+kf.Field), nil, alternative)
+ return
+ }
+ keys[kf.Field] = paramValue
+ }
+ obj.SetKeys(keys) // if the type assertion of a key fails it will be should be set to the zero value of the type and the delete should fail (this means the code is not written properly no changes of user input should cause this.)
+ }
+
+ if t, ok := obj.(Tenantable); ok {
+ authorized, err := t.IsTenantAuthorized(inf.User)
+ if err != nil {
+ HandleDeprecatedErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("checking tenant authorized: "+err.Error()), alternative)
+ return
+ }
+ if !authorized {
+ HandleDeprecatedErr(w, r, inf.Tx.Tx, http.StatusForbidden, errors.New("not authorized on this tenant"), nil, alternative)
+ return
+ }
+ }
+
+ if deleteKeyOptionExists {
+ obj := reflect.New(objectType).Interface().(OptionsDeleter)
+ obj.SetInfo(inf)
+ userErr, sysErr, errCode = obj.OptionsDelete()
+ } else {
+ userErr, sysErr, errCode = obj.Delete()
+ }
+
+ if userErr != nil || sysErr != nil {
+ HandleDeprecatedErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr, alternative)
+ return
+ }
+
+ log.Debugf("changelog for delete on object")
+ if err := CreateChangeLog(ApiChange, Deleted, obj, inf.User, inf.Tx.Tx); err != nil {
+ HandleDeprecatedErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("inserting changelog: "+err.Error()), alternative)
+ return
+ }
+ alerts := CreateDeprecationAlerts(alternative)
+ alerts.AddNewAlert(tc.SuccessLevel, obj.GetType()+" was deleted.")
+
+ WriteAlerts(w, r, http.StatusOK, alerts)
+ }
+}
+
// CreateHandler creates a handler function from the pointer to a struct implementing the Creator interface
// this generic handler encapsulates the logic for handling:
// *current user
diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go
index 3141bc7..7e6c0b8 100644
--- a/traffic_ops/traffic_ops_golang/routing/routes.go
+++ b/traffic_ops/traffic_ops_golang/routing/routes.go
@@ -275,11 +275,9 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
//Region: CRUDs
{api.Version{2, 0}, http.MethodGet, `regions/?$`, api.ReadHandler(®ion.TORegion{}), auth.PrivLevelReadOnly, Authenticated, nil, 210037085, noPerlBypass},
- {api.Version{2, 0}, http.MethodGet, `regions/{id}$`, api.ReadHandler(®ion.TORegion{}), auth.PrivLevelReadOnly, Authenticated, nil, 2224440051, noPerlBypass},
{api.Version{2, 0}, http.MethodPut, `regions/{id}$`, api.UpdateHandler(®ion.TORegion{}), auth.PrivLevelOperations, Authenticated, nil, 222308224, noPerlBypass},
{api.Version{2, 0}, http.MethodPost, `regions/?$`, api.CreateHandler(®ion.TORegion{}), auth.PrivLevelOperations, Authenticated, nil, 2288334488, noPerlBypass},
{api.Version{2, 0}, http.MethodDelete, `regions/?$`, api.DeleteHandler(®ion.TORegion{}), auth.PrivLevelOperations, Authenticated, nil, 2232626758, noPerlBypass},
- {api.Version{2, 0}, http.MethodDelete, `regions/{id}$`, api.DeleteHandler(®ion.TORegion{}), auth.PrivLevelOperations, Authenticated, nil, 2181575271, noPerlBypass},
{api.Version{2, 0}, http.MethodDelete, `deliveryservice_server/{dsid}/{serverid}`, dsserver.Delete, auth.PrivLevelOperations, Authenticated, nil, 2532184523, noPerlBypass},
@@ -668,13 +666,13 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
//Region: CRUDs
{api.Version{1, 1}, http.MethodGet, `regions/?(\.json)?$`, api.ReadHandler(®ion.TORegion{}), auth.PrivLevelReadOnly, Authenticated, nil, 410037085, noPerlBypass},
- {api.Version{1, 1}, http.MethodGet, `regions/{id}$`, api.ReadHandler(®ion.TORegion{}), auth.PrivLevelReadOnly, Authenticated, nil, 2024440051, noPerlBypass},
+ {api.Version{1, 1}, http.MethodGet, `regions/{id}$`, api.DeprecatedReadHandler(®ion.TORegion{}, util.StrPtr("GET /regions with query parameter id")), auth.PrivLevelReadOnly, Authenticated, nil, 2024440051, noPerlBypass},
{api.Version{1, 1}, http.MethodGet, `regions/name/{name}/?(\.json)?$`, region.GetName, auth.PrivLevelReadOnly, Authenticated, nil, 503583197, noPerlBypass},
{api.Version{1, 1}, http.MethodPut, `regions/{id}$`, api.UpdateHandler(®ion.TORegion{}), auth.PrivLevelOperations, Authenticated, nil, 226308224, noPerlBypass},
{api.Version{1, 1}, http.MethodPost, `regions/?$`, api.CreateHandler(®ion.TORegion{}), auth.PrivLevelOperations, Authenticated, nil, 1288334488, noPerlBypass},
{api.Version{1, 5}, http.MethodDelete, `regions/?$`, api.DeleteHandler(®ion.TORegion{}), auth.PrivLevelOperations, Authenticated, nil, 2032626758, noPerlBypass},
{api.Version{1, 1}, http.MethodDelete, `regions/name/{name}$`, handlerToFunc(proxyHandler), 0, NoAuth, nil, 1925881096, noPerlBypass},
- {api.Version{1, 1}, http.MethodDelete, `regions/{id}$`, api.DeleteHandler(®ion.TORegion{}), auth.PrivLevelOperations, Authenticated, nil, 1181575271, noPerlBypass},
+ {api.Version{1, 1}, http.MethodDelete, `regions/{id}$`, api.DeprecatedDeleteHandler(®ion.TORegion{}, util.StrPtr("DELETE /regions with query parameter id")), auth.PrivLevelOperations, Authenticated, nil, 1181575271, noPerlBypass},
{api.Version{1, 1}, http.MethodDelete, `deliveryservice_server/{dsid}/{serverid}`, dsserver.Delete, auth.PrivLevelOperations, Authenticated, nil, 1532184523, noPerlBypass},