You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by mi...@apache.org on 2020/03/16 14:53:25 UTC

[trafficcontrol] branch master updated: Rework cdns/name/:name/dnsseckeys/delete (#4482)

This is an automated email from the ASF dual-hosted git repository.

mitchell852 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 7714637  Rework cdns/name/:name/dnsseckeys/delete (#4482)
7714637 is described below

commit 7714637a9f137fd0e57f2127e5819ad2d21de6a8
Author: Steve Hamrick <sh...@users.noreply.github.com>
AuthorDate: Mon Mar 16 08:53:12 2020 -0600

    Rework cdns/name/:name/dnsseckeys/delete (#4482)
    
    * Rework dnsseckeys delete WIP
    
    * Use correct functions
    
    * Missed comma
    
    * Consistency
    
    * Fix bad docstring
---
 .../api/v1/cdns_name_name_dnsseckeys_delete.rst    |  7 ++++
 docs/source/api/v2/cdns_name_name_dnsseckeys.rst   | 27 ++++++++++++
 .../api/v2/cdns_name_name_dnsseckeys_delete.rst    | 48 ----------------------
 .../clients/python/trafficops/tosession.py         |  4 +-
 traffic_ops/app/db/seeds.sql                       |  1 +
 traffic_ops/traffic_ops_golang/api/api.go          | 23 +++++++----
 traffic_ops/traffic_ops_golang/api/api_test.go     | 47 +++++++++++++++++++++
 traffic_ops/traffic_ops_golang/cdn/dnssec.go       | 37 ++++++++++++++---
 traffic_ops/traffic_ops_golang/routing/routes.go   |  4 +-
 9 files changed, 132 insertions(+), 66 deletions(-)

diff --git a/docs/source/api/v1/cdns_name_name_dnsseckeys_delete.rst b/docs/source/api/v1/cdns_name_name_dnsseckeys_delete.rst
index 24e164c..87606f5 100644
--- a/docs/source/api/v1/cdns_name_name_dnsseckeys_delete.rst
+++ b/docs/source/api/v1/cdns_name_name_dnsseckeys_delete.rst
@@ -23,6 +23,9 @@
 =======
 Delete DNSSEC keys for a CDN and all associated :term:`Delivery Services`.
 
+.. deprecated:: ATCv4
+	Use the ``DELETE`` method of :ref:`to-api-cdns-name-name-dnsseckeys` instead.
+
 :Auth. Required: Yes
 :Roles Required: "admin"
 :Response Type:  Object (string)
@@ -43,6 +46,10 @@ Response Structure
 	:caption: Response Example
 
 	{
+		"alerts": [{
+			"level": "warning",
+			"text": "This endpoint is deprected, please use DELETE /cdns/name/{name}/dnsseckeys instead"
+		}],
 		"response": "Successfully deleted dnssec keys for test"
 	}
 
diff --git a/docs/source/api/v2/cdns_name_name_dnsseckeys.rst b/docs/source/api/v2/cdns_name_name_dnsseckeys.rst
index 107ae8e..fbceaf0 100644
--- a/docs/source/api/v2/cdns_name_name_dnsseckeys.rst
+++ b/docs/source/api/v2/cdns_name_name_dnsseckeys.rst
@@ -111,3 +111,30 @@ Response Structure
 		}
 	}}
 
