From 37f974b8cec56691950cb04a3aba952460fc8401 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Wed, 13 Sep 2023 17:06:40 -0600 Subject: [PATCH 01/20] Fix 304 Not Modified responses written with body instead of LastModified header --- traffic_ops/traffic_ops_golang/api/api.go | 7 ++++--- traffic_ops/traffic_ops_golang/api/api_test.go | 2 +- traffic_ops/traffic_ops_golang/server/servers.go | 7 ++----- traffic_ops/traffic_ops_golang/util/ims/ims.go | 7 ++++--- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/api/api.go b/traffic_ops/traffic_ops_golang/api/api.go index 94a37ff538..7c9c014442 100644 --- a/traffic_ops/traffic_ops_golang/api/api.go +++ b/traffic_ops/traffic_ops_golang/api/api.go @@ -720,13 +720,14 @@ func (inf APIInfo) WriteOKResponseWithSummary(resp any, count uint64) (int, erro } // WriteNotModifiedResponse writes a 304 Not Modified response with the given -// object as the 'response' property of the response body. +// time as the last modified time in the headers. // // This CANNOT be used by any APIInfo that wasn't constructed for the caller by // Wrap - ing a Handler (yet). -func (inf APIInfo) WriteNotModifiedResponse(resp any) (int, error, error) { +func (inf APIInfo) WriteNotModifiedResponse(lastModified time.Time) (int, error, error) { + inf.w.Header().Set(rfc.LastModified, FormatLastModified(lastModified)) inf.w.WriteHeader(http.StatusNotModified) - WriteResp(inf.w, inf.request, resp) + setRespWritten(inf.request) return http.StatusNotModified, nil, nil } diff --git a/traffic_ops/traffic_ops_golang/api/api_test.go b/traffic_ops/traffic_ops_golang/api/api_test.go index e852cdeaaa..56d8a5ce7c 100644 --- a/traffic_ops/traffic_ops_golang/api/api_test.go +++ b/traffic_ops/traffic_ops_golang/api/api_test.go @@ -321,7 +321,7 @@ func TestAPIInfo_WriteNotModifiedResponse(t *testing.T) { request: r, w: w, } - code, userErr, sysErr := inf.WriteNotModifiedResponse("test") + code, userErr, sysErr := inf.WriteNotModifiedResponse(time.Time{}) if code != http.StatusNotModified { t.Errorf("WriteNotModifiedResponse should return a %d %s code, got: %d %s", http.StatusNotModified, http.StatusText(http.StatusNotModified), code, http.StatusText(code)) } diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go index 718f2d1870..5601df03fb 100644 --- a/traffic_ops/traffic_ops_golang/server/servers.go +++ b/traffic_ops/traffic_ops_golang/server/servers.go @@ -674,11 +674,8 @@ func Read(inf *api.APIInfo) (int, error, error) { version := inf.Version servers, serverCount, userErr, sysErr, errCode, maxTime := getServers(inf.RequestHeaders(), inf.Params, inf.Tx, inf.User, useIMS, *version, inf.Config.RoleBasedPermissions) - if useIMS && maxTime != nil { - inf.SetLastModified(*maxTime) - } - if errCode == http.StatusNotModified { - return inf.WriteNotModifiedResponse([]tc.ServerV5{}) + if useIMS && maxTime != nil && errCode == http.StatusNotModified { + return inf.WriteNotModifiedResponse(*maxTime) } if userErr != nil || sysErr != nil { return errCode, userErr, sysErr diff --git a/traffic_ops/traffic_ops_golang/util/ims/ims.go b/traffic_ops/traffic_ops_golang/util/ims/ims.go index 53dbbbddbe..f53e76ec7c 100644 --- a/traffic_ops/traffic_ops_golang/util/ims/ims.go +++ b/traffic_ops/traffic_ops_golang/util/ims/ims.go @@ -2,6 +2,7 @@ package ims import ( "database/sql" + "errors" "net/http" "time" @@ -61,13 +62,13 @@ func TryIfModifiedSinceQuery(tx *sqlx.Tx, h http.Header, queryValues map[string] if rows != nil { defer rows.Close() } + if errors.Is(err, sql.ErrNoRows) { + return dontRunSecond, maxTime + } if err != nil { log.Errorf("Couldn't get the max last updated time: %v", err) return runSecond, maxTime } - if err == sql.ErrNoRows { - return dontRunSecond, maxTime - } // This should only ever contain one row if rows.Next() { v := &LatestTimestamp{} From 05b76a2c83d284a7175f3b489c491406e1e9de29 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Thu, 14 Sep 2023 11:20:22 -0600 Subject: [PATCH 02/20] add V5 structures --- lib/go-tc/federation.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/lib/go-tc/federation.go b/lib/go-tc/federation.go index 8447ed658b..b565f3fb97 100644 --- a/lib/go-tc/federation.go +++ b/lib/go-tc/federation.go @@ -24,6 +24,7 @@ import ( "errors" "fmt" "net" + "time" "github.com/apache/trafficcontrol/lib/go-util" @@ -69,6 +70,39 @@ type CDNFederation struct { *DeliveryServiceIDs `json:"deliveryService,omitempty"` } +// CDNFederationDeliveryService holds information about an assigned Delivery +// Service within a CDNFederationV5 structure. +type CDNFederationDeliveryService = struct { + ID int `json:"id" db:"ds_id"` + XMLID string `json:"xmlID" db:"xml_id"` +} + +// CDNFederationV5 represents a Federation of some CDN as it appears in version +// 5 of the Traffic Ops API. +type CDNFederationV5 struct { + ID int `json:"id" db:"id"` + CName string `json:"cname" db:"cname"` + TTL int `json:"ttl" db:"ttl"` + Description *string `json:"description" db:"description"` + LastUpdated time.Time `json:"lastUpdated" db:"last_updated"` + + DeliveryService *CDNFederationDeliveryService `json:"deliveryService,omitempty"` +} + +// CDNFederationsV5Response represents a Traffic Ops APIv5 response to a request +// for one or more of a CDN's Federations. +type CDNFederationsV5Response struct { + Response []CDNFederationV5 `json:"response"` + Alerts +} + +// CDNFederationV5Response represents a Traffic Ops APIv5 response to a request +// for a single CDN's Federations. +type CDNFederationV5Response struct { + Response CDNFederationV5 `json:"response"` + Alerts +} + // DeliveryServiceIDs are pairs of identifiers for Delivery Services. type DeliveryServiceIDs struct { DsId *int `json:"id,omitempty" db:"ds_id"` From 218761506711eb556f9bad496801126eba175b9d Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Thu, 14 Sep 2023 13:43:59 -0600 Subject: [PATCH 03/20] Update GET handlers to serve v5 structures to v5-requesting clients --- .dependency_license | 1 + docs/source/api/v5/cdns_name_federations.rst | 40 ++-- .../api/v5/cdns_name_federations_id.rst | 78 +++++++ .../cdnfederation/cdnfederations.go | 186 ++++++++++++++-- .../cdnfederation/cdnfederations_test.go | 208 ++++++++++++++++++ .../cdnfederation/select.sql | 19 ++ .../traffic_ops_golang/routing/routes.go | 3 +- 7 files changed, 495 insertions(+), 40 deletions(-) create mode 100644 traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go create mode 100644 traffic_ops/traffic_ops_golang/cdnfederation/select.sql diff --git a/.dependency_license b/.dependency_license index f39518e10a..8374928b4f 100644 --- a/.dependency_license +++ b/.dependency_license @@ -55,6 +55,7 @@ traffic_router/core/src/test/resources/api/.*/steering*, Apache-2.0 traffic_router/core/src/test/resources/api/.*/federations/all, Apache-2.0 BUILD_NUMBER$, Apache-2.0 \.jks, Apache-2.0 # Java Key Store +traffic_ops/traffic_ops_golang/**/*.sql, Apache-2.0 # embedded sql queries - can't have comment blocks due to github.com/jmoiron/sqlx library limitations # Images, created for this project or used under an Apache license. ^misc/logos/ATC-PNG\.png, Apache-2.0 diff --git a/docs/source/api/v5/cdns_name_federations.rst b/docs/source/api/v5/cdns_name_federations.rst index e9188086fa..cbe2db3331 100644 --- a/docs/source/api/v5/cdns_name_federations.rst +++ b/docs/source/api/v5/cdns_name_federations.rst @@ -32,18 +32,24 @@ Request Structure ----------------- .. table:: Request Path Parameters - +-----------+---------------------------------------------------------------------------------------------------------------+ - | Name | Description | - +===========+===============================================================================================================+ - | name | The name of the CDN for which federations will be listed | - +-----------+---------------------------------------------------------------------------------------------------------------+ + +-----------+------------------------------------------------------------------+ + | Name | Description | + +===========+==================================================================+ + | name | The name of the CDN for which :term:`Federations` will be listed | + +-----------+------------------------------------------------------------------+ .. table:: Request Query Parameters +-----------+---------------------------------------------------------------------------------------------------------------+ | Name | Description | +===========+===============================================================================================================+ - | id | Return only the federation that has this id | + | id | Return only the :term:`Federation` that has this ID | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | cname | Return only those :term:`Federations` that have this CNAME | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | dsID | Return only those :term:`Federations` assigned to a :term:`Delivery Service` that has this ID | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | xmlID | Return only those :term:`Federations` assigned to a :term:`Delivery Service` that has this :ref:`ds-xmlid` | +-----------+---------------------------------------------------------------------------------------------------------------+ | orderby | Choose the ordering of the results - must be the name of one of the fields of the objects in the ``response`` | | | array | @@ -70,18 +76,22 @@ Request Structure Response Structure ------------------ -:cname: The Canonical Name (CNAME) used by the federation -:deliveryService: An object with keys that provide identifying information for the :term:`Delivery Service` using this federation +:cname: The :abbr:`CNAME (Canonical Name)` used by the :term:`Federation` +:deliveryService: An object with keys that provide identifying information for the :term:`Delivery Service` using this :term:`Federation` :id: The integral, unique identifier for the :term:`Delivery Service` - :xmlId: The :term:`Delivery Service`'s uniquely identifying 'xml_id' + :xmlID: The :term:`Delivery Service`'s uniquely identifying :ref:`ds-xmlid` -:description: An optionally-present field containing a description of the field + .. versionchanged:: 5.0 + Prior to version 5, this field was known by the name ``xmlId`` - improperly formatted camelCase. - .. note:: This key will only be present if the description was provided when the federation was created. Refer to the ``POST`` method of this endpoint to see how federations can be created. +:description: A human-readable description of the :term:`Federation`. This can be ``null`` as well as an empty string. +:lastUpdated: The date and time at which this :term:`Federation` was last modified, in :RFC:`3339` format -:lastUpdated: The date and time at which this federation was last modified, in :ref:`non-rfc-datetime` -:ttl: Time to Live (TTL) for the ``cname``, in hours + .. versionchanged:: 5.0 + In earlier versions of the API, this field was given in :ref:`non-rfc-datetime`. + +:ttl: :abbr:`TTL (Time to Live)` for the ``cname``, in hours .. code-block:: http :caption: Response Example @@ -104,10 +114,10 @@ Response Structure "cname": "test.quest.", "ttl": 48, "description": "A test federation", - "lastUpdated": "2018-12-05 00:05:16+00", + "lastUpdated": "2018-12-05T00:05:16Z", "deliveryService": { "id": 1, - "xmlId": "demo1" + "xmlID": "demo1" } } ]} diff --git a/docs/source/api/v5/cdns_name_federations_id.rst b/docs/source/api/v5/cdns_name_federations_id.rst index 6fee0b5b72..5f291de2c1 100644 --- a/docs/source/api/v5/cdns_name_federations_id.rst +++ b/docs/source/api/v5/cdns_name_federations_id.rst @@ -19,6 +19,84 @@ ``cdns/{{name}}/federations/{{ID}}`` ************************************ +``GET`` +======= +Retrieves a list of federations in use by a specific CDN. + +.. versionadded:: 5.0 + +:Auth. Required: Yes +:Roles Required: None +:Permissions Required: CDN:READ, FEDERATION:READ, DELIVERY-SERVICE:READ +:Response Type: Object + +Request Structure +----------------- +.. table:: Request Path Parameters + + +------+---------------------------------------------------------------------------------------------+ + | Name | Description | + +======+=============================================================================================+ + | name | The name of the CDN for which the :term:`Federation` identified by ``ID`` will be inspected | + +------+---------------------------------------------------------------------------------------------+ + | ID | An integral, unique identifier for the :term:`Federation` to be inspected | + +------+---------------------------------------------------------------------------------------------+ + +.. code-block:: http + :caption: Request Example + + GET /api/5.0/cdns/CDN-in-a-Box/federations/1 HTTP/1.1 + Host: trafficops.infra.ciab.test + User-Agent: curl/7.62.0 + Accept: */* + Cookie: mojolicious=... + +Response Structure +------------------ +:cname: The :abbr:`CNAME (Canonical Name)` used by the :term:`Federation` +:deliveryService: An object with keys that provide identifying information for the :term:`Delivery Service` using this :term:`Federation` + + :id: The integral, unique identifier for the :term:`Delivery Service` + :xmlID: The :term:`Delivery Service`'s uniquely identifying :ref:`ds-xmlid` + + .. versionchanged:: 5.0 + Prior to version 5, this field was known by the name ``xmlId`` - improperly formatted camelCase. + +:description: A human-readable description of the :term:`Federation`. This can be ``null`` as well as an empty string. +:lastUpdated: The date and time at which this :term:`Federation` was last modified, in :RFC:`3339` format + + .. versionchanged:: 5.0 + In earlier versions of the API, this field was given in :ref:`non-rfc-datetime`. + +:ttl: :abbr:`TTL (Time to Live)` for the ``cname``, in hours + +.. code-block:: http + :caption: Response Example + + HTTP/1.1 200 OK + access-control-allow-credentials: true + access-control-allow-headers: Origin, X-Requested-With, Content-Type, Accept, Set-Cookie, Cookie + access-control-allow-methods: POST,GET,OPTIONS,PUT,DELETE + access-control-allow-origin: * + content-type: application/json + set-cookie: mojolicious=...; Path=/; HttpOnly + whole-content-sha512: SJA7G+7G5KcOfCtnE3Dq5DCobWtGRUKSppiDkfLZoG5+paq4E1aZGqUb6vGVsd+TpPg75MLlhyqfdfCHnhLX/g== + x-server-name: traffic_ops_golang/ + content-length: 170 + date: Wed, 05 Dec 2018 00:35:40 GMT + + { "response": { + "id": 1, + "cname": "test.quest.", + "ttl": 48, + "description": "A test federation", + "lastUpdated": "2018-12-05T00:05:16Z", + "deliveryService": { + "id": 1, + "xmlID": "demo1" + } + }} + ``PUT`` ======= Updates a federation. diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go index b15b61283b..abe7e687a1 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go @@ -1,3 +1,5 @@ +// Package cdnfederation is one of many, many packages that contain logic +// pertaining to federations of CDNs and/or parts of CDNs. package cdnfederation /* @@ -21,6 +23,7 @@ package cdnfederation import ( "database/sql" + _ "embed" // needed to embed SQL queries within Go variables "errors" "fmt" "net/http" @@ -28,14 +31,18 @@ import ( "strings" "time" + "github.com/apache/trafficcontrol/lib/go-log" "github.com/apache/trafficcontrol/lib/go-tc" "github.com/apache/trafficcontrol/lib/go-tc/tovalidate" "github.com/apache/trafficcontrol/lib/go-util" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims" + "github.com/asaskevich/govalidator" validation "github.com/go-ozzo/ozzo-validation" + "github.com/lib/pq" ) // we need a type alias to define functions on @@ -49,9 +56,7 @@ func (v *TOCDNFederation) GetLastUpdated() (*time.Time, bool, error) { return api.GetLastUpdated(v.APIInfo().Tx, *v.ID, "federation") } -func (v *TOCDNFederation) SetLastUpdated(t tc.TimeNoMod) { v.LastUpdated = &t } -func (v *TOCDNFederation) InsertQuery() string { return insertQuery() } -func (v *TOCDNFederation) SelectMaxLastUpdatedQuery(where, orderBy, pagination, tableName string) string { +func selectMaxLastUpdatedQuery(where, orderBy, pagination string) string { return `SELECT max(t) from ( SELECT max(federation.last_updated) as t from federation join federation_deliveryservice fds on fds.federation = federation.id @@ -61,17 +66,57 @@ func (v *TOCDNFederation) SelectMaxLastUpdatedQuery(where, orderBy, pagination, select max(last_updated) as t from last_deleted l where l.table_name='federation') as res` } +func (v *TOCDNFederation) SetLastUpdated(t tc.TimeNoMod) { v.LastUpdated = &t } +func (v *TOCDNFederation) InsertQuery() string { return insertQuery() } +func (v *TOCDNFederation) SelectMaxLastUpdatedQuery(where, orderBy, pagination, _ string) string { + return selectMaxLastUpdatedQuery(where, orderBy, pagination) +} + func (v *TOCDNFederation) NewReadObj() interface{} { return &TOCDNFederation{} } func (v *TOCDNFederation) SelectQuery() string { return selectByCDNName() } -func (v *TOCDNFederation) ParamColumns() map[string]dbhelpers.WhereColumnInfo { - cols := map[string]dbhelpers.WhereColumnInfo{ - "id": dbhelpers.WhereColumnInfo{Column: "federation.id", Checker: api.IsInt}, - "name": dbhelpers.WhereColumnInfo{Column: "c.name", Checker: nil}, - "cname": dbhelpers.WhereColumnInfo{Column: "federation.cname", Checker: nil}, + +func paramColumnInfo(v api.Version) map[string]dbhelpers.WhereColumnInfo { + if v.GreaterThanOrEqualTo(&api.Version{Major: 5}) { + return map[string]dbhelpers.WhereColumnInfo{ + "id": { + Column: "federation.id", + Checker: api.IsInt, + }, + "name": { + Column: "c.name", + }, + "cname": { + Column: "federation.cname", + }, + "xmlID": { + Column: "ds.xml_id", + }, + "dsID": { + Column: "ds.id", + Checker: api.IsInt, + }, + } } - return cols + return map[string]dbhelpers.WhereColumnInfo{ + "id": { + Column: "federation.id", + Checker: api.IsInt, + }, + "name": { + Column: "c.name", + Checker: nil, + }, + "cname": { + Column: "federation.cname", + Checker: nil, + }, + } +} + +func (v *TOCDNFederation) ParamColumns() map[string]dbhelpers.WhereColumnInfo { + return paramColumnInfo(*v.ReqInfo.Version) } func (v *TOCDNFederation) DeleteQuery() string { return deleteQuery() } func (v *TOCDNFederation) UpdateQuery() string { return updateQuery() } @@ -348,22 +393,11 @@ func selectByID() string { // WHERE federation.id = :id (determined by dbhelper) } +//go:embed select.sql +var selectQuery string + func selectByCDNName() string { - return ` - SELECT - ds.tenant_id, - federation.id AS id, - federation.cname, - federation.ttl, - federation.description, - federation.last_updated, - ds.id AS ds_id, - ds.xml_id - FROM federation - JOIN federation_deliveryservice AS fd ON federation.id = fd.federation - JOIN deliveryservice AS ds ON ds.id = fd.deliveryservice - JOIN cdn c ON c.id = ds.cdn_id` - // WHERE cdn.name = :cdn_name (determined by dbhelper) + return selectQuery } func updateQuery() string { @@ -393,3 +427,107 @@ func insertQuery() string { func deleteQuery() string { return `DELETE FROM federation WHERE id = :id` } + +func addTenancyStmt(where string) string { + if where == "" { + where = "WHERE " + } else { + where += " AND " + } + where += "ds.tenant_id = ANY(:tenantIDs)" + return where +} + +func getCDNFederations(inf *api.APIInfo) ([]tc.CDNFederationV5, time.Time, int, error, error) { + tenantList, err := tenant.GetUserTenantIDListTx(inf.Tx.Tx, inf.User.TenantID) + if err != nil { + return nil, time.Time{}, http.StatusInternalServerError, nil, fmt.Errorf("getting tenant list for user: %w", err) + } + + cols := paramColumnInfo(*inf.Version) + + where, orderBy, pagination, queryValues, errs := dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, cols) + if len(errs) > 0 { + return nil, time.Time{}, http.StatusBadRequest, util.JoinErrs(errs), nil + } + queryValues["tenantIDs"] = pq.Array(tenantList) + + where = addTenancyStmt(where) + query := selectQuery + where + orderBy + pagination + + if inf.UseIMS() { + cont, max := ims.TryIfModifiedSinceQuery(inf.Tx, inf.RequestHeaders(), queryValues, query) + if !cont { + log.Debugln("IMS HIT") + return nil, max, http.StatusNotModified, nil, nil + } + log.Debugln("IMS MISS") + } else { + log.Debugln("Non IMS request") + } + + rows, err := inf.Tx.NamedQuery(query, queryValues) + if err != nil { + userErr, sysErr, code := api.ParseDBError(err) + return nil, time.Time{}, code, userErr, sysErr + } + defer log.Close(rows, "closing CDNFederation rows") + + ret := []tc.CDNFederationV5{} + for rows.Next() { + fed := tc.CDNFederationV5{ + DeliveryService: new(tc.CDNFederationDeliveryService), + } + var tenantID int + err := rows.Scan( + &tenantID, + &fed.ID, + &fed.CName, + &fed.TTL, + &fed.Description, + &fed.LastUpdated, + &fed.DeliveryService.ID, + &fed.DeliveryService.XMLID, + ) + if err != nil { + return nil, time.Time{}, http.StatusInternalServerError, nil, fmt.Errorf("scanning a CDN Federation: %w", err) + } + + ret = append(ret, fed) + } + + return ret, time.Time{}, http.StatusOK, nil, nil +} + +// Read handles GET requests to `cdns/{{name}}/federations`. +func Read(inf *api.APIInfo) (int, error, error) { + api.DefaultSort(inf, "cname") + feds, max, code, userErr, sysErr := getCDNFederations(inf) + if userErr != nil || sysErr != nil { + return code, userErr, sysErr + } + if feds == nil { + return inf.WriteNotModifiedResponse(max) + } + return inf.WriteOKResponse(feds) +} + +// ReadID handles GET requests to `cdns/{{name}}/federations/{{ID}}`. +func ReadID(inf *api.APIInfo) (int, error, error) { + feds, max, code, userErr, sysErr := getCDNFederations(inf) + if userErr != nil || sysErr != nil { + return code, userErr, sysErr + } + if feds == nil { + return inf.WriteNotModifiedResponse(max) + } + + id := inf.IntParams["id"] + if len(feds) == 0 { + return http.StatusNotFound, fmt.Errorf("no such Federation #%d in CDN %s", id, inf.Params["name"]), nil + } + if len(feds) > 1 { + return http.StatusInternalServerError, nil, fmt.Errorf("%d CDN federations found by ID: %d", len(feds), id) + } + return inf.WriteOKResponse(feds[0]) +} diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go new file mode 100644 index 0000000000..f8a4e0326c --- /dev/null +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go @@ -0,0 +1,208 @@ +package cdnfederation + +/* + * 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. + */ + +import ( + "errors" + "net/http" + "strings" + "testing" + "time" + + "github.com/apache/trafficcontrol/lib/go-tc" + "github.com/apache/trafficcontrol/lib/go-util" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth" + "github.com/jmoiron/sqlx" + "gopkg.in/DATA-DOG/go-sqlmock.v1" +) + +func TestAddTenancyStmt(t *testing.T) { + where := "" + output := addTenancyStmt(where) + if expected := "WHERE ds.tenant_id = ANY(:tenantIDs)"; output != expected { + t.Errorf("Incorrect statement from blank WHERE; want: '%s', got: '%s", expected, output) + } + where = "WHERE cname=:cname" + output = addTenancyStmt(where) + if expected := "WHERE cname=:cname AND ds.tenant_id = ANY(:tenantIDs)"; output != expected { + t.Errorf("Incorrect statement from blank WHERE; want: '%s', got: '%s", expected, output) + } +} + +func TestParamColumnInfo(t *testing.T) { + params := paramColumnInfo(api.Version{Major: 4}) + if l := len(params); l != 3 { + t.Errorf("Expected versions prior to 5 to support 3 query params, found support for: %d", l) + } + for _, param := range [3]string{"cname", "id", "name"} { + if _, ok := params[param]; !ok { + t.Errorf("Expected versions prior to 5 to support the '%s' query param, but support for such wasn't found", param) + } + } + + params = paramColumnInfo(api.Version{Major: 5}) + if l := len(params); l != 5 { + t.Errorf("Expected versions 5 and later to support 5 query params, found support for: %d", l) + } + for _, param := range [5]string{"cname", "dsID", "id", "name", "xmlID"} { + if _, ok := params[param]; !ok { + t.Errorf("Expected versions 5 and later to support the '%s' query param, but support for such wasn't found", param) + } + } +} + +func getMockTx(t *testing.T) (sqlmock.Sqlmock, *sqlx.Tx, *sqlx.DB) { + t.Helper() + mockDB, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("an error '%s' was not expected when opening a stub database connection", err) + } + + db := sqlx.NewDb(mockDB, "sqlmock") + mock.ExpectBegin() + + return mock, db.MustBegin(), db +} + +func cleanup(t *testing.T, mock sqlmock.Sqlmock, db *sqlx.DB) { + t.Helper() + mock.ExpectClose() + db.Close() + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("expectations were not met: %v", err) + } +} + +func gettingUserTenantListFails(t *testing.T) { + mock, tx, db := getMockTx(t) + defer cleanup(t, mock, db) + defer func() { + mock.ExpectRollback() + tx.Rollback() + }() + + err := errors.New("unknown failure") + mock.ExpectQuery("WITH RECURSIVE").WillReturnError(err) + _, _, code, userErr, sysErr := getCDNFederations(&api.APIInfo{Tx: tx, User: &auth.CurrentUser{TenantID: 1}}) + + if code != http.StatusInternalServerError { + t.Errorf("Incorrect response code when getting user tenants fails; want: %d, got: %d", http.StatusInternalServerError, code) + } + if userErr != nil { + t.Errorf("Unexpected user-facing error: %v", userErr) + } + if sysErr == nil { + t.Fatal("Expected a system error but didn't get one") + } + + // You can't use `errors.Is` here because sqlmock doesn't wrap the error you + // give it, so we have to resort to comparing text and praying there's no + // weird coincidence going on behind the scenes. + if !strings.Contains(sysErr.Error(), err.Error()) { + t.Errorf("Incorrect system error returned; want: %v, got: %v", err, sysErr) + } +} + +func buildingQueryPartsFails(t *testing.T) { + mock, tx, db := getMockTx(t) + defer cleanup(t, mock, db) + defer func() { + mock.ExpectRollback() + tx.Rollback() + }() + + rows := sqlmock.NewRows([]string{"id"}) + rows.AddRow(1) + + mock.ExpectQuery("WITH RECURSIVE").WillReturnRows(rows) + + inf := api.APIInfo{ + Params: map[string]string{ + "dsID": "not an integer", + }, + Tx: tx, + User: &auth.CurrentUser{TenantID: 1}, + Version: &api.Version{Major: 5}, + } + _, _, code, userErr, sysErr := getCDNFederations(&inf) + if code != http.StatusBadRequest { + t.Errorf("Incorrect response code when getting user tenants fails; want: %d, got: %d", http.StatusBadRequest, code) + } + if sysErr != nil { + t.Errorf("Unexpected system error: %v", sysErr) + } + if userErr == nil { + t.Fatal("Expected a user-facing error, but didn't get one") + } + if !strings.Contains(userErr.Error(), "dsID") { + t.Errorf("Incorrect user error; expected it to mention 'dsID', but it didn't: %v", userErr) + } +} + +func everythingWorks(t *testing.T) { + mock, tx, db := getMockTx(t) + defer cleanup(t, mock, db) + defer func() { + mock.ExpectCommit() + tx.Commit() + }() + + rows := sqlmock.NewRows([]string{"id"}) + rows.AddRow(1) + + mock.ExpectQuery("WITH RECURSIVE").WillReturnRows(rows) + + fedRows := sqlmock.NewRows([]string{"tenant_id", "id", "cname", "ttl", "description", "last_updated", "ds_id", "xml_id"}) + fed := tc.CDNFederationV5{ + CName: "test.quest.", + Description: util.Ptr("a non-blank description"), + DeliveryService: &tc.CDNFederationDeliveryService{ + ID: 1, + XMLID: "some-xmlid", + }, + ID: 1, + LastUpdated: time.Time{}.Add(time.Hour), + TTL: 5, + } + + fedRows.AddRow(1, fed.ID, fed.CName, fed.TTL, fed.Description, fed.LastUpdated, fed.DeliveryService.ID, fed.DeliveryService.XMLID) + mock.ExpectQuery("SELECT").WillReturnRows(fedRows) + + feds, _, _, userErr, sysErr := getCDNFederations(&api.APIInfo{Tx: tx, User: &auth.CurrentUser{TenantID: 1}, Version: &api.Version{Major: 5}}) + if userErr != nil { + t.Errorf("Unexpected user-facing error: %v", userErr) + } + if sysErr != nil { + t.Errorf("Unexpected system error: %v", sysErr) + } + if l := len(feds); l != 1 { + t.Fatalf("Expected one federation to be returned; got: %d", l) + } + if feds[0] != fed { + t.Errorf("expected a federation like '%#v', but instead found: %#v", fed, feds[0]) + } +} + +func TestGetCDNFederations(t *testing.T) { + t.Run("getting user Tenant list fails", gettingUserTenantListFails) + t.Run("building where/orderby/pagination fails", buildingQueryPartsFails) + t.Run("everything works", everythingWorks) +} diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/select.sql b/traffic_ops/traffic_ops_golang/cdnfederation/select.sql new file mode 100644 index 0000000000..346587bd1e --- /dev/null +++ b/traffic_ops/traffic_ops_golang/cdnfederation/select.sql @@ -0,0 +1,19 @@ +SELECT + ds.tenant_id, + federation.id AS id, + federation.cname, + federation.ttl, + federation.description, + federation.last_updated, + ds.id AS ds_id, + ds.xml_id +FROM federation +JOIN + federation_deliveryservice AS fd + ON federation.id = fd.federation +JOIN + deliveryservice AS ds ON + ds.id = fd.deliveryservice +JOIN + cdn AS c + ON c.id = ds.cdn_id diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go index 2aa1b328e1..796f32689a 100644 --- a/traffic_ops/traffic_ops_golang/routing/routes.go +++ b/traffic_ops/traffic_ops_golang/routing/routes.go @@ -395,8 +395,9 @@ func Routes(d ServerData) ([]Route, http.Handler, error) { {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}, // 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.MethodGet, Path: `cdns/{name}/federations/?$`, Handler: api.Wrap(cdnfederation.Read, []string{"name"}, nil), 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}, + {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `cdns/{name}/federations/{id}/?$`, Handler: api.Wrap(cdnfederation.ReadID, []string{"name"}, []string{"id"}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"CDN:READ", "FEDERATION:READ", "DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 48922503232}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPut, Path: `cdns/{name}/federations/{id}$`, Handler: api.UpdateHandler(&cdnfederation.TOCDNFederation{}), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:UPDATE", "FEDERATION:READ", "CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 42606546631}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodDelete, Path: `cdns/{name}/federations/{id}$`, Handler: api.DeleteHandler(&cdnfederation.TOCDNFederation{}), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:DELETE", "FEDERATION:READ", "CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 444285290231}, From 353bd29c0f75d16bb408ac52aa284c4e36491486 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Thu, 14 Sep 2023 15:02:07 -0600 Subject: [PATCH 04/20] Update POST handler to support v5 --- docs/source/api/v5/cdns_name_federations.rst | 36 +++++----- .../cdnfederation/cdnfederations.go | 66 +++++++++++++---- .../cdnfederation/cdnfederations_test.go | 72 +++++++++++++++++++ .../cdnfederation/insert.sql | 12 ++++ .../traffic_ops_golang/routing/routes.go | 2 +- 5 files changed, 157 insertions(+), 31 deletions(-) create mode 100644 traffic_ops/traffic_ops_golang/cdnfederation/insert.sql diff --git a/docs/source/api/v5/cdns_name_federations.rst b/docs/source/api/v5/cdns_name_federations.rst index cbe2db3331..f0d5e4fe43 100644 --- a/docs/source/api/v5/cdns_name_federations.rst +++ b/docs/source/api/v5/cdns_name_federations.rst @@ -124,7 +124,11 @@ Response Structure ``POST`` ======== -Creates a new federation. +Creates a new :term:`Federation`. + +.. caution:: Despite the URL of this endpoint, this does `**not**` create a :term:`Federation` within any particular CDN. A :term:`Federation` is associated with a CDN purely because any :term:`Delivery Service` to which it is assigned is scoped to a CDN. Therefore, upon creation a :term:`Federation` is not associated with any CDN in particular. + +.. warning:: There is no restriction on using the special "ALL" CDN for :term:`Federations` - but this is highly discouraged, because many things treat that CDN specially and may not work properly if it is used as though it were a normal CDN. :Auth. Required: Yes :Roles Required: "admin" @@ -135,15 +139,15 @@ Request Structure ----------------- .. table:: Request Path Parameters - +------+----------------------------------------------------------------+ - | Name | Description | - +======+================================================================+ - | name | The name of the CDN for which a new federation will be created | - +------+----------------------------------------------------------------+ + +------+------------------------------------------------------------------------+ + | Name | Description | + +======+========================================================================+ + | name | The name of the CDN for which a new :term:`Federation` will be created | + +------+------------------------------------------------------------------------+ -:cname: The Canonical Name (CNAME) used by the federation +:cname: The :abbr:`CNAME (Canonical Name)` used by the :term:`Federation` - .. note:: The CNAME must end with a "``.``" + .. tip:: The CNAME must end with a "``.``" :description: An optional description of the federation :ttl: Time to Live (TTL) for the name record used for ``cname`` @@ -169,14 +173,14 @@ Request Structure Response Structure ------------------ :id: The integral, unique identifier of the :term:`Federation` -:cname: The Canonical Name (CNAME) used by the federation -:description: An optionally-present field containing a description of the field - - .. note:: This key will only be present if the description was provided when the federation was created +:cname: The :abbr:`CNAME (Canonical Name)` used by the :term:`Federation` +:description: The description of the :term:`Federation` +:lastUpdated: The date and time at which this federation was last modified, in :RFC:`3339` format -:lastUpdated: The date and time at which this federation was last modified, in :ref:`non-rfc-datetime` -:ttl: Time to Live (TTL) for the ``cname``, in hours + .. versionchanged:: 5.0 + In earlier versions of the API, this field was given in :ref:`non-rfc-datetime`. +:ttl: :abbr:`TTL (Time to Live)` for the ``cname``, in hours .. code-block:: http :caption: Response Example @@ -195,7 +199,7 @@ Response Structure { "alerts": [ { - "text": "cdnfederation was created.", + "text": "Federation was created", "level": "success" } ], @@ -204,5 +208,5 @@ Response Structure "cname": "test.quest.", "ttl": 48, "description": "A test federation", - "lastUpdated": "2018-12-05 00:05:16+00" + "lastUpdated": "2018-12-05T00:05:16Z" }} diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go index abe7e687a1..c94c4b26f9 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go @@ -42,6 +42,7 @@ import ( "github.com/asaskevich/govalidator" validation "github.com/go-ozzo/ozzo-validation" + "github.com/go-ozzo/ozzo-validation/is" "github.com/lib/pq" ) @@ -67,7 +68,18 @@ func selectMaxLastUpdatedQuery(where, orderBy, pagination string) string { } func (v *TOCDNFederation) SetLastUpdated(t tc.TimeNoMod) { v.LastUpdated = &t } -func (v *TOCDNFederation) InsertQuery() string { return insertQuery() } +func (*TOCDNFederation) InsertQuery() string { + return ` + INSERT INTO federation ( + cname, + ttl, + description + ) VALUES ( + :cname, + :ttl, + :description + ) RETURNING id, last_updated` +} func (v *TOCDNFederation) SelectMaxLastUpdatedQuery(where, orderBy, pagination, _ string) string { return selectMaxLastUpdatedQuery(where, orderBy, pagination) } @@ -396,6 +408,9 @@ func selectByID() string { //go:embed select.sql var selectQuery string +//go:embed insert.sql +var insertQuery string + func selectByCDNName() string { return selectQuery } @@ -411,19 +426,6 @@ WHERE RETURNING last_updated` } -func insertQuery() string { - return ` - INSERT INTO federation ( - cname, - ttl, - description - ) VALUES ( - :cname, - :ttl, - :description - ) RETURNING id, last_updated` -} - func deleteQuery() string { return `DELETE FROM federation WHERE id = :id` } @@ -531,3 +533,39 @@ func ReadID(inf *api.APIInfo) (int, error, error) { } return inf.WriteOKResponse(feds[0]) } + +func validate(fed tc.CDNFederationV5) error { + endsWithDot := validation.NewStringRule( + func(str string) bool { + return strings.HasSuffix(str, ".") + }, + "must end with a period", + ) + + validateErrs := validation.Errors{ + "cname": validation.Validate(fed.CName, validation.Required, is.DNSName, endsWithDot), + "ttl": validation.Validate(fed.TTL, validation.Required, validation.Min(0)), + } + + return util.JoinErrs(tovalidate.ToErrors(validateErrs)) +} + +// Create handles POST requests to `cdns/{{name}}/federations`. +func Create(inf *api.APIInfo) (int, error, error) { + var fed tc.CDNFederationV5 + if err := inf.DecodeBody(&fed); err != nil { + return http.StatusBadRequest, fmt.Errorf("parsing request body: %w", err), nil + } + + err := validate(fed) + if err != nil { + return http.StatusBadRequest, err, nil + } + + err = inf.Tx.Tx.QueryRow(insertQuery, fed.CName, fed.TTL, fed.Description).Scan(&fed.ID, &fed.LastUpdated) + if err != nil { + return http.StatusInternalServerError, nil, fmt.Errorf("inserting a CDN Federation: %w", err) + } + + return inf.WriteCreatedResponse(fed, "Federation was created", "federations/"+strconv.Itoa(fed.ID)) +} diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go index f8a4e0326c..59c96f8f7e 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go @@ -20,8 +20,12 @@ package cdnfederation */ import ( + "bytes" + "context" + "encoding/json" "errors" "net/http" + "net/http/httptest" "strings" "testing" "time" @@ -30,6 +34,8 @@ import ( "github.com/apache/trafficcontrol/lib/go-util" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/auth" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/config" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/trafficvault/backends/disabled" "github.com/jmoiron/sqlx" "gopkg.in/DATA-DOG/go-sqlmock.v1" ) @@ -206,3 +212,69 @@ func TestGetCDNFederations(t *testing.T) { t.Run("building where/orderby/pagination fails", buildingQueryPartsFails) t.Run("everything works", everythingWorks) } + +func wrapContext(r *http.Request, key any, value any) *http.Request { + return r.WithContext(context.WithValue(r.Context(), key, value)) +} + +func testingInf(t *testing.T, body []byte) (*http.Request, sqlmock.Sqlmock, *sqlx.DB) { + t.Helper() + mockDB, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("failed to open a stub database connection: %v", err) + } + + db := sqlx.NewDb(mockDB, "sqlmock") + + r := httptest.NewRequest(http.MethodPost, "/api/5.0/cdns/ALL/federations", bytes.NewReader(body)) + r = wrapContext(r, api.ConfigContextKey, &config.Config{ConfigTrafficOpsGolang: config.ConfigTrafficOpsGolang{DBQueryTimeoutSeconds: 1000}}) + r = wrapContext(r, api.DBContextKey, db) + r = wrapContext(r, api.TrafficVaultContextKey, &disabled.Disabled{}) + r = wrapContext(r, api.ReqIDContextKey, uint64(0)) + r = wrapContext(r, auth.CurrentUserKey, auth.CurrentUser{}) + r = wrapContext(r, api.PathParamsKey, make(map[string]string)) + + mock.ExpectBegin() + + return r, mock, db +} + +func TestCreate(t *testing.T) { + newFed := tc.CDNFederationV5{ + CName: "test.quest.", + TTL: 5, + Description: nil, + } + bts, err := json.Marshal(newFed) + if err != nil { + t.Fatalf("marshaling testing request body: %v", err) + } + + newFed.ID = 1 + newFed.LastUpdated = time.Time{}.Add(time.Hour) + + r, mock, db := testingInf(t, bts) + defer cleanup(t, mock, db) + + rows := sqlmock.NewRows([]string{"id", "last_updated"}) + rows.AddRow(newFed.ID, newFed.LastUpdated) + mock.ExpectQuery("INSERT").WillReturnRows(rows) + + f := api.Wrap(Create, nil, nil) + w := httptest.NewRecorder() + f(w, r) + + if w.Code != http.StatusCreated { + t.Errorf("Incorrect response code; want: %d, got: %d", http.StatusCreated, w.Code) + } + + var created tc.CDNFederationV5Response + err = json.Unmarshal(w.Body.Bytes(), &created) + if err != nil { + t.Fatalf("Unmarshaling response: %v", err) + } + + if created.Response != newFed { + t.Errorf("Didn't create the expected Federation; want: %#v, got: %#v", newFed, created.Response) + } +} diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/insert.sql b/traffic_ops/traffic_ops_golang/cdnfederation/insert.sql new file mode 100644 index 0000000000..c2393fd56e --- /dev/null +++ b/traffic_ops/traffic_ops_golang/cdnfederation/insert.sql @@ -0,0 +1,12 @@ +INSERT INTO federation ( + cname, + ttl, + "description" +) VALUES ( + $1, + $2, + $3 +) +RETURNING + id, + last_updated diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go index 796f32689a..141d2cb115 100644 --- a/traffic_ops/traffic_ops_golang/routing/routes.go +++ b/traffic_ops/traffic_ops_golang/routing/routes.go @@ -396,7 +396,7 @@ func Routes(d ServerData) ([]Route, http.Handler, error) { // Federations by CDN (the actual table for federation) {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `cdns/{name}/federations/?$`, Handler: api.Wrap(cdnfederation.Read, []string{"name"}, nil), 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}, + {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPost, Path: `cdns/{name}/federations/?$`, Handler: api.Wrap(cdnfederation.Create, nil, nil), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:CREATE", "FEDERATION:READ, CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 495489421931}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `cdns/{name}/federations/{id}/?$`, Handler: api.Wrap(cdnfederation.ReadID, []string{"name"}, []string{"id"}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"CDN:READ", "FEDERATION:READ", "DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 48922503232}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPut, Path: `cdns/{name}/federations/{id}$`, Handler: api.UpdateHandler(&cdnfederation.TOCDNFederation{}), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:UPDATE", "FEDERATION:READ", "CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 42606546631}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodDelete, Path: `cdns/{name}/federations/{id}$`, Handler: api.DeleteHandler(&cdnfederation.TOCDNFederation{}), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:DELETE", "FEDERATION:READ", "CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 444285290231}, From a5ab1fbfc463967050b22307c9464a42b8102234 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Thu, 14 Sep 2023 23:21:38 -0600 Subject: [PATCH 05/20] Update PUT handler to support v5 --- .../api/v5/cdns_name_federations_id.rst | 34 ++++++----- .../cdnfederation/cdnfederations.go | 60 +++++++++++++++---- .../cdnfederation/update.sql | 8 +++ .../traffic_ops_golang/routing/routes.go | 2 +- 4 files changed, 75 insertions(+), 29 deletions(-) create mode 100644 traffic_ops/traffic_ops_golang/cdnfederation/update.sql diff --git a/docs/source/api/v5/cdns_name_federations_id.rst b/docs/source/api/v5/cdns_name_federations_id.rst index 5f291de2c1..612a45c1e7 100644 --- a/docs/source/api/v5/cdns_name_federations_id.rst +++ b/docs/source/api/v5/cdns_name_federations_id.rst @@ -99,7 +99,7 @@ Response Structure ``PUT`` ======= -Updates a federation. +Updates a :term:`Federation`. :Auth. Required: Yes :Roles Required: "admin" @@ -110,13 +110,15 @@ Request Structure ----------------- .. table:: Request Path Parameters - +------+-------------------------------------------------------------------------------------+ - | Name | Description | - +======+=====================================================================================+ - | name | The name of the CDN for which the federation identified by ``ID`` will be inspected | - +------+-------------------------------------------------------------------------------------+ - | ID | An integral, unique identifier for the federation to be inspected | - +------+-------------------------------------------------------------------------------------+ + +------+-------------------------------------------------------------------------------------------+ + | Name | Description | + +======+===========================================================================================+ + | name | The name of the CDN for which the :term:`Federation` identified by ``ID`` will be updated | + +------+-------------------------------------------------------------------------------------------+ + | ID | An integral, unique identifier for the :term:`Federation` to be updated | + +------+-------------------------------------------------------------------------------------------+ + +.. caution:: The name of the CDN doesn't actually matter. It doesn't even need to be the name of any existing CDN. :cname: The Canonical Name (CNAME) used by the federation @@ -144,14 +146,14 @@ Request Structure Response Structure ------------------ -:cname: The Canonical Name (CNAME) used by the federation -:description: An optionally-present field containing a description of the field - - .. note:: This key will only be present if the description was provided when the federation was created +:cname: The :abbr:`CNAME (Canonical Name)` used by the :term:`Federation` +:description: A human-readable description of the :term:`Federation`. This can be ``null`` as well as an empty string. +:lastUpdated: The date and time at which this :term:`Federation` was last modified, in :RFC:`3339` format -:lastUpdated: The date and time at which this federation was last modified, in :ref:`non-rfc-datetime` -:ttl: Time to Live (TTL) for the ``cname``, in hours + .. versionchanged:: 5.0 + In earlier versions of the API, this field was given in :ref:`non-rfc-datetime`. +:ttl: :abbr:`TTL (Time to Live)` for the ``cname``, in hours .. code-block:: http :caption: Response Example @@ -170,7 +172,7 @@ Response Structure { "alerts": [ { - "text": "cdnfederation was updated.", + "text": "Federation was updated", "level": "success" } ], @@ -179,7 +181,7 @@ Response Structure "cname": "foo.bar.", "ttl": 48, "description": null, - "lastUpdated": "2018-12-05 01:03:40+00" + "lastUpdated": "2018-12-05T01:03:40Z" }} diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go index c94c4b26f9..f14785e866 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go @@ -131,7 +131,16 @@ func (v *TOCDNFederation) ParamColumns() map[string]dbhelpers.WhereColumnInfo { return paramColumnInfo(*v.ReqInfo.Version) } func (v *TOCDNFederation) DeleteQuery() string { return deleteQuery() } -func (v *TOCDNFederation) UpdateQuery() string { return updateQuery() } +func (*TOCDNFederation) UpdateQuery() string { + return ` +UPDATE federation SET + cname = :cname, + ttl = :ttl, + description = :description +WHERE + id=:id +RETURNING last_updated` +} // Fufills `Identifier' interface func (fed TOCDNFederation) GetKeyFieldsInfo() []api.KeyFieldInfo { @@ -415,16 +424,8 @@ func selectByCDNName() string { return selectQuery } -func updateQuery() string { - return ` -UPDATE federation SET - cname = :cname, - ttl = :ttl, - description = :description -WHERE - id=:id -RETURNING last_updated` -} +//go:embed update.sql +var updateQuery string func deleteQuery() string { return `DELETE FROM federation WHERE id = :id` @@ -557,9 +558,15 @@ func Create(inf *api.APIInfo) (int, error, error) { return http.StatusBadRequest, fmt.Errorf("parsing request body: %w", err), nil } + // You can't set this at creation time, but if it was in the request it + // would be shown in the response - we're supposed to ignore extra fields. + // This doesn't do that exactly, but it helps. + fed.DeliveryService = nil + err := validate(fed) if err != nil { - return http.StatusBadRequest, err, nil + userErr, sysErr, code := api.ParseDBError(err) + return code, userErr, sysErr } err = inf.Tx.Tx.QueryRow(insertQuery, fed.CName, fed.TTL, fed.Description).Scan(&fed.ID, &fed.LastUpdated) @@ -569,3 +576,32 @@ func Create(inf *api.APIInfo) (int, error, error) { return inf.WriteCreatedResponse(fed, "Federation was created", "federations/"+strconv.Itoa(fed.ID)) } + +// Update handles PUT requests to `cdns/{{name}}/federations/{{id}}`. +func Update(inf *api.APIInfo) (int, error, error) { + var fed tc.CDNFederationV5 + if err := inf.DecodeBody(&fed); err != nil { + return http.StatusBadRequest, fmt.Errorf("parsing request body: %w", err), nil + } + + id := inf.IntParams["id"] + + // You can't set this via a PUT request, but if it was in the request it + // would be shown in the response - we're supposed to ignore extra fields. + // This doesn't do that exactly, but it helps. + fed.DeliveryService = nil + fed.ID = id + + err := validate(fed) + if err != nil { + return http.StatusBadRequest, err, nil + } + + err = inf.Tx.Tx.QueryRow(updateQuery, fed.CName, fed.TTL, fed.Description, id).Scan(&fed.LastUpdated) + if err != nil { + userErr, sysErr, code := api.ParseDBError(err) + return code, userErr, sysErr + } + + return inf.WriteSuccessResponse(fed, "Federation was updated") +} diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/update.sql b/traffic_ops/traffic_ops_golang/cdnfederation/update.sql new file mode 100644 index 0000000000..bdd3566b8e --- /dev/null +++ b/traffic_ops/traffic_ops_golang/cdnfederation/update.sql @@ -0,0 +1,8 @@ +UPDATE federation SET + cname = $1, + ttl = $2, + "description" = $3 +WHERE + id = $4 +RETURNING + last_updated diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go index 141d2cb115..18b877993e 100644 --- a/traffic_ops/traffic_ops_golang/routing/routes.go +++ b/traffic_ops/traffic_ops_golang/routing/routes.go @@ -398,8 +398,8 @@ func Routes(d ServerData) ([]Route, http.Handler, error) { {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `cdns/{name}/federations/?$`, Handler: api.Wrap(cdnfederation.Read, []string{"name"}, nil), 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.Wrap(cdnfederation.Create, nil, nil), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:CREATE", "FEDERATION:READ, CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 495489421931}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `cdns/{name}/federations/{id}/?$`, Handler: api.Wrap(cdnfederation.ReadID, []string{"name"}, []string{"id"}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"CDN:READ", "FEDERATION:READ", "DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 48922503232}, - {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPut, Path: `cdns/{name}/federations/{id}$`, Handler: api.UpdateHandler(&cdnfederation.TOCDNFederation{}), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:UPDATE", "FEDERATION:READ", "CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 42606546631}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodDelete, Path: `cdns/{name}/federations/{id}$`, Handler: api.DeleteHandler(&cdnfederation.TOCDNFederation{}), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:DELETE", "FEDERATION:READ", "CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 444285290231}, + {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPut, Path: `cdns/{name}/federations/{id}$`, Handler: api.Wrap(cdnfederation.Update, nil, []string{"id"}), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:UPDATE", "FEDERATION:READ", "CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 42606546631}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPost, Path: `cdns/{name}/dnsseckeys/ksk/generate$`, Handler: cdn.GenerateKSK, RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"DNS-SEC:CREATE", "CDN:UPDATE", "CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 47292428131}, From 2edfa6f19b4cea99cd31ebea934f0547911b4a58 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Thu, 14 Sep 2023 23:23:43 -0600 Subject: [PATCH 06/20] Update DELETE handler to support v5 --- .../api/v5/cdns_name_federations_id.rst | 38 ++++++++++++++----- .../cdnfederation/cdnfederations.go | 21 ++++++++-- .../cdnfederation/delete.sql | 8 ++++ .../traffic_ops_golang/routing/routes.go | 2 +- 4 files changed, 54 insertions(+), 15 deletions(-) create mode 100644 traffic_ops/traffic_ops_golang/cdnfederation/delete.sql diff --git a/docs/source/api/v5/cdns_name_federations_id.rst b/docs/source/api/v5/cdns_name_federations_id.rst index 612a45c1e7..813d400814 100644 --- a/docs/source/api/v5/cdns_name_federations_id.rst +++ b/docs/source/api/v5/cdns_name_federations_id.rst @@ -192,19 +192,24 @@ Deletes a specific federation. :Auth. Required: Yes :Roles Required: "admin" :Permissions Required: FEDERATION:DELETE, FEDERATION:READ, CDN:READ -:Response Type: ``undefined`` +:Response Type: Object + +.. versionchanged:: 5.0 + In earlier API versions, no ``response`` property is present in these responses. Request Structure ----------------- .. table:: Request Path Parameters - +------+-------------------------------------------------------------------------------------+ - | Name | Description | - +======+=====================================================================================+ - | name | The name of the CDN for which the federation identified by ``ID`` will be inspected | - +------+-------------------------------------------------------------------------------------+ - | ID | An integral, unique identifier for the federation to be inspected | - +------+-------------------------------------------------------------------------------------+ + +------+-------------------------------------------------------------------------------------------+ + | Name | Description | + +======+===========================================================================================+ + | name | The name of the CDN for which the :term:`Federation` identified by ``ID`` will be deleted | + +------+-------------------------------------------------------------------------------------------+ + | ID | An integral, unique identifier for the :term:`Federation` to be deleted | + +------+-------------------------------------------------------------------------------------------+ + +.. caution:: The name of the CDN doesn't actually matter. It doesn't even need to be the name of any existing CDN. .. code-block:: http :caption: Request Example @@ -217,6 +222,12 @@ Request Structure Response Structure ------------------ +:cname: The :abbr:`CNAME (Canonical Name)` used by the :term:`Federation` +:description: A human-readable description of the :term:`Federation`. This can be ``null`` as well as an empty string. +:lastUpdated: The date and time at which this :term:`Federation` was last modified, in :RFC:`3339` format +:ttl: :abbr:`TTL (Time to Live)` for the ``cname``, in hours + + .. code-block:: http :caption: Response Example @@ -234,7 +245,14 @@ Response Structure { "alerts": [ { - "text": "cdnfederation was deleted.", + "text": "Federation was deleted", "level": "success" } - ]} + ], + "response": { + "id": 1, + "cname": "foo.bar.", + "ttl": 48, + "description": null, + "lastUpdated": "2018-12-05T01:03:40Z" + }} diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go index f14785e866..c93966eb77 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go @@ -130,7 +130,7 @@ func paramColumnInfo(v api.Version) map[string]dbhelpers.WhereColumnInfo { func (v *TOCDNFederation) ParamColumns() map[string]dbhelpers.WhereColumnInfo { return paramColumnInfo(*v.ReqInfo.Version) } -func (v *TOCDNFederation) DeleteQuery() string { return deleteQuery() } +func (*TOCDNFederation) DeleteQuery() string { return `DELETE FROM federation WHERE id = :id` } func (*TOCDNFederation) UpdateQuery() string { return ` UPDATE federation SET @@ -427,9 +427,8 @@ func selectByCDNName() string { //go:embed update.sql var updateQuery string -func deleteQuery() string { - return `DELETE FROM federation WHERE id = :id` -} +//go:embed delete.sql +var deleteQuery string func addTenancyStmt(where string) string { if where == "" { @@ -605,3 +604,17 @@ func Update(inf *api.APIInfo) (int, error, error) { return inf.WriteSuccessResponse(fed, "Federation was updated") } + +// Delete handles DELETE requests to `cdns/{{name}}/federations/{{id}}`. +func Delete(inf *api.APIInfo) (int, error, error) { + id := inf.IntParams["id"] + + var fed tc.CDNFederationV5 + err := inf.Tx.QueryRow(deleteQuery, id).Scan(&fed.CName, &fed.Description, &fed.ID, &fed.LastUpdated, &fed.TTL) + if err != nil { + userErr, sysErr, code := api.ParseDBError(err) + return code, userErr, sysErr + } + + return inf.WriteSuccessResponse(fed, "Federation was deleted") +} diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/delete.sql b/traffic_ops/traffic_ops_golang/cdnfederation/delete.sql new file mode 100644 index 0000000000..26a28d20c0 --- /dev/null +++ b/traffic_ops/traffic_ops_golang/cdnfederation/delete.sql @@ -0,0 +1,8 @@ +DELETE FROM federation +WHERE id = $1 +RETURNING + cname, + "description", + id, + last_updated, + ttl diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go index 18b877993e..23d70f83c6 100644 --- a/traffic_ops/traffic_ops_golang/routing/routes.go +++ b/traffic_ops/traffic_ops_golang/routing/routes.go @@ -398,8 +398,8 @@ func Routes(d ServerData) ([]Route, http.Handler, error) { {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `cdns/{name}/federations/?$`, Handler: api.Wrap(cdnfederation.Read, []string{"name"}, nil), 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.Wrap(cdnfederation.Create, nil, nil), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:CREATE", "FEDERATION:READ, CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 495489421931}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodGet, Path: `cdns/{name}/federations/{id}/?$`, Handler: api.Wrap(cdnfederation.ReadID, []string{"name"}, []string{"id"}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"CDN:READ", "FEDERATION:READ", "DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 48922503232}, - {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodDelete, Path: `cdns/{name}/federations/{id}$`, Handler: api.DeleteHandler(&cdnfederation.TOCDNFederation{}), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:DELETE", "FEDERATION:READ", "CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 444285290231}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPut, Path: `cdns/{name}/federations/{id}$`, Handler: api.Wrap(cdnfederation.Update, nil, []string{"id"}), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:UPDATE", "FEDERATION:READ", "CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 42606546631}, + {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodDelete, Path: `cdns/{name}/federations/{id}$`, Handler: api.Wrap(cdnfederation.Delete, nil, []string{"id"}), RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"FEDERATION:DELETE", "FEDERATION:READ", "CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 444285290231}, {Version: api.Version{Major: 5, Minor: 0}, Method: http.MethodPost, Path: `cdns/{name}/dnsseckeys/ksk/generate$`, Handler: cdn.GenerateKSK, RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"DNS-SEC:CREATE", "CDN:UPDATE", "CDN:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 47292428131}, From 33c71c6e8662adadac9d73c2f08eaa737eb380a0 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Thu, 14 Sep 2023 23:25:31 -0600 Subject: [PATCH 07/20] Fix inaccuracies in old API version docs for cdnfederations --- docs/source/api/v3/cdns_name_federations.rst | 14 +++++--------- docs/source/api/v4/cdns_name_federations.rst | 14 +++++--------- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/docs/source/api/v3/cdns_name_federations.rst b/docs/source/api/v3/cdns_name_federations.rst index 2167fcb6a0..29cf413b1a 100644 --- a/docs/source/api/v3/cdns_name_federations.rst +++ b/docs/source/api/v3/cdns_name_federations.rst @@ -42,7 +42,9 @@ Request Structure +-----------+---------------------------------------------------------------------------------------------------------------+ | Name | Description | +===========+===============================================================================================================+ - | id | Return only the federation that has this id | + | id | Return only the :term:`Federation` that has this ID | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | cname | Return only those :term:`Federations` that have this CNAME | +-----------+---------------------------------------------------------------------------------------------------------------+ | orderby | Choose the ordering of the results - must be the name of one of the fields of the objects in the ``response`` | | | array | @@ -75,10 +77,7 @@ Response Structure :id: The integral, unique identifier for the :term:`Delivery Service` :xmlId: The :term:`Delivery Service`'s uniquely identifying 'xml_id' -:description: An optionally-present field containing a description of the field - - .. note:: This key will only be present if the description was provided when the federation was created. Refer to the ``POST`` method of this endpoint to see how federations can be created. - +:description: A human-readable description of the :term:`Federation`. This can be ``null`` as well as an empty string. :lastUpdated: The date and time at which this federation was last modified, in :ref:`non-rfc-datetime` :ttl: Time to Live (TTL) for the ``cname``, in hours @@ -158,10 +157,7 @@ Response Structure ------------------ :id: The integral, unique identifier of the :term:`Federation` :cname: The Canonical Name (CNAME) used by the federation -:description: An optionally-present field containing a description of the field - - .. note:: This key will only be present if the description was provided when the federation was created - +:description: The description of the :term:`Federation` :lastUpdated: The date and time at which this federation was last modified, in :ref:`non-rfc-datetime` :ttl: Time to Live (TTL) for the ``cname``, in hours diff --git a/docs/source/api/v4/cdns_name_federations.rst b/docs/source/api/v4/cdns_name_federations.rst index 00ac1e7f8b..5148a69337 100644 --- a/docs/source/api/v4/cdns_name_federations.rst +++ b/docs/source/api/v4/cdns_name_federations.rst @@ -43,7 +43,9 @@ Request Structure +-----------+---------------------------------------------------------------------------------------------------------------+ | Name | Description | +===========+===============================================================================================================+ - | id | Return only the federation that has this id | + | id | Return only the :term:`Federation` that has this ID | + +-----------+---------------------------------------------------------------------------------------------------------------+ + | cname | Return only those :term:`Federations` that have this CNAME | +-----------+---------------------------------------------------------------------------------------------------------------+ | orderby | Choose the ordering of the results - must be the name of one of the fields of the objects in the ``response`` | | | array | @@ -76,10 +78,7 @@ Response Structure :id: The integral, unique identifier for the :term:`Delivery Service` :xmlId: The :term:`Delivery Service`'s uniquely identifying 'xml_id' -:description: An optionally-present field containing a description of the field - - .. note:: This key will only be present if the description was provided when the federation was created. Refer to the ``POST`` method of this endpoint to see how federations can be created. - +:description: A human-readable description of the :term:`Federation`. This can be ``null`` as well as an empty string. :lastUpdated: The date and time at which this federation was last modified, in :ref:`non-rfc-datetime` :ttl: Time to Live (TTL) for the ``cname``, in hours @@ -160,10 +159,7 @@ Response Structure ------------------ :id: The intergral, unique identifier of the :term:`Federation` :cname: The Canonical Name (CNAME) used by the federation -:description: An optionally-present field containing a description of the field - - .. note:: This key will only be present if the description was provided when the federation was created - +:description: The description of the :term:`Federation` :lastUpdated: The date and time at which this federation was last modified, in :ref:`non-rfc-datetime` :ttl: Time to Live (TTL) for the ``cname``, in hours From ee6538d43885e8ac4f2bd96080a9e96ea310ca50 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Thu, 14 Sep 2023 23:26:20 -0600 Subject: [PATCH 08/20] Fix RFC roles not using grave accents --- docs/source/api/index.rst | 2 +- docs/source/api/v4/deliveryservices.rst | 4 ++-- docs/source/api/v4/deliveryservices_id.rst | 2 +- docs/source/api/v4/deliveryservices_id_safe.rst | 2 +- docs/source/api/v4/servers_id_deliveryservices.rst | 2 +- docs/source/api/v5/coordinates.rst | 8 ++++---- docs/source/api/v5/deliveryservices.rst | 4 ++-- docs/source/api/v5/deliveryservices_id.rst | 2 +- docs/source/api/v5/deliveryservices_id_safe.rst | 2 +- docs/source/api/v5/servers.rst | 4 ++-- docs/source/api/v5/servers_id.rst | 4 ++-- docs/source/api/v5/servers_id_deliveryservices.rst | 2 +- docs/source/api/v5/statuses_id.rst | 2 +- 13 files changed, 20 insertions(+), 20 deletions(-) diff --git a/docs/source/api/index.rst b/docs/source/api/index.rst index 136db1986e..808847a294 100644 --- a/docs/source/api/index.rst +++ b/docs/source/api/index.rst @@ -89,7 +89,7 @@ The following, reserved properties of ``summary`` are guaranteed to always have Traffic Ops's Custom Date/Time Format ------------------------------------- -Traffic Ops will often return responses from its API that include dates. As of the time of this writing, the vast majority of those dates are written in a non-:rfc:3339 format (this is tracked by :issue:`5911`). This is most commonly the case in the ``last_updated`` properties of objects returned as JSON-encoded documents. The format used is :samp:`{YYYY}-{MM}-{DD} {hh}:{mm}:{ss}±{ZZ}`, where ``YYYY`` is the 4-digit year, ``MM`` is the two-digit (zero padded) month, ``DD`` is the two-digit (zero padded) day of the month, ``hh`` is the two-digit (zero padded) hour of the day, ``mm`` is the two-digit (zero padded) minute of the hour, ``ss`` is the two-digit (zero padded) second of the minute, and ``ZZ`` is the two-digit (zero padded) timezone offset in hours of the date/time's local timezone from UTC (the offset can be positive or negative as indicated by a ``+`` or a ``-`` directly before it, where the sample has a ``±``). +Traffic Ops will often return responses from its API that include dates. As of the time of this writing, the vast majority of those dates are written in a non-:RFC:`3339` format (this is tracked by :issue:`5911`). This is most commonly the case in the ``last_updated`` properties of objects returned as JSON-encoded documents. The format used is :samp:`{YYYY}-{MM}-{DD} {hh}:{mm}:{ss}±{ZZ}`, where ``YYYY`` is the 4-digit year, ``MM`` is the two-digit (zero padded) month, ``DD`` is the two-digit (zero padded) day of the month, ``hh`` is the two-digit (zero padded) hour of the day, ``mm`` is the two-digit (zero padded) minute of the hour, ``ss`` is the two-digit (zero padded) second of the minute, and ``ZZ`` is the two-digit (zero padded) timezone offset in hours of the date/time's local timezone from UTC (the offset can be positive or negative as indicated by a ``+`` or a ``-`` directly before it, where the sample has a ``±``). .. note:: In practice, all Traffic Ops API responses use the UTC timezone (offset ``+00``), but do note that this custom format is not capable of representing all timezones. diff --git a/docs/source/api/v4/deliveryservices.rst b/docs/source/api/v4/deliveryservices.rst index 9f98b62155..df87c5d9a5 100644 --- a/docs/source/api/v4/deliveryservices.rst +++ b/docs/source/api/v4/deliveryservices.rst @@ -116,7 +116,7 @@ Response Structure :innerHeaderRewrite: A set of :ref:`ds-inner-header-rw-rules` :ipv6RoutingEnabled: A boolean that defines the :ref:`ds-ipv6-routing` setting on this :term:`Delivery Service` :lastHeaderRewrite: A set of :ref:`ds-last-header-rw-rules` -:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :rfc:3339 format +:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :RFC:`3339` format .. versionchanged:: 4.0 Prior to API version 4.0, this property used :ref:`non-rfc-datetime`. @@ -473,7 +473,7 @@ Response Structure :innerHeaderRewrite: A set of :ref:`ds-inner-header-rw-rules` :ipv6RoutingEnabled: A boolean that defines the :ref:`ds-ipv6-routing` setting on this :term:`Delivery Service` :lastHeaderRewrite: A set of :ref:`ds-last-header-rw-rules` -:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :rfc:3339 format +:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :RFC:`3339` format .. versionchanged:: 4.0 Prior to API version 4.0, this property used :ref:`non-rfc-datetime`. diff --git a/docs/source/api/v4/deliveryservices_id.rst b/docs/source/api/v4/deliveryservices_id.rst index 0865427028..a677c7da27 100644 --- a/docs/source/api/v4/deliveryservices_id.rst +++ b/docs/source/api/v4/deliveryservices_id.rst @@ -223,7 +223,7 @@ Response Structure :innerHeaderRewrite: A set of :ref:`ds-inner-header-rw-rules` :ipv6RoutingEnabled: A boolean that defines the :ref:`ds-ipv6-routing` setting on this :term:`Delivery Service` :lastHeaderRewrite: A set of :ref:`ds-last-header-rw-rules` -:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :rfc:3339 format +:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :RFC:`3339` format .. versionchanged:: 4.0 Prior to API version 4.0, this property used :ref:`non-rfc-datetime`. diff --git a/docs/source/api/v4/deliveryservices_id_safe.rst b/docs/source/api/v4/deliveryservices_id_safe.rst index 8bafe33110..bbbc1eea6e 100644 --- a/docs/source/api/v4/deliveryservices_id_safe.rst +++ b/docs/source/api/v4/deliveryservices_id_safe.rst @@ -95,7 +95,7 @@ Response Structure :innerHeaderRewrite: A set of :ref:`ds-inner-header-rw-rules` :ipv6RoutingEnabled: A boolean that defines the :ref:`ds-ipv6-routing` setting on this :term:`Delivery Service` :lastHeaderRewrite: A set of :ref:`ds-last-header-rw-rules` -:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :rfc:3339 format +:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :RFC:`3339` format .. versionchanged:: 4.0 Prior to API version 4.0, this property used :ref:`non-rfc-datetime`. diff --git a/docs/source/api/v4/servers_id_deliveryservices.rst b/docs/source/api/v4/servers_id_deliveryservices.rst index 84515033c4..cc4aac7673 100644 --- a/docs/source/api/v4/servers_id_deliveryservices.rst +++ b/docs/source/api/v4/servers_id_deliveryservices.rst @@ -103,7 +103,7 @@ Response Structure :innerHeaderRewrite: A set of :ref:`ds-inner-header-rw-rules` :ipv6RoutingEnabled: A boolean that defines the :ref:`ds-ipv6-routing` setting on this :term:`Delivery Service` :lastHeaderRewrite: A set of :ref:`ds-last-header-rw-rules` -:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :rfc:3339 format +:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :RFC:`3339` format .. versionchanged:: 4.0 Prior to API version 4.0, this property used :ref:`non-rfc-datetime`. diff --git a/docs/source/api/v5/coordinates.rst b/docs/source/api/v5/coordinates.rst index dc966fbeab..8f7c885bce 100644 --- a/docs/source/api/v5/coordinates.rst +++ b/docs/source/api/v5/coordinates.rst @@ -56,7 +56,7 @@ Request Structure Response Structure ------------------ :id: Integral, unique, identifier for this coordinate pair -:lastUpdated: The time and date at which this entry was last updated, in :rfc:3339 format +:lastUpdated: The time and date at which this entry was last updated, in :RFC:`3339` format :latitude: Latitude of the coordinate :longitude: Longitude of the coordinate :name: The name of the coordinate - typically this just reflects the name of the Cache Group for which the coordinate was created @@ -159,7 +159,7 @@ Request Structure Response Structure ------------------ :id: Integral, unique, identifier for the newly created coordinate pair -:lastUpdated: The time and date at which this entry was last updated, in :rfc:3339 format +:lastUpdated: The time and date at which this entry was last updated, in :RFC:`3339` format :latitude: Latitude of the newly created coordinate :longitude: Longitude of the newly created coordinate :name: The name of the coordinate @@ -233,7 +233,7 @@ Request Structure Response Structure ------------------ :id: Integral, unique, identifier for the coordinate pair -:lastUpdated: The time and date at which this entry was last updated, in :rfc:3339 format +:lastUpdated: The time and date at which this entry was last updated, in :RFC:`3339` format :latitude: Latitude of the coordinate :longitude: Longitude of the coordinate :name: The name of the coordinate @@ -279,7 +279,7 @@ Deletes a coordinate Request Structure ----------------- :id: Integral, unique, identifier for the coordinate pair -:lastUpdated: The time and date at which this entry was last updated, in :rfc:3339 format +:lastUpdated: The time and date at which this entry was last updated, in :RFC:`3339` format :latitude: Latitude of the coordinate :longitude: Longitude of the coordinate :name: The name of the coordinate diff --git a/docs/source/api/v5/deliveryservices.rst b/docs/source/api/v5/deliveryservices.rst index bc75722c32..21a0c01799 100644 --- a/docs/source/api/v5/deliveryservices.rst +++ b/docs/source/api/v5/deliveryservices.rst @@ -116,7 +116,7 @@ Response Structure :innerHeaderRewrite: A set of :ref:`ds-inner-header-rw-rules` :ipv6RoutingEnabled: A boolean that defines the :ref:`ds-ipv6-routing` setting on this :term:`Delivery Service` :lastHeaderRewrite: A set of :ref:`ds-last-header-rw-rules` -:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :rfc:3339 format +:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :RFC:`3339` format :logsEnabled: A boolean that defines the :ref:`ds-logs-enabled` setting on this :term:`Delivery Service` :longDesc: The :ref:`ds-longdesc` of this :term:`Delivery Service` :matchList: The :term:`Delivery Service`'s :ref:`ds-matchlist` @@ -459,7 +459,7 @@ Response Structure :innerHeaderRewrite: A set of :ref:`ds-inner-header-rw-rules` :ipv6RoutingEnabled: A boolean that defines the :ref:`ds-ipv6-routing` setting on this :term:`Delivery Service` :lastHeaderRewrite: A set of :ref:`ds-last-header-rw-rules` -:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :rfc:3339 format +:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :RFC:`3339` format :logsEnabled: A boolean that defines the :ref:`ds-logs-enabled` setting on this :term:`Delivery Service` :longDesc: The :ref:`ds-longdesc` of this :term:`Delivery Service` :matchList: The :term:`Delivery Service`'s :ref:`ds-matchlist` diff --git a/docs/source/api/v5/deliveryservices_id.rst b/docs/source/api/v5/deliveryservices_id.rst index 52e41c83ae..a832b67bd4 100644 --- a/docs/source/api/v5/deliveryservices_id.rst +++ b/docs/source/api/v5/deliveryservices_id.rst @@ -218,7 +218,7 @@ Response Structure :innerHeaderRewrite: A set of :ref:`ds-inner-header-rw-rules` :ipv6RoutingEnabled: A boolean that defines the :ref:`ds-ipv6-routing` setting on this :term:`Delivery Service` :lastHeaderRewrite: A set of :ref:`ds-last-header-rw-rules` -:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :rfc:3339 format +:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :RFC:`3339` format :logsEnabled: A boolean that defines the :ref:`ds-logs-enabled` setting on this :term:`Delivery Service` :longDesc: The :ref:`ds-longdesc` of this :term:`Delivery Service` :matchList: The :term:`Delivery Service`'s :ref:`ds-matchlist` diff --git a/docs/source/api/v5/deliveryservices_id_safe.rst b/docs/source/api/v5/deliveryservices_id_safe.rst index d5b4cdfe7b..3405c693d6 100644 --- a/docs/source/api/v5/deliveryservices_id_safe.rst +++ b/docs/source/api/v5/deliveryservices_id_safe.rst @@ -95,7 +95,7 @@ Response Structure :innerHeaderRewrite: A set of :ref:`ds-inner-header-rw-rules` :ipv6RoutingEnabled: A boolean that defines the :ref:`ds-ipv6-routing` setting on this :term:`Delivery Service` :lastHeaderRewrite: A set of :ref:`ds-last-header-rw-rules` -:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :rfc:3339 format +:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :RFC:`3339` format :logsEnabled: A boolean that defines the :ref:`ds-logs-enabled` setting on this :term:`Delivery Service` :longDesc: The :ref:`ds-longdesc` of this :term:`Delivery Service` :matchList: The :term:`Delivery Service`'s :ref:`ds-matchlist` diff --git a/docs/source/api/v5/servers.rst b/docs/source/api/v5/servers.rst index 1166d01146..cbf5665c06 100644 --- a/docs/source/api/v5/servers.rst +++ b/docs/source/api/v5/servers.rst @@ -142,7 +142,7 @@ Response Structure :routerPortName: The human-readable name of the router responsible for reaching this server's interface. :routerPortName: The human-readable name of the port used by the router responsible for reaching this server's interface. -:lastUpdated: The date and time at which this server description was last modified, in :RFC:3339 format +:lastUpdated: The date and time at which this server description was last modified, in :RFC:`3339` format .. versionchanged:: 5.0 In earlier versions of the API, this field was given in :ref:`non-rfc-datetime`. @@ -480,7 +480,7 @@ Response Structure :routerPortName: The human-readable name of the router responsible for reaching this server's interface. :routerPortName: The human-readable name of the port used by the router responsible for reaching this server's interface. -:lastUpdated: The date and time at which this server description was last modified, in :RFC:3339 format +:lastUpdated: The date and time at which this server description was last modified, in :RFC:`3339` format .. versionchanged:: 5.0 In earlier versions of the API, this field was given in :ref:`non-rfc-datetime`. diff --git a/docs/source/api/v5/servers_id.rst b/docs/source/api/v5/servers_id.rst index f6275e6484..2573a4260a 100644 --- a/docs/source/api/v5/servers_id.rst +++ b/docs/source/api/v5/servers_id.rst @@ -236,7 +236,7 @@ Response Structure :routerPortName: The human-readable name of the router responsible for reaching this server's interface. :routerPortName: The human-readable name of the port used by the router responsible for reaching this server's interface. -:lastUpdated: The date and time at which this server description was last modified, in :RFC:3339 format +:lastUpdated: The date and time at which this server description was last modified, in :RFC:`3339` format .. versionchanged:: 5.0 In earlier versions of the API, this field was given in :ref:`non-rfc-datetime`. @@ -466,7 +466,7 @@ Response Structure :routerPortName: The human-readable name of the router responsible for reaching this server's interface. :routerPortName: The human-readable name of the port used by the router responsible for reaching this server's interface. -:lastUpdated: The date and time at which this server description was last modified, in :RFC:3339 format +:lastUpdated: The date and time at which this server description was last modified, in :RFC:`3339` format .. versionchanged:: 5.0 In earlier versions of the API, this field was given in :ref:`non-rfc-datetime`. diff --git a/docs/source/api/v5/servers_id_deliveryservices.rst b/docs/source/api/v5/servers_id_deliveryservices.rst index d62b6516d6..a43dfd50dd 100644 --- a/docs/source/api/v5/servers_id_deliveryservices.rst +++ b/docs/source/api/v5/servers_id_deliveryservices.rst @@ -102,7 +102,7 @@ Response Structure :innerHeaderRewrite: A set of :ref:`ds-inner-header-rw-rules` :ipv6RoutingEnabled: A boolean that defines the :ref:`ds-ipv6-routing` setting on this :term:`Delivery Service` :lastHeaderRewrite: A set of :ref:`ds-last-header-rw-rules` -:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :rfc:3339 format +:lastUpdated: The date and time at which this :term:`Delivery Service` was last updated, in :RFC:`3339` format :logsEnabled: A boolean that defines the :ref:`ds-logs-enabled` setting on this :term:`Delivery Service` :longDesc: The :ref:`ds-longdesc` of this :term:`Delivery Service` :matchList: The :term:`Delivery Service`'s :ref:`ds-matchlist` diff --git a/docs/source/api/v5/statuses_id.rst b/docs/source/api/v5/statuses_id.rst index dcca4425b3..4742251aff 100644 --- a/docs/source/api/v5/statuses_id.rst +++ b/docs/source/api/v5/statuses_id.rst @@ -48,7 +48,7 @@ Response Structure ------------------ :description: A short description of the status :id: The integral, unique identifier of this status -:lastUpdated: The date and time at which this status was last modified, in :rfc:3339 format +:lastUpdated: The date and time at which this status was last modified, in :RFC:`3339` format :name: The name of the status .. code-block:: http From a7ccd0e276da89028e7bd8c05db470344d954317 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Thu, 14 Sep 2023 23:36:24 -0600 Subject: [PATCH 09/20] Update client and client tests --- .../testing/api/v5/cdn_federations_test.go | 54 +++++++++---------- traffic_ops/testing/api/v5/tc-fixtures.json | 6 +-- .../testing/api/v5/traffic_control_test.go | 2 +- traffic_ops/v5-client/cdnfederations.go | 16 +++--- 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/traffic_ops/testing/api/v5/cdn_federations_test.go b/traffic_ops/testing/api/v5/cdn_federations_test.go index 2878b3dfe8..1bad115414 100644 --- a/traffic_ops/testing/api/v5/cdn_federations_test.go +++ b/traffic_ops/testing/api/v5/cdn_federations_test.go @@ -45,7 +45,7 @@ func TestCDNFederations(t *testing.T) { currentTimeRFC := currentTime.Format(time.RFC1123) tomorrow := currentTime.AddDate(0, 0, 1).Format(time.RFC1123) - methodTests := utils.TestCase[client.Session, client.RequestOptions, tc.CDNFederation]{ + methodTests := utils.TestCase[client.Session, client.RequestOptions, tc.CDNFederationV5]{ "GET": { "NOT MODIFIED when NO CHANGES made": { ClientSession: TOSession, @@ -107,9 +107,9 @@ func TestCDNFederations(t *testing.T) { "OK when VALID request": { EndpointID: GetFederationID(t, "google.com."), ClientSession: TOSession, - RequestBody: tc.CDNFederation{ - CName: util.Ptr("new.cname."), - TTL: util.Ptr(34), + RequestBody: tc.CDNFederationV5{ + CName: "new.cname.", + TTL: 34, Description: util.Ptr("updated"), }, Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCDNFederationUpdateFields(map[string]interface{}{"CName": "new.cname."})), @@ -118,9 +118,9 @@ func TestCDNFederations(t *testing.T) { EndpointID: GetFederationID(t, "booya.com."), ClientSession: TOSession, RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfUnmodifiedSince: {currentTimeRFC}}}, - RequestBody: tc.CDNFederation{ - CName: util.Ptr("booya.com."), - TTL: util.Ptr(34), + RequestBody: tc.CDNFederationV5{ + CName: "booya.com.", + TTL: 34, Description: util.Ptr("fooya"), }, Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)), @@ -128,9 +128,9 @@ func TestCDNFederations(t *testing.T) { "PRECONDITION FAILED when updating with IFMATCH ETAG Header": { EndpointID: GetFederationID(t, "booya.com."), ClientSession: TOSession, - RequestBody: tc.CDNFederation{ - CName: util.Ptr("new.cname."), - TTL: util.Ptr(34), + RequestBody: tc.CDNFederationV5{ + CName: "new.cname.", + TTL: 34, Description: util.Ptr("updated"), }, RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}}}, @@ -145,7 +145,7 @@ func TestCDNFederations(t *testing.T) { switch method { case "GET": t.Run(name, func(t *testing.T) { - resp, reqInf, err := testCase.ClientSession.GetCDNFederationsByName(cdnName, testCase.RequestOpts) + resp, reqInf, err := testCase.ClientSession.GetCDNFederations(cdnName, testCase.RequestOpts) for _, check := range testCase.Expectations { check(t, reqInf, resp.Response, resp.Alerts, err) } @@ -181,12 +181,11 @@ func TestCDNFederations(t *testing.T) { func validateCDNFederationUpdateFields(expectedResp map[string]interface{}) utils.CkReqFunc { return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) { assert.RequireNotNil(t, resp, "Expected CDN Federation response to not be nil.") - CDNFederationResp := resp.(tc.CDNFederation) + CDNFederationResp := resp.(tc.CDNFederationV5) for field, expected := range expectedResp { switch field { case "CName": - assert.RequireNotNil(t, CDNFederationResp.CName, "Expected CName to not be nil.") - assert.Equal(t, expected, *CDNFederationResp.CName, "Expected CName to be %v, but got %s", expected, *CDNFederationResp.CName) + assert.Equal(t, expected, CDNFederationResp.CName, "Expected CName to be %v, but got %s", expected, CDNFederationResp.CName) default: t.Errorf("Expected field: %v, does not exist in response", field) } @@ -196,11 +195,11 @@ func validateCDNFederationUpdateFields(expectedResp map[string]interface{}) util func validateCDNFederationPagination(paginationParam string) utils.CkReqFunc { return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) { - paginationResp := resp.([]tc.CDNFederation) + paginationResp := resp.([]tc.CDNFederationV5) opts := client.NewRequestOptions() opts.QueryParameters.Set("orderby", "id") - respBase, _, err := TOSession.GetCDNFederationsByName(cdnName, opts) + respBase, _, err := TOSession.GetCDNFederations(cdnName, opts) assert.RequireNoError(t, err, "Cannot get Federation Users: %v - alerts: %+v", err, respBase.Alerts) CDNfederations := respBase.Response @@ -220,10 +219,9 @@ func validateCDNFederationCNameSort() utils.CkReqFunc { return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, alerts tc.Alerts, _ error) { assert.RequireNotNil(t, resp, "Expected CDN Federation response to not be nil.") var federationCNames []string - CDNFederationResp := resp.([]tc.CDNFederation) - for _, CDNFederation := range CDNFederationResp { - assert.RequireNotNil(t, CDNFederation.CName, "Expected CDN Federation CName to not be nil.") - federationCNames = append(federationCNames, *CDNFederation.CName) + CDNFederationResp := resp.([]tc.CDNFederationV5) + for _, CDNFederationV5 := range CDNFederationResp { + federationCNames = append(federationCNames, CDNFederationV5.CName) } assert.Equal(t, true, sort.StringsAreSorted(federationCNames), "List is not sorted by their names: %v", federationCNames) } @@ -233,9 +231,9 @@ func validateCDNFederationIDDescSort() utils.CkReqFunc { return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, alerts tc.Alerts, _ error) { assert.RequireNotNil(t, resp, "Expected CDN Federation response to not be nil.") var CDNFederationIDs []int - CDNFederationResp := resp.([]tc.CDNFederation) + CDNFederationResp := resp.([]tc.CDNFederationV5) for _, federation := range CDNFederationResp { - CDNFederationIDs = append([]int{*federation.ID}, CDNFederationIDs...) + CDNFederationIDs = append([]int{federation.ID}, CDNFederationIDs...) } assert.Equal(t, true, sort.IntsAreSorted(CDNFederationIDs), "List is not sorted by their ids: %v", CDNFederationIDs) } @@ -249,16 +247,14 @@ func GetFederationID(t *testing.T, cname string) func() int { } } -func setFederationID(t *testing.T, cdnFederation tc.CDNFederation) { - assert.RequireNotNil(t, cdnFederation.CName, "Federation CName was nil after posting.") - assert.RequireNotNil(t, cdnFederation.ID, "Federation ID was nil after posting.") - fedIDs[*cdnFederation.CName] = *cdnFederation.ID +func setFederationID(t *testing.T, cdnFederation tc.CDNFederationV5) { + fedIDs[cdnFederation.CName] = cdnFederation.ID } func CreateTestCDNFederations(t *testing.T) { for _, federation := range testData.Federations { opts := client.NewRequestOptions() - opts.QueryParameters.Set("xmlId", *federation.DeliveryServiceIDs.XmlId) + opts.QueryParameters.Set("xmlID", federation.DeliveryService.XMLID) dsResp, _, err := TOSession.GetDeliveryServices(opts) assert.RequireNoError(t, err, "Could not get Delivery Service by XML ID: %v", err) assert.RequireEqual(t, 1, len(dsResp.Response), "Expected one Delivery Service, but got %d", len(dsResp.Response)) @@ -271,7 +267,7 @@ func CreateTestCDNFederations(t *testing.T) { setFederationID(t, resp.Response) assert.RequireNotNil(t, resp.Response.ID, "Federation ID was nil after posting.") assert.RequireNotNil(t, dsResp.Response[0].ID, "Delivery Service ID was nil.") - _, _, err = TOSession.CreateFederationDeliveryServices(*resp.Response.ID, []int{*dsResp.Response[0].ID}, false, client.NewRequestOptions()) + _, _, err = TOSession.CreateFederationDeliveryServices(resp.Response.ID, []int{*dsResp.Response[0].ID}, false, client.NewRequestOptions()) assert.NoError(t, err, "Could not create Federation Delivery Service: %v", err) } } @@ -283,7 +279,7 @@ func DeleteTestCDNFederations(t *testing.T) { assert.NoError(t, err, "Cannot delete federation #%d: %v - alerts: %+v", id, err, resp.Alerts) opts.QueryParameters.Set("id", strconv.Itoa(id)) - data, _, err := TOSession.GetCDNFederationsByName(cdnName, opts) + data, _, err := TOSession.GetCDNFederations(cdnName, opts) assert.Equal(t, 0, len(data.Response), "expected federation to be deleted") } fedIDs = make(map[string]int) // reset the global variable for the next test diff --git a/traffic_ops/testing/api/v5/tc-fixtures.json b/traffic_ops/testing/api/v5/tc-fixtures.json index 3fd3bba2ae..6672e3922d 100644 --- a/traffic_ops/testing/api/v5/tc-fixtures.json +++ b/traffic_ops/testing/api/v5/tc-fixtures.json @@ -1867,7 +1867,7 @@ "ttl": 48, "description": "the description", "deliveryService": { - "xmlId": "ds1" + "xmlID": "ds1" } }, { @@ -1875,7 +1875,7 @@ "ttl": 34, "description": "fooya", "deliveryService": { - "xmlId": "ds1" + "xmlID": "ds1" } }, { @@ -1883,7 +1883,7 @@ "ttl": 30, "description": "google", "deliveryService": { - "xmlId": "ds1" + "xmlID": "ds1" } } ], diff --git a/traffic_ops/testing/api/v5/traffic_control_test.go b/traffic_ops/testing/api/v5/traffic_control_test.go index ae2a4452ba..3fca22db40 100644 --- a/traffic_ops/testing/api/v5/traffic_control_test.go +++ b/traffic_ops/testing/api/v5/traffic_control_test.go @@ -35,7 +35,7 @@ type TrafficControl struct { DeliveryServiceServerAssignments []tc.DeliveryServiceServers `json:"deliveryServiceServerAssignments"` TopologyBasedDeliveryServicesRequiredCapabilities []tc.DeliveryServicesRequiredCapability `json:"topologyBasedDeliveryServicesRequiredCapabilities"` Divisions []tc.DivisionV5 `json:"divisions"` - Federations []tc.CDNFederation `json:"federations"` + Federations []tc.CDNFederationV5 `json:"federations"` FederationResolvers []tc.FederationResolverV5 `json:"federation_resolvers"` Jobs []tc.InvalidationJobCreateV4 `json:"jobs"` Origins []tc.OriginV5 `json:"origins"` diff --git a/traffic_ops/v5-client/cdnfederations.go b/traffic_ops/v5-client/cdnfederations.go index dcd6bfcb66..c574af79eb 100644 --- a/traffic_ops/v5-client/cdnfederations.go +++ b/traffic_ops/v5-client/cdnfederations.go @@ -30,8 +30,8 @@ import ( // CreateCDNFederation creates the given Federation in the CDN with the given // name. -func (to *Session) CreateCDNFederation(f tc.CDNFederation, cdnName string, opts RequestOptions) (tc.CreateCDNFederationResponse, toclientlib.ReqInf, error) { - var data tc.CreateCDNFederationResponse +func (to *Session) CreateCDNFederation(f tc.CDNFederationV5, cdnName string, opts RequestOptions) (tc.CDNFederationV5Response, toclientlib.ReqInf, error) { + var data tc.CDNFederationV5Response route := "/cdns/" + url.PathEscape(cdnName) + "/federations" inf, err := to.post(route, opts, f, &data) return data, inf, err @@ -39,8 +39,8 @@ func (to *Session) CreateCDNFederation(f tc.CDNFederation, cdnName string, opts // GetCDNFederationsByName retrieves all Federations in the CDN with the given // name. -func (to *Session) GetCDNFederationsByName(cdnName string, opts RequestOptions) (tc.CDNFederationResponse, toclientlib.ReqInf, error) { - var data tc.CDNFederationResponse +func (to *Session) GetCDNFederations(cdnName string, opts RequestOptions) (tc.CDNFederationsV5Response, toclientlib.ReqInf, error) { + var data tc.CDNFederationsV5Response route := "/cdns/" + url.PathEscape(cdnName) + "/federations" inf, err := to.get(route, opts, &data) return data, inf, err @@ -48,8 +48,8 @@ func (to *Session) GetCDNFederationsByName(cdnName string, opts RequestOptions) // UpdateCDNFederation replaces the Federation with the given ID in the CDN // with the given name with the provided Federation. -func (to *Session) UpdateCDNFederation(f tc.CDNFederation, cdnName string, id int, opts RequestOptions) (tc.UpdateCDNFederationResponse, toclientlib.ReqInf, error) { - var data tc.UpdateCDNFederationResponse +func (to *Session) UpdateCDNFederation(f tc.CDNFederationV5, cdnName string, id int, opts RequestOptions) (tc.CDNFederationV5Response, toclientlib.ReqInf, error) { + var data tc.CDNFederationV5Response route := fmt.Sprintf("/cdns/%s/federations/%d", url.PathEscape(cdnName), id) inf, err := to.put(route, opts, f, &data) return data, inf, err @@ -57,8 +57,8 @@ func (to *Session) UpdateCDNFederation(f tc.CDNFederation, cdnName string, id in // DeleteCDNFederation deletes the Federation with the given ID in the CDN // with the given name. -func (to *Session) DeleteCDNFederation(cdnName string, id int, opts RequestOptions) (tc.DeleteCDNFederationResponse, toclientlib.ReqInf, error) { - var data tc.DeleteCDNFederationResponse +func (to *Session) DeleteCDNFederation(cdnName string, id int, opts RequestOptions) (tc.CDNFederationV5Response, toclientlib.ReqInf, error) { + var data tc.CDNFederationV5Response route := fmt.Sprintf("/cdns/%s/federations/%d", url.PathEscape(cdnName), id) inf, err := to.del(route, opts, &data) return data, inf, err From b897ca0d388e2dd670db6f1ed3a9ff3f6424a3e1 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Thu, 14 Sep 2023 23:48:36 -0600 Subject: [PATCH 10/20] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e3bacca95..e86a2b1ba1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ## [unreleased] - [#7665](https://github.com/apache/trafficcontrol/pull/7665) *Automation* Changes to Ansible role dataset_loader to add ATS 9 support - [#7802](https://github.com/apache/trafficcontrol/pull/7802) *Traffic Control Health Client* Fixed ReadMe.md typos and duplicates. +- [#7806](https://github.com/apache/trafficcontrol/pull/7806) *Traffic Ops* `cdns/{{name}}/federations` and `cdns/{{name}}/federations/{{ID}}` use RFC3339 timestamps in APIv5 ### Added - [#7672](https://github.com/apache/trafficcontrol/pull/7672) *Traffic Control Health Client* Added peer monitor flag while using `strategies.yaml`. - [#7609](https://github.com/apache/trafficcontrol/pull/7609) *Traffic Portal* Added Scope Query Param to SSO login. From 15244971920e8c028308e5633b9909333728d062 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 15 Sep 2023 09:34:25 -0600 Subject: [PATCH 11/20] Update TOTest --- lib/go-tc/totest/federations.go | 12 +++++------- lib/go-tc/totest/traffic_control.go | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/go-tc/totest/federations.go b/lib/go-tc/totest/federations.go index 99a1be0bce..9f90efab36 100644 --- a/lib/go-tc/totest/federations.go +++ b/lib/go-tc/totest/federations.go @@ -45,16 +45,14 @@ func GetFederationID(t *testing.T, cname string) func() int { } } -func setFederationID(t *testing.T, cdnFederation tc.CDNFederation) { - assert.RequireNotNil(t, cdnFederation.CName, "Federation CName was nil after posting.") - assert.RequireNotNil(t, cdnFederation.ID, "Federation ID was nil after posting.") - fedIDs[*cdnFederation.CName] = *cdnFederation.ID +func setFederationID(t *testing.T, cdnFederation tc.CDNFederationV5) { + fedIDs[cdnFederation.CName] = cdnFederation.ID } func CreateTestCDNFederations(t *testing.T, cl *toclient.Session, dat TrafficControl) { for _, federation := range dat.Federations { opts := toclient.NewRequestOptions() - opts.QueryParameters.Set("xmlId", *federation.DeliveryServiceIDs.XmlId) + opts.QueryParameters.Set("xmlId", federation.DeliveryService.XMLID) dsResp, _, err := cl.GetDeliveryServices(opts) assert.RequireNoError(t, err, "Could not get Delivery Service by XML ID: %v", err) assert.RequireEqual(t, 1, len(dsResp.Response), "Expected one Delivery Service, but got %d", len(dsResp.Response)) @@ -67,7 +65,7 @@ func CreateTestCDNFederations(t *testing.T, cl *toclient.Session, dat TrafficCon setFederationID(t, resp.Response) assert.RequireNotNil(t, resp.Response.ID, "Federation ID was nil after posting.") assert.RequireNotNil(t, dsResp.Response[0].ID, "Delivery Service ID was nil.") - _, _, err = cl.CreateFederationDeliveryServices(*resp.Response.ID, []int{*dsResp.Response[0].ID}, false, toclient.NewRequestOptions()) + _, _, err = cl.CreateFederationDeliveryServices(resp.Response.ID, []int{*dsResp.Response[0].ID}, false, toclient.NewRequestOptions()) assert.NoError(t, err, "Could not create Federation Delivery Service: %v", err) } } @@ -79,7 +77,7 @@ func DeleteTestCDNFederations(t *testing.T, cl *toclient.Session) { assert.NoError(t, err, "Cannot delete federation #%d: %v - alerts: %+v", id, err, resp.Alerts) opts.QueryParameters.Set("id", strconv.Itoa(id)) - data, _, err := cl.GetCDNFederationsByName(FederationCDNName, opts) + data, _, err := cl.GetCDNFederations(FederationCDNName, opts) assert.Equal(t, 0, len(data.Response), "expected federation to be deleted") } fedIDs = make(map[string]int) // reset the global variable for the next test diff --git a/lib/go-tc/totest/traffic_control.go b/lib/go-tc/totest/traffic_control.go index a6260a48d9..bd384718ff 100644 --- a/lib/go-tc/totest/traffic_control.go +++ b/lib/go-tc/totest/traffic_control.go @@ -37,7 +37,7 @@ type TrafficControl struct { DeliveryServiceServerAssignments []tc.DeliveryServiceServerV5 `json:"deliveryServiceServerAssignments"` TopologyBasedDeliveryServicesRequiredCapabilities []tc.DeliveryServicesRequiredCapability `json:"topologyBasedDeliveryServicesRequiredCapabilities"` Divisions []tc.DivisionV5 `json:"divisions"` - Federations []tc.CDNFederation `json:"federations"` + Federations []tc.CDNFederationV5 `json:"federations"` FederationResolvers []tc.FederationResolverV5 `json:"federation_resolvers"` Jobs []tc.InvalidationJobCreateV4 `json:"jobs"` Origins []tc.OriginV5 `json:"origins"` From 05509fdf554dc63213fb6a0f472f96ac288823e5 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 15 Sep 2023 09:36:39 -0600 Subject: [PATCH 12/20] Fix dependency_license pattern --- .dependency_license | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.dependency_license b/.dependency_license index 8374928b4f..0adb340f0c 100644 --- a/.dependency_license +++ b/.dependency_license @@ -55,7 +55,7 @@ traffic_router/core/src/test/resources/api/.*/steering*, Apache-2.0 traffic_router/core/src/test/resources/api/.*/federations/all, Apache-2.0 BUILD_NUMBER$, Apache-2.0 \.jks, Apache-2.0 # Java Key Store -traffic_ops/traffic_ops_golang/**/*.sql, Apache-2.0 # embedded sql queries - can't have comment blocks due to github.com/jmoiron/sqlx library limitations +traffic_ops/traffic_ops_golang/.*\.sql, Apache-2.0 # embedded sql queries - can't have comment blocks due to github.com/jmoiron/sqlx library limitations # Images, created for this project or used under an Apache license. ^misc/logos/ATC-PNG\.png, Apache-2.0 From 8a38de271e41204b75e74d0bfb3569718b51226f Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 15 Sep 2023 09:40:59 -0600 Subject: [PATCH 13/20] Update enroller --- .../cdn-in-a-box/enroller/enroller.go | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/infrastructure/cdn-in-a-box/enroller/enroller.go b/infrastructure/cdn-in-a-box/enroller/enroller.go index 926a9db478..7af02ccd9d 100644 --- a/infrastructure/cdn-in-a-box/enroller/enroller.go +++ b/infrastructure/cdn-in-a-box/enroller/enroller.go @@ -812,7 +812,12 @@ func enrollFederation(toSession *session, r io.Reader) error { } opts := client.NewRequestOptions() for _, mapping := range federation.Mappings { - var cdnFederation tc.CDNFederation + if mapping.CName == nil || mapping.TTL == nil { + err = fmt.Errorf("mapping found with null or undefined CName and/or TTL: %+v", mapping) + log.Errorln(err) + return err + } + var cdnFederation tc.CDNFederationV5 var cdnName string { xmlID := string(federation.DeliveryService) @@ -831,9 +836,9 @@ func enrollFederation(toSession *session, r io.Reader) error { } deliveryService := deliveryServices.Response[0] cdnName = *deliveryService.CDNName - cdnFederation = tc.CDNFederation{ - CName: mapping.CName, - TTL: mapping.TTL, + cdnFederation = tc.CDNFederationV5{ + CName: *mapping.CName, + TTL: *mapping.TTL, } resp, _, err := toSession.CreateCDNFederation(cdnFederation, cdnName, client.RequestOptions{}) if err != nil { @@ -842,13 +847,8 @@ func enrollFederation(toSession *session, r io.Reader) error { return err } cdnFederation = resp.Response - if cdnFederation.ID == nil { - err = fmt.Errorf("federation returned from creation through Traffic Ops with null or undefined ID") - log.Infoln(err) - return err - } - if alerts, _, err := toSession.CreateFederationDeliveryServices(*cdnFederation.ID, []int{*deliveryService.ID}, true, client.RequestOptions{}); err != nil { - err = fmt.Errorf("assigning Delivery Service %s to Federation with ID %d: %v - alerts: %+v", xmlID, *cdnFederation.ID, err, alerts.Alerts) + if alerts, _, err := toSession.CreateFederationDeliveryServices(cdnFederation.ID, []int{*deliveryService.ID}, true, client.RequestOptions{}); err != nil { + err = fmt.Errorf("assigning Delivery Service %s to Federation with ID %d: %v - alerts: %+v", xmlID, cdnFederation.ID, err, alerts.Alerts) log.Infoln(err) return err } @@ -865,10 +865,10 @@ func enrollFederation(toSession *session, r io.Reader) error { log.Infoln(err) return err } - resp, _, err := toSession.CreateFederationUsers(*cdnFederation.ID, []int{*user.Response.ID}, true, client.RequestOptions{}) + resp, _, err := toSession.CreateFederationUsers(cdnFederation.ID, []int{*user.Response.ID}, true, client.RequestOptions{}) if err != nil { username := user.Response.Username - err = fmt.Errorf("assigning User '%s' to Federation with ID %d: %v - alerts: %+v", username, *cdnFederation.ID, err, resp.Alerts) + err = fmt.Errorf("assigning User '%s' to Federation with ID %d: %v - alerts: %+v", username, cdnFederation.ID, err, resp.Alerts) log.Infoln(err) return err } @@ -885,20 +885,20 @@ func enrollFederation(toSession *session, r io.Reader) error { allResolverIDs = append(allResolverIDs, resolverIDs...) } } - if resp, _, err := toSession.AssignFederationFederationResolver(*cdnFederation.ID, allResolverIDs, true, client.RequestOptions{}); err != nil { - err = fmt.Errorf("assigning Federation Resolvers to Federation with ID %d: %v - alerts: %+v", *cdnFederation.ID, err, resp.Alerts) + if resp, _, err := toSession.AssignFederationFederationResolver(cdnFederation.ID, allResolverIDs, true, client.RequestOptions{}); err != nil { + err = fmt.Errorf("assigning Federation Resolvers to Federation with ID %d: %v - alerts: %+v", cdnFederation.ID, err, resp.Alerts) log.Infoln(err) return err } - opts.QueryParameters.Set("id", strconv.Itoa(*cdnFederation.ID)) - response, _, err := toSession.GetCDNFederationsByName(cdnName, opts) + opts.QueryParameters.Set("id", strconv.Itoa(cdnFederation.ID)) + response, _, err := toSession.GetCDNFederations(cdnName, opts) opts.QueryParameters.Del("id") if err != nil { - err = fmt.Errorf("getting CDN Federation with ID %d: %v - alerts: %+v", *cdnFederation.ID, err, response.Alerts) + err = fmt.Errorf("getting CDN Federation with ID %d: %v - alerts: %+v", cdnFederation.ID, err, response.Alerts) return err } if len(response.Response) < 1 { - err = fmt.Errorf("unable to GET a CDN Federation ID %d in CDN %s", *cdnFederation.ID, cdnName) + err = fmt.Errorf("unable to GET a CDN Federation ID %d in CDN %s", cdnFederation.ID, cdnName) log.Infoln(err) return err } @@ -908,7 +908,7 @@ func enrollFederation(toSession *session, r io.Reader) error { enc.SetIndent("", " ") err = enc.Encode(&cdnFederation) if err != nil { - err = fmt.Errorf("encoding CDNFederation %s with ID %d: %v", *cdnFederation.CName, *cdnFederation.ID, err) + err = fmt.Errorf("encoding CDNFederation %s with ID %d: %v", cdnFederation.CName, cdnFederation.ID, err) log.Infoln(err) return err } From 0bb5652b594202d45cc71919ddba807f1e7ba1a3 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 15 Sep 2023 09:57:58 -0600 Subject: [PATCH 14/20] Fix comparison using pointers instead of values in tests --- .../traffic_ops_golang/cdnfederation/cdnfederations_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go index 59c96f8f7e..bd9157e0d4 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go @@ -26,6 +26,7 @@ import ( "errors" "net/http" "net/http/httptest" + "reflect" "strings" "testing" "time" @@ -202,7 +203,7 @@ func everythingWorks(t *testing.T) { if l := len(feds); l != 1 { t.Fatalf("Expected one federation to be returned; got: %d", l) } - if feds[0] != fed { + if !reflect.DeepEqual(feds[0], fed) { t.Errorf("expected a federation like '%#v', but instead found: %#v", fed, feds[0]) } } From f3b0d71c080bef89b23756323214db2323009fdc Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 15 Sep 2023 09:59:08 -0600 Subject: [PATCH 15/20] Fix improperly cased query string parameter --- traffic_ops/testing/api/v5/cdn_federations_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/traffic_ops/testing/api/v5/cdn_federations_test.go b/traffic_ops/testing/api/v5/cdn_federations_test.go index 1bad115414..da2198520c 100644 --- a/traffic_ops/testing/api/v5/cdn_federations_test.go +++ b/traffic_ops/testing/api/v5/cdn_federations_test.go @@ -254,7 +254,7 @@ func setFederationID(t *testing.T, cdnFederation tc.CDNFederationV5) { func CreateTestCDNFederations(t *testing.T) { for _, federation := range testData.Federations { opts := client.NewRequestOptions() - opts.QueryParameters.Set("xmlID", federation.DeliveryService.XMLID) + opts.QueryParameters.Set("xmlId", federation.DeliveryService.XMLID) dsResp, _, err := TOSession.GetDeliveryServices(opts) assert.RequireNoError(t, err, "Could not get Delivery Service by XML ID: %v", err) assert.RequireEqual(t, 1, len(dsResp.Response), "Expected one Delivery Service, but got %d", len(dsResp.Response)) From 26763bd564263804a0bf5ca43e3ab02bd58b5f4d Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 15 Sep 2023 13:13:41 -0600 Subject: [PATCH 16/20] fix the selectMaxLastUpdatedQuery - and actually use it for that purpose --- .../cdnfederation/cdnfederations.go | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go index c93966eb77..738ba9312a 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go @@ -58,13 +58,26 @@ func (v *TOCDNFederation) GetLastUpdated() (*time.Time, bool, error) { } func selectMaxLastUpdatedQuery(where, orderBy, pagination string) string { - return `SELECT max(t) from ( - SELECT max(federation.last_updated) as t from federation - join federation_deliveryservice fds on fds.federation = federation.id - join deliveryservice ds on ds.id = fds.deliveryservice - join cdn c on c.id = ds.cdn_id ` + where + orderBy + pagination + - ` UNION ALL - select max(last_updated) as t from last_deleted l where l.table_name='federation') as res` + return ` + SELECT max(t) + FROM ( + ( + SELECT federation.last_updated AS t + FROM federation + JOIN federation_deliveryservice fds + ON fds.federation = federation.id + JOIN deliveryservice ds + ON ds.id = fds.deliveryservice + JOIN cdn c + ON c.id = ds.cdn_id ` + where + orderBy + pagination + + `) + UNION ALL + ( + SELECT max(last_updated) AS t + FROM last_deleted l + WHERE l.table_name='federation' + ) + ) AS res` } func (v *TOCDNFederation) SetLastUpdated(t tc.TimeNoMod) { v.LastUpdated = &t } @@ -455,9 +468,9 @@ func getCDNFederations(inf *api.APIInfo) ([]tc.CDNFederationV5, time.Time, int, queryValues["tenantIDs"] = pq.Array(tenantList) where = addTenancyStmt(where) - query := selectQuery + where + orderBy + pagination if inf.UseIMS() { + query := selectMaxLastUpdatedQuery(where, orderBy, pagination) cont, max := ims.TryIfModifiedSinceQuery(inf.Tx, inf.RequestHeaders(), queryValues, query) if !cont { log.Debugln("IMS HIT") @@ -468,6 +481,7 @@ func getCDNFederations(inf *api.APIInfo) ([]tc.CDNFederationV5, time.Time, int, log.Debugln("Non IMS request") } + query := selectQuery + where + orderBy + pagination rows, err := inf.Tx.NamedQuery(query, queryValues) if err != nil { userErr, sysErr, code := api.ParseDBError(err) From 38b78d5805430a24208c00c90abe88e8b5994aa8 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Fri, 15 Sep 2023 14:22:32 -0600 Subject: [PATCH 17/20] Implement IUS/ETag --- .../cdnfederation/cdnfederations.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go index 738ba9312a..f86c664e1d 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go @@ -599,13 +599,22 @@ func Update(inf *api.APIInfo) (int, error, error) { id := inf.IntParams["id"] + var lastModified time.Time + err := inf.Tx.QueryRow("SELECT last_updated FROM federation WHERE id = $1", id).Scan(&lastModified) + if err != nil { + return http.StatusInternalServerError, nil, fmt.Errorf("getting last modified time for Federation #%d: %w", id, err) + } + if !api.IsUnmodified(inf.RequestHeaders(), lastModified) { + return http.StatusPreconditionFailed, api.ResourceModifiedError, nil + } + // You can't set this via a PUT request, but if it was in the request it // would be shown in the response - we're supposed to ignore extra fields. // This doesn't do that exactly, but it helps. fed.DeliveryService = nil fed.ID = id - err := validate(fed) + err = validate(fed) if err != nil { return http.StatusBadRequest, err, nil } From 0f6d05dec59297bf538a5085407e2205d64e2b01 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Mon, 18 Sep 2023 14:50:27 -0600 Subject: [PATCH 18/20] Revert embedding queries from files --- .../cdnfederation/cdnfederations.go | 68 ++++++++++++++++--- .../cdnfederation/delete.sql | 8 --- .../cdnfederation/insert.sql | 12 ---- .../cdnfederation/select.sql | 19 ------ .../cdnfederation/update.sql | 8 --- 5 files changed, 57 insertions(+), 58 deletions(-) delete mode 100644 traffic_ops/traffic_ops_golang/cdnfederation/delete.sql delete mode 100644 traffic_ops/traffic_ops_golang/cdnfederation/insert.sql delete mode 100644 traffic_ops/traffic_ops_golang/cdnfederation/select.sql delete mode 100644 traffic_ops/traffic_ops_golang/cdnfederation/update.sql diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go index f86c664e1d..4f65cc9821 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go @@ -23,7 +23,6 @@ package cdnfederation import ( "database/sql" - _ "embed" // needed to embed SQL queries within Go variables "errors" "fmt" "net/http" @@ -427,21 +426,68 @@ func selectByID() string { // WHERE federation.id = :id (determined by dbhelper) } -//go:embed select.sql -var selectQuery string - -//go:embed insert.sql -var insertQuery string +const selectQuery = ` +SELECT + ds.tenant_id, + federation.id AS id, + federation.cname, + federation.ttl, + federation.description, + federation.last_updated, + ds.id AS ds_id, + ds.xml_id +FROM federation +JOIN + federation_deliveryservice AS fd + ON federation.id = fd.federation +JOIN + deliveryservice AS ds + ON ds.id = fd.deliveryservice +JOIN + cdn AS c + ON c.id = ds.cdn_id +` + +const insertQuery = ` +INSERT INTO federation ( + cname, + ttl, + "description" +) VALUES ( + $1, + $2, + $3 +) +RETURNING + id, + last_updated +` func selectByCDNName() string { return selectQuery } -//go:embed update.sql -var updateQuery string - -//go:embed delete.sql -var deleteQuery string +const updateQuery = ` +UPDATE federation SET + cname = $1, + ttl = $2, + "description" = $3 +WHERE + id = $4 +RETURNING + last_updated +` + +const deleteQuery = ` +DELETE FROM federation +WHERE id = $1 +RETURNING + cname, + "description", + id, + last_updated, + ttl +` func addTenancyStmt(where string) string { if where == "" { diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/delete.sql b/traffic_ops/traffic_ops_golang/cdnfederation/delete.sql deleted file mode 100644 index 26a28d20c0..0000000000 --- a/traffic_ops/traffic_ops_golang/cdnfederation/delete.sql +++ /dev/null @@ -1,8 +0,0 @@ -DELETE FROM federation -WHERE id = $1 -RETURNING - cname, - "description", - id, - last_updated, - ttl diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/insert.sql b/traffic_ops/traffic_ops_golang/cdnfederation/insert.sql deleted file mode 100644 index c2393fd56e..0000000000 --- a/traffic_ops/traffic_ops_golang/cdnfederation/insert.sql +++ /dev/null @@ -1,12 +0,0 @@ -INSERT INTO federation ( - cname, - ttl, - "description" -) VALUES ( - $1, - $2, - $3 -) -RETURNING - id, - last_updated diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/select.sql b/traffic_ops/traffic_ops_golang/cdnfederation/select.sql deleted file mode 100644 index 346587bd1e..0000000000 --- a/traffic_ops/traffic_ops_golang/cdnfederation/select.sql +++ /dev/null @@ -1,19 +0,0 @@ -SELECT - ds.tenant_id, - federation.id AS id, - federation.cname, - federation.ttl, - federation.description, - federation.last_updated, - ds.id AS ds_id, - ds.xml_id -FROM federation -JOIN - federation_deliveryservice AS fd - ON federation.id = fd.federation -JOIN - deliveryservice AS ds ON - ds.id = fd.deliveryservice -JOIN - cdn AS c - ON c.id = ds.cdn_id diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/update.sql b/traffic_ops/traffic_ops_golang/cdnfederation/update.sql deleted file mode 100644 index bdd3566b8e..0000000000 --- a/traffic_ops/traffic_ops_golang/cdnfederation/update.sql +++ /dev/null @@ -1,8 +0,0 @@ -UPDATE federation SET - cname = $1, - ttl = $2, - "description" = $3 -WHERE - id = $4 -RETURNING - last_updated From 1825a5561d78ba5e9c6d18f0c46927ff14106e6c Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Tue, 19 Sep 2023 11:59:30 -0600 Subject: [PATCH 19/20] Add TTL minimum --- .../cdnfederation/cdnfederations.go | 2 +- .../cdnfederation/cdnfederations_test.go | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go index 4f65cc9821..a075343d34 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go @@ -604,7 +604,7 @@ func validate(fed tc.CDNFederationV5) error { validateErrs := validation.Errors{ "cname": validation.Validate(fed.CName, validation.Required, is.DNSName, endsWithDot), - "ttl": validation.Validate(fed.TTL, validation.Required, validation.Min(0)), + "ttl": validation.Validate(fed.TTL, validation.Required, validation.Min(60)), } return util.JoinErrs(tovalidate.ToErrors(validateErrs)) diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go index bd9157e0d4..3709e0de3d 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go @@ -279,3 +279,23 @@ func TestCreate(t *testing.T) { t.Errorf("Didn't create the expected Federation; want: %#v, got: %#v", newFed, created.Response) } } + +func TestValidate(t *testing.T) { + fed := tc.CDNFederationV5{ + CName: "test.quest.", + TTL: 60, + } + err := validate(fed) + if err != nil { + t.Errorf("Unexpected validation error: %v", err) + } + + fed.TTL-- + err = validate(fed) + if err == nil { + t.Fatal("Expected an error for TTL below minimum, but didn't get one") + } + if !strings.Contains(err.Error(), "ttl") { + t.Errorf("Expected error message to mention 'ttl': %v", err) + } +} From e569f84652a233b61f21652461467a4c9011ff41 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Tue, 19 Sep 2023 14:08:33 -0600 Subject: [PATCH 20/20] Fix testing data to fit new restriction --- docs/source/api/v5/cdns_name_federations.rst | 11 +++++++---- docs/source/api/v5/cdns_name_federations_id.rst | 16 +++++++++------- .../testing/api/v5/cdn_federations_test.go | 6 +++--- traffic_ops/testing/api/v5/federations_test.go | 7 ++++--- traffic_ops/testing/api/v5/tc-fixtures.json | 6 +++--- .../cdnfederation/cdnfederations.go | 6 +++--- .../cdnfederation/cdnfederations_test.go | 13 ++++++++++++- 7 files changed, 41 insertions(+), 24 deletions(-) diff --git a/docs/source/api/v5/cdns_name_federations.rst b/docs/source/api/v5/cdns_name_federations.rst index f0d5e4fe43..d5dce307f0 100644 --- a/docs/source/api/v5/cdns_name_federations.rst +++ b/docs/source/api/v5/cdns_name_federations.rst @@ -112,7 +112,7 @@ Response Structure { "id": 1, "cname": "test.quest.", - "ttl": 48, + "ttl": 68, "description": "A test federation", "lastUpdated": "2018-12-05T00:05:16Z", "deliveryService": { @@ -150,7 +150,10 @@ Request Structure .. tip:: The CNAME must end with a "``.``" :description: An optional description of the federation -:ttl: Time to Live (TTL) for the name record used for ``cname`` +:ttl: Time to Live (TTL) for the name record used for ``cname`` - minimum of 60 + + .. versionchanged:: 5.0 + In earlier API versions, there is no enforced minimum (although Traffic Portal would never allow a value under 60). .. code-block:: http :caption: Request Example @@ -165,7 +168,7 @@ Request Structure { "cname": "test.quest.", - "ttl": 48, + "ttl": 68, "description": "A test federation" } @@ -206,7 +209,7 @@ Response Structure "response": { "id": 1, "cname": "test.quest.", - "ttl": 48, + "ttl": 68, "description": "A test federation", "lastUpdated": "2018-12-05T00:05:16Z" }} diff --git a/docs/source/api/v5/cdns_name_federations_id.rst b/docs/source/api/v5/cdns_name_federations_id.rst index 813d400814..89545e92d3 100644 --- a/docs/source/api/v5/cdns_name_federations_id.rst +++ b/docs/source/api/v5/cdns_name_federations_id.rst @@ -88,7 +88,7 @@ Response Structure { "response": { "id": 1, "cname": "test.quest.", - "ttl": 48, + "ttl": 68, "description": "A test federation", "lastUpdated": "2018-12-05T00:05:16Z", "deliveryService": { @@ -120,12 +120,15 @@ Request Structure .. caution:: The name of the CDN doesn't actually matter. It doesn't even need to be the name of any existing CDN. -:cname: The Canonical Name (CNAME) used by the federation +:cname: The :abbr:`CNAME (Canonical Name)` used by the :term:`Federation` .. note:: The CNAME must end with a "``.``" :description: An optional description of the federation -:ttl: Time to Live (TTL) for the name record used for ``cname`` +:ttl: Time to Live (TTL) for the name record used for ``cname`` - minimum of 60 + + .. versionchanged:: 5.0 + In earlier API versions, there is no enforced minimum (although Traffic Portal would never allow a value under 60). .. code-block:: http :caption: Request Example @@ -140,7 +143,7 @@ Request Structure { "cname": "foo.bar.", - "ttl": 48 + "ttl": 68 } @@ -179,7 +182,7 @@ Response Structure "response": { "id": 1, "cname": "foo.bar.", - "ttl": 48, + "ttl": 68, "description": null, "lastUpdated": "2018-12-05T01:03:40Z" }} @@ -227,7 +230,6 @@ Response Structure :lastUpdated: The date and time at which this :term:`Federation` was last modified, in :RFC:`3339` format :ttl: :abbr:`TTL (Time to Live)` for the ``cname``, in hours - .. code-block:: http :caption: Response Example @@ -252,7 +254,7 @@ Response Structure "response": { "id": 1, "cname": "foo.bar.", - "ttl": 48, + "ttl": 68, "description": null, "lastUpdated": "2018-12-05T01:03:40Z" }} diff --git a/traffic_ops/testing/api/v5/cdn_federations_test.go b/traffic_ops/testing/api/v5/cdn_federations_test.go index da2198520c..d0a554774e 100644 --- a/traffic_ops/testing/api/v5/cdn_federations_test.go +++ b/traffic_ops/testing/api/v5/cdn_federations_test.go @@ -109,7 +109,7 @@ func TestCDNFederations(t *testing.T) { ClientSession: TOSession, RequestBody: tc.CDNFederationV5{ CName: "new.cname.", - TTL: 34, + TTL: 64, Description: util.Ptr("updated"), }, Expectations: utils.CkRequest(utils.NoError(), utils.HasStatus(http.StatusOK), validateCDNFederationUpdateFields(map[string]interface{}{"CName": "new.cname."})), @@ -120,7 +120,7 @@ func TestCDNFederations(t *testing.T) { RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfUnmodifiedSince: {currentTimeRFC}}}, RequestBody: tc.CDNFederationV5{ CName: "booya.com.", - TTL: 34, + TTL: 64, Description: util.Ptr("fooya"), }, Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusPreconditionFailed)), @@ -130,7 +130,7 @@ func TestCDNFederations(t *testing.T) { ClientSession: TOSession, RequestBody: tc.CDNFederationV5{ CName: "new.cname.", - TTL: 34, + TTL: 64, Description: util.Ptr("updated"), }, RequestOpts: client.RequestOptions{Header: http.Header{rfc.IfMatch: {rfc.ETag(currentTime)}}}, diff --git a/traffic_ops/testing/api/v5/federations_test.go b/traffic_ops/testing/api/v5/federations_test.go index 4bf2dce381..51dcb6d5fe 100644 --- a/traffic_ops/testing/api/v5/federations_test.go +++ b/traffic_ops/testing/api/v5/federations_test.go @@ -49,18 +49,19 @@ func TestFederations(t *testing.T) { validateAllFederationsFields([]map[string]interface{}{ { "DeliveryService": "ds1", + // TODO: Why are these hard-coded copies of the test data? "Mappings": []map[string]interface{}{ { "CName": "the.cname.com.", - "TTL": 48, + "TTL": 68, }, { "CName": "booya.com.", - "TTL": 34, + "TTL": 64, }, { "CName": "google.com.", - "TTL": 30, + "TTL": 60, }, }, }, diff --git a/traffic_ops/testing/api/v5/tc-fixtures.json b/traffic_ops/testing/api/v5/tc-fixtures.json index 6672e3922d..05a885f81e 100644 --- a/traffic_ops/testing/api/v5/tc-fixtures.json +++ b/traffic_ops/testing/api/v5/tc-fixtures.json @@ -1864,7 +1864,7 @@ "federations": [ { "cname": "the.cname.com.", - "ttl": 48, + "ttl": 68, "description": "the description", "deliveryService": { "xmlID": "ds1" @@ -1872,7 +1872,7 @@ }, { "cname": "booya.com.", - "ttl": 34, + "ttl": 64, "description": "fooya", "deliveryService": { "xmlID": "ds1" @@ -1880,7 +1880,7 @@ }, { "cname": "google.com.", - "ttl": 30, + "ttl": 60, "description": "google", "deliveryService": { "xmlID": "ds1" diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go index a075343d34..514ae7c5a3 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations.go @@ -624,13 +624,13 @@ func Create(inf *api.APIInfo) (int, error, error) { err := validate(fed) if err != nil { - userErr, sysErr, code := api.ParseDBError(err) - return code, userErr, sysErr + return http.StatusBadRequest, err, nil } err = inf.Tx.Tx.QueryRow(insertQuery, fed.CName, fed.TTL, fed.Description).Scan(&fed.ID, &fed.LastUpdated) if err != nil { - return http.StatusInternalServerError, nil, fmt.Errorf("inserting a CDN Federation: %w", err) + userErr, sysErr, code := api.ParseDBError(err) + return code, userErr, fmt.Errorf("inserting a CDN Federation: %w", sysErr) } return inf.WriteCreatedResponse(fed, "Federation was created", "federations/"+strconv.Itoa(fed.ID)) diff --git a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go index 3709e0de3d..e671122a00 100644 --- a/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go +++ b/traffic_ops/traffic_ops_golang/cdnfederation/cdnfederations_test.go @@ -243,7 +243,7 @@ func testingInf(t *testing.T, body []byte) (*http.Request, sqlmock.Sqlmock, *sql func TestCreate(t *testing.T) { newFed := tc.CDNFederationV5{ CName: "test.quest.", - TTL: 5, + TTL: 420, Description: nil, } bts, err := json.Marshal(newFed) @@ -298,4 +298,15 @@ func TestValidate(t *testing.T) { if !strings.Contains(err.Error(), "ttl") { t.Errorf("Expected error message to mention 'ttl': %v", err) } + + fed.TTL = 60 + fed.CName = "test.quest" + err = validate(fed) + if err == nil { + t.Fatal("Expected an error for a CNAME without a terminating '.', but didn't get one") + } + if !strings.Contains(err.Error(), "cname") { + t.Errorf("Expected error message to mention 'cname': %v", err) + } + }