From c68e05bd961fcbd87479deed50e313087218ed66 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Tue, 22 Nov 2022 16:45:20 -0700 Subject: [PATCH 01/10] initial commit --- CHANGELOG.md | 1 + cache-config/testing/ort-tests/tcdata/todb.go | 1 - ...deliveryservices_required_capabilities.rst | 6 + .../api/v4/deliveryservice_requests.rst | 16 ++- .../v4/deliveryservice_requests_id_assign.rst | 6 +- .../v4/deliveryservice_requests_id_status.rst | 6 +- docs/source/api/v4/deliveryservices.rst | 21 ++- docs/source/api/v4/deliveryservices_id.rst | 14 +- ...deliveryservices_required_capabilities.rst | 6 + .../api/v4/servers_id_deliveryservices.rst | 5 + lib/go-tc/deliveryservice_requests.go | 121 ++++++++++++++++-- lib/go-tc/deliveryservice_requests_test.go | 4 +- lib/go-tc/deliveryservices.go | 10 +- ...0_add_required_capabilities_to_ds.down.sql | 36 ++++++ ...300_add_required_capabilities_to_ds.up.sql | 30 +++++ .../testing/api/v3/deliveryservices_test.go | 5 +- traffic_ops/testing/api/v3/todb_test.go | 1 - .../api/v4/deliveryservice_requests_test.go | 7 +- .../testing/api/v4/deliveryservices_test.go | 5 +- traffic_ops/testing/api/v4/todb_test.go | 1 - .../testing/api/v5/deliveryservices_test.go | 5 +- traffic_ops/testing/api/v5/todb_test.go | 1 - .../traffic_ops_golang/api/shared_handlers.go | 67 +++++++--- .../crconfig/deliveryservice.go | 4 +- .../dbhelpers/db_helpers.go | 33 ++--- .../deliveryservice/deliveryservices.go | 72 ++++++++--- .../deliveryservices_required_capabilities.go | 58 ++++----- ...veryservices_required_capabilities_test.go | 9 +- .../deliveryservice/deliveryservices_test.go | 1 + .../deliveryservice/eligible.go | 4 +- .../deliveryservice/request/assign.go | 8 +- .../deliveryservice/request/requests.go | 53 ++++++-- .../deliveryservice/request/status.go | 8 +- .../traffic_ops_golang/routing/routes.go | 7 +- .../traffic_ops_golang/server/servers.go | 25 +++- .../server/servers_server_capability.go | 15 +-- .../traffic_ops_golang/topology/topologies.go | 5 +- .../test/integration/Data/deliveryservices.ts | 4 +- 38 files changed, 508 insertions(+), 173 deletions(-) create mode 100644 traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.down.sql create mode 100644 traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.up.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 50e949a7a3..afb4032f8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#7113](https://github.com/apache/trafficcontrol/pull/7113) *Traffic Portal* Minimize the Server Server Capability part of the *Traffic Servers* section of the Snapshot Diff ### Changed +- *Traffic Ops* Required Parameters are now a part of the `DeliveryService` structure. - [#7063](https://github.com/apache/trafficcontrol/pull/7063) *Traffic Ops* Python client now uses Traffic Ops API 4.1 by default. - [#6981](https://github.com/apache/trafficcontrol/pull/6981) *Traffic Portal* Obscures sensitive text in Delivery Service "Raw Remap" fields, private SSL keys, "Header Rewrite" rules, and ILO interface passwords by default. - [#7037](https://github.com/apache/trafficcontrol/pull/7037) *Traffic Router* Uses Traffic Ops API 4.0 by default diff --git a/cache-config/testing/ort-tests/tcdata/todb.go b/cache-config/testing/ort-tests/tcdata/todb.go index 0bbb27b76c..b526c0416c 100644 --- a/cache-config/testing/ort-tests/tcdata/todb.go +++ b/cache-config/testing/ort-tests/tcdata/todb.go @@ -247,7 +247,6 @@ func (r *TCData) Teardown(db *sql.DB) error { sqlStmt := ` DELETE FROM api_capability; - DELETE FROM deliveryservices_required_capability; DELETE FROM server_server_capability; DELETE FROM server_capability; DELETE FROM to_extension; diff --git a/docs/source/api/v3/deliveryservices_required_capabilities.rst b/docs/source/api/v3/deliveryservices_required_capabilities.rst index 9df9088bf3..66a1b0e4dc 100644 --- a/docs/source/api/v3/deliveryservices_required_capabilities.rst +++ b/docs/source/api/v3/deliveryservices_required_capabilities.rst @@ -19,6 +19,8 @@ ``deliveryservices_required_capabilities`` ****************************************** +.. deprecated:: ATCv7 + ``GET`` ======= Gets all associations of :term:`Server Capability` to :term:`Delivery Services`. @@ -102,6 +104,8 @@ Response Structure ] } +.. deprecated:: ATCv7 + ``POST`` ======== Associates a :term:`Server Capability` with a :term:`Delivery Service`. @@ -168,6 +172,8 @@ Response Structure } } +.. deprecated:: ATCv7 + ``DELETE`` ========== Dissociate a :term:`Server Capability` from a :term:`Delivery Service`. diff --git a/docs/source/api/v4/deliveryservice_requests.rst b/docs/source/api/v4/deliveryservice_requests.rst index dd628cd4d3..c88fb7ebdf 100644 --- a/docs/source/api/v4/deliveryservice_requests.rst +++ b/docs/source/api/v4/deliveryservice_requests.rst @@ -72,7 +72,7 @@ Request Structure .. code-block:: http :caption: Request Example - GET /api/4.0/deliveryservice_requests?status=draft HTTP/1.1 + GET /api/4.1/deliveryservice_requests?status=draft HTTP/1.1 User-Agent: python-requests/2.22.0 Accept-Encoding: gzip, deflate Accept: */* @@ -163,6 +163,7 @@ The response is an array of representations of :term:`Delivery Service Requests` "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -213,7 +214,7 @@ The request must be a well-formed representation of a :term:`Delivery Service Re .. code-block:: http :caption: Request Example - POST /api/4.0/deliveryservice_requests HTTP/1.1 + POST /api/4.1/deliveryservice_requests HTTP/1.1 User-Agent: python-requests/2.22.0 Accept-Encoding: gzip, deflate Accept: */* @@ -280,6 +281,7 @@ The request must be a well-formed representation of a :term:`Delivery Service Re "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -328,7 +330,7 @@ The response will be a representation of the created :term:`Delivery Service Req Content-Encoding: gzip Content-Type: application/json Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 24 Feb 2020 20:11:12 GMT; Max-Age=3600; HttpOnly - Location: /api/4.0/deliveryservice_requests/2 + Location: /api/4.1/deliveryservice_requests/2 X-Server-Name: traffic_ops_golang/ Date: Mon, 24 Feb 2020 19:11:12 GMT Content-Length: 901 @@ -405,6 +407,7 @@ The response will be a representation of the created :term:`Delivery Service Req "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -490,6 +493,7 @@ The response will be a representation of the created :term:`Delivery Service Req "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -550,7 +554,7 @@ The request body must be a representation of a :term:`Delivery Service Request` .. code-block:: http :caption: Request Example - PUT /api/4.0/deliveryservice_requests?id=1 HTTP/1.1 + PUT /api/4.1/deliveryservice_requests?id=1 HTTP/1.1 User-Agent: python-requests/2.22.0 Accept-Encoding: gzip, deflate Accept: */* @@ -667,6 +671,7 @@ The response is a full representation of the edited :term:`Delivery Service Requ "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -748,6 +753,7 @@ The response is a full representation of the edited :term:`Delivery Service Requ "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "cdn", "signed": false, "sslKeyVersion": null, @@ -800,7 +806,7 @@ Request Structure .. code-block:: http :caption: Request Example - DELETE /api/4.0/deliveryservice_requests?id=1 HTTP/1.1 + DELETE /api/4.1/deliveryservice_requests?id=1 HTTP/1.1 User-Agent: python-requests/2.22.0 Accept-Encoding: gzip, deflate Accept: */* diff --git a/docs/source/api/v4/deliveryservice_requests_id_assign.rst b/docs/source/api/v4/deliveryservice_requests_id_assign.rst index 9a9710acec..89b7e00018 100644 --- a/docs/source/api/v4/deliveryservice_requests_id_assign.rst +++ b/docs/source/api/v4/deliveryservice_requests_id_assign.rst @@ -42,7 +42,7 @@ Request Structure .. code-block:: http :caption: Request Example - GET /api/4.0/deliveryservice_requests/1/assign HTTP/1.1 + GET /api/4.1/deliveryservice_requests/1/assign HTTP/1.1 User-Agent: python-requests/2.24.0 Accept-Encoding: gzip, deflate Accept: */* @@ -103,7 +103,7 @@ Request Structure .. code-block:: http :caption: Request Example - PUT /api/4.0/deliveryservice_requests/1/assign HTTP/1.1 + PUT /api/4.1/deliveryservice_requests/1/assign HTTP/1.1 User-Agent: python-requests/2.24.0 Accept-Encoding: gzip, deflate Accept: */* @@ -198,6 +198,7 @@ The response contains a full representation of the newly assigned :term:`Deliver "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -279,6 +280,7 @@ The response contains a full representation of the newly assigned :term:`Deliver "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "cdn", "signed": false, "sslKeyVersion": null, diff --git a/docs/source/api/v4/deliveryservice_requests_id_status.rst b/docs/source/api/v4/deliveryservice_requests_id_status.rst index 933e8abe25..35b5b6b82a 100644 --- a/docs/source/api/v4/deliveryservice_requests_id_status.rst +++ b/docs/source/api/v4/deliveryservice_requests_id_status.rst @@ -45,7 +45,7 @@ Request Structure .. code-block:: http :caption: Request Example - GET /api/4.0/deliveryservice_requests/1/status HTTP/1.1 + GET /api/4.1/deliveryservice_requests/1/status HTTP/1.1 User-Agent: python-requests/2.24.0 Accept-Encoding: gzip, deflate Accept: */* @@ -98,7 +98,7 @@ Request Structure .. code-block:: http :caption: Request Example - PUT /api/4.0/deliveryservice_requests/1/status HTTP/1.1 + PUT /api/4.1/deliveryservice_requests/1/status HTTP/1.1 User-Agent: python-requests/2.22.0 Accept-Encoding: gzip, deflate Accept: */* @@ -195,6 +195,7 @@ The response is a full representation of the modified :term:`DSR`. "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -276,6 +277,7 @@ The response is a full representation of the modified :term:`DSR`. "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "cdn", "signed": false, "sslKeyVersion": null, diff --git a/docs/source/api/v4/deliveryservices.rst b/docs/source/api/v4/deliveryservices.rst index f6e3934d9b..48e64b0261 100644 --- a/docs/source/api/v4/deliveryservices.rst +++ b/docs/source/api/v4/deliveryservices.rst @@ -73,7 +73,7 @@ Request Structure .. code-block:: http :caption: Request Example - GET /api/4.0/deliveryservices?xmlId=demo2 HTTP/1.1 + GET /api/4.1/deliveryservices?xmlId=demo2 HTTP/1.1 Host: trafficops.infra.ciab.test User-Agent: python-requests/2.24.0 Accept-Encoding: gzip, deflate @@ -148,6 +148,10 @@ Response Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` +:requiredCapabilities: An array of the capabilities that this delivery service requires. + + .. versionchanged:: 4.1 + :serviceCategory: The name of the :ref:`ds-service-category` with which the :term:`Delivery Service` is associated :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise :signingAlgorithm: Either a :ref:`ds-signing-algorithm` or ``null`` to indicate URL/URI signing is not implemented on this :term:`Delivery Service` @@ -250,6 +254,7 @@ Response Structure "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "serviceCategory": null, "signed": false, @@ -328,6 +333,10 @@ Request Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` +:requiredCapabilities: An array of the capabilities that this delivery service requires. + + .. versionchanged:: 4.1 + :serviceCategory: The name of the :ref:`ds-service-category` with which the :term:`Delivery Service` is associated - or ``null`` if there is to be no such category :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise :signingAlgorithm: Either a :ref:`ds-signing-algorithm` or ``null`` to indicate URL/URI signing is not implemented on this :term:`Delivery Service` @@ -348,7 +357,7 @@ Request Structure .. code-block:: http :caption: Request Example - POST /api/4.0/deliveryservices HTTP/1.1 + POST /api/4.1/deliveryservices HTTP/1.1 User-Agent: python-requests/2.24.0 Accept-Encoding: gzip, deflate Accept: */* @@ -407,6 +416,7 @@ Request Structure "regexRemap": null, "regional": false, "regionalGeoBlocking": false, + "requiredCapabilities": null, "routingName": "test", "serviceCategory": null, "signed": false, @@ -495,6 +505,10 @@ Response Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` +:requiredCapabilities: An array of the capabilities that this delivery service requires. + + .. versionchanged:: 4.1 + :serviceCategory: The name of the :ref:`ds-service-category` with which the :term:`Delivery Service` is associated :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise :signingAlgorithm: Either a :ref:`ds-signing-algorithm` or ``null`` to indicate URL/URI signing is not implemented on this :term:`Delivery Service` @@ -522,7 +536,7 @@ Response Structure Access-Control-Allow-Origin: * Content-Encoding: gzip Content-Type: application/json - Location: /api/4.0/deliveryservices?id=6 + Location: /api/4.1/deliveryservices?id=6 Permissions-Policy: interest-cohort=() Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 07 Jun 2021 23:37:37 GMT; Max-Age=3600; HttpOnly Vary: Accept-Encoding @@ -606,6 +620,7 @@ Response Structure "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "test", "serviceCategory": null, "signed": false, diff --git a/docs/source/api/v4/deliveryservices_id.rst b/docs/source/api/v4/deliveryservices_id.rst index c2ce373d9f..823a5a953e 100644 --- a/docs/source/api/v4/deliveryservices_id.rst +++ b/docs/source/api/v4/deliveryservices_id.rst @@ -81,6 +81,10 @@ Request Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` +:requiredCapabilities: An array of the capabilities that this delivery service requires. + + .. versionchanged:: 4.1 + :routingName: The :ref:`ds-routing-name` of this :term:`Delivery Service` .. note:: If the Delivery Service has SSL Keys, then ``routingName`` is not allowed to change as that would invalidate the SSL Key @@ -106,7 +110,7 @@ Request Structure .. code-block:: http :caption: Request Example - PUT /api/4.0/deliveryservices/6 HTTP/1.1 + PUT /api/4.1/deliveryservices/6 HTTP/1.1 Host: trafficops.infra.ciab.test User-Agent: python-requests/2.24.0 Accept-Encoding: gzip, deflate @@ -165,6 +169,7 @@ Request Structure "regexRemap": null, "regional": false, "regionalGeoBlocking": false, + "requiredCapabilities": null, "routingName": "test", "serviceCategory": null, "signed": false, @@ -249,6 +254,10 @@ Response Structure :regexRemap: A :ref:`ds-regex-remap` :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` +:requiredCapabilities: An array of the capabilities that this delivery service requires. + + .. versionchanged:: 4.1 + :remapText: :ref:`ds-raw-remap` :serviceCategory: The name of the :ref:`ds-service-category` with which the :term:`Delivery Service` is associated :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise @@ -355,6 +364,7 @@ Response Structure "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "test", "serviceCategory": null, "signed": false, @@ -394,7 +404,7 @@ Request Structure .. code-block:: http :caption: Request Example - DELETE /api/4.0/deliveryservices/2 HTTP/1.1 + DELETE /api/4.1/deliveryservices/2 HTTP/1.1 Host: trafficops.infra.ciab.test User-Agent: curl/7.47.0 Accept: */* diff --git a/docs/source/api/v4/deliveryservices_required_capabilities.rst b/docs/source/api/v4/deliveryservices_required_capabilities.rst index 1bf4b4d81a..40b2532e13 100644 --- a/docs/source/api/v4/deliveryservices_required_capabilities.rst +++ b/docs/source/api/v4/deliveryservices_required_capabilities.rst @@ -19,6 +19,8 @@ ``deliveryservices_required_capabilities`` ****************************************** +.. deprecated:: ATCv7 + ``GET`` ======= Gets all associations of :term:`Server Capability` to :term:`Delivery Services`. @@ -103,6 +105,8 @@ Response Structure ] } +.. deprecated:: ATCv7 + ``POST`` ======== Associates a :term:`Server Capability` with a :term:`Delivery Service`. @@ -170,6 +174,8 @@ Response Structure } } +.. deprecated:: ATCv7 + ``DELETE`` ========== Dissociate a :term:`Server Capability` from a :term:`Delivery Service`. diff --git a/docs/source/api/v4/servers_id_deliveryservices.rst b/docs/source/api/v4/servers_id_deliveryservices.rst index 386de7532a..b434f7ea5e 100644 --- a/docs/source/api/v4/servers_id_deliveryservices.rst +++ b/docs/source/api/v4/servers_id_deliveryservices.rst @@ -134,6 +134,10 @@ Response Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` +:requiredCapabilities: An array of the capabilities that this delivery service requires. + + .. versionchanged:: 4.1 + :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise :signingAlgorithm: Either a :ref:`ds-signing-algorithm` or ``null`` to indicate URL/URI signing is not implemented on this :term:`Delivery Service` :rangeSliceBlockSize: An integer that defines the byte block size for the ATS Slice Plugin. It can only and must be set if ``rangeRequestHandling`` is set to 3. @@ -234,6 +238,7 @@ Response Structure "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "cdn", "serviceCategory": null, "signed": false, diff --git a/lib/go-tc/deliveryservice_requests.go b/lib/go-tc/deliveryservice_requests.go index 0c6ae2662c..dbf0cb9280 100644 --- a/lib/go-tc/deliveryservice_requests.go +++ b/lib/go-tc/deliveryservice_requests.go @@ -435,6 +435,54 @@ type DeliveryServiceRequestNullable struct { XMLID *string `json:"-" db:"xml_id"` } +// Downgrade will convert an instance of DeliveryServiceRequestV4 to DeliveryServiceRequestV40 +func (dsr DeliveryServiceRequestV4) Downgrade() DeliveryServiceRequestV40 { + var dsrV40 DeliveryServiceRequestV40 + dsrV40.Assignee = dsr.Assignee + dsrV40.AssigneeID = dsr.AssigneeID + dsrV40.Author = dsr.Author + dsrV40.AuthorID = dsr.AuthorID + dsrV40.ChangeType = dsr.ChangeType + dsrV40.CreatedAt = dsr.CreatedAt + dsrV40.ID = dsr.ID + dsrV40.LastEditedBy = dsr.LastEditedBy + dsrV40.LastEditedByID = dsr.LastEditedByID + dsrV40.LastUpdated = dsr.LastUpdated + if dsr.Original != nil { + dsrV40.Original = &dsr.Original.DeliveryServiceV40 + } + if dsr.Requested != nil { + dsrV40.Requested = &dsr.Requested.DeliveryServiceV40 + } + dsrV40.Status = dsr.Status + dsrV40.XMLID = dsr.XMLID + return dsrV40 +} + +// Upgrade will convert an instance of DeliveryServiceRequestV40 to DeliveryServiceRequestV4 +func (dsrV40 DeliveryServiceRequestV40) Upgrade() DeliveryServiceRequestV4 { + var dsrV4 DeliveryServiceRequestV41 + dsrV4.Assignee = dsrV40.Assignee + dsrV4.AssigneeID = dsrV40.AssigneeID + dsrV4.Author = dsrV40.Author + dsrV4.AuthorID = dsrV40.AuthorID + dsrV4.ChangeType = dsrV40.ChangeType + dsrV4.CreatedAt = dsrV40.CreatedAt + dsrV4.ID = dsrV40.ID + dsrV4.LastEditedBy = dsrV40.LastEditedBy + dsrV4.LastEditedByID = dsrV40.LastEditedByID + dsrV4.LastUpdated = dsrV40.LastUpdated + if dsrV40.Original != nil { + dsrV4.Original = &DeliveryServiceV4{DeliveryServiceV40: *dsrV40.Original} + } + if dsrV40.Requested != nil { + dsrV4.Requested = &DeliveryServiceV4{DeliveryServiceV40: *dsrV40.Requested} + } + dsrV4.Status = dsrV40.Status + dsrV4.XMLID = dsrV40.XMLID + return dsrV4 +} + // Upgrade coerces the DeliveryServiceRequestNullable to the newer // DeliveryServiceRequestV40 structure. // @@ -468,11 +516,13 @@ func (dsr DeliveryServiceRequestNullable) Upgrade() DeliveryServiceRequestV40 { } if dsr.DeliveryService != nil { if upgraded.ChangeType == DSRChangeTypeDelete { - upgraded.Original = new(DeliveryServiceV4) - *upgraded.Original = dsr.DeliveryService.UpgradeToV4() + upgraded.Original = new(DeliveryServiceV40) + orig := dsr.DeliveryService.UpgradeToV4().DeliveryServiceV40 + upgraded.Original = &orig } else { - upgraded.Requested = new(DeliveryServiceV4) - *upgraded.Requested = dsr.DeliveryService.UpgradeToV4() + upgraded.Requested = new(DeliveryServiceV40) + requested := dsr.DeliveryService.UpgradeToV4().DeliveryServiceV40 + upgraded.Requested = &requested } } if dsr.ID != nil { @@ -693,6 +743,53 @@ func (dsrct *DSRChangeType) UnmarshalJSON(b []byte) error { return nil } +// DeliveryServiceRequestV41 is the type of a Delivery Service Request in +// Traffic Ops API version 4.1. +type DeliveryServiceRequestV41 struct { + // Assignee is the username of the user assigned to the Delivery Service + // Request, if any. + Assignee *string `json:"assignee"` + // AssigneeID is the integral, unique identifier of the user assigned to the + // Delivery Service Request, if any. + AssigneeID *int `json:"-" db:"assignee_id"` + // Author is the username of the user who created the Delivery Service + // Request. + Author string `json:"author"` + // AuthorID is the integral, unique identifier of the user who created the + // Delivery Service Request, if/when it is known. + AuthorID *int `json:"-" db:"author_id"` + // ChangeType represents the type of change being made, must be one of + // "create", "change" or "delete". + ChangeType DSRChangeType `json:"changeType" db:"change_type"` + // CreatedAt is the date/time at which the Delivery Service Request was + // created. + CreatedAt time.Time `json:"createdAt" db:"created_at"` + // ID is the integral, unique identifier for the Delivery Service Request + // if/when it is known. + ID *int `json:"id" db:"id"` + // LastEditedBy is the username of the user by whom the Delivery Service + // Request was last edited. + LastEditedBy string `json:"lastEditedBy"` + // LastEditedByID is the integral, unique identifier of the user by whom the + // Delivery Service Request was last edited, if/when it is known. + LastEditedByID *int `json:"-" db:"last_edited_by_id"` + // LastUpdated is the date/time at which the Delivery Service was last + // modified. + LastUpdated time.Time `json:"lastUpdated" db:"last_updated"` + // Original is the original Delivery Service for which changes are + // requested. This is present in responses only for ChangeTypes 'change' and + // 'delete', and is only required in requests where ChangeType is 'delete'. + Original *DeliveryServiceV41 `json:"original,omitempty" db:"original"` + // Requested is the set of requested changes. This is present in responses + // only for ChangeTypes 'change' and 'create', and is only required in + // requests in those cases. + Requested *DeliveryServiceV41 `json:"requested,omitempty" db:"deliveryservice"` + // Status is the status of the Delivery Service Request. + Status RequestStatus `json:"status" db:"status"` + // Used internally to define the affected Delivery Service. + XMLID string `json:"-"` +} + // DeliveryServiceRequestV40 is the type of a Delivery Service Request in // Traffic Ops API version 4.0. type DeliveryServiceRequestV40 struct { @@ -729,11 +826,11 @@ type DeliveryServiceRequestV40 struct { // Original is the original Delivery Service for which changes are // requested. This is present in responses only for ChangeTypes 'change' and // 'delete', and is only required in requests where ChangeType is 'delete'. - Original *DeliveryServiceV4 `json:"original,omitempty" db:"original"` + Original *DeliveryServiceV40 `json:"original,omitempty" db:"original"` // Requested is the set of requested changes. This is present in responses // only for ChangeTypes 'change' and 'create', and is only required in // requests in those cases. - Requested *DeliveryServiceV4 `json:"requested,omitempty" db:"deliveryservice"` + Requested *DeliveryServiceV40 `json:"requested,omitempty" db:"deliveryservice"` // Status is the status of the Delivery Service Request. Status RequestStatus `json:"status" db:"status"` // Used internally to define the affected Delivery Service. @@ -742,7 +839,7 @@ type DeliveryServiceRequestV40 struct { // DeliveryServiceRequestV4 is the type of a Delivery Service Request as it // appears in API version 4. -type DeliveryServiceRequestV4 = DeliveryServiceRequestV40 +type DeliveryServiceRequestV4 = DeliveryServiceRequestV41 // IsOpen returns whether or not the Delivery Service Request is still "open" - // i.e. has not been rejected or completed. @@ -791,10 +888,16 @@ func (dsr DeliveryServiceRequestV40) Downgrade() DeliveryServiceRequestNullable downgraded.CreatedAt = TimeNoModFromTime(dsr.CreatedAt) if dsr.Requested != nil { downgraded.DeliveryService = new(DeliveryServiceNullableV30) - *downgraded.DeliveryService = dsr.Requested.DowngradeToV31() + if dsr.Requested != nil { + dsV4 := DeliveryServiceV4{DeliveryServiceV40: *dsr.Requested} + *downgraded.DeliveryService = dsV4.DowngradeToV31() + } } else if dsr.Original != nil { downgraded.DeliveryService = new(DeliveryServiceNullableV30) - *downgraded.DeliveryService = dsr.Original.DowngradeToV31() + if dsr.Original != nil { + dsV4 := DeliveryServiceV4{DeliveryServiceV40: *dsr.Original} + *downgraded.DeliveryService = dsV4.DowngradeToV31() + } } if dsr.ID != nil { downgraded.ID = new(int) diff --git a/lib/go-tc/deliveryservice_requests_test.go b/lib/go-tc/deliveryservice_requests_test.go index 44b62f8b93..3f4178e32c 100644 --- a/lib/go-tc/deliveryservice_requests_test.go +++ b/lib/go-tc/deliveryservice_requests_test.go @@ -151,7 +151,7 @@ func TestDeliveryServiceRequestV40_Downgrade(t *testing.T) { LastEditedBy: "last edited by", LastEditedByID: nil, LastUpdated: time.Now(), - Requested: &DeliveryServiceV4{}, + Requested: &DeliveryServiceV40{}, Status: RequestStatusComplete, } dsr.Requested.XMLID = &xmlid @@ -216,7 +216,7 @@ func ExampleDeliveryServiceRequestV40_SetXMLID() { var dsr DeliveryServiceRequestV40 fmt.Println(dsr.XMLID == "") - dsr.Requested = new(DeliveryServiceV4) + dsr.Requested = new(DeliveryServiceV40) dsr.Requested.XMLID = new(string) *dsr.Requested.XMLID = "test" dsr.SetXMLID() diff --git a/lib/go-tc/deliveryservices.go b/lib/go-tc/deliveryservices.go index b3a999ddf6..97e96c1661 100644 --- a/lib/go-tc/deliveryservices.go +++ b/lib/go-tc/deliveryservices.go @@ -283,7 +283,8 @@ type DeliveryServiceV41 struct { // Regional indicates whether the Delivery Service's MaxOriginConnections is // only per Cache Group, rather than divided over all Cache Servers in child // Cache Groups of the Origin. - Regional bool `json:"regional" db:"regional"` + Regional bool `json:"regional" db:"regional"` + RequiredCapabilities []string `json:"requiredCapabilities" db:"required_capabilities"` } // DeliveryServiceV4 is a Delivery Service as it appears in version 4 of the @@ -1437,6 +1438,8 @@ type DeliveryServiceV50 struct { // use, because the input is in no way restricted, validated, or limited in // scope to the Delivery Service. RemapText *string `json:"remapText" db:"remap_text"` + // RequiredCapabilities is an array of capabilities required for this delivery service + RequiredCapabilities []string `json:"requiredCapabilities" db:"required_capabilities"` // RoutingName defines the lowest-level DNS label used by the Delivery // Service, e.g. if trafficcontrol.apache.org were a Delivery Service, it // would have a RoutingName of "trafficcontrol". @@ -1608,7 +1611,8 @@ func (ds DeliveryServiceV5) Downgrade() DeliveryServiceV4 { }, TLSVersions: make([]string, len(ds.TLSVersions)), }, - Regional: ds.Regional, + Regional: ds.Regional, + RequiredCapabilities: ds.RequiredCapabilities, } *downgraded.Active = ds.Active == DSActiveStateActive @@ -1691,6 +1695,7 @@ func (ds DeliveryServiceV4) Upgrade() DeliveryServiceV5 { Regional: ds.Regional, RegionalGeoBlocking: coalesceBool(ds.RegionalGeoBlocking, false), RemapText: copyStringIfNotNil(ds.RemapText), + RequiredCapabilities: make([]string, len(ds.RequiredCapabilities)), RoutingName: coalesceString(ds.RoutingName, ""), ServiceCategory: copyStringIfNotNil(ds.ServiceCategory), Signed: ds.Signed, @@ -1726,6 +1731,7 @@ func (ds DeliveryServiceV4) Upgrade() DeliveryServiceV5 { copy(upgraded.MatchList, *ds.MatchList) } copy(upgraded.TLSVersions, ds.TLSVersions) + copy(upgraded.RequiredCapabilities, ds.RequiredCapabilities) return upgraded } diff --git a/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.down.sql b/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.down.sql new file mode 100644 index 0000000000..f76d291cf9 --- /dev/null +++ b/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.down.sql @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you 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. + */ + +CREATE TABLE IF NOT EXISTS deliveryservices_required_capability ( + required_capability TEXT NOT NULL, + deliveryservice_id bigint NOT NULL, + last_updated timestamp with time zone DEFAULT now() NOT NULL, + + PRIMARY KEY (deliveryservice_id, required_capability) + ); + +DO $$ +DECLARE temprow RECORD; +BEGIN FOR temprow IN +select id as deliveryservice_id, unnest(required_capabilities) as required_capability from deliveryservice d group by d.id, d.required_capabilities + LOOP +insert into deliveryservices_required_capability ("deliveryservice_id", "required_capability") values (temprow.deliveryservice_id, temprow.required_capability); +END LOOP; +END $$; + +ALTER TABLE public.deliveryservice +DROP COLUMN required_capabilities; diff --git a/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.up.sql b/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.up.sql new file mode 100644 index 0000000000..75da8e4dad --- /dev/null +++ b/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.up.sql @@ -0,0 +1,30 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you 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. + */ + +ALTER TABLE public.deliveryservice + ADD COLUMN required_capabilities text[]; + +DO $$ +DECLARE temprow RECORD; +BEGIN FOR temprow IN +select deliveryservice_id, array_agg(required_capability) as required_capabilities from deliveryservices_required_capability drc group by drc.deliveryservice_id + LOOP +update deliveryservice d set required_capabilities = temprow.required_capabilities where d.id = temprow.deliveryservice_id; +END LOOP; +END $$; + +DROP TABLE public.deliveryservices_required_capability; diff --git a/traffic_ops/testing/api/v3/deliveryservices_test.go b/traffic_ops/testing/api/v3/deliveryservices_test.go index 975df24669..694ce642ef 100644 --- a/traffic_ops/testing/api/v3/deliveryservices_test.go +++ b/traffic_ops/testing/api/v3/deliveryservices_test.go @@ -226,8 +226,9 @@ func TestDeliveryServices(t *testing.T) { "BAD REQUEST when ADDING TOPOLOGY to DS with DS REQUIRED CAPABILITY": { EndpointID: GetDeliveryServiceId(t, "ds1"), ClientSession: TOSession, RequestBody: generateDeliveryService(t, map[string]interface{}{ - "topology": "top-for-ds-req", - "xmlId": "ds1", + "requiredCapabilities": []string{"foo"}, + "topology": "top-for-ds-req", + "xmlId": "ds1", }), Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), }, diff --git a/traffic_ops/testing/api/v3/todb_test.go b/traffic_ops/testing/api/v3/todb_test.go index f42ae00ad4..123e94fa25 100644 --- a/traffic_ops/testing/api/v3/todb_test.go +++ b/traffic_ops/testing/api/v3/todb_test.go @@ -253,7 +253,6 @@ func Teardown(db *sql.DB) error { sqlStmt := ` DELETE FROM api_capability; - DELETE FROM deliveryservices_required_capability; DELETE FROM server_server_capability; DELETE FROM server_capability; DELETE FROM to_extension; diff --git a/traffic_ops/testing/api/v4/deliveryservice_requests_test.go b/traffic_ops/testing/api/v4/deliveryservice_requests_test.go index 9f60632ce4..6e27b7b8f0 100644 --- a/traffic_ops/testing/api/v4/deliveryservice_requests_test.go +++ b/traffic_ops/testing/api/v4/deliveryservice_requests_test.go @@ -370,9 +370,10 @@ func validatePutDSRequestFields(expectedResp map[string]interface{}) utils.CkReq func CreateTestDeliveryServiceRequests(t *testing.T) { for _, dsr := range testData.DeliveryServiceRequests { - resetDS(dsr.Original) - resetDS(dsr.Requested) - respDSR, _, err := TOSession.CreateDeliveryServiceRequest(dsr, client.RequestOptions{}) + dsrV41 := dsr.Upgrade() + resetDS(dsrV41.Original) + resetDS(dsrV41.Requested) + respDSR, _, err := TOSession.CreateDeliveryServiceRequest(dsrV41, client.RequestOptions{}) assert.NoError(t, err, "Could not create Delivery Service Requests: %v - alerts: %+v", err, respDSR.Alerts) } } diff --git a/traffic_ops/testing/api/v4/deliveryservices_test.go b/traffic_ops/testing/api/v4/deliveryservices_test.go index d5c68acd6e..facb71aedd 100644 --- a/traffic_ops/testing/api/v4/deliveryservices_test.go +++ b/traffic_ops/testing/api/v4/deliveryservices_test.go @@ -354,8 +354,9 @@ func TestDeliveryServices(t *testing.T) { "BAD REQUEST when ADDING TOPOLOGY to DS with DS REQUIRED CAPABILITY": { EndpointID: GetDeliveryServiceId(t, "ds1"), ClientSession: TOSession, RequestBody: generateDeliveryService(t, map[string]interface{}{ - "topology": "top-for-ds-req", - "xmlId": "ds1", + "requiredCapabilities": []string{"foo"}, + "topology": "top-for-ds-req", + "xmlId": "ds1", }), Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), }, diff --git a/traffic_ops/testing/api/v4/todb_test.go b/traffic_ops/testing/api/v4/todb_test.go index 7f9104a8a3..a80a3e997a 100644 --- a/traffic_ops/testing/api/v4/todb_test.go +++ b/traffic_ops/testing/api/v4/todb_test.go @@ -385,7 +385,6 @@ func Teardown(db *sql.DB) error { sqlStmt := ` DELETE FROM api_capability; - DELETE FROM deliveryservices_required_capability; DELETE FROM server_server_capability; DELETE FROM server_capability; DELETE FROM to_extension; diff --git a/traffic_ops/testing/api/v5/deliveryservices_test.go b/traffic_ops/testing/api/v5/deliveryservices_test.go index ded0f46730..19ea86e240 100644 --- a/traffic_ops/testing/api/v5/deliveryservices_test.go +++ b/traffic_ops/testing/api/v5/deliveryservices_test.go @@ -362,8 +362,9 @@ func TestDeliveryServices(t *testing.T) { "BAD REQUEST when ADDING TOPOLOGY to DS with DS REQUIRED CAPABILITY": { EndpointID: GetDeliveryServiceId(t, "ds1"), ClientSession: TOSession, RequestBody: generateDeliveryService(t, map[string]interface{}{ - "topology": "top-for-ds-req", - "xmlId": "ds1", + "topology": "top-for-ds-req", + "xmlId": "ds1", + "requiredCapabilities": []string{"foo"}, }), Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), }, diff --git a/traffic_ops/testing/api/v5/todb_test.go b/traffic_ops/testing/api/v5/todb_test.go index 2b09fa2903..aed6c19387 100644 --- a/traffic_ops/testing/api/v5/todb_test.go +++ b/traffic_ops/testing/api/v5/todb_test.go @@ -385,7 +385,6 @@ func Teardown(db *sql.DB) error { sqlStmt := ` DELETE FROM api_capability; - DELETE FROM deliveryservices_required_capability; DELETE FROM server_server_capability; DELETE FROM server_capability; DELETE FROM to_extension; diff --git a/traffic_ops/traffic_ops_golang/api/shared_handlers.go b/traffic_ops/traffic_ops_golang/api/shared_handlers.go index 2f7e139801..a593f8329b 100644 --- a/traffic_ops/traffic_ops_golang/api/shared_handlers.go +++ b/traffic_ops/traffic_ops_golang/api/shared_handlers.go @@ -142,6 +142,7 @@ func SetLastModifiedHeader(r *http.Request, useIMS bool) bool { type errWriterFunc func(w http.ResponseWriter, r *http.Request, tx *sql.Tx, statusCode int, userErr error, sysErr error) type readSuccessWriterFunc func(w http.ResponseWriter, r *http.Request, statusCode int, results interface{}) type deleteSuccessWriterFunc func(w http.ResponseWriter, r *http.Request, message string) +type createSuccessWriterFunc func(w http.ResponseWriter, r *http.Request, statusCode int, alerts tc.Alerts, results interface{}) // ReadHandler creates a handler function from the pointer to a struct implementing the Reader interface // @@ -471,17 +472,33 @@ func deleteHandlerHelper(deleter Deleter, errHandler errWriterFunc, successHandl // *change log entry // *forming and writing the body over the wire func CreateHandler(creator Creator) http.HandlerFunc { + return createHandlerHelper( + creator, + HandleErr, + func(w http.ResponseWriter, r *http.Request, statusCode int, alerts tc.Alerts, results interface{}) { + w.WriteHeader(statusCode) + if len(alerts.Alerts) > 0 { + WriteAlertsObj(w, r, statusCode, alerts, results) + } else { + WriteResp(w, r, results) + } + + }, + ) +} + +func createHandlerHelper(creator Creator, errHandler errWriterFunc, successHandler createSuccessWriterFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { inf, userErr, sysErr, errCode := NewInfo(r, nil, nil) if userErr != nil || sysErr != nil { - HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr) + errHandler(w, r, inf.Tx.Tx, errCode, userErr, sysErr) return } defer inf.Close() interfacePtr := reflect.ValueOf(creator) if interfacePtr.Kind() != reflect.Ptr { - HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("reflect: can only indirect from a pointer")) + errHandler(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("reflect: can only indirect from a pointer")) return } objectType := reflect.Indirect(interfacePtr).Type() @@ -491,7 +508,7 @@ func CreateHandler(creator Creator) http.HandlerFunc { if c, ok := obj.(MultipleCreator); ok && c.AllowMultipleCreates() { data, err := ioutil.ReadAll(r.Body) if err != nil { - HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, err, nil) + errHandler(w, r, inf.Tx.Tx, http.StatusBadRequest, err, nil) return } @@ -502,7 +519,7 @@ func CreateHandler(creator Creator) http.HandlerFunc { objSlice, err := parseMultipleCreates(data, objectType, inf) if err != nil { - HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err) + errHandler(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err) return } @@ -515,30 +532,30 @@ func CreateHandler(creator Creator) http.HandlerFunc { if sysErr != nil { code = http.StatusInternalServerError } - HandleErr(w, r, inf.Tx.Tx, code, userErr, sysErr) + errHandler(w, r, inf.Tx.Tx, code, userErr, sysErr) return } if t, ok := objElem.(Tenantable); ok { authorized, err := t.IsTenantAuthorized(inf.User) if err != nil { - HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("checking tenant authorized: "+err.Error())) + errHandler(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("checking tenant authorized: "+err.Error())) return } if !authorized { - HandleErr(w, r, inf.Tx.Tx, http.StatusForbidden, errors.New("not authorized on this tenant"), nil) + errHandler(w, r, inf.Tx.Tx, http.StatusForbidden, errors.New("not authorized on this tenant"), nil) return } } userErr, sysErr, errCode = objElem.Create() if userErr != nil || sysErr != nil { - HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr) + errHandler(w, r, inf.Tx.Tx, errCode, userErr, sysErr) return } if err = CreateChangeLog(ApiChange, Created, objElem, inf.User, inf.Tx.Tx); err != nil { - HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("inserting changelog: %w", err)) + errHandler(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("inserting changelog: %w", err)) return } } @@ -562,7 +579,7 @@ func CreateHandler(creator Creator) http.HandlerFunc { alerts.AddAlerts(objElem.(AlertsResponse).GetAlerts()) } } - WriteAlertsObj(w, r, http.StatusOK, alerts, responseObj) + successHandler(w, r, http.StatusOK, alerts, responseObj) } else { userErr, sysErr := decodeAndValidateRequestBody(r, obj) @@ -571,41 +588,59 @@ func CreateHandler(creator Creator) http.HandlerFunc { if sysErr != nil { code = http.StatusInternalServerError } - HandleErr(w, r, inf.Tx.Tx, code, userErr, sysErr) + errHandler(w, r, inf.Tx.Tx, code, userErr, sysErr) return } if t, ok := obj.(Tenantable); ok { authorized, err := t.IsTenantAuthorized(inf.User) if err != nil { - HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("checking tenant authorized: "+err.Error())) + errHandler(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("checking tenant authorized: "+err.Error())) return } if !authorized { - HandleErr(w, r, inf.Tx.Tx, http.StatusForbidden, errors.New("not authorized on this tenant"), nil) + errHandler(w, r, inf.Tx.Tx, http.StatusForbidden, errors.New("not authorized on this tenant"), nil) return } } userErr, sysErr, errCode = obj.Create() if userErr != nil || sysErr != nil { - HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr) + errHandler(w, r, inf.Tx.Tx, errCode, userErr, sysErr) return } if err := CreateChangeLog(ApiChange, Created, obj, inf.User, inf.Tx.Tx); err != nil { - HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("inserting changelog: %w", err)) + errHandler(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("inserting changelog: %w", err)) return } alerts := tc.CreateAlerts(tc.SuccessLevel, obj.GetType()+" was created.") if alertsObj, hasAlerts := obj.(AlertsResponse); hasAlerts { alerts.AddAlerts(alertsObj.GetAlerts()) } - WriteAlertsObj(w, r, http.StatusOK, alerts, obj) + successHandler(w, r, http.StatusOK, alerts, obj) } } } +// DeprecatedCreateHandler creates a net/http.HandlerFunc for the passed Creator object, and adds a deprecation +// notice, optionally with a passed alternative route suggestion. +func DeprecatedCreateHandler(creator Creator, alternative *string) http.HandlerFunc { + return createHandlerHelper( + creator, + func(w http.ResponseWriter, r *http.Request, tx *sql.Tx, statusCode int, userErr error, sysErr error) { + HandleDeprecatedErr(w, r, tx, statusCode, userErr, sysErr, alternative) + }, + func(w http.ResponseWriter, r *http.Request, statusCode int, alerts tc.Alerts, results interface{}) { + depAlerts := CreateDeprecationAlerts(alternative) + for _, al := range alerts.Alerts { + depAlerts.Alerts = append(depAlerts.Alerts, al) + } + WriteAlertsObj(w, r, statusCode, depAlerts, results) + }, + ) +} + func parseMultipleCreates(data []byte, desiredType reflect.Type, inf *APIInfo) ([]Creator, error) { buf := ioutil.NopCloser(bytes.NewReader(data)) diff --git a/traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go b/traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go index 94875ac764..17805994a7 100644 --- a/traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go +++ b/traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go @@ -131,9 +131,7 @@ SELECT d.anonymous_blocking_enabled, d.miss_long, p.name AS profile, d.protocol, - (SELECT ARRAY_AGG(required_capability ORDER BY required_capability) - FROM deliveryservices_required_capability - WHERE deliveryservice_id = d.id) AS required_capabilities, + d.required_capabilities, d.topology, d.tr_request_headers, d.tr_response_headers, diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go index 7b1dae9fd0..a71337f1e6 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go +++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go @@ -850,13 +850,11 @@ func GetRequiredCapabilitiesOfDeliveryServices(ids []int, tx *sql.Tx) (map[int][ dsCaps := make(map[int][]string, len(ids)) q := ` SELECT - d.id, - ARRAY_REMOVE(ARRAY_AGG(dsrc.required_capability ORDER BY dsrc.required_capability), NULL) AS required_capabilities -FROM deliveryservice d -LEFT JOIN deliveryservices_required_capability dsrc on d.id = dsrc.deliveryservice_id -WHERE - d.id = ANY($1) -GROUP BY d.id + ds.id, + ARRAY_REMOVE((ds.required_capabilities), NULL) AS required_capabilities +FROM deliveryservice ds +WHERE ds.id = ANY($1) +GROUP BY ds.id, ds.required_capabilities ` rows, err := tx.Query(q, pq.Array(&queryIDs)) if err != nil { @@ -897,7 +895,7 @@ func DSRExistsWithXMLID(xmlid string, tx *sql.Tx) (bool, error) { return exists, err } -// ScanCachegroupsServerCapabilities, given rows of (server ID, CDN ID, cachegroup name, server capabilities), +// ScanCachegroupsServerCapabilities : given rows of (server ID, CDN ID, cachegroup name, server capabilities), // returns a map of cachegroup names to server IDs, a map of server IDs to a map of their capabilities, // a map of server IDs to CDN IDs, and an error (if one occurs). func ScanCachegroupsServerCapabilities(rows *sql.Rows) (map[string][]int, map[int]map[string]struct{}, map[int]int, error) { @@ -927,10 +925,10 @@ func ScanCachegroupsServerCapabilities(rows *sql.Rows) (map[string][]int, map[in // GetDSRequiredCapabilitiesFromID returns the server's capabilities. func GetDSRequiredCapabilitiesFromID(id int, tx *sql.Tx) ([]string, error) { q := ` - SELECT required_capability - FROM deliveryservices_required_capability - WHERE deliveryservice_id = $1 - ORDER BY required_capability` + SELECT required_capabilities + FROM deliveryservice + WHERE id = $1 + ORDER BY required_capabilities` rows, err := tx.Query(q, id) if err != nil { return nil, errors.New("querying deliveryservice required capabilities from id: " + err.Error()) @@ -939,11 +937,9 @@ func GetDSRequiredCapabilitiesFromID(id int, tx *sql.Tx) ([]string, error) { caps := []string{} for rows.Next() { - var cap string - if err := rows.Scan(&cap); err != nil { + if err := rows.Scan(pq.Array(&caps)); err != nil { return nil, errors.New("scanning capability: " + err.Error()) } - caps = append(caps, cap) } return caps, nil } @@ -1546,7 +1542,7 @@ func GetDeliveryServiceTypeAndCDNName(dsID int, tx *sql.Tx) (tc.DSType, string, return dsType, cdnName, true, nil } -// GetDeliveryServiceTypeAndTopology returns the type of the deliveryservice and the name of its topology. +// GetDeliveryServiceTypeRequiredCapabilitiesAndTopology returns the type of the deliveryservice and the name of its topology. func GetDeliveryServiceTypeRequiredCapabilitiesAndTopology(dsID int, tx *sql.Tx) (tc.DSType, []string, *string, bool, error) { var dsType tc.DSType var reqCap []string @@ -1554,13 +1550,12 @@ func GetDeliveryServiceTypeRequiredCapabilitiesAndTopology(dsID int, tx *sql.Tx) q := ` SELECT t.name, - ARRAY_REMOVE(ARRAY_AGG(dsrc.required_capability ORDER BY dsrc.required_capability), NULL) AS required_capabilities, + ARRAY_REMOVE(ds.required_capabilities, NULL) AS required_capabilities, ds.topology FROM deliveryservice AS ds -LEFT JOIN deliveryservices_required_capability AS dsrc ON dsrc.deliveryservice_id = ds.id JOIN type t ON ds.type = t.id WHERE ds.id = $1 -GROUP BY t.name, ds.topology +GROUP BY t.name, ds.topology, ds.required_capabilities ` if err := tx.QueryRow(q, dsID).Scan(&dsType, pq.Array(&reqCap), &topology); err != nil { if err == sql.ErrNoRows { diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go index 1cc3525545..429cd185c7 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go @@ -347,7 +347,7 @@ func recreateTLSVersions(versions []string, dsid int, tx *sql.Tx) error { return nil } -// create creates the given ds in the database, and returns the DS with its id and other fields created on insert set. On error, the HTTP status code, user error, and system error are returned. The status code SHOULD NOT be used, if both errors are nil. +// createV40 creates the given ds in the database, and returns the DS with its id and other fields created on insert set. On error, the HTTP status code, user error, and system error are returned. The status code SHOULD NOT be used, if both errors are nil. func createV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV4 tc.DeliveryServiceV40, omitExtraLongDescFields bool) (*tc.DeliveryServiceV40, int, error, error) { ds, code, userErr, sysErr := createV41(w, r, inf, tc.DeliveryServiceV41{DeliveryServiceV40: dsV4}, omitExtraLongDescFields) if userErr != nil || sysErr != nil || ds == nil { @@ -469,6 +469,7 @@ func createV50(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, ds tc.D ds.LastHeaderRewrite, ds.ServiceCategory, ds.MaxRequestHeaderBytes, + pq.Array(ds.RequiredCapabilities), ) } else { resultRows, err = tx.Query(insertQuery(), @@ -532,6 +533,7 @@ func createV50(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, ds tc.D ds.LastHeaderRewrite, ds.ServiceCategory, ds.MaxRequestHeaderBytes, + pq.Array(ds.RequiredCapabilities), ) } @@ -916,6 +918,13 @@ func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 * dsNull := tc.DeliveryServiceNullableV30(*dsV31) ds := dsNull.UpgradeToV4() dsV41 := ds + dsMap, err := dbhelpers.GetRequiredCapabilitiesOfDeliveryServices([]int{*dsV31.ID}, inf.Tx.Tx) + if err != nil { + return nil, http.StatusInternalServerError, nil, err + } + if caps, ok := dsMap[*dsV31.ID]; ok { + dsV41.RequiredCapabilities = caps + } if dsV41.ID == nil { return nil, http.StatusInternalServerError, nil, errors.New("cannot update a Delivery Service with nil ID") } @@ -926,11 +935,11 @@ func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 * return nil, http.StatusInternalServerError, nil, fmt.Errorf("getting TLS versions for DS #%d in API version < 4.0: %w", *dsV41.ID, sysErr) } - res, status, usrErr, sysErr := updateV40(w, r, inf, &dsV41.DeliveryServiceV40, false) + res, status, usrErr, sysErr := updateV41(w, r, inf, &dsV41, false) if res == nil || usrErr != nil || sysErr != nil { return nil, status, usrErr, sysErr } - ds.DeliveryServiceV40 = *res + ds.DeliveryServiceV40 = res.DeliveryServiceV40 if dsV31.CacheURL != nil { _, err := tx.Exec("UPDATE deliveryservice SET cacheurl = $1 WHERE id = $2", *dsV31.CacheURL, @@ -1057,12 +1066,8 @@ func updateV50(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, ds *tc. } if ds.Topology != nil { - requiredCapabilities, err := dbhelpers.GetDSRequiredCapabilitiesFromID(*ds.ID, tx) - if err != nil { - return nil, http.StatusInternalServerError, nil, fmt.Errorf("getting existing DS required capabilities: %w", err) - } - if len(requiredCapabilities) > 0 { - if userErr, sysErr, status := EnsureTopologyBasedRequiredCapabilities(tx, *ds.ID, *ds.Topology, requiredCapabilities); userErr != nil || sysErr != nil { + if len(ds.RequiredCapabilities) > 0 { + if userErr, sysErr, status := EnsureTopologyBasedRequiredCapabilities(tx, *ds.ID, *ds.Topology, ds.RequiredCapabilities); userErr != nil || sysErr != nil { return nil, status, userErr, sysErr } } @@ -1141,6 +1146,7 @@ func updateV50(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, ds *tc. ds.LastHeaderRewrite, ds.ServiceCategory, ds.MaxRequestHeaderBytes, + pq.Array(ds.RequiredCapabilities), ds.ID, ) } else { @@ -1205,6 +1211,7 @@ func updateV50(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, ds *tc. ds.LastHeaderRewrite, ds.ServiceCategory, ds.MaxRequestHeaderBytes, + pq.Array(ds.RequiredCapabilities), ds.ID, ) } @@ -1584,12 +1591,37 @@ func Validate(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { if err := validateTypeFields(tx, ds); err != nil { errs = append(errs, fmt.Errorf("type fields: %w", err)) } + if err := validateRequiredCapabilities(tx, ds); err != nil { + errs = append(errs, errors.New("required capabilities: "+err.Error())) + } if len(errs) == 0 { return nil } return util.JoinErrs(errs) } +func validateRequiredCapabilities(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { + var valid bool + query := `SELECT $1 <@ (SELECT ARRAY_AGG(name) FROM server_capability)` + if ds.RequiredCapabilities != nil && len(ds.RequiredCapabilities) > 0 { + rows, err := tx.Query(query, pq.Array(ds.RequiredCapabilities)) + if err != nil { + return err + } + defer rows.Close() + for rows.Next() { + err = rows.Scan(&valid) + if err != nil { + return err + } + if !valid { + return errors.New("one or more of the required capabilities do not exist") + } + } + } + return nil +} + func validateGeoLimitCountries(ds *tc.DeliveryServiceV5) error { var IsLetter = regexp.MustCompile(`^[A-Z]+$`).MatchString if len(ds.GeoLimitCountries) == 0 { @@ -1947,6 +1979,7 @@ func GetDeliveryServices(query string, queryValues map[string]interface{}, tx *s &ds.Regional, &ds.RegionalGeoBlocking, &ds.RemapText, + pq.Array(&ds.RequiredCapabilities), &ds.RoutingName, &ds.ServiceCategory, &ds.SigningAlgorithm, @@ -2484,6 +2517,7 @@ ds.active, ds.regional, ds.regional_geo_blocking, ds.remap_text, + ds.required_capabilities, ds.routing_name, ds.service_category, ds.signing_algorithm, @@ -2569,8 +2603,9 @@ first_header_rewrite=$56, inner_header_rewrite=$57, last_header_rewrite=$58, service_category=$59, -max_request_header_bytes=$60 -WHERE id=$61 +max_request_header_bytes=$60, +required_capabilities=$61 +WHERE id=$62 RETURNING last_updated ` } @@ -2636,8 +2671,9 @@ first_header_rewrite=$54, inner_header_rewrite=$55, last_header_rewrite=$56, service_category=$57, -max_request_header_bytes=$58 -WHERE id=$59 +max_request_header_bytes=$58, +required_capabilities=$59 +WHERE id=$60 RETURNING last_updated ` } @@ -2704,9 +2740,10 @@ first_header_rewrite, inner_header_rewrite, last_header_rewrite, service_category, -max_request_header_bytes +max_request_header_bytes, +required_capabilities ) -VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25,$26,$27,$28,$29,$30,$31,$32,$33,$34,$35,$36,$37,$38,$39,$40,$41,$42,$43,$44,$45,$46,$47,$48,$49,$50,$51,$52,$53,$54,$55,$56,$57,$58,$59,$60) +VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25,$26,$27,$28,$29,$30,$31,$32,$33,$34,$35,$36,$37,$38,$39,$40,$41,$42,$43,$44,$45,$46,$47,$48,$49,$50,$51,$52,$53,$54,$55,$56,$57,$58,$59,$60,$61) RETURNING id, last_updated ` } @@ -2771,9 +2808,10 @@ first_header_rewrite, inner_header_rewrite, last_header_rewrite, service_category, -max_request_header_bytes +max_request_header_bytes, +required_capabilities ) -VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25,$26,$27,$28,$29,$30,$31,$32,$33,$34,$35,$36,$37,$38,$39,$40,$41,$42,$43,$44,$45,$46,$47,$48,$49,$50,$51,$52,$53,$54,$55,$56,$57,$58) +VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25,$26,$27,$28,$29,$30,$31,$32,$33,$34,$35,$36,$37,$38,$39,$40,$41,$42,$43,$44,$45,$46,$47,$48,$49,$50,$51,$52,$53,$54,$55,$56,$57,$58,$59) RETURNING id, last_updated ` } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go index b6b75b3dd8..aac113ea1a 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go @@ -65,36 +65,31 @@ func (rc *RequiredCapability) NewReadObj() interface{} { // SelectQuery implements the api.GenericReader interface. func (rc *RequiredCapability) SelectQuery() string { return `SELECT - rc.required_capability, - rc.deliveryservice_id, + UNNEST(ds.required_capabilities) as required_capability, + ds.id as deliveryservice_id, ds.xml_id, - rc.last_updated - FROM deliveryservices_required_capability rc - JOIN deliveryservice ds ON ds.id = rc.deliveryservice_id` + ds.last_updated + FROM deliveryservice ds` } // ParamColumns implements the api.GenericReader interface. func (rc *RequiredCapability) ParamColumns() map[string]dbhelpers.WhereColumnInfo { return map[string]dbhelpers.WhereColumnInfo{ deliveryServiceQueryParam: dbhelpers.WhereColumnInfo{ - Column: "rc.deliveryservice_id", + Column: "ds.id", Checker: api.IsInt, }, xmlIDQueryParam: dbhelpers.WhereColumnInfo{ Column: "ds.xml_id", Checker: nil, }, - requiredCapabilityQueryParam: dbhelpers.WhereColumnInfo{ - Column: "rc.required_capability", - Checker: nil, - }, } } // DeleteQuery implements the api.GenericDeleter interface. func (rc *RequiredCapability) DeleteQuery() string { - return `DELETE FROM deliveryservices_required_capability - WHERE deliveryservice_id = :deliveryservice_id AND required_capability = :required_capability` + return `UPDATE deliveryservice ds SET required_capabilities = ARRAY_REMOVE((select required_capabilities from deliveryservice WHERE id=:deliveryservice_id), :required_capability) +WHERE id=:deliveryservice_id AND EXISTS(SELECT 1 FROM deliveryservice ds WHERE id=:deliveryservice_id AND :required_capability = ANY(ds.required_capabilities))` } // GetKeyFieldsInfo implements the api.Identifier interface. @@ -209,7 +204,7 @@ func (rc *RequiredCapability) getCapabilities(h http.Header, tenantIDs []int, us where, queryValues = dbhelpers.AddTenancyCheck(where, queryValues, "ds.tenant_id", tenantIDs) if useIMS { - runSecond, maxTime = ims.TryIfModifiedSinceQuery(rc.APIInfo().Tx, h, queryValues, selectMaxLastUpdatedQueryRC(where, orderBy, pagination)) + runSecond, maxTime = ims.TryIfModifiedSinceQuery(rc.APIInfo().Tx, h, queryValues, selectMaxLastUpdatedQuery(where)) if !runSecond { log.Debugln("IMS HIT") return results, nil, nil, http.StatusNotModified, &maxTime @@ -220,6 +215,10 @@ func (rc *RequiredCapability) getCapabilities(h http.Header, tenantIDs []int, us } query := rc.SelectQuery() + where + orderBy + pagination + if reqdCap, ok := rc.APIInfo().Params[requiredCapabilityQueryParam]; ok { + query = `WITH res AS (` + query + `) SELECT res.required_capability, res.deliveryservice_id, res.xml_id, res.last_updated FROM res WHERE res.required_capability=:requiredCapability` + queryValues[requiredCapabilityQueryParam] = reqdCap + } rows, err := rc.APIInfo().Tx.NamedQuery(query, queryValues) if err != nil { return nil, nil, err, http.StatusInternalServerError, nil @@ -239,10 +238,10 @@ func (rc *RequiredCapability) getCapabilities(h http.Header, tenantIDs []int, us func selectMaxLastUpdatedQueryRC(where string, orderBy string, pagination string) string { return `SELECT max(t) from ( - SELECT max(rc.last_updated) as t FROM deliveryservices_required_capability rc - JOIN deliveryservice ds ON ds.id = rc.deliveryservice_id ` + where + orderBy + pagination + + SELECT max(d.last_updated) as t FROM deliveryservice d` + + where + orderBy + pagination + ` UNION ALL - select max(last_updated) as t from last_deleted l where l.table_name='deliveryservices_required_capability') as res` + select max(last_updated) as t from last_deleted l where l.table_name='deliveryservice') as res` } // Delete implements the api.CRUDer interface. @@ -292,7 +291,7 @@ func (rc *RequiredCapability) Create() (error, error, int) { } if !dsType.IsHTTP() && !dsType.IsDNS() { - return errors.New("Invalid DS type. Only DNS and HTTP delivery services can have required capabilities"), nil, http.StatusBadRequest + return errors.New("invalid DS type. Only DNS and HTTP delivery services can have required capabilities"), nil, http.StatusBadRequest } usrErr, sysErr, rCode := rc.checkServerCap() @@ -300,6 +299,11 @@ func (rc *RequiredCapability) Create() (error, error, int) { return usrErr, sysErr, rCode } + for _, reqCap := range reqCaps { + if reqCap == *rc.RequiredCapability { + return fmt.Errorf("capability %s already exists for delivery service with ID: %d", *rc.RequiredCapability, *rc.DeliveryServiceID), nil, http.StatusBadRequest + } + } if topology == nil { usrErr, sysErr, rCode = rc.ensureDSServerCap() if usrErr != nil || sysErr != nil { @@ -325,7 +329,7 @@ func (rc *RequiredCapability) Create() (error, error, int) { rowsAffected := 0 for rows.Next() { rowsAffected++ - if err := rows.StructScan(&rc); err != nil { + if err := rows.Scan(&rc.DeliveryServiceID, &rc.LastUpdated); err != nil { return nil, fmt.Errorf("%s create scanning: %s", rc.GetType(), err.Error()), http.StatusInternalServerError } } @@ -518,18 +522,12 @@ func (rc *RequiredCapability) isTenantAuthorized() (bool, error) { } func rcInsertQuery() string { - return `INSERT INTO deliveryservices_required_capability ( -required_capability, -deliveryservice_id) VALUES ( -:required_capability, -:deliveryservice_id) RETURNING deliveryservice_id, required_capability, last_updated` + return `UPDATE deliveryservice ds SET required_capabilities = array_append((select required_capabilities from deliveryservice WHERE id=:deliveryservice_id), :required_capability) +WHERE id=:deliveryservice_id RETURNING ds.id, ds.last_updated` } -// language=SQL -const HasRequiredCapabilitiesQuery = ` -SELECT EXISTS( - SELECT drc.required_capability - FROM deliveryservices_required_capability drc - WHERE drc.deliveryservice_id = $1 -) +const GetRequiredCapabilitiesQuery = ` +SELECT ds.required_capabilities +FROM deliveryservice ds +WHERE ds.id = $1 ` diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities_test.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities_test.go index af60e2075f..5c480ea690 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities_test.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities_test.go @@ -92,12 +92,11 @@ func TestCreateDeliveryServicesRequiredCapability(t *testing.T) { arrayRows := sqlmock.NewRows([]string{"array"}) mock.ExpectQuery("SELECT ds.server FROM deliveryservice_server").WillReturnRows(arrayRows) - rows := sqlmock.NewRows([]string{"required_capability", "deliveryservice_id", "last_updated"}).AddRow( - util.StrPtr("mem"), + rows := sqlmock.NewRows([]string{"deliveryservice_id", "last_updated"}).AddRow( util.IntPtr(1), time.Now(), ) - mock.ExpectQuery("INSERT INTO deliveryservices_required_capability").WillReturnRows(rows) + mock.ExpectQuery("UPDATE deliveryservice").WillReturnRows(rows) userErr, sysErr, errCode := rc.Create() if userErr != nil { @@ -197,7 +196,7 @@ func TestReadDeliveryServicesRequiredCapability(t *testing.T) { capability.XMLID, time.Now(), ) - mock.ExpectQuery("SELECT .* FROM deliveryservices_required_capability").WillReturnRows(rows) + mock.ExpectQuery("SELECT .* FROM deliveryservice").WillReturnRows(rows) results, userErr, sysErr, errCode, _ := rc.Read(nil, false) if userErr != nil { @@ -248,7 +247,7 @@ func TestDeleteDeliveryServicesRequiredCapability(t *testing.T) { mock.ExpectQuery("SELECT").WillReturnRows(sqlmock.NewRows([]string{"xml_id", "name"}).AddRow("name", "cdnName")) mock.ExpectQuery("SELECT c.username").WillReturnRows(sqlmock.NewRows(nil)) - mock.ExpectExec("DELETE").WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec("UPDATE").WillReturnResult(sqlmock.NewResult(1, 1)) rc := RequiredCapability{ api.APIInfoImpl{ diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go index 01e9fd1fe2..b397c1c53d 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go @@ -321,6 +321,7 @@ func TestReadGetDeliveryServices(t *testing.T) { {"regional", false}, {"regional_geo_blocking", false}, {"remap_text", nil}, + {"required_capabilities", nil}, {"routing_name", "video"}, {"service_category", nil}, {"signing_algorithm", nil}, diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/eligible.go b/traffic_ops/traffic_ops_golang/deliveryservice/eligible.go index d863aa423b..691a1ae7ba 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/eligible.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/eligible.go @@ -147,7 +147,7 @@ t.name as server_type, s.type as server_type_id, s.config_update_time > s.config_apply_time AS upd_pending, ARRAY(select ssc.server_capability from server_server_capability ssc where ssc.server = s.id order by ssc.server_capability) as server_capabilities, -ARRAY(select drc.required_capability from deliveryservices_required_capability drc where drc.deliveryservice_id = (select v from ds_id) order by drc.required_capability) as deliveryservice_capabilities +(SELECT ds.required_capabilities FROM deliveryservice ds WHERE ds.id = $2) AS deliveryservice_capabilities ` idRows, err := tx.Query(fmt.Sprintf(queryFormatString, "", queryWhereClause), dsID) if err != nil { @@ -168,7 +168,7 @@ ARRAY(select drc.required_capability from deliveryservices_required_capability d return nil, errors.New("unable to get server interfaces: " + err.Error()) } - rows, err := tx.Query(fmt.Sprintf(queryFormatString, dataFetchQuery, queryWhereClause), dsID) + rows, err := tx.Query(fmt.Sprintf(queryFormatString, dataFetchQuery, queryWhereClause), dsID, dsID) if err != nil { return nil, errors.New("querying delivery service eligible servers: " + err.Error()) } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/assign.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/assign.go index d34d138c72..69a56dbcc5 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/request/assign.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/assign.go @@ -225,9 +225,13 @@ func PutAssignment(w http.ResponseWriter, r *http.Request) { if inf.Version.Major >= 5 { resp = dsr } else if inf.Version.Major >= 4 { - resp = dsr.Downgrade() + if inf.Version.Major >= 1 { + resp = dsr.Downgrade() + } else { + resp = dsr.Downgrade().Downgrade() + } } else { - resp = dsr.Downgrade().Downgrade() + resp = dsr.Downgrade().Downgrade().Downgrade() } api.WriteRespAlertObj(w, r, tc.SuccessLevel, message, resp) diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go index 4b3ddac65c..8aa1da8c7c 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go @@ -286,9 +286,17 @@ func Get(w http.ResponseWriter, r *http.Request) { api.WriteResp(w, r, dsrs) return } + if version.Minor >= 1 { + downgraded := make([]tc.DeliveryServiceRequestV4, 0, len(dsrs)) + for _, dsr := range dsrs { + downgraded = append(downgraded, dsr.Downgrade()) + } + api.WriteResp(w, r, downgraded) + return + } downgraded := make([]tc.DeliveryServiceRequestV40, 0, len(dsrs)) for _, dsr := range dsrs { - downgraded = append(downgraded, dsr.Downgrade()) + downgraded = append(downgraded, dsr.Downgrade().Downgrade()) } api.WriteResp(w, r, downgraded) return @@ -296,7 +304,7 @@ func Get(w http.ResponseWriter, r *http.Request) { downgraded := make([]tc.DeliveryServiceRequestNullable, 0, len(dsrs)) for _, dsr := range dsrs { - downgraded = append(downgraded, dsr.Downgrade().Downgrade()) + downgraded = append(downgraded, dsr.Downgrade().Downgrade().Downgrade()) } api.WriteResp(w, r, downgraded) @@ -498,7 +506,8 @@ func createV4(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result return } - dsr.SetXMLID() + upgraded.SetXMLID() + dsr.XMLID = upgraded.XMLID if ok, err = dbhelpers.DSRExistsWithXMLID(dsr.XMLID, tx); err != nil { err = fmt.Errorf("checking for existence of DSR with xmlid '%s'", dsr.XMLID) api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err) @@ -536,7 +545,11 @@ func createV4(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result w.Header().Set("Location", fmt.Sprintf("/api/%d.%d/deliveryservice_requests/%d", inf.Version.Major, inf.Version.Minor, *dsr.ID)) w.WriteHeader(http.StatusCreated) - api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Delivery Service request created", dsr) + if inf.Version.Minor >= 1 { + api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Delivery Service request created", dsr) + } else { + api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Delivery Service request created", dsr.Downgrade()) + } result.Successful = true result.Assignee = dsr.Assignee @@ -559,7 +572,7 @@ func createLegacy(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (res return } - upgraded := dsr.Upgrade().Upgrade() + upgraded := dsr.Upgrade().Upgrade().Upgrade() authorized, err := isTenantAuthorized(upgraded, inf) if err != nil { sysErr := fmt.Errorf("checking tenant authorized: %w", err) @@ -856,12 +869,23 @@ func putV50(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result ds return } -func putV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result dsrManipulationResult) { +func putV4(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result dsrManipulationResult) { tx := inf.Tx.Tx - var dsr tc.DeliveryServiceRequestV40 - if err := json.NewDecoder(r.Body).Decode(&dsr); err != nil { - api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("decoding: %w", err), nil) - return + var dsr tc.DeliveryServiceRequestV4 + var dsrV40 tc.DeliveryServiceRequestV40 + + if inf.Version.Minor == 0 { + if err := json.NewDecoder(r.Body).Decode(&dsrV40); err != nil { + api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("decoding: %v", err), nil) + return + } + dsr = dsrV40.Upgrade() + } else { + if err := json.NewDecoder(r.Body).Decode(&dsr); err != nil { + api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("decoding: %v", err), nil) + return + } + dsrV40 = dsr.Downgrade() } if userErr, sysErr := validateV4(dsr, tx); userErr != nil || sysErr != nil { api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, sysErr) @@ -936,8 +960,8 @@ func putV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result ds api.HandleErr(w, r, tx, errCode, userErr, sysErr) return } - dsr = upgraded.Downgrade() - dsr.SetXMLID() + upgraded.SetXMLID() + dsr.XMLID = upgraded.XMLID if dsr.ChangeType == tc.DSRChangeTypeUpdate { query := deliveryservice.SelectDeliveryServicesQuery + `WHERE xml_id=:XMLID` @@ -997,7 +1021,7 @@ func putLegacy(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result dsr.LastEditedByID = new(tc.IDNoMod) *dsr.LastEditedByID = tc.IDNoMod(inf.User.ID) - upgraded := dsr.Upgrade().Upgrade() + upgraded := dsr.Upgrade().Upgrade().Upgrade() authorized, err := isTenantAuthorized(upgraded, inf) if err != nil { @@ -1032,6 +1056,7 @@ func putLegacy(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result return } upgraded.SetXMLID() + dsr.XMLID = &upgraded.XMLID api.WriteRespAlertObj(w, r, tc.SuccessLevel, fmt.Sprintf("Delivery Service Request #%d updated", inf.IntParams["id"]), dsr) result.Action = api.Updated @@ -1107,7 +1132,7 @@ func Put(w http.ResponseWriter, r *http.Request) { case 5: result = putV50(w, r, inf) case 4: - result = putV40(w, r, inf) + result = putV4(w, r, inf) case 3: result = putLegacy(w, r, inf) } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/status.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/status.go index d252a3906d..695f744742 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/request/status.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/status.go @@ -227,9 +227,13 @@ func PutStatus(w http.ResponseWriter, r *http.Request) { if inf.Version.Major >= 5 { resp = dsr } else if inf.Version.Major >= 4 { - resp = dsr.Downgrade() + if inf.Version.Minor >= 1 { + resp = dsr.Downgrade() + } else { + resp = dsr.Downgrade().Downgrade() + } } else { - resp = dsr.Downgrade().Downgrade() + resp = dsr.Downgrade().Downgrade().Downgrade() } api.WriteRespAlertObj(w, r, tc.SuccessLevel, message, resp) diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go index 87703283d0..b4515a0ef9 100644 --- a/traffic_ops/traffic_ops_golang/routing/routes.go +++ b/traffic_ops/traffic_ops_golang/routing/routes.go @@ -30,6 +30,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/about" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/acme" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" @@ -798,9 +799,9 @@ func Routes(d ServerData) ([]Route, http.Handler, error) { {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodDelete, Path: `deliveryservices/{xmlID}/urisignkeys$`, Handler: urisigning.RemoveDeliveryServiceURIKeysHandler, RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"DS-SECURITY-KEY:DELETE", "DS-SECURITY-KEY:READ", "DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 4299254173}, //Delivery Service Required Capabilities: CRUD - {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodGet, Path: `deliveryservices_required_capabilities/?$`, Handler: api.ReadHandler(&deliveryservice.RequiredCapability{}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 41585222273}, - {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodPost, Path: `deliveryservices_required_capabilities/?$`, Handler: api.CreateHandler(&deliveryservice.RequiredCapability{}), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 40968739923}, - {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodDelete, Path: `deliveryservices_required_capabilities/?$`, Handler: api.DeleteHandler(&deliveryservice.RequiredCapability{}), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 44962893043}, + {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodGet, Path: `deliveryservices_required_capabilities/?$`, Handler: api.DeprecatedReadHandler(&deliveryservice.RequiredCapability{}, util.StrPtr("the deliveryservices endpoint")), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 41585222273}, + {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodPost, Path: `deliveryservices_required_capabilities/?$`, Handler: api.DeprecatedCreateHandler(&deliveryservice.RequiredCapability{}, util.StrPtr("the deliveryservices endpoint")), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 40968739923}, + {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodDelete, Path: `deliveryservices_required_capabilities/?$`, Handler: api.DeprecatedDeleteHandler(&deliveryservice.RequiredCapability{}, util.StrPtr("the deliveryservices endpoint")), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 44962893043}, // Federations by CDN (the actual table for federation) {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodGet, Path: `cdns/{name}/federations/?$`, Handler: api.ReadHandler(&cdnfederation.TOCDNFederation{}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"CDN:READ", "FEDERATION:READ", "DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 4892250323}, diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go index e576aeb7e1..5442ad5744 100644 --- a/traffic_ops/traffic_ops_golang/server/servers.go +++ b/traffic_ops/traffic_ops_golang/server/servers.go @@ -104,9 +104,9 @@ AND ( FROM server_server_capability ssc WHERE ssc."server" = s.id ) @> ( - SELECT ARRAY_AGG(drc.required_capability) - FROM deliveryservices_required_capability drc - WHERE drc.deliveryservice_id = d.id + SELECT d.required_capabilities + FROM deliveryservice d + WHERE d.id = :dsId ) ` @@ -803,6 +803,7 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth usesMids := false queryAddition := "" dsHasRequiredCapabilities := false + var requiredCapabilities []string var dsID int var cdnID int var serverCount uint64 @@ -825,10 +826,20 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth } var joinSubQuery string - if err = tx.QueryRow(deliveryservice.HasRequiredCapabilitiesQuery, dsID).Scan(&dsHasRequiredCapabilities); err != nil { + rows, err := tx.Query(deliveryservice.GetRequiredCapabilitiesQuery, dsID) + if err != nil { err = fmt.Errorf("unable to get required capabilities for deliveryservice %d: %s", dsID, err) return nil, 0, nil, err, http.StatusInternalServerError, nil } + for rows.Next() { + if err = rows.Scan(pq.Array(&requiredCapabilities)); err != nil { + err = fmt.Errorf("unable to scan required capabilities for deliveryservice %d: %s", dsID, err) + return nil, 0, nil, err, http.StatusInternalServerError, nil + } + } + if requiredCapabilities != nil && len(requiredCapabilities) > 0 { + dsHasRequiredCapabilities = true + } joinSubQuery = dssTopologiesJoinSubquery // only if dsId is part of params: add join on deliveryservice_server table queryAddition = fmt.Sprintf(deliveryServiceServersJoin, joinSubQuery) @@ -1111,9 +1122,9 @@ func getMidServers(edgeIDs []int, servers map[int]tc.ServerV40, dsID int, cdnID capabilities.array_agg @> ( - SELECT ARRAY_AGG(drc.required_capability) - FROM deliveryservices_required_capability drc - WHERE drc.deliveryservice_id=:ds_id) + SELECT ds.required_capabilities + FROM deliveryservice ds + WHERE ds.id=:ds_id) )` } else { // TODO: include secondary parent? diff --git a/traffic_ops/traffic_ops_golang/server/servers_server_capability.go b/traffic_ops/traffic_ops_golang/server/servers_server_capability.go index ed52f21ad9..b1a5ee56ab 100644 --- a/traffic_ops/traffic_ops_golang/server/servers_server_capability.go +++ b/traffic_ops/traffic_ops_golang/server/servers_server_capability.go @@ -363,13 +363,13 @@ server) VALUES ( func checkDSReqCapQuery() string { return ` SELECT ARRAY( - SELECT dsrc.deliveryservice_id - FROM deliveryservices_required_capability as dsrc - WHERE deliveryservice_id IN ( + SELECT ds.id + FROM deliveryservice as ds + WHERE id IN ( SELECT deliveryservice FROM deliveryservice_server WHERE server = $1) - AND dsrc.required_capability = $2)` + AND $2 = ANY(ds.required_capabilities))` } // get the topology-based DSes (with all their required capabilities) that a given @@ -380,15 +380,14 @@ SELECT ds.xml_id, ds.topology, ds.tenant_id, - ARRAY_AGG(dsrc.required_capability) AS req_caps + ds.required_capabilities AS req_caps FROM server s JOIN cachegroup c ON s.cachegroup = c.id JOIN topology_cachegroup tc ON c.name = tc.cachegroup JOIN deliveryservice ds ON ds.topology = tc.topology -JOIN deliveryservices_required_capability dsrc ON dsrc.deliveryservice_id = ds.id WHERE s.id = $1 -GROUP BY ds.xml_id, ds.tenant_id, ds.topology -HAVING $2 = ANY(ARRAY_AGG(dsrc.required_capability)) +GROUP BY ds.xml_id, ds.tenant_id, ds.topology, ds.required_capabilities +HAVING $2 = ANY(ds.required_capabilities) ` } diff --git a/traffic_ops/traffic_ops_golang/topology/topologies.go b/traffic_ops/traffic_ops_golang/topology/topologies.go index 988913cc9d..9bbdf533be 100644 --- a/traffic_ops/traffic_ops_golang/topology/topologies.go +++ b/traffic_ops/traffic_ops_golang/topology/topologies.go @@ -340,12 +340,11 @@ func getDSRequiredCapabilitiesByTopology(name string, tx *sql.Tx) (map[string][] SELECT d.xml_id, d.cdn_id, - ARRAY_AGG(drc.required_capability) AS required_capabilities + d.required_capabilities FROM deliveryservice d -JOIN deliveryservices_required_capability drc ON d.id = drc.deliveryservice_id WHERE d.topology = $1 -GROUP BY d.xml_id, d.cdn_id +GROUP BY d.xml_id, d.cdn_id, d.required_capabilities ` rows, err := tx.Query(q, name) if err != nil { diff --git a/traffic_portal/test/integration/Data/deliveryservices.ts b/traffic_portal/test/integration/Data/deliveryservices.ts index 06bc4da2f6..4514acb7ed 100644 --- a/traffic_portal/test/integration/Data/deliveryservices.ts +++ b/traffic_portal/test/integration/Data/deliveryservices.ts @@ -361,7 +361,7 @@ export const deliveryservices = { { rcName: "DSTestCap", xmlID: "tpdservice2", - validationMessage: "deliveryservice.RequiredCapability was created." + validationMessage: "This endpoint is deprecated, please use the deliveryservices endpoint instead" } ], remove: [ @@ -469,7 +469,7 @@ export const deliveryservices = { { rcName: "DSTestCap", xmlID: "optpdservice2", - validationMessage: "deliveryservice.RequiredCapability was created." + validationMessage: "This endpoint is deprecated, please use the deliveryservices endpoint instead" } ], remove: [ From f1f820567e1ff53e5997f4deec692e3a6107e40b Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Mon, 28 Nov 2022 13:25:19 -0700 Subject: [PATCH 02/10] fix unit test --- lib/go-tc/deliveryservices.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/go-tc/deliveryservices.go b/lib/go-tc/deliveryservices.go index 97e96c1661..92b2a39968 100644 --- a/lib/go-tc/deliveryservices.go +++ b/lib/go-tc/deliveryservices.go @@ -1731,7 +1731,11 @@ func (ds DeliveryServiceV4) Upgrade() DeliveryServiceV5 { copy(upgraded.MatchList, *ds.MatchList) } copy(upgraded.TLSVersions, ds.TLSVersions) - copy(upgraded.RequiredCapabilities, ds.RequiredCapabilities) + if ds.RequiredCapabilities == nil { + upgraded.RequiredCapabilities = nil + } else { + copy(upgraded.RequiredCapabilities, ds.RequiredCapabilities) + } return upgraded } From bdcb2a265ee9b097f5582c919e0b9deac97a69f9 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Mon, 5 Dec 2022 23:37:38 -0700 Subject: [PATCH 03/10] code review --- CHANGELOG.md | 2 +- .../api/v4/deliveryservice_requests.rst | 12 ++--- .../v4/deliveryservice_requests_id_assign.rst | 4 +- .../v4/deliveryservice_requests_id_status.rst | 4 +- docs/source/api/v4/deliveryservices.rst | 12 ++--- docs/source/api/v4/deliveryservices_id.rst | 8 ++-- .../api/v4/servers_id_deliveryservices.rst | 4 +- lib/go-tc/deliveryservice_requests.go | 44 ++++++++++--------- lib/go-tc/deliveryservices.go | 6 +-- ...0_add_required_capabilities_to_ds.down.sql | 15 ++----- ...300_add_required_capabilities_to_ds.up.sql | 6 +-- .../testing/api/v3/deliveryservices_test.go | 5 +-- .../api/v4/deliveryservice_requests_test.go | 7 ++- .../testing/api/v4/traffic_control_test.go | 2 +- .../traffic_ops_golang/api/shared_handlers.go | 7 ++- .../dbhelpers/db_helpers.go | 11 +---- .../deliveryservice/deliveryservices.go | 22 ++++++---- .../traffic_ops_golang/server/servers.go | 2 +- 18 files changed, 82 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12a0c17a00..25e624bc27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#7113](https://github.com/apache/trafficcontrol/pull/7113) *Traffic Portal* Minimize the Server Server Capability part of the *Traffic Servers* section of the Snapshot Diff ### Changed -- *Traffic Ops* Required Parameters are now a part of the `DeliveryService` structure. +- [#7224](https://github.com/apache/trafficcontrol/pull/7224) *Traffic Ops* Required Capabilities are now a part of the `DeliveryService` structure. - [#7063](https://github.com/apache/trafficcontrol/pull/7063) *Traffic Ops* Python client now uses Traffic Ops API 4.1 by default. - [#6981](https://github.com/apache/trafficcontrol/pull/6981) *Traffic Portal* Obscures sensitive text in Delivery Service "Raw Remap" fields, private SSL keys, "Header Rewrite" rules, and ILO interface passwords by default. - [#7037](https://github.com/apache/trafficcontrol/pull/7037) *Traffic Router* Uses Traffic Ops API 4.0 by default diff --git a/docs/source/api/v4/deliveryservice_requests.rst b/docs/source/api/v4/deliveryservice_requests.rst index c88fb7ebdf..5748454f9b 100644 --- a/docs/source/api/v4/deliveryservice_requests.rst +++ b/docs/source/api/v4/deliveryservice_requests.rst @@ -163,7 +163,7 @@ The response is an array of representations of :term:`Delivery Service Requests` "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -281,7 +281,7 @@ The request must be a well-formed representation of a :term:`Delivery Service Re "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -407,7 +407,7 @@ The response will be a representation of the created :term:`Delivery Service Req "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -493,7 +493,7 @@ The response will be a representation of the created :term:`Delivery Service Req "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -671,7 +671,7 @@ The response is a full representation of the edited :term:`Delivery Service Requ "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -753,7 +753,7 @@ The response is a full representation of the edited :term:`Delivery Service Requ "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "cdn", "signed": false, "sslKeyVersion": null, diff --git a/docs/source/api/v4/deliveryservice_requests_id_assign.rst b/docs/source/api/v4/deliveryservice_requests_id_assign.rst index 89b7e00018..7737b67881 100644 --- a/docs/source/api/v4/deliveryservice_requests_id_assign.rst +++ b/docs/source/api/v4/deliveryservice_requests_id_assign.rst @@ -198,7 +198,7 @@ The response contains a full representation of the newly assigned :term:`Deliver "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -280,7 +280,7 @@ The response contains a full representation of the newly assigned :term:`Deliver "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "cdn", "signed": false, "sslKeyVersion": null, diff --git a/docs/source/api/v4/deliveryservice_requests_id_status.rst b/docs/source/api/v4/deliveryservice_requests_id_status.rst index 35b5b6b82a..fc1333918d 100644 --- a/docs/source/api/v4/deliveryservice_requests_id_status.rst +++ b/docs/source/api/v4/deliveryservice_requests_id_status.rst @@ -195,7 +195,7 @@ The response is a full representation of the modified :term:`DSR`. "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -277,7 +277,7 @@ The response is a full representation of the modified :term:`DSR`. "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "cdn", "signed": false, "sslKeyVersion": null, diff --git a/docs/source/api/v4/deliveryservices.rst b/docs/source/api/v4/deliveryservices.rst index 48e64b0261..d16ce62fe5 100644 --- a/docs/source/api/v4/deliveryservices.rst +++ b/docs/source/api/v4/deliveryservices.rst @@ -150,7 +150,7 @@ Response Structure :remapText: :ref:`ds-raw-remap` :requiredCapabilities: An array of the capabilities that this delivery service requires. - .. versionchanged:: 4.1 + .. versionadded:: 4.1 :serviceCategory: The name of the :ref:`ds-service-category` with which the :term:`Delivery Service` is associated :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise @@ -254,7 +254,7 @@ Response Structure "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "video", "serviceCategory": null, "signed": false, @@ -335,7 +335,7 @@ Request Structure :remapText: :ref:`ds-raw-remap` :requiredCapabilities: An array of the capabilities that this delivery service requires. - .. versionchanged:: 4.1 + .. versionadded:: 4.1 :serviceCategory: The name of the :ref:`ds-service-category` with which the :term:`Delivery Service` is associated - or ``null`` if there is to be no such category :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise @@ -416,7 +416,7 @@ Request Structure "regexRemap": null, "regional": false, "regionalGeoBlocking": false, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "test", "serviceCategory": null, "signed": false, @@ -507,7 +507,7 @@ Response Structure :remapText: :ref:`ds-raw-remap` :requiredCapabilities: An array of the capabilities that this delivery service requires. - .. versionchanged:: 4.1 + .. versionadded:: 4.1 :serviceCategory: The name of the :ref:`ds-service-category` with which the :term:`Delivery Service` is associated :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise @@ -620,7 +620,7 @@ Response Structure "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "test", "serviceCategory": null, "signed": false, diff --git a/docs/source/api/v4/deliveryservices_id.rst b/docs/source/api/v4/deliveryservices_id.rst index 823a5a953e..9fa17d22c4 100644 --- a/docs/source/api/v4/deliveryservices_id.rst +++ b/docs/source/api/v4/deliveryservices_id.rst @@ -83,7 +83,7 @@ Request Structure :remapText: :ref:`ds-raw-remap` :requiredCapabilities: An array of the capabilities that this delivery service requires. - .. versionchanged:: 4.1 + .. versionadded:: 4.1 :routingName: The :ref:`ds-routing-name` of this :term:`Delivery Service` @@ -169,7 +169,7 @@ Request Structure "regexRemap": null, "regional": false, "regionalGeoBlocking": false, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "test", "serviceCategory": null, "signed": false, @@ -256,7 +256,7 @@ Response Structure :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :requiredCapabilities: An array of the capabilities that this delivery service requires. - .. versionchanged:: 4.1 + .. versionadded:: 4.1 :remapText: :ref:`ds-raw-remap` :serviceCategory: The name of the :ref:`ds-service-category` with which the :term:`Delivery Service` is associated @@ -364,7 +364,7 @@ Response Structure "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "test", "serviceCategory": null, "signed": false, diff --git a/docs/source/api/v4/servers_id_deliveryservices.rst b/docs/source/api/v4/servers_id_deliveryservices.rst index b434f7ea5e..116342e44e 100644 --- a/docs/source/api/v4/servers_id_deliveryservices.rst +++ b/docs/source/api/v4/servers_id_deliveryservices.rst @@ -136,7 +136,7 @@ Response Structure :remapText: :ref:`ds-raw-remap` :requiredCapabilities: An array of the capabilities that this delivery service requires. - .. versionchanged:: 4.1 + .. versionadded:: 4.1 :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise :signingAlgorithm: Either a :ref:`ds-signing-algorithm` or ``null`` to indicate URL/URI signing is not implemented on this :term:`Delivery Service` @@ -238,7 +238,7 @@ Response Structure "regional": false, "regionalGeoBlocking": false, "remapText": null, - "requiredCapabilities": null, + "requiredCapabilities": [], "routingName": "cdn", "serviceCategory": null, "signed": false, diff --git a/lib/go-tc/deliveryservice_requests.go b/lib/go-tc/deliveryservice_requests.go index dbf0cb9280..46524ce3db 100644 --- a/lib/go-tc/deliveryservice_requests.go +++ b/lib/go-tc/deliveryservice_requests.go @@ -435,23 +435,25 @@ type DeliveryServiceRequestNullable struct { XMLID *string `json:"-" db:"xml_id"` } -// Downgrade will convert an instance of DeliveryServiceRequestV4 to DeliveryServiceRequestV40 -func (dsr DeliveryServiceRequestV4) Downgrade() DeliveryServiceRequestV40 { +// Downgrade will convert an instance of DeliveryServiceRequestV41 to DeliveryServiceRequestV40 +func (dsr DeliveryServiceRequestV41) Downgrade() DeliveryServiceRequestV40 { var dsrV40 DeliveryServiceRequestV40 - dsrV40.Assignee = dsr.Assignee - dsrV40.AssigneeID = dsr.AssigneeID + dsrV40.Assignee = copyStringIfNotNil(dsr.Assignee) + dsrV40.AssigneeID = copyIntIfNotNil(dsr.AssigneeID) dsrV40.Author = dsr.Author - dsrV40.AuthorID = dsr.AuthorID + dsrV40.AuthorID = copyIntIfNotNil(dsr.AuthorID) dsrV40.ChangeType = dsr.ChangeType dsrV40.CreatedAt = dsr.CreatedAt - dsrV40.ID = dsr.ID + dsrV40.ID = copyIntIfNotNil(dsr.ID) dsrV40.LastEditedBy = dsr.LastEditedBy - dsrV40.LastEditedByID = dsr.LastEditedByID + dsrV40.LastEditedByID = copyIntIfNotNil(dsr.LastEditedByID) dsrV40.LastUpdated = dsr.LastUpdated if dsr.Original != nil { + dsrV40.Original = new(DeliveryServiceV40) dsrV40.Original = &dsr.Original.DeliveryServiceV40 } if dsr.Requested != nil { + dsrV40.Requested = new(DeliveryServiceV40) dsrV40.Requested = &dsr.Requested.DeliveryServiceV40 } dsrV40.Status = dsr.Status @@ -459,23 +461,25 @@ func (dsr DeliveryServiceRequestV4) Downgrade() DeliveryServiceRequestV40 { return dsrV40 } -// Upgrade will convert an instance of DeliveryServiceRequestV40 to DeliveryServiceRequestV4 -func (dsrV40 DeliveryServiceRequestV40) Upgrade() DeliveryServiceRequestV4 { +// Upgrade will convert an instance of DeliveryServiceRequestV40 to DeliveryServiceRequestV41 +func (dsrV40 DeliveryServiceRequestV40) Upgrade() DeliveryServiceRequestV41 { var dsrV4 DeliveryServiceRequestV41 - dsrV4.Assignee = dsrV40.Assignee - dsrV4.AssigneeID = dsrV40.AssigneeID + dsrV4.Assignee = copyStringIfNotNil(dsrV40.Assignee) + dsrV4.AssigneeID = copyIntIfNotNil(dsrV40.AssigneeID) dsrV4.Author = dsrV40.Author - dsrV4.AuthorID = dsrV40.AuthorID + dsrV4.AuthorID = copyIntIfNotNil(dsrV40.AuthorID) dsrV4.ChangeType = dsrV40.ChangeType dsrV4.CreatedAt = dsrV40.CreatedAt - dsrV4.ID = dsrV40.ID + dsrV4.ID = copyIntIfNotNil(dsrV40.ID) dsrV4.LastEditedBy = dsrV40.LastEditedBy - dsrV4.LastEditedByID = dsrV40.LastEditedByID + dsrV4.LastEditedByID = copyIntIfNotNil(dsrV40.LastEditedByID) dsrV4.LastUpdated = dsrV40.LastUpdated if dsrV40.Original != nil { + dsrV4.Original = new(DeliveryServiceV41) dsrV4.Original = &DeliveryServiceV4{DeliveryServiceV40: *dsrV40.Original} } if dsrV40.Requested != nil { + dsrV4.Requested = new(DeliveryServiceV41) dsrV4.Requested = &DeliveryServiceV4{DeliveryServiceV40: *dsrV40.Requested} } dsrV4.Status = dsrV40.Status @@ -1116,7 +1120,7 @@ func (dsr DeliveryServiceRequestV5) Downgrade() DeliveryServiceRequestV4 { // Original) are copied using the DeliveryServiceV4.Upgrade method (which is // also deep). func (dsr DeliveryServiceRequestV4) Upgrade() DeliveryServiceRequestV5 { - downgraded := DeliveryServiceRequestV5{ + upgraded := DeliveryServiceRequestV5{ Assignee: copyStringIfNotNil(dsr.Assignee), AssigneeID: copyIntIfNotNil(dsr.AssigneeID), Author: dsr.Author, @@ -1131,13 +1135,13 @@ func (dsr DeliveryServiceRequestV4) Upgrade() DeliveryServiceRequestV5 { XMLID: dsr.XMLID, } if dsr.Requested != nil { - downgraded.Requested = new(DeliveryServiceV5) - *downgraded.Requested = dsr.Requested.Upgrade() + upgraded.Requested = new(DeliveryServiceV5) + *upgraded.Requested = dsr.Requested.Upgrade() } else if dsr.Original != nil { - downgraded.Original = new(DeliveryServiceV5) - *downgraded.Original = dsr.Original.Upgrade() + upgraded.Original = new(DeliveryServiceV5) + *upgraded.Original = dsr.Original.Upgrade() } - return downgraded + return upgraded } // IsOpen returns whether or not the Delivery Service Request is still "open" - diff --git a/lib/go-tc/deliveryservices.go b/lib/go-tc/deliveryservices.go index 92b2a39968..59824d170e 100644 --- a/lib/go-tc/deliveryservices.go +++ b/lib/go-tc/deliveryservices.go @@ -1438,7 +1438,7 @@ type DeliveryServiceV50 struct { // use, because the input is in no way restricted, validated, or limited in // scope to the Delivery Service. RemapText *string `json:"remapText" db:"remap_text"` - // RequiredCapabilities is an array of capabilities required for this delivery service + // RequiredCapabilities is an array of capabilities required for this delivery service. RequiredCapabilities []string `json:"requiredCapabilities" db:"required_capabilities"` // RoutingName defines the lowest-level DNS label used by the Delivery // Service, e.g. if trafficcontrol.apache.org were a Delivery Service, it @@ -1731,8 +1731,8 @@ func (ds DeliveryServiceV4) Upgrade() DeliveryServiceV5 { copy(upgraded.MatchList, *ds.MatchList) } copy(upgraded.TLSVersions, ds.TLSVersions) - if ds.RequiredCapabilities == nil { - upgraded.RequiredCapabilities = nil + if len(ds.RequiredCapabilities) == 0 { + upgraded.RequiredCapabilities = make([]string, 0) } else { copy(upgraded.RequiredCapabilities, ds.RequiredCapabilities) } diff --git a/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.down.sql b/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.down.sql index f76d291cf9..18dd306f19 100644 --- a/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.down.sql +++ b/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.down.sql @@ -15,22 +15,15 @@ * the License. */ -CREATE TABLE IF NOT EXISTS deliveryservices_required_capability ( +CREATE TABLE IF NOT EXISTS public.deliveryservices_required_capability ( required_capability TEXT NOT NULL, - deliveryservice_id bigint NOT NULL, + deliveryservice_id BIGINT NOT NULL, last_updated timestamp with time zone DEFAULT now() NOT NULL, - PRIMARY KEY (deliveryservice_id, required_capability) + CONSTRAINT ds_capability_primary PRIMARY KEY (deliveryservice_id, required_capability) ); -DO $$ -DECLARE temprow RECORD; -BEGIN FOR temprow IN -select id as deliveryservice_id, unnest(required_capabilities) as required_capability from deliveryservice d group by d.id, d.required_capabilities - LOOP -insert into deliveryservices_required_capability ("deliveryservice_id", "required_capability") values (temprow.deliveryservice_id, temprow.required_capability); -END LOOP; -END $$; +INSERT INTO public.deliveryservices_required_capability ("deliveryservice_id", "required_capability") SELECT id AS deliveryservice_id, UNNEST(required_capabilities) AS required_capability FROM deliveryservice d GROUP BY d.id, d.required_capabilities; ALTER TABLE public.deliveryservice DROP COLUMN required_capabilities; diff --git a/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.up.sql b/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.up.sql index 75da8e4dad..e92f060394 100644 --- a/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.up.sql +++ b/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.up.sql @@ -16,14 +16,14 @@ */ ALTER TABLE public.deliveryservice - ADD COLUMN required_capabilities text[]; + ADD COLUMN required_capabilities TEXT[]; DO $$ DECLARE temprow RECORD; BEGIN FOR temprow IN -select deliveryservice_id, array_agg(required_capability) as required_capabilities from deliveryservices_required_capability drc group by drc.deliveryservice_id +SELECT deliveryservice_id, ARRAY_AGG(required_capability) AS required_capabilities FROM deliveryservices_required_capability drc GROUP BY drc.deliveryservice_id LOOP -update deliveryservice d set required_capabilities = temprow.required_capabilities where d.id = temprow.deliveryservice_id; +UPDATE deliveryservice d SET required_capabilities = temprow.required_capabilities WHERE d.id = temprow.deliveryservice_id; END LOOP; END $$; diff --git a/traffic_ops/testing/api/v3/deliveryservices_test.go b/traffic_ops/testing/api/v3/deliveryservices_test.go index 694ce642ef..975df24669 100644 --- a/traffic_ops/testing/api/v3/deliveryservices_test.go +++ b/traffic_ops/testing/api/v3/deliveryservices_test.go @@ -226,9 +226,8 @@ func TestDeliveryServices(t *testing.T) { "BAD REQUEST when ADDING TOPOLOGY to DS with DS REQUIRED CAPABILITY": { EndpointID: GetDeliveryServiceId(t, "ds1"), ClientSession: TOSession, RequestBody: generateDeliveryService(t, map[string]interface{}{ - "requiredCapabilities": []string{"foo"}, - "topology": "top-for-ds-req", - "xmlId": "ds1", + "topology": "top-for-ds-req", + "xmlId": "ds1", }), Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), }, diff --git a/traffic_ops/testing/api/v4/deliveryservice_requests_test.go b/traffic_ops/testing/api/v4/deliveryservice_requests_test.go index 643dee30b7..947a338791 100644 --- a/traffic_ops/testing/api/v4/deliveryservice_requests_test.go +++ b/traffic_ops/testing/api/v4/deliveryservice_requests_test.go @@ -408,10 +408,9 @@ func validatePutDSRequestFields(expectedResp map[string]interface{}) utils.CkReq func CreateTestDeliveryServiceRequests(t *testing.T) { for _, dsr := range testData.DeliveryServiceRequests { - dsrV41 := dsr.Upgrade() - resetDS(dsrV41.Original) - resetDS(dsrV41.Requested) - respDSR, _, err := TOSession.CreateDeliveryServiceRequest(dsrV41, client.RequestOptions{}) + resetDS(dsr.Original) + resetDS(dsr.Requested) + respDSR, _, err := TOSession.CreateDeliveryServiceRequest(dsr, client.RequestOptions{}) assert.NoError(t, err, "Could not create Delivery Service Requests: %v - alerts: %+v", err, respDSR.Alerts) } } diff --git a/traffic_ops/testing/api/v4/traffic_control_test.go b/traffic_ops/testing/api/v4/traffic_control_test.go index ddbc8b06a0..cd08c61af9 100644 --- a/traffic_ops/testing/api/v4/traffic_control_test.go +++ b/traffic_ops/testing/api/v4/traffic_control_test.go @@ -28,7 +28,7 @@ type TrafficControl struct { Capabilities []tc.Capability `json:"capability"` Coordinates []tc.Coordinate `json:"coordinates"` DeliveryServicesRegexes []tc.DeliveryServiceRegexesTest `json:"deliveryServicesRegexes"` - DeliveryServiceRequests []tc.DeliveryServiceRequestV40 `json:"deliveryServiceRequests"` + DeliveryServiceRequests []tc.DeliveryServiceRequestV4 `json:"deliveryServiceRequests"` DeliveryServiceRequestComments []tc.DeliveryServiceRequestComment `json:"deliveryServiceRequestComments"` DeliveryServices []tc.DeliveryServiceV4 `json:"deliveryservices"` DeliveryServicesRequiredCapabilities []tc.DeliveryServicesRequiredCapability `json:"deliveryservicesRequiredCapabilities"` diff --git a/traffic_ops/traffic_ops_golang/api/shared_handlers.go b/traffic_ops/traffic_ops_golang/api/shared_handlers.go index a593f8329b..782e7204ac 100644 --- a/traffic_ops/traffic_ops_golang/api/shared_handlers.go +++ b/traffic_ops/traffic_ops_golang/api/shared_handlers.go @@ -633,10 +633,9 @@ func DeprecatedCreateHandler(creator Creator, alternative *string) http.HandlerF }, func(w http.ResponseWriter, r *http.Request, statusCode int, alerts tc.Alerts, results interface{}) { depAlerts := CreateDeprecationAlerts(alternative) - for _, al := range alerts.Alerts { - depAlerts.Alerts = append(depAlerts.Alerts, al) - } - WriteAlertsObj(w, r, statusCode, depAlerts, results) + al := tc.Alerts{Alerts: depAlerts.Alerts} + al.AddAlerts(alerts) + WriteAlertsObj(w, r, statusCode, al, results) }, ) } diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go index a71337f1e6..09f9b30a36 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go +++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go @@ -929,17 +929,10 @@ func GetDSRequiredCapabilitiesFromID(id int, tx *sql.Tx) ([]string, error) { FROM deliveryservice WHERE id = $1 ORDER BY required_capabilities` - rows, err := tx.Query(q, id) - if err != nil { - return nil, errors.New("querying deliveryservice required capabilities from id: " + err.Error()) - } - defer rows.Close() caps := []string{} - for rows.Next() { - if err := rows.Scan(pq.Array(&caps)); err != nil { - return nil, errors.New("scanning capability: " + err.Error()) - } + if err := tx.QueryRow(q, id).Scan(pq.Array(&caps)); err != nil { + return nil, errors.New("getting/ scanning capability: " + err.Error()) } return caps, nil } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go index 8cd9dce765..0486219b48 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go @@ -1591,8 +1591,12 @@ func Validate(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { if err := validateTypeFields(tx, ds); err != nil { errs = append(errs, fmt.Errorf("type fields: %w", err)) } - if err := validateRequiredCapabilities(tx, ds); err != nil { - errs = append(errs, errors.New("required capabilities: "+err.Error())) + userErr, sysErr := validateRequiredCapabilities(tx, ds) + if userErr != nil { + errs = append(errs, errors.New("required capabilities: "+userErr.Error())) + } + if sysErr != nil { + errs = append(errs, errors.New("reading/ scanning required capabilities: "+sysErr.Error())) } if len(errs) == 0 { return nil @@ -1600,26 +1604,26 @@ func Validate(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { return util.JoinErrs(errs) } -func validateRequiredCapabilities(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { +func validateRequiredCapabilities(tx *sql.Tx, ds *tc.DeliveryServiceV5) (error, error) { var valid bool query := `SELECT $1 <@ (SELECT ARRAY_AGG(name) FROM server_capability)` - if ds.RequiredCapabilities != nil && len(ds.RequiredCapabilities) > 0 { + if len(ds.RequiredCapabilities) > 0 { rows, err := tx.Query(query, pq.Array(ds.RequiredCapabilities)) if err != nil { - return err + return nil, err } defer rows.Close() for rows.Next() { err = rows.Scan(&valid) if err != nil { - return err + return nil, err } if !valid { - return errors.New("one or more of the required capabilities do not exist") + return errors.New("one or more of the required capabilities do not exist"), nil } } } - return nil + return nil, nil } func validateGeoLimitCountries(ds *tc.DeliveryServiceV5) error { @@ -2517,7 +2521,7 @@ ds.active, ds.regional, ds.regional_geo_blocking, ds.remap_text, - ds.required_capabilities, + COALESCE(ds.required_capabilities, '{}'), ds.routing_name, ds.service_category, ds.signing_algorithm, diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go index 5442ad5744..d45413272c 100644 --- a/traffic_ops/traffic_ops_golang/server/servers.go +++ b/traffic_ops/traffic_ops_golang/server/servers.go @@ -833,7 +833,7 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth } for rows.Next() { if err = rows.Scan(pq.Array(&requiredCapabilities)); err != nil { - err = fmt.Errorf("unable to scan required capabilities for deliveryservice %d: %s", dsID, err) + err = fmt.Errorf("unable to scan required capabilities for deliveryservice %d: %w", dsID, err) return nil, 0, nil, err, http.StatusInternalServerError, nil } } From cc5cacd079ebc6d5f8d82b90e434955ff0070a26 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Tue, 6 Dec 2022 17:38:13 -0700 Subject: [PATCH 04/10] code review round 2 --- ...deliveryservices_required_capabilities.rst | 6 - ...deliveryservices_required_capabilities.rst | 7 +- ...veryservices_required_capabilities_test.go | 424 +++++++++--------- .../testing/api/v5/deliveryservices_test.go | 2 +- .../api/v5/deliveryserviceservers_test.go | 2 +- .../deliveryservice/deliveryservices.go | 22 +- .../traffic_ops_golang/routing/routes.go | 5 - 7 files changed, 233 insertions(+), 235 deletions(-) diff --git a/docs/source/api/v3/deliveryservices_required_capabilities.rst b/docs/source/api/v3/deliveryservices_required_capabilities.rst index 66a1b0e4dc..9df9088bf3 100644 --- a/docs/source/api/v3/deliveryservices_required_capabilities.rst +++ b/docs/source/api/v3/deliveryservices_required_capabilities.rst @@ -19,8 +19,6 @@ ``deliveryservices_required_capabilities`` ****************************************** -.. deprecated:: ATCv7 - ``GET`` ======= Gets all associations of :term:`Server Capability` to :term:`Delivery Services`. @@ -104,8 +102,6 @@ Response Structure ] } -.. deprecated:: ATCv7 - ``POST`` ======== Associates a :term:`Server Capability` with a :term:`Delivery Service`. @@ -172,8 +168,6 @@ Response Structure } } -.. deprecated:: ATCv7 - ``DELETE`` ========== Dissociate a :term:`Server Capability` from a :term:`Delivery Service`. diff --git a/docs/source/api/v4/deliveryservices_required_capabilities.rst b/docs/source/api/v4/deliveryservices_required_capabilities.rst index 40b2532e13..d010dfe977 100644 --- a/docs/source/api/v4/deliveryservices_required_capabilities.rst +++ b/docs/source/api/v4/deliveryservices_required_capabilities.rst @@ -19,7 +19,8 @@ ``deliveryservices_required_capabilities`` ****************************************** -.. deprecated:: ATCv7 +.. deprecated:: 4.1 + This endpoint will be removed in a future release, in favor of ``required_capabilities`` being a part of :term:`Delivery Services`. ``GET`` ======= @@ -105,8 +106,6 @@ Response Structure ] } -.. deprecated:: ATCv7 - ``POST`` ======== Associates a :term:`Server Capability` with a :term:`Delivery Service`. @@ -174,8 +173,6 @@ Response Structure } } -.. deprecated:: ATCv7 - ``DELETE`` ========== Dissociate a :term:`Server Capability` from a :term:`Delivery Service`. diff --git a/traffic_ops/testing/api/v5/deliveryservices_required_capabilities_test.go b/traffic_ops/testing/api/v5/deliveryservices_required_capabilities_test.go index fba249641b..bb78a7adff 100644 --- a/traffic_ops/testing/api/v5/deliveryservices_required_capabilities_test.go +++ b/traffic_ops/testing/api/v5/deliveryservices_required_capabilities_test.go @@ -16,217 +16,211 @@ package v5 */ import ( - "net/http" - "net/url" - "strconv" - "testing" - "time" - - "github.com/apache/trafficcontrol/lib/go-rfc" + "fmt" "github.com/apache/trafficcontrol/lib/go-tc" - "github.com/apache/trafficcontrol/lib/go-util" "github.com/apache/trafficcontrol/traffic_ops/testing/api/assert" "github.com/apache/trafficcontrol/traffic_ops/testing/api/utils" "github.com/apache/trafficcontrol/traffic_ops/toclientlib" client "github.com/apache/trafficcontrol/traffic_ops/v5-client" + "testing" ) -func TestDeliveryServicesRequiredCapabilities(t *testing.T) { - WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, ServerCapabilities, Topologies, ServiceCategories, DeliveryServices, DeliveryServiceServerAssignments, ServerServerCapabilities, DeliveryServicesRequiredCapabilities}, func() { - - currentTime := time.Now().UTC().Add(-15 * time.Second) - currentTimeRFC := currentTime.Format(time.RFC1123) - tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123) - - methodTests := utils.TestCase[client.Session, client.RequestOptions, tc.DeliveryServicesRequiredCapability]{ - "GET": { - "NOT MODIFIED when NO CHANGES made": { - ClientSession: TOSession, - RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfModifiedSince: {tomorrow}}}, - Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusNotModified)), - }, - "OK when VALID request": { - ClientSession: TOSession, - Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)), - }, - "OK when VALID DELIVERYSERVICEID parameter": { - ClientSession: TOSession, - RequestOpts: client.RequestOptions{QueryParameters: url.Values{"deliveryServiceId": {strconv.Itoa(GetDeliveryServiceId(t, "ds1")())}}}, - Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseLengthGreaterOrEqual(1), - validateDSRCExpectedFields(map[string]interface{}{"DeliveryServiceId": "ds1"})), - }, - "OK when VALID XMLID parameter": { - ClientSession: TOSession, - RequestOpts: client.RequestOptions{QueryParameters: url.Values{"xmlID": {"ds2"}}}, - Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseLengthGreaterOrEqual(1), - validateDSRCExpectedFields(map[string]interface{}{"XMLID": "ds2"})), - }, - "OK when VALID REQUIREDCAPABILITY parameter": { - ClientSession: TOSession, - RequestOpts: client.RequestOptions{QueryParameters: url.Values{"requiredCapability": {"bar"}}}, - Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseLengthGreaterOrEqual(1), - validateDSRCExpectedFields(map[string]interface{}{"RequiredCapability": "bar"})), - }, - "FIRST RESULT when LIMIT=1": { - ClientSession: TOSession, - RequestOpts: client.RequestOptions{QueryParameters: url.Values{"orderby": {"requiredCapability"}, "limit": {"1"}}}, - Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateDSRCPagination("limit")), - }, - "SECOND RESULT when LIMIT=1 OFFSET=1": { - ClientSession: TOSession, - RequestOpts: client.RequestOptions{QueryParameters: url.Values{"orderby": {"requiredCapability"}, "limit": {"1"}, "offset": {"1"}}}, - Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateDSRCPagination("offset")), - }, - "SECOND RESULT when LIMIT=1 PAGE=2": { - ClientSession: TOSession, - RequestOpts: client.RequestOptions{QueryParameters: url.Values{"orderby": {"requiredCapability"}, "limit": {"1"}, "page": {"2"}}}, - Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateDSRCPagination("page")), - }, - "BAD REQUEST when INVALID LIMIT parameter": { - ClientSession: TOSession, - RequestOpts: client.RequestOptions{QueryParameters: url.Values{"limit": {"-2"}}}, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), - }, - "BAD REQUEST when INVALID OFFSET parameter": { - ClientSession: TOSession, - RequestOpts: client.RequestOptions{QueryParameters: url.Values{"limit": {"1"}, "offset": {"0"}}}, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), - }, - "BAD REQUEST when INVALID PAGE parameter": { - ClientSession: TOSession, - RequestOpts: client.RequestOptions{QueryParameters: url.Values{"limit": {"1"}, "page": {"0"}}}, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), - }, - "OK when CHANGES made": { - ClientSession: TOSession, - RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfModifiedSince: {currentTimeRFC}}}, - Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)), - }, - }, - "POST": { - "BAD REQUEST when REASSIGNING REQUIRED CAPABILITY to DELIVERY SERVICE": { - ClientSession: TOSession, - RequestBody: tc.DeliveryServicesRequiredCapability{ - DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "ds1")()), - RequiredCapability: util.StrPtr("foo"), - }, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), - }, - "BAD REQUEST when SERVERS DONT have CAPABILITY": { - ClientSession: TOSession, - RequestBody: tc.DeliveryServicesRequiredCapability{ - DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "test-ds-server-assignments")()), - RequiredCapability: util.StrPtr("disk"), - }, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), - }, - "BAD REQUEST when DELIVERY SERVICE HAS TOPOLOGY where SERVERS DONT have CAPABILITY": { - ClientSession: TOSession, - RequestBody: tc.DeliveryServicesRequiredCapability{ - DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "ds-top-req-cap")()), - RequiredCapability: util.StrPtr("bar"), - }, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), - }, - "BAD REQUEST when DELIVERY SERVICE ID EMPTY": { - ClientSession: TOSession, - RequestBody: tc.DeliveryServicesRequiredCapability{ - RequiredCapability: util.StrPtr("bar"), - }, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), - }, - "BAD REQUEST when REQUIRED CAPABILITY EMPTY": { - ClientSession: TOSession, - RequestBody: tc.DeliveryServicesRequiredCapability{ - DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "ds1")()), - }, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), - }, - "NOT FOUND when NON-EXISTENT REQUIRED CAPABILITY": { - ClientSession: TOSession, - RequestBody: tc.DeliveryServicesRequiredCapability{ - DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "ds1")()), - RequiredCapability: util.StrPtr("bogus"), - }, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusNotFound)), - }, - "NOT FOUND when NON-EXISTENT DELIVERY SERVICE ID": { - ClientSession: TOSession, - RequestBody: tc.DeliveryServicesRequiredCapability{ - DeliveryServiceID: util.IntPtr(-1), - RequiredCapability: util.StrPtr("foo"), - }, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusNotFound)), - }, - "BAD REQUEST when INVALID DELIVERY SERVICE TYPE": { - ClientSession: TOSession, - RequestBody: tc.DeliveryServicesRequiredCapability{ - DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "anymap-ds")()), - RequiredCapability: util.StrPtr("foo"), - }, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), - }, - }, - "DELETE": { - "OK when VALID request": { - ClientSession: TOSession, - RequestBody: tc.DeliveryServicesRequiredCapability{ - DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "ds-top-req-cap")()), - RequiredCapability: util.StrPtr("ram"), - }, - Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)), - }, - "NOT FOUND when NON-EXISTENT DELIVERYSERVICEID parameter": { - ClientSession: TOSession, - RequestBody: tc.DeliveryServicesRequiredCapability{ - DeliveryServiceID: util.IntPtr(-1), - RequiredCapability: util.StrPtr("foo"), - }, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusNotFound)), - }, - "NOT FOUND when NON-EXISTENT REQUIREDCAPABILITY parameter": { - ClientSession: TOSession, - RequestBody: tc.DeliveryServicesRequiredCapability{ - DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "ds1")()), - RequiredCapability: util.StrPtr("bogus"), - }, - Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusNotFound)), - }, - }, - } - - for method, testCases := range methodTests { - t.Run(method, func(t *testing.T) { - for name, testCase := range testCases { - switch method { - case "GET": - t.Run(name, func(t *testing.T) { - resp, reqInf, err := testCase.ClientSession.GetDeliveryServicesRequiredCapabilities(testCase.RequestOpts) - for _, check := range testCase.Expectations { - check(t, reqInf, resp.Response, resp.Alerts, err) - } - }) - case "POST": - t.Run(name, func(t *testing.T) { - alerts, reqInf, err := testCase.ClientSession.CreateDeliveryServicesRequiredCapability(testCase.RequestBody, testCase.RequestOpts) - for _, check := range testCase.Expectations { - check(t, reqInf, nil, alerts, err) - } - }) - case "DELETE": - t.Run(name, func(t *testing.T) { - alerts, reqInf, err := testCase.ClientSession.DeleteDeliveryServicesRequiredCapability(*testCase.RequestBody.DeliveryServiceID, *testCase.RequestBody.RequiredCapability, testCase.RequestOpts) - for _, check := range testCase.Expectations { - check(t, reqInf, nil, alerts, err) - } - }) - } - } - }) - } - }) - -} +//func TestDeliveryServicesRequiredCapabilities(t *testing.T) { +// WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, ServerCapabilities, Topologies, ServiceCategories, DeliveryServices, DeliveryServiceServerAssignments, ServerServerCapabilities, DeliveryServicesRequiredCapabilities}, func() { +// +// currentTime := time.Now().UTC().Add(-15 * time.Second) +// currentTimeRFC := currentTime.Format(time.RFC1123) +// tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123) +// +// methodTests := utils.TestCase[client.Session, client.RequestOptions, tc.DeliveryServicesRequiredCapability]{ +// "GET": { +// "NOT MODIFIED when NO CHANGES made": { +// ClientSession: TOSession, +// RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfModifiedSince: {tomorrow}}}, +// Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusNotModified)), +// }, +// "OK when VALID request": { +// ClientSession: TOSession, +// Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)), +// }, +// "OK when VALID DELIVERYSERVICEID parameter": { +// ClientSession: TOSession, +// RequestOpts: client.RequestOptions{QueryParameters: url.Values{"deliveryServiceId": {strconv.Itoa(GetDeliveryServiceId(t, "ds1")())}}}, +// Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseLengthGreaterOrEqual(1), +// validateDSRCExpectedFields(map[string]interface{}{"DeliveryServiceId": "ds1"})), +// }, +// "OK when VALID XMLID parameter": { +// ClientSession: TOSession, +// RequestOpts: client.RequestOptions{QueryParameters: url.Values{"xmlID": {"ds2"}}}, +// Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseLengthGreaterOrEqual(1), +// validateDSRCExpectedFields(map[string]interface{}{"XMLID": "ds2"})), +// }, +// "OK when VALID REQUIREDCAPABILITY parameter": { +// ClientSession: TOSession, +// RequestOpts: client.RequestOptions{QueryParameters: url.Values{"requiredCapability": {"bar"}}}, +// Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), utils.ResponseLengthGreaterOrEqual(1), +// validateDSRCExpectedFields(map[string]interface{}{"RequiredCapability": "bar"})), +// }, +// "FIRST RESULT when LIMIT=1": { +// ClientSession: TOSession, +// RequestOpts: client.RequestOptions{QueryParameters: url.Values{"orderby": {"requiredCapability"}, "limit": {"1"}}}, +// Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateDSRCPagination("limit")), +// }, +// "SECOND RESULT when LIMIT=1 OFFSET=1": { +// ClientSession: TOSession, +// RequestOpts: client.RequestOptions{QueryParameters: url.Values{"orderby": {"requiredCapability"}, "limit": {"1"}, "offset": {"1"}}}, +// Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateDSRCPagination("offset")), +// }, +// "SECOND RESULT when LIMIT=1 PAGE=2": { +// ClientSession: TOSession, +// RequestOpts: client.RequestOptions{QueryParameters: url.Values{"orderby": {"requiredCapability"}, "limit": {"1"}, "page": {"2"}}}, +// Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateDSRCPagination("page")), +// }, +// "BAD REQUEST when INVALID LIMIT parameter": { +// ClientSession: TOSession, +// RequestOpts: client.RequestOptions{QueryParameters: url.Values{"limit": {"-2"}}}, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), +// }, +// "BAD REQUEST when INVALID OFFSET parameter": { +// ClientSession: TOSession, +// RequestOpts: client.RequestOptions{QueryParameters: url.Values{"limit": {"1"}, "offset": {"0"}}}, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), +// }, +// "BAD REQUEST when INVALID PAGE parameter": { +// ClientSession: TOSession, +// RequestOpts: client.RequestOptions{QueryParameters: url.Values{"limit": {"1"}, "page": {"0"}}}, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), +// }, +// "OK when CHANGES made": { +// ClientSession: TOSession, +// RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfModifiedSince: {currentTimeRFC}}}, +// Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)), +// }, +// }, +// "POST": { +// "BAD REQUEST when REASSIGNING REQUIRED CAPABILITY to DELIVERY SERVICE": { +// ClientSession: TOSession, +// RequestBody: tc.DeliveryServicesRequiredCapability{ +// DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "ds1")()), +// RequiredCapability: util.StrPtr("foo"), +// }, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), +// }, +// "BAD REQUEST when SERVERS DONT have CAPABILITY": { +// ClientSession: TOSession, +// RequestBody: tc.DeliveryServicesRequiredCapability{ +// DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "test-ds-server-assignments")()), +// RequiredCapability: util.StrPtr("disk"), +// }, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), +// }, +// "BAD REQUEST when DELIVERY SERVICE HAS TOPOLOGY where SERVERS DONT have CAPABILITY": { +// ClientSession: TOSession, +// RequestBody: tc.DeliveryServicesRequiredCapability{ +// DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "ds-top-req-cap")()), +// RequiredCapability: util.StrPtr("bar"), +// }, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), +// }, +// "BAD REQUEST when DELIVERY SERVICE ID EMPTY": { +// ClientSession: TOSession, +// RequestBody: tc.DeliveryServicesRequiredCapability{ +// RequiredCapability: util.StrPtr("bar"), +// }, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), +// }, +// "BAD REQUEST when REQUIRED CAPABILITY EMPTY": { +// ClientSession: TOSession, +// RequestBody: tc.DeliveryServicesRequiredCapability{ +// DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "ds1")()), +// }, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), +// }, +// "NOT FOUND when NON-EXISTENT REQUIRED CAPABILITY": { +// ClientSession: TOSession, +// RequestBody: tc.DeliveryServicesRequiredCapability{ +// DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "ds1")()), +// RequiredCapability: util.StrPtr("bogus"), +// }, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusNotFound)), +// }, +// "NOT FOUND when NON-EXISTENT DELIVERY SERVICE ID": { +// ClientSession: TOSession, +// RequestBody: tc.DeliveryServicesRequiredCapability{ +// DeliveryServiceID: util.IntPtr(-1), +// RequiredCapability: util.StrPtr("foo"), +// }, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusNotFound)), +// }, +// "BAD REQUEST when INVALID DELIVERY SERVICE TYPE": { +// ClientSession: TOSession, +// RequestBody: tc.DeliveryServicesRequiredCapability{ +// DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "anymap-ds")()), +// RequiredCapability: util.StrPtr("foo"), +// }, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), +// }, +// }, +// "DELETE": { +// "OK when VALID request": { +// ClientSession: TOSession, +// RequestBody: tc.DeliveryServicesRequiredCapability{ +// DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "ds-top-req-cap")()), +// RequiredCapability: util.StrPtr("ram"), +// }, +// Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK)), +// }, +// "NOT FOUND when NON-EXISTENT DELIVERYSERVICEID parameter": { +// ClientSession: TOSession, +// RequestBody: tc.DeliveryServicesRequiredCapability{ +// DeliveryServiceID: util.IntPtr(-1), +// RequiredCapability: util.StrPtr("foo"), +// }, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusNotFound)), +// }, +// "NOT FOUND when NON-EXISTENT REQUIREDCAPABILITY parameter": { +// ClientSession: TOSession, +// RequestBody: tc.DeliveryServicesRequiredCapability{ +// DeliveryServiceID: util.IntPtr(GetDeliveryServiceId(t, "ds1")()), +// RequiredCapability: util.StrPtr("bogus"), +// }, +// Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusNotFound)), +// }, +// }, +// } +// +// for method, testCases := range methodTests { +// t.Run(method, func(t *testing.T) { +// for name, testCase := range testCases { +// switch method { +// case "GET": +// t.Run(name, func(t *testing.T) { +// resp, reqInf, err := testCase.ClientSession.GetDeliveryServicesRequiredCapabilities(testCase.RequestOpts) +// for _, check := range testCase.Expectations { +// check(t, reqInf, resp.Response, resp.Alerts, err) +// } +// }) +// case "POST": +// t.Run(name, func(t *testing.T) { +// alerts, reqInf, err := testCase.ClientSession.CreateDeliveryServicesRequiredCapability(testCase.RequestBody, testCase.RequestOpts) +// for _, check := range testCase.Expectations { +// check(t, reqInf, nil, alerts, err) +// } +// }) +// case "DELETE": +// t.Run(name, func(t *testing.T) { +// alerts, reqInf, err := testCase.ClientSession.DeleteDeliveryServicesRequiredCapability(*testCase.RequestBody.DeliveryServiceID, *testCase.RequestBody.RequiredCapability, testCase.RequestOpts) +// for _, check := range testCase.Expectations { +// check(t, reqInf, nil, alerts, err) +// } +// }) +// } +// } +// }) +// } +// }) +// +//} func validateDSRCExpectedFields(expectedResp map[string]interface{}) utils.CkReqFunc { return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) { @@ -276,18 +270,24 @@ func CreateTestDeliveryServicesRequiredCapabilities(t *testing.T) { DeliveryServiceID: &dsId, RequiredCapability: dsrc.RequiredCapability, } - resp, _, err := TOSession.CreateDeliveryServicesRequiredCapability(dsrc, client.RequestOptions{}) + opts := client.NewRequestOptions() + opts.QueryParameters.Set("id", fmt.Sprint(dsId)) + resp, _, err := TOSession.GetDeliveryServices(opts) + assert.NoError(t, err, "Error getting delivery services: %v - alerts: %v", err, resp.Alerts) + assert.Equal(t, len(resp.Response), 1, "Expected response to have exactly 1 delivery service, but got %d", len(resp.Response)) + ds := resp.Response[0] + ds.RequiredCapabilities = []string{*dsrc.RequiredCapability} + _, _, err = TOSession.UpdateDeliveryService(dsId, ds, client.NewRequestOptions()) assert.NoError(t, err, "Unexpected error creating a Delivery Service/Required Capability relationship: %v - alerts: %+v", err, resp.Alerts) } } func DeleteTestDeliveryServicesRequiredCapabilities(t *testing.T) { - // Get Required Capabilities to delete them - dsrcs, _, err := TOSession.GetDeliveryServicesRequiredCapabilities(client.RequestOptions{}) - assert.NoError(t, err, "Error getting Delivery Service/Required Capability relationships: %v - alerts: %+v", err, dsrcs.Alerts) - - for _, dsrc := range dsrcs.Response { - alerts, _, err := TOSession.DeleteDeliveryServicesRequiredCapability(*dsrc.DeliveryServiceID, *dsrc.RequiredCapability, client.RequestOptions{}) - assert.NoError(t, err, "Error deleting a relationship between a Delivery Service and a Capability: %v - alerts: %+v", err, alerts.Alerts) + resp, _, err := TOSession.GetDeliveryServices(client.RequestOptions{}) + assert.NoError(t, err, "Error getting delivery services: %v - alerts: %v", err, resp.Alerts) + for _, r := range resp.Response { + r.RequiredCapabilities = []string{} + response, _, err := TOSession.UpdateDeliveryService(*r.ID, r, client.RequestOptions{}) + assert.NoError(t, err, "Error removing Delivery Service/ Required Capability relationship: %v - alerts: %v", err, response.Alerts) } } diff --git a/traffic_ops/testing/api/v5/deliveryservices_test.go b/traffic_ops/testing/api/v5/deliveryservices_test.go index 19ea86e240..3b9f574057 100644 --- a/traffic_ops/testing/api/v5/deliveryservices_test.go +++ b/traffic_ops/testing/api/v5/deliveryservices_test.go @@ -32,7 +32,7 @@ import ( ) func TestDeliveryServices(t *testing.T) { - WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, ServerCapabilities, ServiceCategories, DeliveryServices, ServerServerCapabilities, DeliveryServicesRequiredCapabilities, DeliveryServiceServerAssignments}, func() { + WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, ServerCapabilities, ServiceCategories, DeliveryServices, ServerServerCapabilities, DeliveryServiceServerAssignments}, func() { currentTime := time.Now().UTC().Add(-15 * time.Second) currentTimeRFC := currentTime.Format(time.RFC1123) diff --git a/traffic_ops/testing/api/v5/deliveryserviceservers_test.go b/traffic_ops/testing/api/v5/deliveryserviceservers_test.go index 7b78249824..8feb52d947 100644 --- a/traffic_ops/testing/api/v5/deliveryserviceservers_test.go +++ b/traffic_ops/testing/api/v5/deliveryserviceservers_test.go @@ -31,7 +31,7 @@ import ( ) func TestDeliveryServiceServers(t *testing.T) { - WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, ServiceCategories, ServerCapabilities, DeliveryServices, DeliveryServiceServerAssignments, ServerServerCapabilities, DeliveryServicesRequiredCapabilities}, func() { + WithObjs(t, []TCObj{CDNs, Types, Tenants, Parameters, Profiles, Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies, ServiceCategories, ServerCapabilities, DeliveryServices, DeliveryServiceServerAssignments, ServerServerCapabilities}, func() { tomorrow := time.Now().UTC().AddDate(0, 0, 1).Format(time.RFC1123) diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go index 0486219b48..d9c14b851e 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go @@ -1605,8 +1605,14 @@ func Validate(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { } func validateRequiredCapabilities(tx *sql.Tx, ds *tc.DeliveryServiceV5) (error, error) { - var valid bool - query := `SELECT $1 <@ (SELECT ARRAY_AGG(name) FROM server_capability)` + missing := make([]string, 0) + var missingCap string + query := `SELECT missing +FROM ( + SELECT UNNEST($1::TEXT[]) + EXCEPT + SELECT UNNEST(ARRAY_AGG(name)) FROM server_capability) t(missing) +` if len(ds.RequiredCapabilities) > 0 { rows, err := tx.Query(query, pq.Array(ds.RequiredCapabilities)) if err != nil { @@ -1614,13 +1620,19 @@ func validateRequiredCapabilities(tx *sql.Tx, ds *tc.DeliveryServiceV5) (error, } defer rows.Close() for rows.Next() { - err = rows.Scan(&valid) + err = rows.Scan(&missingCap) if err != nil { return nil, err } - if !valid { - return errors.New("one or more of the required capabilities do not exist"), nil + missing = append(missing, missingCap) + } + if len(missing) > 0 { + var msg string + for _, m := range missing { + msg = msg + m + " " } + userErr := fmt.Errorf("the following capabilities do not exist: %s", msg) + return userErr, nil } } return nil, nil diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go index b4515a0ef9..ff4e4b0dcb 100644 --- a/traffic_ops/traffic_ops_golang/routing/routes.go +++ b/traffic_ops/traffic_ops_golang/routing/routes.go @@ -396,11 +396,6 @@ func Routes(d ServerData) ([]Route, http.Handler, error) { {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPut, Path: `deliveryservices/{xmlID}/urisignkeys$`, Handler: urisigning.SaveDeliveryServiceURIKeysHandler, RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"DS-SECURITY-KEY:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 4764896931}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodDelete, Path: `deliveryservices/{xmlID}/urisignkeys$`, Handler: urisigning.RemoveDeliveryServiceURIKeysHandler, RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"DS-SECURITY-KEY:DELETE", "DS-SECURITY-KEY:READ", "DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 42992541731}, - //Delivery Service Required Capabilities: CRUD - {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `deliveryservices_required_capabilities/?$`, Handler: api.ReadHandler(&deliveryservice.RequiredCapability{}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 415852222731}, - {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPost, Path: `deliveryservices_required_capabilities/?$`, Handler: api.CreateHandler(&deliveryservice.RequiredCapability{}), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 409687399231}, - {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodDelete, Path: `deliveryservices_required_capabilities/?$`, Handler: api.DeleteHandler(&deliveryservice.RequiredCapability{}), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 449628930431}, - // Federations by CDN (the actual table for federation) {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `cdns/{name}/federations/?$`, Handler: api.ReadHandler(&cdnfederation.TOCDNFederation{}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"CDN:READ", "FEDERATION:READ", "DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 48922503231}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPost, Path: `cdns/{name}/federations/?$`, Handler: api.CreateHandler(&cdnfederation.TOCDNFederation{}), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:CREATE", "FEDERATION:READ, CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 495489421931}, From 8507899dd1a638b9e5147d40355da0307d90f51d Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Wed, 7 Dec 2022 10:33:33 -0700 Subject: [PATCH 05/10] fix unit test --- lib/go-tc/deliveryservices.go | 6 +++++- lib/go-tc/deliveryservices_test.go | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/go-tc/deliveryservices.go b/lib/go-tc/deliveryservices.go index 59824d170e..673ad05641 100644 --- a/lib/go-tc/deliveryservices.go +++ b/lib/go-tc/deliveryservices.go @@ -1632,7 +1632,11 @@ func (ds DeliveryServiceV5) Downgrade() DeliveryServiceV4 { copy(*downgraded.MatchList, ds.MatchList) } copy(downgraded.TLSVersions, ds.TLSVersions) - + if len(ds.RequiredCapabilities) == 0 { + downgraded.RequiredCapabilities = make([]string, 0) + } else { + copy(downgraded.RequiredCapabilities, ds.RequiredCapabilities) + } return downgraded } diff --git a/lib/go-tc/deliveryservices_test.go b/lib/go-tc/deliveryservices_test.go index 03f6e05721..eda06c9ef3 100644 --- a/lib/go-tc/deliveryservices_test.go +++ b/lib/go-tc/deliveryservices_test.go @@ -473,6 +473,7 @@ func dsUpgradeAndDowngradeTestingPair() (DeliveryServiceNullableV30, DeliverySer typ := DSTypeDNS typeID := 22 xmlid := "xmlid" + requiredCapabilities := make([]string, 0) newDS := DeliveryServiceV4{} newDS.Active = new(bool) @@ -547,6 +548,7 @@ func dsUpgradeAndDowngradeTestingPair() (DeliveryServiceNullableV30, DeliverySer newDS.Type = &typ newDS.TypeID = &typeID newDS.XMLID = &xmlid + newDS.RequiredCapabilities = requiredCapabilities active := false oldDS := DeliveryServiceNullableV30{ From 6b3bea5f2fa0d8ed2a7a02fc8827f7dcc8f1b1dd Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Wed, 7 Dec 2022 12:38:56 -0700 Subject: [PATCH 06/10] add allocation --- lib/go-tc/deliveryservices.go | 2 ++ lib/go-tc/deliveryservices_test.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/go-tc/deliveryservices.go b/lib/go-tc/deliveryservices.go index 673ad05641..d95ac781fb 100644 --- a/lib/go-tc/deliveryservices.go +++ b/lib/go-tc/deliveryservices.go @@ -1635,6 +1635,7 @@ func (ds DeliveryServiceV5) Downgrade() DeliveryServiceV4 { if len(ds.RequiredCapabilities) == 0 { downgraded.RequiredCapabilities = make([]string, 0) } else { + downgraded.RequiredCapabilities = make([]string, len(ds.RequiredCapabilities)) copy(downgraded.RequiredCapabilities, ds.RequiredCapabilities) } return downgraded @@ -1738,6 +1739,7 @@ func (ds DeliveryServiceV4) Upgrade() DeliveryServiceV5 { if len(ds.RequiredCapabilities) == 0 { upgraded.RequiredCapabilities = make([]string, 0) } else { + upgraded.RequiredCapabilities = make([]string, len(ds.RequiredCapabilities)) copy(upgraded.RequiredCapabilities, ds.RequiredCapabilities) } diff --git a/lib/go-tc/deliveryservices_test.go b/lib/go-tc/deliveryservices_test.go index eda06c9ef3..51d491cd1d 100644 --- a/lib/go-tc/deliveryservices_test.go +++ b/lib/go-tc/deliveryservices_test.go @@ -473,7 +473,7 @@ func dsUpgradeAndDowngradeTestingPair() (DeliveryServiceNullableV30, DeliverySer typ := DSTypeDNS typeID := 22 xmlid := "xmlid" - requiredCapabilities := make([]string, 0) + requiredCapabilities := []string{"foo"} newDS := DeliveryServiceV4{} newDS.Active = new(bool) From e526e637009c8154138a26e22b1be12c58bd5114 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Tue, 13 Dec 2022 21:45:34 -0700 Subject: [PATCH 07/10] code review --- docs/source/api/v4/deliveryservices.rst | 6 +- docs/source/api/v4/deliveryservices_id.rst | 4 +- ...deliveryservices_required_capabilities.rst | 2 +- .../api/v4/servers_id_deliveryservices.rst | 2 +- ...deliveryservices_required_capabilities.rst | 227 ------------------ lib/go-tc/deliveryservice_requests.go | 6 +- lib/go-tc/deliveryservices.go | 12 +- ...300_add_required_capabilities_to_ds.up.sql | 9 +- .../dbhelpers/db_helpers.go | 2 +- 9 files changed, 16 insertions(+), 254 deletions(-) delete mode 100644 docs/source/api/v5/deliveryservices_required_capabilities.rst diff --git a/docs/source/api/v4/deliveryservices.rst b/docs/source/api/v4/deliveryservices.rst index d16ce62fe5..9f98b62155 100644 --- a/docs/source/api/v4/deliveryservices.rst +++ b/docs/source/api/v4/deliveryservices.rst @@ -148,7 +148,7 @@ Response Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` -:requiredCapabilities: An array of the capabilities that this delivery service requires. +:requiredCapabilities: An array of the capabilities that this Delivery Service requires. .. versionadded:: 4.1 @@ -333,7 +333,7 @@ Request Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` -:requiredCapabilities: An array of the capabilities that this delivery service requires. +:requiredCapabilities: An array of the capabilities that this Delivery Service requires. .. versionadded:: 4.1 @@ -505,7 +505,7 @@ Response Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` -:requiredCapabilities: An array of the capabilities that this delivery service requires. +:requiredCapabilities: An array of the capabilities that this Delivery Service requires. .. versionadded:: 4.1 diff --git a/docs/source/api/v4/deliveryservices_id.rst b/docs/source/api/v4/deliveryservices_id.rst index 9fa17d22c4..0865427028 100644 --- a/docs/source/api/v4/deliveryservices_id.rst +++ b/docs/source/api/v4/deliveryservices_id.rst @@ -81,7 +81,7 @@ Request Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` -:requiredCapabilities: An array of the capabilities that this delivery service requires. +:requiredCapabilities: An array of the capabilities that this Delivery Service requires. .. versionadded:: 4.1 @@ -254,7 +254,7 @@ Response Structure :regexRemap: A :ref:`ds-regex-remap` :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` -:requiredCapabilities: An array of the capabilities that this delivery service requires. +:requiredCapabilities: An array of the capabilities that this Delivery Service requires. .. versionadded:: 4.1 diff --git a/docs/source/api/v4/deliveryservices_required_capabilities.rst b/docs/source/api/v4/deliveryservices_required_capabilities.rst index d010dfe977..f3a2dd9fd7 100644 --- a/docs/source/api/v4/deliveryservices_required_capabilities.rst +++ b/docs/source/api/v4/deliveryservices_required_capabilities.rst @@ -20,7 +20,7 @@ ****************************************** .. deprecated:: 4.1 - This endpoint will be removed in a future release, in favor of ``required_capabilities`` being a part of :term:`Delivery Services`. + This endpoint will be removed in a future release, in favor of :ref:`ds-required-capabilities` being a part of :term:`Delivery Services`. ``GET`` ======= diff --git a/docs/source/api/v4/servers_id_deliveryservices.rst b/docs/source/api/v4/servers_id_deliveryservices.rst index 116342e44e..9b5d6adb3e 100644 --- a/docs/source/api/v4/servers_id_deliveryservices.rst +++ b/docs/source/api/v4/servers_id_deliveryservices.rst @@ -134,7 +134,7 @@ Response Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` -:requiredCapabilities: An array of the capabilities that this delivery service requires. +:requiredCapabilities: An array of the capabilities that this Delivery Service requires. .. versionadded:: 4.1 diff --git a/docs/source/api/v5/deliveryservices_required_capabilities.rst b/docs/source/api/v5/deliveryservices_required_capabilities.rst deleted file mode 100644 index 6761728b5f..0000000000 --- a/docs/source/api/v5/deliveryservices_required_capabilities.rst +++ /dev/null @@ -1,227 +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-deliveryservices-required-capabilities: - -****************************************** -``deliveryservices_required_capabilities`` -****************************************** - -``GET`` -======= -Gets all associations of :term:`Server Capability` to :term:`Delivery Services`. - -:Auth. Required: Yes -:Roles Required: None -:Permissions Required: DELIVERY-SERVICE:READ -:Response Type: Array - -Request Structure ------------------ -.. table:: Request Query Parameters - - +--------------------+----------+---------------------------------------------------------------------------------------------------------------+ - | Name | Required | Description | - +====================+==========+===============================================================================================================+ - | deliveryServiceID | no | Filter :term:`Server Capability` associations by :term:`Delivery Service` integral, unique identifier | - +--------------------+----------+---------------------------------------------------------------------------------------------------------------+ - | xmlID | no | Filter :term:`Server Capability` associations by :term:`Delivery Service` :ref:`ds-xmlid` | - +--------------------+----------+---------------------------------------------------------------------------------------------------------------+ - | requiredCapability | no | Filter :term:`Server Capability` associations by :term:`Server Capability` name | - +--------------------+----------+---------------------------------------------------------------------------------------------------------------+ - | orderby | no | Choose the ordering of the results - must be the name of one of the fields of the objects in the ``response`` | - | | | array | - +--------------------+----------+---------------------------------------------------------------------------------------------------------------+ - | sortOrder | no | Changes the order of sorting. Either ascending (default or "asc") or descending ("desc") | - +--------------------+----------+---------------------------------------------------------------------------------------------------------------+ - | limit | no | Choose the maximum number of results to return | - +--------------------+----------+---------------------------------------------------------------------------------------------------------------+ - | offset | no | The number of results to skip before beginning to return results. Must use in conjunction with limit. | - +--------------------+----------+---------------------------------------------------------------------------------------------------------------+ - | page | no | Return the n\ :sup:`th` page of results, where "n" is the value of this parameter, pages are ``limit`` long | - | | | and the first page is 1. If ``offset`` was defined, this query parameter has no effect. ``limit`` must be | - | | | defined to make use of ``page``. | - +--------------------+----------+---------------------------------------------------------------------------------------------------------------+ - -.. code-block:: http - :caption: Request Example - - GET /api/5.0/deliveryservices_required_capabilities HTTP/1.1 - Host: trafficops.infra.ciab.test - User-Agent: curl/7.47.0 - Accept: */* - Cookie: mojolicious=... - -Response Structure ------------------- -:deliveryServiceID: The associated :term:`Delivery Service`'s integral, unique identifier -:xmlID: The associated :term:`Delivery Service`'s :ref:`ds-xmlid` -:lastUpdated: The date and time at which this association between the :term:`Delivery Service` and the :term:`Server Capability` was last updated, in :ref:`non-rfc-datetime` -:requiredCapability: The :term:`Server Capability`'s 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,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: UFO3/jcBFmFZM7CsrsIwTfPc5v8gUiXqJm6BNp1boPb4EQBnWNXZh/DbBwhMAOJoeqDImoDlrLnrVjQGO4AooA== - X-Server-Name: traffic_ops_golang/ - Date: Mon, 07 Oct 2019 22:15:11 GMT - Content-Length: 396 - - { - "response": [ - { - "deliveryServiceID": 1, - "lastUpdated": "2019-10-07 22:05:31+00", - "requiredCapability": "ram", - "xmlId": "example_ds-1" - }, - { - "deliveryServiceID": 2, - "lastUpdated": "2019-10-07 22:05:31+00", - "requiredCapability": "disk", - "xmlId": "example_ds-2" - } - ] - } - -``POST`` -======== -Associates a :term:`Server Capability` with a :term:`Delivery Service`. - -:Auth. Required: Yes -:Roles Required: "admin" or "operations" -:Permissions Required: DELIVERY-SERVICE:READ, DELIVERY-SERVICE:UPDATE -:Response Type: Object - -.. note:: A :term:`Server Capability` can only be made required on a :term:`Delivery Service` if its associated Servers already have that :term:`Server Capability` assigned. - -Request Structure ------------------ -:deliveryServiceID: The integral, unique identifier of the :term:`Delivery Service` to be associated -:requiredCapability: The name of the :term:`Server Capability` to be associated - -.. code-block:: http - :caption: Request Example - - POST /api/5.0/deliveryservices_required_capabilities HTTP/1.1 - Host: trafficops.infra.ciab.test - User-Agent: curl/7.47.0 - Accept: */* - Cookie: mojolicious=... - Content-Length: 56 - Content-Type: application/json - - { - "deliveryServiceID": 1, - "requiredCapability": "disk" - } - -Response Structure ------------------- -:deliveryServiceID: The newly associated :term:`Delivery Service`'s integral, unique identifier -:lastUpdated: The date and time at which this association between the :term:`Delivery Service` and the :term:`Server Capability` was last updated, in :ref:`non-rfc-datetime` -:requiredCapability: The newly associated :term:`Server Capability`'s 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,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: eQrl48zWids0kDpfCYmmtYMpegjnFxfOVvlBYxxLSfp7P7p6oWX4uiC+/Cfh2X9i3G+MQ36eH95gukJqOBOGbQ== - X-Server-Name: traffic_ops_golang/ - Date: Mon, 07 Oct 2019 22:15:11 GMT - Content-Length: 287 - - { - "alerts": [ - { - "level": "success", - "text": "deliveryservice.RequiredCapability was created." - } - ], - "response": { - "deliveryServiceID": 1, - "lastUpdated": "2019-10-07 22:15:11+00", - "requiredCapability": "disk" - } - } - -``DELETE`` -========== -Dissociate a :term:`Server Capability` from a :term:`Delivery Service`. - -:Auth. Required: Yes -:Roles Required: "admin" or "operations" -:Permissions Required: DELIVERY-SERVICE:READ, DELIVERY-SERVICE:UPDATE -:Response Type: ``undefined`` - -Request Structure ------------------ -:deliveryServiceID: The integral, unique identifier of the :term:`Delivery Service` from which a :term:`Server Capability` will be dissociated -:requiredCapability: The name of the :term:`Server Capability` to dissociate - -.. code-block:: http - :caption: Request Example - - POST /api/5.0/deliveryservices_required_capabilities HTTP/1.1 - Host: trafficops.infra.ciab.test - User-Agent: curl/7.47.0 - Accept: */* - Cookie: mojolicious=... - Content-Length: 56 - Content-Type: application/json - - { - "deliveryServiceID": 1, - "requiredCapability": "disk" - } - -Response Structure ------------------- -.. 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,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: eQrl48zWids0kDpfCYmmtYMpegjnFxfOVvlBYxxLSfp7P7p6oWX4uiC+/Cfh2X9i3G+MQ36eH95gukJqOBOGbQ== - X-Server-Name: traffic_ops_golang/ - Date: Mon, 07 Oct 2019 22:15:11 GMT - Content-Length: 127 - - { - "alerts": [ - { - "level": "success", - "text": "deliveryservice.RequiredCapability was deleted." - } - ] - } diff --git a/lib/go-tc/deliveryservice_requests.go b/lib/go-tc/deliveryservice_requests.go index 46524ce3db..d787a90072 100644 --- a/lib/go-tc/deliveryservice_requests.go +++ b/lib/go-tc/deliveryservice_requests.go @@ -435,7 +435,8 @@ type DeliveryServiceRequestNullable struct { XMLID *string `json:"-" db:"xml_id"` } -// Downgrade will convert an instance of DeliveryServiceRequestV41 to DeliveryServiceRequestV40 +// Downgrade will convert an instance of DeliveryServiceRequestV41 to DeliveryServiceRequestV40. +// Note that this function does a shallow copy of the requested and original Delivery Service structures. func (dsr DeliveryServiceRequestV41) Downgrade() DeliveryServiceRequestV40 { var dsrV40 DeliveryServiceRequestV40 dsrV40.Assignee = copyStringIfNotNil(dsr.Assignee) @@ -461,7 +462,8 @@ func (dsr DeliveryServiceRequestV41) Downgrade() DeliveryServiceRequestV40 { return dsrV40 } -// Upgrade will convert an instance of DeliveryServiceRequestV40 to DeliveryServiceRequestV41 +// Upgrade will convert an instance of DeliveryServiceRequestV40 to DeliveryServiceRequestV41. +// Note that this function does a shallow copy of the requested and original Delivery Service structures. func (dsrV40 DeliveryServiceRequestV40) Upgrade() DeliveryServiceRequestV41 { var dsrV4 DeliveryServiceRequestV41 dsrV4.Assignee = copyStringIfNotNil(dsrV40.Assignee) diff --git a/lib/go-tc/deliveryservices.go b/lib/go-tc/deliveryservices.go index d95ac781fb..daf61e8f9b 100644 --- a/lib/go-tc/deliveryservices.go +++ b/lib/go-tc/deliveryservices.go @@ -1612,7 +1612,7 @@ func (ds DeliveryServiceV5) Downgrade() DeliveryServiceV4 { TLSVersions: make([]string, len(ds.TLSVersions)), }, Regional: ds.Regional, - RequiredCapabilities: ds.RequiredCapabilities, + RequiredCapabilities: make([]string, len(ds.RequiredCapabilities)), } *downgraded.Active = ds.Active == DSActiveStateActive @@ -1632,10 +1632,7 @@ func (ds DeliveryServiceV5) Downgrade() DeliveryServiceV4 { copy(*downgraded.MatchList, ds.MatchList) } copy(downgraded.TLSVersions, ds.TLSVersions) - if len(ds.RequiredCapabilities) == 0 { - downgraded.RequiredCapabilities = make([]string, 0) - } else { - downgraded.RequiredCapabilities = make([]string, len(ds.RequiredCapabilities)) + if len(ds.RequiredCapabilities) > 0 { copy(downgraded.RequiredCapabilities, ds.RequiredCapabilities) } return downgraded @@ -1736,10 +1733,7 @@ func (ds DeliveryServiceV4) Upgrade() DeliveryServiceV5 { copy(upgraded.MatchList, *ds.MatchList) } copy(upgraded.TLSVersions, ds.TLSVersions) - if len(ds.RequiredCapabilities) == 0 { - upgraded.RequiredCapabilities = make([]string, 0) - } else { - upgraded.RequiredCapabilities = make([]string, len(ds.RequiredCapabilities)) + if len(ds.RequiredCapabilities) > 0 { copy(upgraded.RequiredCapabilities, ds.RequiredCapabilities) } diff --git a/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.up.sql b/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.up.sql index e92f060394..17c840f5a6 100644 --- a/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.up.sql +++ b/traffic_ops/app/db/migrations/2022112215022300_add_required_capabilities_to_ds.up.sql @@ -18,13 +18,6 @@ ALTER TABLE public.deliveryservice ADD COLUMN required_capabilities TEXT[]; -DO $$ -DECLARE temprow RECORD; -BEGIN FOR temprow IN -SELECT deliveryservice_id, ARRAY_AGG(required_capability) AS required_capabilities FROM deliveryservices_required_capability drc GROUP BY drc.deliveryservice_id - LOOP -UPDATE deliveryservice d SET required_capabilities = temprow.required_capabilities WHERE d.id = temprow.deliveryservice_id; -END LOOP; -END $$; +UPDATE public.deliveryservice d SET required_capabilities = (SELECT ARRAY_AGG(required_capability) AS required_capabilities FROM deliveryservices_required_capability drc WHERE drc.deliveryservice_id = d.id); DROP TABLE public.deliveryservices_required_capability; diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go index 09f9b30a36..d4e493fa7b 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go +++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go @@ -895,7 +895,7 @@ func DSRExistsWithXMLID(xmlid string, tx *sql.Tx) (bool, error) { return exists, err } -// ScanCachegroupsServerCapabilities : given rows of (server ID, CDN ID, cachegroup name, server capabilities), +// ScanCachegroupsServerCapabilities, given rows of (server ID, CDN ID, cachegroup name, server capabilities), // returns a map of cachegroup names to server IDs, a map of server IDs to a map of their capabilities, // a map of server IDs to CDN IDs, and an error (if one occurs). func ScanCachegroupsServerCapabilities(rows *sql.Rows) (map[string][]int, map[int]map[string]struct{}, map[int]int, error) { From 040833c6f1a63b32e0af9c9ac185741d74acdbf9 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Wed, 14 Dec 2022 12:36:26 -0700 Subject: [PATCH 08/10] code review --- .../deliveryservice/deliveryservices.go | 40 +++++++++---------- .../deliveryservice/request/requests.go | 18 +++++++-- .../deliveryservice/request/validate.go | 27 ++++++------- .../traffic_ops_golang/server/servers.go | 11 +---- 4 files changed, 49 insertions(+), 47 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go index d9c14b851e..1ead836a1c 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go @@ -377,12 +377,15 @@ func createV41(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, ds tc.D // error are returned. The status code SHOULD NOT be used, if both errors are // nil. func createV50(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, ds tc.DeliveryServiceV5, omitExtraLongDescFields bool, longDesc1, longDesc2 *string) (*tc.DeliveryServiceV5, int, error, error) { + var err error tx := inf.Tx.Tx - err := Validate(tx, &ds) - if err != nil { - return nil, http.StatusBadRequest, fmt.Errorf("invalid request: %w", err), nil + userErr, sysErr := Validate(tx, &ds) + if userErr != nil { + return nil, http.StatusBadRequest, fmt.Errorf("invalid request: %w", userErr), nil + } + if sysErr != nil { + return nil, http.StatusInternalServerError, nil, sysErr } - if authorized, err := isTenantAuthorized(inf, &ds); err != nil { return nil, http.StatusInternalServerError, nil, fmt.Errorf("checking tenant: %w", err) } else if !authorized { @@ -1000,8 +1003,12 @@ func updateV41(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV4 *t func updateV50(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, ds *tc.DeliveryServiceV5, omitExtraLongDescFields bool, longDesc1, longDesc2 *string) (*tc.DeliveryServiceV5, int, error, error) { tx := inf.Tx.Tx user := inf.User - if err := Validate(tx, ds); err != nil { - return nil, http.StatusBadRequest, fmt.Errorf("invalid request: %w", err), nil + userErr, sysErr := Validate(tx, ds) + if userErr != nil { + return nil, http.StatusBadRequest, fmt.Errorf("invalid request: %w", userErr), nil + } + if sysErr != nil { + return nil, http.StatusInternalServerError, nil, sysErr } if authorized, err := isTenantAuthorized(inf, ds); err != nil { @@ -1023,8 +1030,6 @@ func updateV50(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, ds *tc. } var errCode int - var userErr error - var sysErr error var oldDetails TODeliveryServiceOldDetails if dsType.HasSSLKeys() { oldDetails, userErr, sysErr, errCode = getOldDetails(*ds.ID, tx) @@ -1537,9 +1542,7 @@ var validTLSVersionPattern = regexp.MustCompile(`^\d+\.\d+$`) // providing default values to some properties when they are zero-valued or nil // references*. This will panic if either argument is nil. The error returned is // safe for clients to see. -// -// TODO: return system errors as well. -func Validate(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { +func Validate(tx *sql.Tx, ds *tc.DeliveryServiceV5) (error, error) { sanitize(ds) neverOrAlways := validation.NewStringRule(tovalidate.IsOneOfStringICase("NEVER", "ALWAYS"), "must be one of 'NEVER' or 'ALWAYS'") @@ -1592,16 +1595,16 @@ func Validate(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { errs = append(errs, fmt.Errorf("type fields: %w", err)) } userErr, sysErr := validateRequiredCapabilities(tx, ds) + if sysErr != nil { + return nil, fmt.Errorf("reading/ scanning required capabilities: %w", sysErr) + } if userErr != nil { errs = append(errs, errors.New("required capabilities: "+userErr.Error())) } - if sysErr != nil { - errs = append(errs, errors.New("reading/ scanning required capabilities: "+sysErr.Error())) - } if len(errs) == 0 { - return nil + return nil, nil } - return util.JoinErrs(errs) + return util.JoinErrs(errs), nil } func validateRequiredCapabilities(tx *sql.Tx, ds *tc.DeliveryServiceV5) (error, error) { @@ -1627,10 +1630,7 @@ FROM ( missing = append(missing, missingCap) } if len(missing) > 0 { - var msg string - for _, m := range missing { - msg = msg + m + " " - } + msg := strings.Join(missing, ",") userErr := fmt.Errorf("the following capabilities do not exist: %s", msg) return userErr, nil } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go index 79f16cfb69..533625aab9 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go @@ -559,8 +559,13 @@ func createLegacy(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (res api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil) return } - if err := validateLegacy(dsr, tx); err != nil { - api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil) + userErr, sysErr := validateLegacy(dsr, tx) + if sysErr != nil { + api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, sysErr) + return + } + if userErr != nil { + api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil) return } @@ -997,8 +1002,13 @@ func putLegacy(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil) return } - if err := validateLegacy(dsr, tx); err != nil { - api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil) + userErr, sysErr := validateLegacy(dsr, tx) + if sysErr != nil { + api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, sysErr) + return + } + if userErr != nil { + api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil) return } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/validate.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/validate.go index 47fb24265d..963543d617 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/request/validate.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/validate.go @@ -35,7 +35,7 @@ import ( // validateLegacy ensures all required fields are present and in correct form. // Also checks request JSON is complete and valid. -func validateLegacy(dsr tc.DeliveryServiceRequestNullable, tx *sql.Tx) error { +func validateLegacy(dsr tc.DeliveryServiceRequestNullable, tx *sql.Tx) (error, error) { if tx == nil { log.Errorln("validating a legacy Delivery Service Request: nil transaction was passed") } @@ -46,7 +46,7 @@ func validateLegacy(dsr tc.DeliveryServiceRequestNullable, tx *sql.Tx) error { if err != nil { log.Errorf("querying for dsr by ID %d: %v", *dsr.ID, err) - return errors.New("unknown error") + return errors.New("unknown error"), nil } } @@ -69,11 +69,13 @@ func validateLegacy(dsr tc.DeliveryServiceRequestNullable, tx *sql.Tx) error { errs := tovalidate.ToErrors(errMap) // ensure the deliveryservice requested is valid upgraded := dsr.DeliveryService.UpgradeToV4().Upgrade() - e := deliveryservice.Validate(tx, &upgraded) - - errs = append(errs, e) + userErr, sysErr := deliveryservice.Validate(tx, &upgraded) + if sysErr != nil { + return nil, sysErr + } + errs = append(errs, userErr) - return util.JoinErrs(errs) + return util.JoinErrs(errs), sysErr } // validateV4 validates a DSR, returning - in order - a user-facing error that @@ -85,6 +87,7 @@ func validateV4(dsr tc.DeliveryServiceRequestV4, tx *sql.Tx) (error, error) { // validateV4 validates a DSR, returning - in order - a user-facing error that // should be shown to the client, and a system error. func validateV5(dsr tc.DeliveryServiceRequestV5, tx *sql.Tx) (error, error) { + var userErr, sysErr error if tx == nil { return nil, errors.New("nil transaction") } @@ -125,11 +128,11 @@ func validateV5(dsr tc.DeliveryServiceRequestV5, tx *sql.Tx) (error, error) { if ds == nil { return fmt.Errorf("required for changeType='%s'", dsr.ChangeType) } - err := deliveryservice.Validate(tx, ds) - if err == nil { + userErr, sysErr = deliveryservice.Validate(tx, ds) + if userErr == nil && sysErr == nil { dsr.XMLID = ds.XMLID } - return err + return userErr }, )), validation.Field(&dsr.Original, validation.By( @@ -182,9 +185,5 @@ func validateV5(dsr tc.DeliveryServiceRequestV5, tx *sql.Tx) (error, error) { }, )), ) - if err != nil { - return err, nil - } - - return err, nil + return err, sysErr } diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go index d45413272c..a2d2461ce2 100644 --- a/traffic_ops/traffic_ops_golang/server/servers.go +++ b/traffic_ops/traffic_ops_golang/server/servers.go @@ -826,17 +826,10 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth } var joinSubQuery string - rows, err := tx.Query(deliveryservice.GetRequiredCapabilitiesQuery, dsID) - if err != nil { - err = fmt.Errorf("unable to get required capabilities for deliveryservice %d: %s", dsID, err) + if err := tx.QueryRow(deliveryservice.GetRequiredCapabilitiesQuery, dsID).Scan(pq.Array(&requiredCapabilities)); err != nil && err != sql.ErrNoRows { + err = fmt.Errorf("unable to get required capabilities for deliveryservice %d: %w", dsID, err) return nil, 0, nil, err, http.StatusInternalServerError, nil } - for rows.Next() { - if err = rows.Scan(pq.Array(&requiredCapabilities)); err != nil { - err = fmt.Errorf("unable to scan required capabilities for deliveryservice %d: %w", dsID, err) - return nil, 0, nil, err, http.StatusInternalServerError, nil - } - } if requiredCapabilities != nil && len(requiredCapabilities) > 0 { dsHasRequiredCapabilities = true } From 0683e1f77c827931c8657990f021bf3976e56839 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Wed, 14 Dec 2022 13:19:02 -0700 Subject: [PATCH 09/10] remove superfluous call to WriteHeader --- traffic_ops/traffic_ops_golang/api/shared_handlers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/traffic_ops/traffic_ops_golang/api/shared_handlers.go b/traffic_ops/traffic_ops_golang/api/shared_handlers.go index 782e7204ac..594b34a62c 100644 --- a/traffic_ops/traffic_ops_golang/api/shared_handlers.go +++ b/traffic_ops/traffic_ops_golang/api/shared_handlers.go @@ -476,10 +476,10 @@ func CreateHandler(creator Creator) http.HandlerFunc { creator, HandleErr, func(w http.ResponseWriter, r *http.Request, statusCode int, alerts tc.Alerts, results interface{}) { - w.WriteHeader(statusCode) if len(alerts.Alerts) > 0 { WriteAlertsObj(w, r, statusCode, alerts, results) } else { + w.WriteHeader(statusCode) WriteResp(w, r, results) } From 9b7a439acc19d422d749a29c03825a769b595463 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Wed, 14 Dec 2022 14:58:43 -0700 Subject: [PATCH 10/10] go fmt --- lib/go-tc/deliveryservice_requests.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/go-tc/deliveryservice_requests.go b/lib/go-tc/deliveryservice_requests.go index 6f8ba8fd00..817cabc0c3 100644 --- a/lib/go-tc/deliveryservice_requests.go +++ b/lib/go-tc/deliveryservice_requests.go @@ -1140,8 +1140,8 @@ func (dsr DeliveryServiceRequestV4) Upgrade() DeliveryServiceRequestV5 { if dsr.Requested != nil { upgraded.Requested = new(DeliveryServiceV5) *upgraded.Requested = dsr.Requested.Upgrade() - } - if dsr.Original != nil { + } + if dsr.Original != nil { upgraded.Original = new(DeliveryServiceV5) *upgraded.Original = dsr.Original.Upgrade() }