+``DELETE``
+==========
+Delete DNSSEC keys for a CDN and all associated :term:`Delivery Services`.
+
+:Auth. Required: Yes
+:Roles Required: "admin"
+:Response Type:  Object (string)
+
+Request Structure
+-----------------
+.. table:: Request Path Parameters
+
+	+------+-----------------------------------------------------------+
+	| Name |                       Description                         |
+	+======+===========================================================+
+	| name | The name of the CDN for which DNSSEC keys will be deleted |
+	+------+-----------------------------------------------------------+
+
+Response Structure
+------------------
+.. code-block:: json
+	:caption: Response Example
+
+	{
+		"response": "Successfully deleted dnssec keys for test"
+	}
+
diff --git a/docs/source/api/v2/cdns_name_name_dnsseckeys_delete.rst b/docs/source/api/v2/cdns_name_name_dnsseckeys_delete.rst
deleted file mode 100644
index da10fdb..0000000
--- a/docs/source/api/v2/cdns_name_name_dnsseckeys_delete.rst
+++ /dev/null
@@ -1,48 +0,0 @@
-..
-..
-.. Licensed under the Apache License, Version 2.0 (the "License");
-.. you may not use this file except in compliance with the License.
-.. You may obtain a copy of the License at
-..
-..     http://www.apache.org/licenses/LICENSE-2.0
-..
-.. Unless required by applicable law or agreed to in writing, software
-.. distributed under the License is distributed on an "AS IS" BASIS,
-.. WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-.. See the License for the specific language governing permissions and
-.. limitations under the License.
-..
-
-.. _to-api-cdns-name-name-dnsseckeys-delete:
-
-****************************************
-``cdns/name/{{name}}/dnsseckeys/delete``
-****************************************
-
-``GET``
-=======
-Delete DNSSEC keys for a CDN and all associated :term:`Delivery Services`.
-
-:Auth. Required: Yes
-:Roles Required: "admin"
-:Response Type:  Object (string)
-
-Request Structure
------------------
-.. table:: Request Path Parameters
-
-	+------+-----------------------------------------------------------+
-	| Name |                       Description                         |
-	+======+===========================================================+
-	| name | The name of the CDN for which DNSSEC keys will be deleted |
-	+------+-----------------------------------------------------------+
-
-Response Structure
-------------------
-.. code-block:: json
-	:caption: Response Example
-
-	{
-		"response": "Successfully deleted dnssec keys for test"
-	}
-
diff --git a/traffic_control/clients/python/trafficops/tosession.py b/traffic_control/clients/python/trafficops/tosession.py
index 02049e3..827b5b4 100644
--- a/traffic_control/clients/python/trafficops/tosession.py
+++ b/traffic_control/clients/python/trafficops/tosession.py
@@ -630,11 +630,11 @@ class TOSession(RestApiSession):
 		:raises: Union[LoginError, OperationError]
 		"""
 
-	@api_request('get', 'cdns/name/{cdn_name:s}/dnsseckeys/delete', ('2.0',))
+	@api_request('delete', 'cdns/name/{cdn_name:s}/dnsseckeys', ('2.0',))
 	def delete_cdn_dns_sec_keys(self, cdn_name=None):
 		"""
 		Delete dnssec keys for a cdn and all associated delivery services
-		:ref:`to-api-cdns-name-name-dnsseckeys-delete`
+		:ref:`to-api-cdns-name-name-dnsseckeys`
 		:param cdn_name: The CDN name to delete dnsseckeys info for
 		:type cdn_name: String
 		:rtype: Tuple[Dict[str, Any], requests.Response]
diff --git a/traffic_ops/app/db/seeds.sql b/traffic_ops/app/db/seeds.sql
index 75b85b8..38a2b59 100644
--- a/traffic_ops/app/db/seeds.sql
+++ b/traffic_ops/app/db/seeds.sql
@@ -483,6 +483,7 @@ insert into api_capability (http_method, route, capability) values ('GET', 'cdns
 insert into api_capability (http_method, route, capability) values ('GET', 'cdns/name/*/dnsseckeys', 'cdn-security-keys-read') ON CONFLICT (http_method, route, capability) DO NOTHING;
 insert into api_capability (http_method, route, capability) values ('POST', 'cdns/dnsseckeys/generate', 'cdn-security-keys-write') ON CONFLICT (http_method, route, capability) DO NOTHING;
 insert into api_capability (http_method, route, capability) values ('GET', 'cdns/name/*/dnsseckeys/delete', 'cdn-security-keys-write') ON CONFLICT (http_method, route, capability) DO NOTHING;
+insert into api_capability (http_method, route, capability) values ('DELETE', 'cdns/name/*/dnsseckeys', 'cdn-security-keys-write') ON CONFLICT (http_method, route, capability) DO NOTHING;
 -- change logs
 insert into api_capability (http_method, route, capability) values ('GET', 'logs', 'change-logs-read') ON CONFLICT (http_method, route, capability) DO NOTHING;
 insert into api_capability (http_method, route, capability) values ('GET', 'logs/*/days', 'change-logs-read') ON CONFLICT (http_method, route, capability) DO NOTHING;
diff --git a/traffic_ops/traffic_ops_golang/api/api.go b/traffic_ops/traffic_ops_golang/api/api.go
index 2e2f2f7..a66707a 100644
--- a/traffic_ops/traffic_ops_golang/api/api.go
+++ b/traffic_ops/traffic_ops_golang/api/api.go
@@ -94,7 +94,7 @@ func WriteRespRaw(w http.ResponseWriter, r *http.Request, v interface{}) {
 		tc.GetHandleErrorsFunc(w, r)(http.StatusInternalServerError, errors.New(http.StatusText(http.StatusInternalServerError)))
 		return
 	}
-	w.Header().Set("Content-Type", "application/json")
+	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
 	w.Write(append(bts, '\n'))
 }
 
@@ -287,7 +287,7 @@ func WriteRespAlertObj(w http.ResponseWriter, r *http.Request, level tc.AlertLev
 		return
 	}
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
-	w.Write(append(respBts, '\n'))
+	_, _ = w.Write(append(respBts, '\n'))
 }
 
 func WriteAlerts(w http.ResponseWriter, r *http.Request, code int, alerts tc.Alerts) {
@@ -297,17 +297,24 @@ func WriteAlerts(w http.ResponseWriter, r *http.Request, code int, alerts tc.Ale
 	}
 	setRespWritten(r)
 
-	resp, err := json.Marshal(alerts)
-	if err != nil {
-		handleSimpleErr(w, r, http.StatusInternalServerError, nil, fmt.Errorf("marshalling JSON: %v", err))
-		return
-	}
 	w.Header().Set(rfc.ContentType, rfc.ApplicationJSON)
 	w.WriteHeader(code)
-	w.Write(append(resp, '\n'))
+	if alerts.HasAlerts() {
+		resp, err := json.Marshal(alerts)
+		if err != nil {
+			handleSimpleErr(w, r, http.StatusInternalServerError, nil, fmt.Errorf("marshalling JSON: %v", err))
+			return
+		}
+		_, _ = w.Write(append(resp, '\n'))
+	}
 }
 
 func WriteAlertsObj(w http.ResponseWriter, r *http.Request, code int, alerts tc.Alerts, obj interface{}) {
+	if !alerts.HasAlerts() {
+		WriteResp(w, r, obj)
+		w.WriteHeader(code)
+		return
+	}
 	if respWritten(r) {
 		log.Errorf("WriteAlertsObj called after a write already occurred! Not double-writing! Path %s", r.URL.Path)
 		return
diff --git a/traffic_ops/traffic_ops_golang/api/api_test.go b/traffic_ops/traffic_ops_golang/api/api_test.go
index 1c3493d..af13c19 100644
--- a/traffic_ops/traffic_ops_golang/api/api_test.go
+++ b/traffic_ops/traffic_ops_golang/api/api_test.go
@@ -20,6 +20,7 @@ package api
  */
 
 import (
+	"bytes"
 	"database/sql"
 	"encoding/json"
 	"errors"
@@ -76,6 +77,52 @@ func TestRespWrittenAfterErrFails(t *testing.T) {
 	}
 }
 
+func TestWriteAlertsObjEmpty(t *testing.T) {
+	w := &MockHTTPResponseWriter{}
+	r := &http.Request{URL: &url.URL{}}
+	a := tc.Alerts{}
+	code := http.StatusAlreadyReported
+
+	WriteAlertsObj(w, r, code, a, code)
+
+	resp := struct {
+		Response interface{} `json:"response"`
+	}{code}
+	serialized, _ := json.Marshal(resp)
+	if !bytes.Equal(append(serialized[:], '\n'), w.Body[:]) {
+		t.Error("expected response to only include object")
+	}
+	writeAlertsCodeTest(t, *w, code)
+}
+
+func TestWriteAlertsObj(t *testing.T) {
+	w := &MockHTTPResponseWriter{}
+	r := &http.Request{URL: &url.URL{}}
+	a := tc.CreateAlerts(tc.WarnLevel, "test")
+	code := http.StatusAlreadyReported
+
+	WriteAlertsObj(w, r, code, a, code)
+
+	resp := struct {
+		tc.Alerts
+		Response interface{} `json:"response"`
+	}{ a, code}
+	serialized, _ := json.Marshal(resp)
+	if !bytes.Equal(append(serialized[:], '\n'), w.Body[:]) {
+		t.Error("expected response to include alert")
+	}
+	writeAlertsCodeTest(t, *w, code)
+}
+
+func writeAlertsCodeTest(t *testing.T, w MockHTTPResponseWriter, code int) {
+	if w.Body == nil || len(w.Body) == 0 {
+		t.Error("expected response body to be written to")
+	}
+	if w.Code != code {
+		t.Errorf("expected response code %v, got %v", code, w.Code)
+	}
+}
+
 func TestWriteResp(t *testing.T) {
 	apiWriteTest(t, func(w http.ResponseWriter, r *http.Request) {
 		WriteResp(w, r, "foo")
diff --git a/traffic_ops/traffic_ops_golang/cdn/dnssec.go b/traffic_ops/traffic_ops_golang/cdn/dnssec.go
index 4a62969..6e0c7b3 100644
--- a/traffic_ops/traffic_ops_golang/cdn/dnssec.go
+++ b/traffic_ops/traffic_ops_golang/cdn/dnssec.go
@@ -29,6 +29,7 @@ import (
 
 	"github.com/apache/trafficcontrol/lib/go-log"
 	"github.com/apache/trafficcontrol/lib/go-tc"
+	"github.com/apache/trafficcontrol/lib/go-util"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/config"
 	"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
@@ -260,6 +261,8 @@ func generateStoreDNSSECKeys(
 	return nil
 }
 
+const API_DNSSECKEYS = "DELETE /cdns/name/:name/dnsseckeys"
+
 type CDNDS struct {
 	Name        string
 	Protocol    *int
@@ -299,35 +302,57 @@ WHERE cdn.name = $1
 }
 
 func DeleteDNSSECKeys(w http.ResponseWriter, r *http.Request) {
+	deleteDNSSECKeys(w, r, false)
+}
+
+func DeleteDNSSECKeysDeprecated(w http.ResponseWriter, r *http.Request) {
+	deleteDNSSECKeys(w, r, true)
+}
+
+func writeError(w http.ResponseWriter, r *http.Request, tx *sql.Tx, statusCode int, userErr error, sysErr error, deprecated bool) {
+	if deprecated {
+		api.HandleDeprecatedErr(w, r, tx, statusCode, userErr, sysErr, util.StrPtr(API_DNSSECKEYS))
+	} else {
+		api.HandleErr(w, r, tx, statusCode, userErr, sysErr)
+	}
+}
+
+func deleteDNSSECKeys(w http.ResponseWriter, r *http.Request, deprecated bool) {
 	inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"name"}, nil)
 	if userErr != nil || sysErr != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+		writeError(w, r, inf.Tx.Tx, errCode, userErr, sysErr, deprecated)
 		return
 	}
 	defer inf.Close()
 
 	cluster, err := riaksvc.GetPooledCluster(inf.Tx.Tx, inf.Config.RiakAuthOptions, inf.Config.RiakPort)
 	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting riak cluster: "+err.Error()))
+		writeError(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting riak cluster: "+err.Error()), deprecated)
 		return
 	}
 
 	key := inf.Params["name"]
 	cdnID, ok, err := getCDNIDFromName(inf.Tx.Tx, tc.CDNName(key))
 	if err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting cdn id: "+err.Error()))
+		writeError(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting cdn id: "+err.Error()), deprecated)
 		return
 	} else if !ok {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, nil, nil)
+		writeError(w, r, inf.Tx.Tx, http.StatusNotFound, nil, nil, deprecated)
 		return
 	}
 
 	if err := riaksvc.DeleteObject(key, CDNDNSSECKeyType, cluster); err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("deleting cdn dnssec keys: "+err.Error()))
+		writeError(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("deleting cdn dnssec keys: "+err.Error()), deprecated)
 		return
 	}
 	api.CreateChangeLogRawTx(api.ApiChange, "CDN: "+key+", ID: "+strconv.Itoa(cdnID)+", ACTION: Deleted DNSSEC keys", inf.User, inf.Tx.Tx)
-	api.WriteResp(w, r, "Successfully deleted "+CDNDNSSECKeyType+" for "+key)
+	successMsg := "Successfully deleted "+CDNDNSSECKeyType+" for "+key
+	if deprecated {
+		api.WriteAlertsObj(w, r, http.StatusOK, api.CreateDeprecationAlerts(util.StrPtr(API_DNSSECKEYS)), successMsg)
+	} else {
+		api.WriteResp(w, r, successMsg)
+
+	}
 }
 
 // getCDNIDFromName returns the CDN's ID if a CDN with the given name exists
diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go
index 7a1dd2b..9f62a2c 100644
--- a/traffic_ops/traffic_ops_golang/routing/routes.go
+++ b/traffic_ops/traffic_ops_golang/routing/routes.go
@@ -199,7 +199,7 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
 		//CDN: queue updates
 		{api.Version{2, 0}, http.MethodPost, `cdns/{id}/queue_update$`, cdn.Queue, auth.PrivLevelOperations, Authenticated, nil, 221515980, noPerlBypass},
 		{api.Version{2, 0}, http.MethodPost, `cdns/dnsseckeys/generate?$`, cdn.CreateDNSSECKeys, auth.PrivLevelAdmin, Authenticated, nil, 275336, noPerlBypass},
-		{api.Version{2, 0}, http.MethodGet, `cdns/name/{name}/dnsseckeys/delete/?$`, cdn.DeleteDNSSECKeys, auth.PrivLevelAdmin, Authenticated, nil, 271104207, noPerlBypass},
+		{api.Version{2, 0}, http.MethodDelete, `cdns/name/{name}/dnsseckeys?$`, cdn.DeleteDNSSECKeys, auth.PrivLevelAdmin, Authenticated, nil, 271104207, noPerlBypass},
 		{api.Version{2, 0}, http.MethodGet, `cdns/name/{name}/dnsseckeys/?$`, cdn.GetDNSSECKeys, auth.PrivLevelAdmin, Authenticated, nil, 279010609, noPerlBypass},
 
 		{api.Version{2, 0}, http.MethodGet, `cdns/dnsseckeys/refresh/?$`, cdn.RefreshDNSSECKeys, auth.PrivLevelOperations, Authenticated, nil, 2771997116, noPerlBypass},
@@ -591,7 +591,7 @@ func Routes(d ServerData) ([]Route, []RawRoute, http.Handler, error) {
 		//CDN: queue updates
 		{api.Version{1, 1}, http.MethodPost, `cdns/{id}/queue_update$`, cdn.Queue, auth.PrivLevelOperations, Authenticated, nil, 271515980, noPerlBypass},
 		{api.Version{1, 1}, http.MethodPost, `cdns/dnsseckeys/generate(\.json)?$`, cdn.CreateDNSSECKeys, auth.PrivLevelAdmin, Authenticated, nil, 675336, noPerlBypass},
-		{api.Version{1, 1}, http.MethodGet, `cdns/name/{name}/dnsseckeys/delete/?(\.json)?$`, cdn.DeleteDNSSECKeys, auth.PrivLevelAdmin, Authenticated, nil, 571104207, noPerlBypass},
+		{api.Version{1, 1}, http.MethodGet, `cdns/name/{name}/dnsseckeys/delete/?(\.json)?$`, cdn.DeleteDNSSECKeysDeprecated, auth.PrivLevelAdmin, Authenticated, nil, 571104207, noPerlBypass},
 		{api.Version{1, 4}, http.MethodGet, `cdns/name/{name}/dnsseckeys/?(\.json)?$`, cdn.GetDNSSECKeys, auth.PrivLevelAdmin, Authenticated, nil, 479010609, noPerlBypass},
 		{api.Version{1, 1}, http.MethodGet, `cdns/name/{name}/dnsseckeys/?(\.json)?$`, cdn.GetDNSSECKeysV11, auth.PrivLevelAdmin, Authenticated, nil, 1427173311, noPerlBypass},