From 2265e25b934c96d13d3084c4022582a680e643ab Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Wed, 9 Nov 2022 17:23:34 -0700 Subject: [PATCH 1/7] initial commit --- cache-config/testing/ort-tests/tcdata/todb.go | 1 - lib/go-tc/deliveryservices.go | 3 +- ...0_add_required_capabilities_to_ds.down.sql | 36 +++++++++++ ...600_add_required_capabilities_to_ds.up.sql | 30 +++++++++ .../crconfig/deliveryservice.go | 4 +- .../dbhelpers/db_helpers.go | 31 ++++------ .../deliveryservice/deliveryservices.go | 62 ++++++++++++++----- .../deliveryservices_required_capabilities.go | 43 +++++++------ .../deliveryservice/eligible.go | 4 +- .../traffic_ops_golang/server/servers.go | 18 +++--- .../server/servers_server_capability.go | 13 ++-- .../traffic_ops_golang/topology/topologies.go | 5 +- 12 files changed, 171 insertions(+), 79 deletions(-) create mode 100644 traffic_ops/app/db/migrations/2022110813211600_add_required_capabilities_to_ds.down.sql create mode 100644 traffic_ops/app/db/migrations/2022110813211600_add_required_capabilities_to_ds.up.sql diff --git a/cache-config/testing/ort-tests/tcdata/todb.go b/cache-config/testing/ort-tests/tcdata/todb.go index 0bbb27b76c..b526c0416c 100644 --- a/cache-config/testing/ort-tests/tcdata/todb.go +++ b/cache-config/testing/ort-tests/tcdata/todb.go @@ -247,7 +247,6 @@ func (r *TCData) Teardown(db *sql.DB) error { sqlStmt := ` DELETE FROM api_capability; - DELETE FROM deliveryservices_required_capability; DELETE FROM server_server_capability; DELETE FROM server_capability; DELETE FROM to_extension; diff --git a/lib/go-tc/deliveryservices.go b/lib/go-tc/deliveryservices.go index 082501a237..f752623102 100644 --- a/lib/go-tc/deliveryservices.go +++ b/lib/go-tc/deliveryservices.go @@ -268,7 +268,8 @@ type DeliveryServiceV41 struct { // Regional indicates whether the Delivery Service's MaxOriginConnections is // only per Cache Group, rather than divided over all Cache Servers in child // Cache Groups of the Origin. - Regional bool `json:"regional" db:"regional"` + Regional bool `json:"regional" db:"regional"` + RequiredCapabilities []string `json:"requiredCapabilities" db:"required_capabilities"` } // DeliveryServiceV4 is a Delivery Service as it appears in version 4 of the diff --git a/traffic_ops/app/db/migrations/2022110813211600_add_required_capabilities_to_ds.down.sql b/traffic_ops/app/db/migrations/2022110813211600_add_required_capabilities_to_ds.down.sql new file mode 100644 index 0000000000..86d98c23a0 --- /dev/null +++ b/traffic_ops/app/db/migrations/2022110813211600_add_required_capabilities_to_ds.down.sql @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +CREATE TABLE IF NOT EXISTS deliveryservices_required_capability ( +required_capability TEXT NOT NULL, +deliveryservice_id bigint NOT NULL, +last_updated timestamp with time zone DEFAULT now() NOT NULL, + +PRIMARY KEY (deliveryservice_id, required_capability) +); + +DO $$ +DECLARE temprow RECORD; +BEGIN FOR temprow IN +select id as deliveryservice_id, unnest(required_capabilities) as required_capability from deliveryservice d group by d.id, d.required_capabilities + LOOP +insert into deliveryservices_required_capability ("deliveryservice_id", "required_capability") values (temprow.deliveryservice_id, temprow.required_capability); +END LOOP; +END $$; + +ALTER TABLE public.deliveryservice +DROP COLUMN required_capabilities; \ No newline at end of file diff --git a/traffic_ops/app/db/migrations/2022110813211600_add_required_capabilities_to_ds.up.sql b/traffic_ops/app/db/migrations/2022110813211600_add_required_capabilities_to_ds.up.sql new file mode 100644 index 0000000000..c7faa0aad3 --- /dev/null +++ b/traffic_ops/app/db/migrations/2022110813211600_add_required_capabilities_to_ds.up.sql @@ -0,0 +1,30 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +ALTER TABLE public.deliveryservice +ADD COLUMN required_capabilities text[]; + +DO $$ +DECLARE temprow RECORD; +BEGIN FOR temprow IN +select deliveryservice_id, array_agg(required_capability) as required_capabilities from deliveryservices_required_capability drc group by drc.deliveryservice_id +LOOP +update deliveryservice d set required_capabilities = temprow.required_capabilities where d.id = temprow.deliveryservice_id; +END LOOP; +END $$; + +DROP TABLE public.deliveryservices_required_capability; \ No newline at end of file diff --git a/traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go b/traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go index dfd45ebaa7..5321442943 100644 --- a/traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go +++ b/traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go @@ -132,9 +132,7 @@ SELECT d.anonymous_blocking_enabled, d.miss_long, p.name AS profile, d.protocol, - (SELECT ARRAY_AGG(required_capability ORDER BY required_capability) - FROM deliveryservices_required_capability - WHERE deliveryservice_id = d.id) AS required_capabilities, + d.required_capabilities, d.topology, d.tr_request_headers, d.tr_response_headers, diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go index f93131ab96..5ed708a2a8 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go +++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go @@ -846,13 +846,11 @@ func GetRequiredCapabilitiesOfDeliveryServices(ids []int, tx *sql.Tx) (map[int][ dsCaps := make(map[int][]string, len(ids)) q := ` SELECT - d.id, - ARRAY_REMOVE(ARRAY_AGG(dsrc.required_capability ORDER BY dsrc.required_capability), NULL) AS required_capabilities -FROM deliveryservice d -LEFT JOIN deliveryservices_required_capability dsrc on d.id = dsrc.deliveryservice_id -WHERE - d.id = ANY($1) -GROUP BY d.id + ds.id, + ARRAY_REMOVE((ds.required_capabilities), NULL) AS required_capabilities +FROM deliveryservice ds +WHERE ds.id = ANY($1) +GROUP BY ds.id, ds.required_capabilities ` rows, err := tx.Query(q, pq.Array(&queryIDs)) if err != nil { @@ -923,10 +921,10 @@ func ScanCachegroupsServerCapabilities(rows *sql.Rows) (map[string][]int, map[in // GetDSRequiredCapabilitiesFromID returns the server's capabilities. func GetDSRequiredCapabilitiesFromID(id int, tx *sql.Tx) ([]string, error) { q := ` - SELECT required_capability - FROM deliveryservices_required_capability - WHERE deliveryservice_id = $1 - ORDER BY required_capability` + SELECT required_capabilities + FROM deliveryservice + WHERE id = $1 + ORDER BY required_capabilities` rows, err := tx.Query(q, id) if err != nil { return nil, errors.New("querying deliveryservice required capabilities from id: " + err.Error()) @@ -935,11 +933,9 @@ func GetDSRequiredCapabilitiesFromID(id int, tx *sql.Tx) ([]string, error) { caps := []string{} for rows.Next() { - var cap string - if err := rows.Scan(&cap); err != nil { + if err := rows.Scan(pq.Array(&caps)); err != nil { return nil, errors.New("scanning capability: " + err.Error()) } - caps = append(caps, cap) } return caps, nil } @@ -1544,7 +1540,7 @@ func GetDeliveryServiceTypeAndCDNName(dsID int, tx *sql.Tx) (tc.DSType, string, return dsType, cdnName, true, nil } -// GetDeliveryServiceTypeAndTopology returns the type of the deliveryservice and the name of its topology. +// GetDeliveryServiceTypeRequiredCapabilitiesAndTopology returns the type of the deliveryservice and the name of its topology. func GetDeliveryServiceTypeRequiredCapabilitiesAndTopology(dsID int, tx *sql.Tx) (tc.DSType, []string, *string, bool, error) { var dsType tc.DSType var reqCap []string @@ -1552,13 +1548,12 @@ func GetDeliveryServiceTypeRequiredCapabilitiesAndTopology(dsID int, tx *sql.Tx) q := ` SELECT t.name, - ARRAY_REMOVE(ARRAY_AGG(dsrc.required_capability ORDER BY dsrc.required_capability), NULL) AS required_capabilities, + ARRAY_REMOVE(ds.required_capabilities, NULL) AS required_capabilities, ds.topology FROM deliveryservice AS ds -LEFT JOIN deliveryservices_required_capability AS dsrc ON dsrc.deliveryservice_id = ds.id JOIN type t ON ds.type = t.id WHERE ds.id = $1 -GROUP BY t.name, ds.topology +GROUP BY t.name, ds.topology, ds.required_capabilities ` if err := tx.QueryRow(q, dsID).Scan(&dsType, pq.Array(&reqCap), &topology); err != nil { if err == sql.ErrNoRows { diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go index b918638c7a..941bb66432 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go @@ -288,7 +288,7 @@ func recreateTLSVersions(versions []string, dsid int, tx *sql.Tx) error { return nil } -// create creates the given ds in the database, and returns the DS with its id and other fields created on insert set. On error, the HTTP status code, user error, and system error are returned. The status code SHOULD NOT be used, if both errors are nil. +// createV40 creates the given ds in the database, and returns the DS with its id and other fields created on insert set. On error, the HTTP status code, user error, and system error are returned. The status code SHOULD NOT be used, if both errors are nil. func createV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV4 tc.DeliveryServiceV40, omitExtraLongDescFields bool) (*tc.DeliveryServiceV40, int, error, error) { ds, code, userErr, sysErr := createV41(w, r, inf, tc.DeliveryServiceV41{DeliveryServiceV40: dsV4}, omitExtraLongDescFields) if userErr != nil || sysErr != nil || ds == nil { @@ -399,6 +399,7 @@ func createV41(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV4 tc ds.LastHeaderRewrite, ds.ServiceCategory, ds.MaxRequestHeaderBytes, + pq.Array(ds.RequiredCapabilities), ) } else { resultRows, err = tx.Query(insertQuery(), @@ -462,6 +463,7 @@ func createV41(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV4 tc ds.LastHeaderRewrite, ds.ServiceCategory, ds.MaxRequestHeaderBytes, + pq.Array(ds.RequiredCapabilities), ) } @@ -896,12 +898,8 @@ func updateV41(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV4 *t } if ds.Topology != nil { - requiredCapabilities, err := dbhelpers.GetDSRequiredCapabilitiesFromID(*ds.ID, tx) - if err != nil { - return nil, http.StatusInternalServerError, nil, errors.New("getting existing DS required capabilities: " + err.Error()) - } - if len(requiredCapabilities) > 0 { - if userErr, sysErr, status := EnsureTopologyBasedRequiredCapabilities(tx, *ds.ID, *ds.Topology, requiredCapabilities); userErr != nil || sysErr != nil { + if len(ds.RequiredCapabilities) > 0 { + if userErr, sysErr, status := EnsureTopologyBasedRequiredCapabilities(tx, *ds.ID, *ds.Topology, ds.RequiredCapabilities); userErr != nil || sysErr != nil { return nil, status, userErr, sysErr } } @@ -981,6 +979,7 @@ func updateV41(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV4 *t ds.LastHeaderRewrite, ds.ServiceCategory, ds.MaxRequestHeaderBytes, + pq.Array(ds.RequiredCapabilities), ds.ID, ) } else { @@ -1045,6 +1044,7 @@ func updateV41(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV4 *t ds.LastHeaderRewrite, ds.ServiceCategory, ds.MaxRequestHeaderBytes, + pq.Array(ds.RequiredCapabilities), ds.ID, ) } @@ -1389,12 +1389,38 @@ func Validate(tx *sql.Tx, ds *tc.DeliveryServiceV4) error { if err := validateTypeFields(tx, ds); err != nil { errs = append(errs, errors.New("type fields: "+err.Error())) } + if err := validateRequiredCapabilities(tx, ds); err != nil { + errs = append(errs, errors.New("required capabilities: "+err.Error())) + } if len(errs) == 0 { return nil } return util.JoinErrs(errs) } +func validateRequiredCapabilities(tx *sql.Tx, ds *tc.DeliveryServiceV4) error { + //ToDo: fix this + var valid bool + query := `SELECT $1 @> (SELECT ARRAY_AGG(name) FROM server_capability)` + if ds.RequiredCapabilities != nil && len(ds.RequiredCapabilities) > 0 { + rows, err := tx.Query(query, pq.Array(ds.RequiredCapabilities)) + if err != nil { + return err + } + defer rows.Close() + for rows.Next() { + err = rows.Scan(&valid) + if err != nil { + return err + } + if !valid { + return errors.New("one or more of the required capabilities do not exist") + } + } + } + return nil +} + func validateGeoLimitCountries(ds *tc.DeliveryServiceV4) error { var IsLetter = regexp.MustCompile(`^[A-Z]+$`).MatchString if ds.GeoLimitCountries == nil { @@ -1738,6 +1764,7 @@ func GetDeliveryServices(query string, queryValues map[string]interface{}, tx *s &ds.Regional, &ds.RegionalGeoBlocking, &ds.RemapText, + pq.Array(&ds.RequiredCapabilities), &ds.RoutingName, &ds.ServiceCategory, &ds.SigningAlgorithm, @@ -2277,6 +2304,7 @@ ds.active, ds.regional, ds.regional_geo_blocking, ds.remap_text, + ds.required_capabilities, ds.routing_name, ds.service_category, ds.signing_algorithm, @@ -2362,8 +2390,9 @@ first_header_rewrite=$56, inner_header_rewrite=$57, last_header_rewrite=$58, service_category=$59, -max_request_header_bytes=$60 -WHERE id=$61 +max_request_header_bytes=$60, +required_capabilities=$61 +WHERE id=$62 RETURNING last_updated ` } @@ -2429,8 +2458,9 @@ first_header_rewrite=$54, inner_header_rewrite=$55, last_header_rewrite=$56, service_category=$57, -max_request_header_bytes=$58 -WHERE id=$59 +max_request_header_bytes=$58, +required_capabilities=$59 +WHERE id=$60 RETURNING last_updated ` } @@ -2497,9 +2527,10 @@ first_header_rewrite, inner_header_rewrite, last_header_rewrite, service_category, -max_request_header_bytes +max_request_header_bytes, +required_capabilities ) -VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25,$26,$27,$28,$29,$30,$31,$32,$33,$34,$35,$36,$37,$38,$39,$40,$41,$42,$43,$44,$45,$46,$47,$48,$49,$50,$51,$52,$53,$54,$55,$56,$57,$58,$59,$60) +VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25,$26,$27,$28,$29,$30,$31,$32,$33,$34,$35,$36,$37,$38,$39,$40,$41,$42,$43,$44,$45,$46,$47,$48,$49,$50,$51,$52,$53,$54,$55,$56,$57,$58,$59,$60,$61) RETURNING id, last_updated ` } @@ -2564,9 +2595,10 @@ first_header_rewrite, inner_header_rewrite, last_header_rewrite, service_category, -max_request_header_bytes +max_request_header_bytes, +required_capabilities ) -VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25,$26,$27,$28,$29,$30,$31,$32,$33,$34,$35,$36,$37,$38,$39,$40,$41,$42,$43,$44,$45,$46,$47,$48,$49,$50,$51,$52,$53,$54,$55,$56,$57,$58) +VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,$14,$15,$16,$17,$18,$19,$20,$21,$22,$23,$24,$25,$26,$27,$28,$29,$30,$31,$32,$33,$34,$35,$36,$37,$38,$39,$40,$41,$42,$43,$44,$45,$46,$47,$48,$49,$50,$51,$52,$53,$54,$55,$56,$57,$58,$59) RETURNING id, last_updated ` } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go index b6b75b3dd8..5ec1af84be 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go @@ -65,19 +65,19 @@ func (rc *RequiredCapability) NewReadObj() interface{} { // SelectQuery implements the api.GenericReader interface. func (rc *RequiredCapability) SelectQuery() string { return `SELECT - rc.required_capability, - rc.deliveryservice_id, + UNNEST(ds.required_capabilities) as required_capability, + ds.id as deliveryservice_id, ds.xml_id, - rc.last_updated - FROM deliveryservices_required_capability rc - JOIN deliveryservice ds ON ds.id = rc.deliveryservice_id` + ds.last_updated + FROM deliveryservice ds + ` } // ParamColumns implements the api.GenericReader interface. func (rc *RequiredCapability) ParamColumns() map[string]dbhelpers.WhereColumnInfo { return map[string]dbhelpers.WhereColumnInfo{ deliveryServiceQueryParam: dbhelpers.WhereColumnInfo{ - Column: "rc.deliveryservice_id", + Column: "ds.id", Checker: api.IsInt, }, xmlIDQueryParam: dbhelpers.WhereColumnInfo{ @@ -93,8 +93,8 @@ func (rc *RequiredCapability) ParamColumns() map[string]dbhelpers.WhereColumnInf // DeleteQuery implements the api.GenericDeleter interface. func (rc *RequiredCapability) DeleteQuery() string { - return `DELETE FROM deliveryservices_required_capability - WHERE deliveryservice_id = :deliveryservice_id AND required_capability = :required_capability` + return `UPDATE deliveryservice ds SET required_capabilities = ARRAY_REMOVE((select required_capabilities from deliveryservice WHERE id=:deliveryservice_id), :required_capability) +WHERE id=:deliveryservice_id` } // GetKeyFieldsInfo implements the api.Identifier interface. @@ -292,7 +292,7 @@ func (rc *RequiredCapability) Create() (error, error, int) { } if !dsType.IsHTTP() && !dsType.IsDNS() { - return errors.New("Invalid DS type. Only DNS and HTTP delivery services can have required capabilities"), nil, http.StatusBadRequest + return errors.New("invalid DS type. Only DNS and HTTP delivery services can have required capabilities"), nil, http.StatusBadRequest } usrErr, sysErr, rCode := rc.checkServerCap() @@ -300,6 +300,11 @@ func (rc *RequiredCapability) Create() (error, error, int) { return usrErr, sysErr, rCode } + for _, reqCap := range reqCaps { + if reqCap == *rc.RequiredCapability { + return fmt.Errorf("capability %s already exists for delivery service with ID: %d", *rc.RequiredCapability, *rc.DeliveryServiceID), nil, http.StatusBadRequest + } + } if topology == nil { usrErr, sysErr, rCode = rc.ensureDSServerCap() if usrErr != nil || sysErr != nil { @@ -325,7 +330,7 @@ func (rc *RequiredCapability) Create() (error, error, int) { rowsAffected := 0 for rows.Next() { rowsAffected++ - if err := rows.StructScan(&rc); err != nil { + if err := rows.Scan(&rc.DeliveryServiceID, &rc.LastUpdated); err != nil { return nil, fmt.Errorf("%s create scanning: %s", rc.GetType(), err.Error()), http.StatusInternalServerError } } @@ -518,18 +523,12 @@ func (rc *RequiredCapability) isTenantAuthorized() (bool, error) { } func rcInsertQuery() string { - return `INSERT INTO deliveryservices_required_capability ( -required_capability, -deliveryservice_id) VALUES ( -:required_capability, -:deliveryservice_id) RETURNING deliveryservice_id, required_capability, last_updated` + return `UPDATE deliveryservice ds SET required_capabilities = array_append((select required_capabilities from deliveryservice WHERE id=:deliveryservice_id), :required_capability) +WHERE id=:deliveryservice_id RETURNING ds.id, ds.last_updated` } -// language=SQL -const HasRequiredCapabilitiesQuery = ` -SELECT EXISTS( - SELECT drc.required_capability - FROM deliveryservices_required_capability drc - WHERE drc.deliveryservice_id = $1 -) +const GetRequiredCapabilitiesQuery = ` +SELECT ds.required_capabilities +FROM deliveryservice ds +WHERE ds.id = $1 ` diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/eligible.go b/traffic_ops/traffic_ops_golang/deliveryservice/eligible.go index d863aa423b..691a1ae7ba 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/eligible.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/eligible.go @@ -147,7 +147,7 @@ t.name as server_type, s.type as server_type_id, s.config_update_time > s.config_apply_time AS upd_pending, ARRAY(select ssc.server_capability from server_server_capability ssc where ssc.server = s.id order by ssc.server_capability) as server_capabilities, -ARRAY(select drc.required_capability from deliveryservices_required_capability drc where drc.deliveryservice_id = (select v from ds_id) order by drc.required_capability) as deliveryservice_capabilities +(SELECT ds.required_capabilities FROM deliveryservice ds WHERE ds.id = $2) AS deliveryservice_capabilities ` idRows, err := tx.Query(fmt.Sprintf(queryFormatString, "", queryWhereClause), dsID) if err != nil { @@ -168,7 +168,7 @@ ARRAY(select drc.required_capability from deliveryservices_required_capability d return nil, errors.New("unable to get server interfaces: " + err.Error()) } - rows, err := tx.Query(fmt.Sprintf(queryFormatString, dataFetchQuery, queryWhereClause), dsID) + rows, err := tx.Query(fmt.Sprintf(queryFormatString, dataFetchQuery, queryWhereClause), dsID, dsID) if err != nil { return nil, errors.New("querying delivery service eligible servers: " + err.Error()) } diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go index f1a827a278..7cfe0fe560 100644 --- a/traffic_ops/traffic_ops_golang/server/servers.go +++ b/traffic_ops/traffic_ops_golang/server/servers.go @@ -104,9 +104,9 @@ AND ( FROM server_server_capability ssc WHERE ssc."server" = s.id ) @> ( - SELECT ARRAY_AGG(drc.required_capability) - FROM deliveryservices_required_capability drc - WHERE drc.deliveryservice_id = d.id + SELECT d.required_capabilities + FROM deliveryservice d + WHERE d.id = :dsId ) ` @@ -803,6 +803,7 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth usesMids := false queryAddition := "" dsHasRequiredCapabilities := false + var requiredCapabilities []string var dsID int var cdnID int var serverCount uint64 @@ -825,10 +826,13 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth } var joinSubQuery string - if err = tx.QueryRow(deliveryservice.HasRequiredCapabilitiesQuery, dsID).Scan(&dsHasRequiredCapabilities); err != nil { + if err = tx.QueryRow(deliveryservice.GetRequiredCapabilitiesQuery, dsID).Scan(pq.Array(&requiredCapabilities)); err != nil { err = fmt.Errorf("unable to get required capabilities for deliveryservice %d: %s", dsID, err) return nil, 0, nil, err, http.StatusInternalServerError, nil } + if requiredCapabilities != nil && len(requiredCapabilities) > 0 { + dsHasRequiredCapabilities = true + } joinSubQuery = dssTopologiesJoinSubquery // only if dsId is part of params: add join on deliveryservice_server table queryAddition = fmt.Sprintf(deliveryServiceServersJoin, joinSubQuery) @@ -1111,9 +1115,9 @@ func getMidServers(edgeIDs []int, servers map[int]tc.ServerV40, dsID int, cdnID capabilities.array_agg @> ( - SELECT ARRAY_AGG(drc.required_capability) - FROM deliveryservices_required_capability drc - WHERE drc.deliveryservice_id=:ds_id) + SELECT ds.required_capabilities + FROM deliveryservice ds + WHERE ds.id=:ds_id) )` } else { // TODO: include secondary parent? diff --git a/traffic_ops/traffic_ops_golang/server/servers_server_capability.go b/traffic_ops/traffic_ops_golang/server/servers_server_capability.go index ed52f21ad9..b57c40c39d 100644 --- a/traffic_ops/traffic_ops_golang/server/servers_server_capability.go +++ b/traffic_ops/traffic_ops_golang/server/servers_server_capability.go @@ -363,13 +363,13 @@ server) VALUES ( func checkDSReqCapQuery() string { return ` SELECT ARRAY( - SELECT dsrc.deliveryservice_id - FROM deliveryservices_required_capability as dsrc - WHERE deliveryservice_id IN ( + SELECT ds.id + FROM deliveryservice as ds + WHERE id IN ( SELECT deliveryservice FROM deliveryservice_server WHERE server = $1) - AND dsrc.required_capability = $2)` + AND $2 = ANY(ds.required_capabilities))` } // get the topology-based DSes (with all their required capabilities) that a given @@ -380,15 +380,14 @@ SELECT ds.xml_id, ds.topology, ds.tenant_id, - ARRAY_AGG(dsrc.required_capability) AS req_caps + ds.required_capabilities AS req_caps FROM server s JOIN cachegroup c ON s.cachegroup = c.id JOIN topology_cachegroup tc ON c.name = tc.cachegroup JOIN deliveryservice ds ON ds.topology = tc.topology -JOIN deliveryservices_required_capability dsrc ON dsrc.deliveryservice_id = ds.id WHERE s.id = $1 GROUP BY ds.xml_id, ds.tenant_id, ds.topology -HAVING $2 = ANY(ARRAY_AGG(dsrc.required_capability)) +HAVING $2 = ANY(ds.required_capabilities) ` } diff --git a/traffic_ops/traffic_ops_golang/topology/topologies.go b/traffic_ops/traffic_ops_golang/topology/topologies.go index 988913cc9d..9bbdf533be 100644 --- a/traffic_ops/traffic_ops_golang/topology/topologies.go +++ b/traffic_ops/traffic_ops_golang/topology/topologies.go @@ -340,12 +340,11 @@ func getDSRequiredCapabilitiesByTopology(name string, tx *sql.Tx) (map[string][] SELECT d.xml_id, d.cdn_id, - ARRAY_AGG(drc.required_capability) AS required_capabilities + d.required_capabilities FROM deliveryservice d -JOIN deliveryservices_required_capability drc ON d.id = drc.deliveryservice_id WHERE d.topology = $1 -GROUP BY d.xml_id, d.cdn_id +GROUP BY d.xml_id, d.cdn_id, d.required_capabilities ` rows, err := tx.Query(q, name) if err != nil { From e636b88f8d1fe1e42a27a9ef69b77c2bca4875ff Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Fri, 11 Nov 2022 11:56:31 -0700 Subject: [PATCH 2/7] fix tests --- .../testing/api/v3/deliveryservices_test.go | 10 ++++++++-- traffic_ops/testing/api/v3/todb_test.go | 1 - .../testing/api/v4/deliveryservices_test.go | 5 +++-- traffic_ops/testing/api/v4/todb_test.go | 1 - .../testing/api/v5/deliveryservices_test.go | 5 +++-- traffic_ops/testing/api/v5/todb_test.go | 1 - .../dbhelpers/db_helpers.go | 2 +- .../deliveryservice/deliveryservices.go | 15 +++++++++++---- .../deliveryservices_required_capabilities.go | 19 +++++++++---------- ...veryservices_required_capabilities_test.go | 9 ++++----- .../deliveryservice/deliveryservices_test.go | 1 + .../traffic_ops_golang/server/servers.go | 9 ++++++++- .../server/servers_server_capability.go | 2 +- 13 files changed, 49 insertions(+), 31 deletions(-) diff --git a/traffic_ops/testing/api/v3/deliveryservices_test.go b/traffic_ops/testing/api/v3/deliveryservices_test.go index bd4e994b67..fe47f2b427 100644 --- a/traffic_ops/testing/api/v3/deliveryservices_test.go +++ b/traffic_ops/testing/api/v3/deliveryservices_test.go @@ -17,6 +17,7 @@ package v3 import ( "encoding/json" + "fmt" "net/http" "net/url" "strconv" @@ -226,8 +227,9 @@ func TestDeliveryServices(t *testing.T) { "BAD REQUEST when ADDING TOPOLOGY to DS with DS REQUIRED CAPABILITY": { EndpointId: GetDeliveryServiceId(t, "ds1"), ClientSession: TOSession, RequestBody: generateDeliveryService(t, map[string]interface{}{ - "topology": "top-for-ds-req", - "xmlId": "ds1", + "requiredCapabilities": []string{"foo"}, + "topology": "top-for-ds-req", + "xmlId": "ds1", }), Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), }, @@ -361,6 +363,10 @@ func TestDeliveryServices(t *testing.T) { }) case "PUT": t.Run(name, func(t *testing.T) { + if name == "BAD REQUEST when ADDING TOPOLOGY to DS with DS REQUIRED CAPABILITY" { + fmt.Println(testCase.RequestBody) + fmt.Println(*ds.Topology) + } resp, reqInf, err := testCase.ClientSession.UpdateDeliveryServiceV30WithHdr(testCase.EndpointId(), ds, testCase.RequestHeaders) for _, check := range testCase.Expectations { check(t, reqInf, resp, tc.Alerts{}, err) diff --git a/traffic_ops/testing/api/v3/todb_test.go b/traffic_ops/testing/api/v3/todb_test.go index f42ae00ad4..123e94fa25 100644 --- a/traffic_ops/testing/api/v3/todb_test.go +++ b/traffic_ops/testing/api/v3/todb_test.go @@ -253,7 +253,6 @@ func Teardown(db *sql.DB) error { sqlStmt := ` DELETE FROM api_capability; - DELETE FROM deliveryservices_required_capability; DELETE FROM server_server_capability; DELETE FROM server_capability; DELETE FROM to_extension; diff --git a/traffic_ops/testing/api/v4/deliveryservices_test.go b/traffic_ops/testing/api/v4/deliveryservices_test.go index 98446dff88..fd6ef056b9 100644 --- a/traffic_ops/testing/api/v4/deliveryservices_test.go +++ b/traffic_ops/testing/api/v4/deliveryservices_test.go @@ -354,8 +354,9 @@ func TestDeliveryServices(t *testing.T) { "BAD REQUEST when ADDING TOPOLOGY to DS with DS REQUIRED CAPABILITY": { EndpointId: GetDeliveryServiceId(t, "ds1"), ClientSession: TOSession, RequestBody: generateDeliveryService(t, map[string]interface{}{ - "topology": "top-for-ds-req", - "xmlId": "ds1", + "requiredCapabilities": []string{"foo"}, + "topology": "top-for-ds-req", + "xmlId": "ds1", }), Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), }, diff --git a/traffic_ops/testing/api/v4/todb_test.go b/traffic_ops/testing/api/v4/todb_test.go index 7f9104a8a3..a80a3e997a 100644 --- a/traffic_ops/testing/api/v4/todb_test.go +++ b/traffic_ops/testing/api/v4/todb_test.go @@ -385,7 +385,6 @@ func Teardown(db *sql.DB) error { sqlStmt := ` DELETE FROM api_capability; - DELETE FROM deliveryservices_required_capability; DELETE FROM server_server_capability; DELETE FROM server_capability; DELETE FROM to_extension; diff --git a/traffic_ops/testing/api/v5/deliveryservices_test.go b/traffic_ops/testing/api/v5/deliveryservices_test.go index 5e400b0be4..2c5f327457 100644 --- a/traffic_ops/testing/api/v5/deliveryservices_test.go +++ b/traffic_ops/testing/api/v5/deliveryservices_test.go @@ -354,8 +354,9 @@ func TestDeliveryServices(t *testing.T) { "BAD REQUEST when ADDING TOPOLOGY to DS with DS REQUIRED CAPABILITY": { EndpointId: GetDeliveryServiceId(t, "ds1"), ClientSession: TOSession, RequestBody: generateDeliveryService(t, map[string]interface{}{ - "topology": "top-for-ds-req", - "xmlId": "ds1", + "topology": "top-for-ds-req", + "xmlId": "ds1", + "requiredCapabilities": []string{"foo"}, }), Expectations: utils.CkRequest(utils.HasError(), utils.HasStatus(http.StatusBadRequest)), }, diff --git a/traffic_ops/testing/api/v5/todb_test.go b/traffic_ops/testing/api/v5/todb_test.go index 2b09fa2903..aed6c19387 100644 --- a/traffic_ops/testing/api/v5/todb_test.go +++ b/traffic_ops/testing/api/v5/todb_test.go @@ -385,7 +385,6 @@ func Teardown(db *sql.DB) error { sqlStmt := ` DELETE FROM api_capability; - DELETE FROM deliveryservices_required_capability; DELETE FROM server_server_capability; DELETE FROM server_capability; DELETE FROM to_extension; diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go index 5ed708a2a8..656c9be55e 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go +++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go @@ -891,7 +891,7 @@ func DSRExistsWithXMLID(xmlid string, tx *sql.Tx) (bool, error) { return exists, err } -// ScanCachegroupsServerCapabilities, given rows of (server ID, CDN ID, cachegroup name, server capabilities), +// ScanCachegroupsServerCapabilities : given rows of (server ID, CDN ID, cachegroup name, server capabilities), // returns a map of cachegroup names to server IDs, a map of server IDs to a map of their capabilities, // a map of server IDs to CDN IDs, and an error (if one occurs). func ScanCachegroupsServerCapabilities(rows *sql.Rows) (map[string][]int, map[int]map[string]struct{}, map[int]int, error) { diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go index 941bb66432..92276f44a5 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go @@ -776,9 +776,17 @@ WHERE return nil, status, userErr, sysErr } func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 *tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, int, error, error) { + //ToDo: Srijeet change here to convert to v41 dsNull := tc.DeliveryServiceNullableV30(*dsV31) ds := dsNull.UpgradeToV4() dsV41 := ds + dsMap, err := dbhelpers.GetRequiredCapabilitiesOfDeliveryServices([]int{*dsV31.ID}, inf.Tx.Tx) + if err != nil { + return nil, http.StatusInternalServerError, nil, err + } + if caps, ok := dsMap[*dsV31.ID]; ok { + dsV41.RequiredCapabilities = caps + } if dsV41.ID == nil { return nil, http.StatusInternalServerError, nil, errors.New("cannot update a Delivery Service with nil ID") } @@ -789,11 +797,11 @@ func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 * return nil, http.StatusInternalServerError, nil, fmt.Errorf("getting TLS versions for DS #%d in API version < 4.0: %w", *dsV41.ID, sysErr) } - res, status, usrErr, sysErr := updateV40(w, r, inf, &dsV41.DeliveryServiceV40, false) + res, status, usrErr, sysErr := updateV41(w, r, inf, &dsV41, false) if res == nil || usrErr != nil || sysErr != nil { return nil, status, usrErr, sysErr } - ds.DeliveryServiceV40 = *res + ds.DeliveryServiceV40 = res.DeliveryServiceV40 if dsV31.CacheURL != nil { _, err := tx.Exec("UPDATE deliveryservice SET cacheurl = $1 WHERE ID = $2", dsV31.CacheURL, @@ -1399,9 +1407,8 @@ func Validate(tx *sql.Tx, ds *tc.DeliveryServiceV4) error { } func validateRequiredCapabilities(tx *sql.Tx, ds *tc.DeliveryServiceV4) error { - //ToDo: fix this var valid bool - query := `SELECT $1 @> (SELECT ARRAY_AGG(name) FROM server_capability)` + query := `SELECT $1 <@ (SELECT ARRAY_AGG(name) FROM server_capability)` if ds.RequiredCapabilities != nil && len(ds.RequiredCapabilities) > 0 { rows, err := tx.Query(query, pq.Array(ds.RequiredCapabilities)) if err != nil { diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go index 5ec1af84be..bcd94ac6c0 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities.go @@ -84,17 +84,13 @@ func (rc *RequiredCapability) ParamColumns() map[string]dbhelpers.WhereColumnInf Column: "ds.xml_id", Checker: nil, }, - requiredCapabilityQueryParam: dbhelpers.WhereColumnInfo{ - Column: "rc.required_capability", - Checker: nil, - }, } } // DeleteQuery implements the api.GenericDeleter interface. func (rc *RequiredCapability) DeleteQuery() string { return `UPDATE deliveryservice ds SET required_capabilities = ARRAY_REMOVE((select required_capabilities from deliveryservice WHERE id=:deliveryservice_id), :required_capability) -WHERE id=:deliveryservice_id` +WHERE id=:deliveryservice_id AND EXISTS(SELECT 1 FROM deliveryservice ds WHERE id=:deliveryservice_id AND :required_capability = ANY(ds.required_capabilities))` } // GetKeyFieldsInfo implements the api.Identifier interface. @@ -209,7 +205,7 @@ func (rc *RequiredCapability) getCapabilities(h http.Header, tenantIDs []int, us where, queryValues = dbhelpers.AddTenancyCheck(where, queryValues, "ds.tenant_id", tenantIDs) if useIMS { - runSecond, maxTime = ims.TryIfModifiedSinceQuery(rc.APIInfo().Tx, h, queryValues, selectMaxLastUpdatedQueryRC(where, orderBy, pagination)) + runSecond, maxTime = ims.TryIfModifiedSinceQuery(rc.APIInfo().Tx, h, queryValues, selectMaxLastUpdatedQuery(where)) if !runSecond { log.Debugln("IMS HIT") return results, nil, nil, http.StatusNotModified, &maxTime @@ -219,7 +215,10 @@ func (rc *RequiredCapability) getCapabilities(h http.Header, tenantIDs []int, us log.Debugln("Non IMS request") } query := rc.SelectQuery() + where + orderBy + pagination - + if reqdCap, ok := rc.APIInfo().Params[requiredCapabilityQueryParam]; ok { + query = `WITH res AS (` + query + `) SELECT res.required_capability, res.deliveryservice_id, res.xml_id, res.last_updated FROM res WHERE res.required_capability=:requiredCapability` + queryValues[requiredCapabilityQueryParam] = reqdCap + } rows, err := rc.APIInfo().Tx.NamedQuery(query, queryValues) if err != nil { return nil, nil, err, http.StatusInternalServerError, nil @@ -239,10 +238,10 @@ func (rc *RequiredCapability) getCapabilities(h http.Header, tenantIDs []int, us func selectMaxLastUpdatedQueryRC(where string, orderBy string, pagination string) string { return `SELECT max(t) from ( - SELECT max(rc.last_updated) as t FROM deliveryservices_required_capability rc - JOIN deliveryservice ds ON ds.id = rc.deliveryservice_id ` + where + orderBy + pagination + + SELECT max(d.last_updated) as t FROM deliveryservice d` + + where + orderBy + pagination + ` UNION ALL - select max(last_updated) as t from last_deleted l where l.table_name='deliveryservices_required_capability') as res` + select max(last_updated) as t from last_deleted l where l.table_name='deliveryservice') as res` } // Delete implements the api.CRUDer interface. diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities_test.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities_test.go index af60e2075f..5c480ea690 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities_test.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_required_capabilities_test.go @@ -92,12 +92,11 @@ func TestCreateDeliveryServicesRequiredCapability(t *testing.T) { arrayRows := sqlmock.NewRows([]string{"array"}) mock.ExpectQuery("SELECT ds.server FROM deliveryservice_server").WillReturnRows(arrayRows) - rows := sqlmock.NewRows([]string{"required_capability", "deliveryservice_id", "last_updated"}).AddRow( - util.StrPtr("mem"), + rows := sqlmock.NewRows([]string{"deliveryservice_id", "last_updated"}).AddRow( util.IntPtr(1), time.Now(), ) - mock.ExpectQuery("INSERT INTO deliveryservices_required_capability").WillReturnRows(rows) + mock.ExpectQuery("UPDATE deliveryservice").WillReturnRows(rows) userErr, sysErr, errCode := rc.Create() if userErr != nil { @@ -197,7 +196,7 @@ func TestReadDeliveryServicesRequiredCapability(t *testing.T) { capability.XMLID, time.Now(), ) - mock.ExpectQuery("SELECT .* FROM deliveryservices_required_capability").WillReturnRows(rows) + mock.ExpectQuery("SELECT .* FROM deliveryservice").WillReturnRows(rows) results, userErr, sysErr, errCode, _ := rc.Read(nil, false) if userErr != nil { @@ -248,7 +247,7 @@ func TestDeleteDeliveryServicesRequiredCapability(t *testing.T) { mock.ExpectQuery("SELECT").WillReturnRows(sqlmock.NewRows([]string{"xml_id", "name"}).AddRow("name", "cdnName")) mock.ExpectQuery("SELECT c.username").WillReturnRows(sqlmock.NewRows(nil)) - mock.ExpectExec("DELETE").WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec("UPDATE").WillReturnResult(sqlmock.NewResult(1, 1)) rc := RequiredCapability{ api.APIInfoImpl{ diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go index 7bfaeaa876..6b8548bf45 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go @@ -320,6 +320,7 @@ func TestReadGetDeliveryServices(t *testing.T) { {"regional", false}, {"regional_geo_blocking", false}, {"remap_text", nil}, + {"required_capabilities", nil}, {"routing_name", "video"}, {"service_category", nil}, {"signing_algorithm", nil}, diff --git a/traffic_ops/traffic_ops_golang/server/servers.go b/traffic_ops/traffic_ops_golang/server/servers.go index 7cfe0fe560..0a05baaf64 100644 --- a/traffic_ops/traffic_ops_golang/server/servers.go +++ b/traffic_ops/traffic_ops_golang/server/servers.go @@ -826,10 +826,17 @@ func getServers(h http.Header, params map[string]string, tx *sqlx.Tx, user *auth } var joinSubQuery string - if err = tx.QueryRow(deliveryservice.GetRequiredCapabilitiesQuery, dsID).Scan(pq.Array(&requiredCapabilities)); err != nil { + rows, err := tx.Query(deliveryservice.GetRequiredCapabilitiesQuery, dsID) + if err != nil { err = fmt.Errorf("unable to get required capabilities for deliveryservice %d: %s", dsID, err) return nil, 0, nil, err, http.StatusInternalServerError, nil } + for rows.Next() { + if err = rows.Scan(pq.Array(&requiredCapabilities)); err != nil { + err = fmt.Errorf("unable to scan required capabilities for deliveryservice %d: %s", dsID, err) + return nil, 0, nil, err, http.StatusInternalServerError, nil + } + } if requiredCapabilities != nil && len(requiredCapabilities) > 0 { dsHasRequiredCapabilities = true } diff --git a/traffic_ops/traffic_ops_golang/server/servers_server_capability.go b/traffic_ops/traffic_ops_golang/server/servers_server_capability.go index b57c40c39d..b1a5ee56ab 100644 --- a/traffic_ops/traffic_ops_golang/server/servers_server_capability.go +++ b/traffic_ops/traffic_ops_golang/server/servers_server_capability.go @@ -386,7 +386,7 @@ JOIN cachegroup c ON s.cachegroup = c.id JOIN topology_cachegroup tc ON c.name = tc.cachegroup JOIN deliveryservice ds ON ds.topology = tc.topology WHERE s.id = $1 -GROUP BY ds.xml_id, ds.tenant_id, ds.topology +GROUP BY ds.xml_id, ds.tenant_id, ds.topology, ds.required_capabilities HAVING $2 = ANY(ds.required_capabilities) ` } From 62c068190818f6004184ecb1ca80148624d1a3ba Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Fri, 11 Nov 2022 15:57:16 -0700 Subject: [PATCH 3/7] cleanup --- traffic_ops/testing/api/v3/deliveryservices_test.go | 5 ----- .../traffic_ops_golang/deliveryservice/deliveryservices.go | 1 - 2 files changed, 6 deletions(-) diff --git a/traffic_ops/testing/api/v3/deliveryservices_test.go b/traffic_ops/testing/api/v3/deliveryservices_test.go index fe47f2b427..0283250c8b 100644 --- a/traffic_ops/testing/api/v3/deliveryservices_test.go +++ b/traffic_ops/testing/api/v3/deliveryservices_test.go @@ -17,7 +17,6 @@ package v3 import ( "encoding/json" - "fmt" "net/http" "net/url" "strconv" @@ -363,10 +362,6 @@ func TestDeliveryServices(t *testing.T) { }) case "PUT": t.Run(name, func(t *testing.T) { - if name == "BAD REQUEST when ADDING TOPOLOGY to DS with DS REQUIRED CAPABILITY" { - fmt.Println(testCase.RequestBody) - fmt.Println(*ds.Topology) - } resp, reqInf, err := testCase.ClientSession.UpdateDeliveryServiceV30WithHdr(testCase.EndpointId(), ds, testCase.RequestHeaders) for _, check := range testCase.Expectations { check(t, reqInf, resp, tc.Alerts{}, err) diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go index 92276f44a5..5c1ad14270 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go @@ -776,7 +776,6 @@ WHERE return nil, status, userErr, sysErr } func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 *tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, int, error, error) { - //ToDo: Srijeet change here to convert to v41 dsNull := tc.DeliveryServiceNullableV30(*dsV31) ds := dsNull.UpgradeToV4() dsV41 := ds From b1400523f0e5838ba85e0ff6c92ddd72f21755e9 Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Fri, 11 Nov 2022 16:01:15 -0700 Subject: [PATCH 4/7] adding changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9b17d4ba7..5eeed2fc5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ### Changed - [#2564](https://github.com/apache/trafficcontrol/issues/2564) *Traffic Ops, Traffic Monitor* Renamed RASCAL references to TRAFFIC_MONITOR +- [#7186](https://github.com/apache/trafficcontrol/pull/7186) *Traffic Ops* Required Parameters are now a part of the `DeliveryService` structure. - [#7063](https://github.com/apache/trafficcontrol/pull/7063) *Traffic Ops* Python client now uses Traffic Ops API 4.1 by default. - [#6981](https://github.com/apache/trafficcontrol/pull/6981) *Traffic Portal* Obscures sensitive text in Delivery Service "Raw Remap" fields, private SSL keys, "Header Rewrite" rules, and ILO interface passwords by default. - [#7037](https://github.com/apache/trafficcontrol/pull/7037) *Traffic Router* Uses Traffic Ops API 4.0 by default From 6ed1c1461990a0f81b4b2b35d4fdd0818518373c Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Tue, 15 Nov 2022 17:41:54 -0700 Subject: [PATCH 5/7] adding doc changes --- ...deliveryservices_required_capabilities.rst | 6 ++ .../api/v4/deliveryservice_requests.rst | 16 +++-- .../v4/deliveryservice_requests_id_assign.rst | 6 +- .../v4/deliveryservice_requests_id_status.rst | 6 +- docs/source/api/v4/deliveryservices.rst | 21 +++++- docs/source/api/v4/deliveryservices_id.rst | 14 +++- ...deliveryservices_required_capabilities.rst | 6 ++ .../api/v4/servers_id_deliveryservices.rst | 5 ++ .../testing/api/conf/traffic-ops-test.conf | 6 +- .../traffic_ops_golang/api/shared_handlers.go | 67 ++++++++++++++----- .../traffic_ops_golang/routing/routes.go | 7 +- 11 files changed, 124 insertions(+), 36 deletions(-) diff --git a/docs/source/api/v3/deliveryservices_required_capabilities.rst b/docs/source/api/v3/deliveryservices_required_capabilities.rst index 9df9088bf3..66a1b0e4dc 100644 --- a/docs/source/api/v3/deliveryservices_required_capabilities.rst +++ b/docs/source/api/v3/deliveryservices_required_capabilities.rst @@ -19,6 +19,8 @@ ``deliveryservices_required_capabilities`` ****************************************** +.. deprecated:: ATCv7 + ``GET`` ======= Gets all associations of :term:`Server Capability` to :term:`Delivery Services`. @@ -102,6 +104,8 @@ Response Structure ] } +.. deprecated:: ATCv7 + ``POST`` ======== Associates a :term:`Server Capability` with a :term:`Delivery Service`. @@ -168,6 +172,8 @@ Response Structure } } +.. deprecated:: ATCv7 + ``DELETE`` ========== Dissociate a :term:`Server Capability` from a :term:`Delivery Service`. diff --git a/docs/source/api/v4/deliveryservice_requests.rst b/docs/source/api/v4/deliveryservice_requests.rst index dd628cd4d3..c88fb7ebdf 100644 --- a/docs/source/api/v4/deliveryservice_requests.rst +++ b/docs/source/api/v4/deliveryservice_requests.rst @@ -72,7 +72,7 @@ Request Structure .. code-block:: http :caption: Request Example - GET /api/4.0/deliveryservice_requests?status=draft HTTP/1.1 + GET /api/4.1/deliveryservice_requests?status=draft HTTP/1.1 User-Agent: python-requests/2.22.0 Accept-Encoding: gzip, deflate Accept: */* @@ -163,6 +163,7 @@ The response is an array of representations of :term:`Delivery Service Requests` "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -213,7 +214,7 @@ The request must be a well-formed representation of a :term:`Delivery Service Re .. code-block:: http :caption: Request Example - POST /api/4.0/deliveryservice_requests HTTP/1.1 + POST /api/4.1/deliveryservice_requests HTTP/1.1 User-Agent: python-requests/2.22.0 Accept-Encoding: gzip, deflate Accept: */* @@ -280,6 +281,7 @@ The request must be a well-formed representation of a :term:`Delivery Service Re "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -328,7 +330,7 @@ The response will be a representation of the created :term:`Delivery Service Req Content-Encoding: gzip Content-Type: application/json Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 24 Feb 2020 20:11:12 GMT; Max-Age=3600; HttpOnly - Location: /api/4.0/deliveryservice_requests/2 + Location: /api/4.1/deliveryservice_requests/2 X-Server-Name: traffic_ops_golang/ Date: Mon, 24 Feb 2020 19:11:12 GMT Content-Length: 901 @@ -405,6 +407,7 @@ The response will be a representation of the created :term:`Delivery Service Req "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -490,6 +493,7 @@ The response will be a representation of the created :term:`Delivery Service Req "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -550,7 +554,7 @@ The request body must be a representation of a :term:`Delivery Service Request` .. code-block:: http :caption: Request Example - PUT /api/4.0/deliveryservice_requests?id=1 HTTP/1.1 + PUT /api/4.1/deliveryservice_requests?id=1 HTTP/1.1 User-Agent: python-requests/2.22.0 Accept-Encoding: gzip, deflate Accept: */* @@ -667,6 +671,7 @@ The response is a full representation of the edited :term:`Delivery Service Requ "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -748,6 +753,7 @@ The response is a full representation of the edited :term:`Delivery Service Requ "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "cdn", "signed": false, "sslKeyVersion": null, @@ -800,7 +806,7 @@ Request Structure .. code-block:: http :caption: Request Example - DELETE /api/4.0/deliveryservice_requests?id=1 HTTP/1.1 + DELETE /api/4.1/deliveryservice_requests?id=1 HTTP/1.1 User-Agent: python-requests/2.22.0 Accept-Encoding: gzip, deflate Accept: */* diff --git a/docs/source/api/v4/deliveryservice_requests_id_assign.rst b/docs/source/api/v4/deliveryservice_requests_id_assign.rst index 9a9710acec..89b7e00018 100644 --- a/docs/source/api/v4/deliveryservice_requests_id_assign.rst +++ b/docs/source/api/v4/deliveryservice_requests_id_assign.rst @@ -42,7 +42,7 @@ Request Structure .. code-block:: http :caption: Request Example - GET /api/4.0/deliveryservice_requests/1/assign HTTP/1.1 + GET /api/4.1/deliveryservice_requests/1/assign HTTP/1.1 User-Agent: python-requests/2.24.0 Accept-Encoding: gzip, deflate Accept: */* @@ -103,7 +103,7 @@ Request Structure .. code-block:: http :caption: Request Example - PUT /api/4.0/deliveryservice_requests/1/assign HTTP/1.1 + PUT /api/4.1/deliveryservice_requests/1/assign HTTP/1.1 User-Agent: python-requests/2.24.0 Accept-Encoding: gzip, deflate Accept: */* @@ -198,6 +198,7 @@ The response contains a full representation of the newly assigned :term:`Deliver "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -279,6 +280,7 @@ The response contains a full representation of the newly assigned :term:`Deliver "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "cdn", "signed": false, "sslKeyVersion": null, diff --git a/docs/source/api/v4/deliveryservice_requests_id_status.rst b/docs/source/api/v4/deliveryservice_requests_id_status.rst index 933e8abe25..35b5b6b82a 100644 --- a/docs/source/api/v4/deliveryservice_requests_id_status.rst +++ b/docs/source/api/v4/deliveryservice_requests_id_status.rst @@ -45,7 +45,7 @@ Request Structure .. code-block:: http :caption: Request Example - GET /api/4.0/deliveryservice_requests/1/status HTTP/1.1 + GET /api/4.1/deliveryservice_requests/1/status HTTP/1.1 User-Agent: python-requests/2.24.0 Accept-Encoding: gzip, deflate Accept: */* @@ -98,7 +98,7 @@ Request Structure .. code-block:: http :caption: Request Example - PUT /api/4.0/deliveryservice_requests/1/status HTTP/1.1 + PUT /api/4.1/deliveryservice_requests/1/status HTTP/1.1 User-Agent: python-requests/2.22.0 Accept-Encoding: gzip, deflate Accept: */* @@ -195,6 +195,7 @@ The response is a full representation of the modified :term:`DSR`. "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "signed": false, "sslKeyVersion": 1, @@ -276,6 +277,7 @@ The response is a full representation of the modified :term:`DSR`. "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "cdn", "signed": false, "sslKeyVersion": null, diff --git a/docs/source/api/v4/deliveryservices.rst b/docs/source/api/v4/deliveryservices.rst index f6e3934d9b..48e64b0261 100644 --- a/docs/source/api/v4/deliveryservices.rst +++ b/docs/source/api/v4/deliveryservices.rst @@ -73,7 +73,7 @@ Request Structure .. code-block:: http :caption: Request Example - GET /api/4.0/deliveryservices?xmlId=demo2 HTTP/1.1 + GET /api/4.1/deliveryservices?xmlId=demo2 HTTP/1.1 Host: trafficops.infra.ciab.test User-Agent: python-requests/2.24.0 Accept-Encoding: gzip, deflate @@ -148,6 +148,10 @@ Response Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` +:requiredCapabilities: An array of the capabilities that this delivery service requires. + + .. versionchanged:: 4.1 + :serviceCategory: The name of the :ref:`ds-service-category` with which the :term:`Delivery Service` is associated :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise :signingAlgorithm: Either a :ref:`ds-signing-algorithm` or ``null`` to indicate URL/URI signing is not implemented on this :term:`Delivery Service` @@ -250,6 +254,7 @@ Response Structure "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "video", "serviceCategory": null, "signed": false, @@ -328,6 +333,10 @@ Request Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` +:requiredCapabilities: An array of the capabilities that this delivery service requires. + + .. versionchanged:: 4.1 + :serviceCategory: The name of the :ref:`ds-service-category` with which the :term:`Delivery Service` is associated - or ``null`` if there is to be no such category :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise :signingAlgorithm: Either a :ref:`ds-signing-algorithm` or ``null`` to indicate URL/URI signing is not implemented on this :term:`Delivery Service` @@ -348,7 +357,7 @@ Request Structure .. code-block:: http :caption: Request Example - POST /api/4.0/deliveryservices HTTP/1.1 + POST /api/4.1/deliveryservices HTTP/1.1 User-Agent: python-requests/2.24.0 Accept-Encoding: gzip, deflate Accept: */* @@ -407,6 +416,7 @@ Request Structure "regexRemap": null, "regional": false, "regionalGeoBlocking": false, + "requiredCapabilities": null, "routingName": "test", "serviceCategory": null, "signed": false, @@ -495,6 +505,10 @@ Response Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` +:requiredCapabilities: An array of the capabilities that this delivery service requires. + + .. versionchanged:: 4.1 + :serviceCategory: The name of the :ref:`ds-service-category` with which the :term:`Delivery Service` is associated :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise :signingAlgorithm: Either a :ref:`ds-signing-algorithm` or ``null`` to indicate URL/URI signing is not implemented on this :term:`Delivery Service` @@ -522,7 +536,7 @@ Response Structure Access-Control-Allow-Origin: * Content-Encoding: gzip Content-Type: application/json - Location: /api/4.0/deliveryservices?id=6 + Location: /api/4.1/deliveryservices?id=6 Permissions-Policy: interest-cohort=() Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 07 Jun 2021 23:37:37 GMT; Max-Age=3600; HttpOnly Vary: Accept-Encoding @@ -606,6 +620,7 @@ Response Structure "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "test", "serviceCategory": null, "signed": false, diff --git a/docs/source/api/v4/deliveryservices_id.rst b/docs/source/api/v4/deliveryservices_id.rst index c2ce373d9f..823a5a953e 100644 --- a/docs/source/api/v4/deliveryservices_id.rst +++ b/docs/source/api/v4/deliveryservices_id.rst @@ -81,6 +81,10 @@ Request Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` +:requiredCapabilities: An array of the capabilities that this delivery service requires. + + .. versionchanged:: 4.1 + :routingName: The :ref:`ds-routing-name` of this :term:`Delivery Service` .. note:: If the Delivery Service has SSL Keys, then ``routingName`` is not allowed to change as that would invalidate the SSL Key @@ -106,7 +110,7 @@ Request Structure .. code-block:: http :caption: Request Example - PUT /api/4.0/deliveryservices/6 HTTP/1.1 + PUT /api/4.1/deliveryservices/6 HTTP/1.1 Host: trafficops.infra.ciab.test User-Agent: python-requests/2.24.0 Accept-Encoding: gzip, deflate @@ -165,6 +169,7 @@ Request Structure "regexRemap": null, "regional": false, "regionalGeoBlocking": false, + "requiredCapabilities": null, "routingName": "test", "serviceCategory": null, "signed": false, @@ -249,6 +254,10 @@ Response Structure :regexRemap: A :ref:`ds-regex-remap` :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` +:requiredCapabilities: An array of the capabilities that this delivery service requires. + + .. versionchanged:: 4.1 + :remapText: :ref:`ds-raw-remap` :serviceCategory: The name of the :ref:`ds-service-category` with which the :term:`Delivery Service` is associated :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise @@ -355,6 +364,7 @@ Response Structure "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "test", "serviceCategory": null, "signed": false, @@ -394,7 +404,7 @@ Request Structure .. code-block:: http :caption: Request Example - DELETE /api/4.0/deliveryservices/2 HTTP/1.1 + DELETE /api/4.1/deliveryservices/2 HTTP/1.1 Host: trafficops.infra.ciab.test User-Agent: curl/7.47.0 Accept: */* diff --git a/docs/source/api/v4/deliveryservices_required_capabilities.rst b/docs/source/api/v4/deliveryservices_required_capabilities.rst index 1bf4b4d81a..40b2532e13 100644 --- a/docs/source/api/v4/deliveryservices_required_capabilities.rst +++ b/docs/source/api/v4/deliveryservices_required_capabilities.rst @@ -19,6 +19,8 @@ ``deliveryservices_required_capabilities`` ****************************************** +.. deprecated:: ATCv7 + ``GET`` ======= Gets all associations of :term:`Server Capability` to :term:`Delivery Services`. @@ -103,6 +105,8 @@ Response Structure ] } +.. deprecated:: ATCv7 + ``POST`` ======== Associates a :term:`Server Capability` with a :term:`Delivery Service`. @@ -170,6 +174,8 @@ Response Structure } } +.. deprecated:: ATCv7 + ``DELETE`` ========== Dissociate a :term:`Server Capability` from a :term:`Delivery Service`. diff --git a/docs/source/api/v4/servers_id_deliveryservices.rst b/docs/source/api/v4/servers_id_deliveryservices.rst index 386de7532a..b434f7ea5e 100644 --- a/docs/source/api/v4/servers_id_deliveryservices.rst +++ b/docs/source/api/v4/servers_id_deliveryservices.rst @@ -134,6 +134,10 @@ Response Structure :regional: A boolean value defining the :ref:`ds-regional` setting on this :term:`Delivery Service` :regionalGeoBlocking: A boolean defining the :ref:`ds-regionalgeo` setting on this :term:`Delivery Service` :remapText: :ref:`ds-raw-remap` +:requiredCapabilities: An array of the capabilities that this delivery service requires. + + .. versionchanged:: 4.1 + :signed: ``true`` if and only if ``signingAlgorithm`` is not ``null``, ``false`` otherwise :signingAlgorithm: Either a :ref:`ds-signing-algorithm` or ``null`` to indicate URL/URI signing is not implemented on this :term:`Delivery Service` :rangeSliceBlockSize: An integer that defines the byte block size for the ATS Slice Plugin. It can only and must be set if ``rangeRequestHandling`` is set to 3. @@ -234,6 +238,7 @@ Response Structure "regional": false, "regionalGeoBlocking": false, "remapText": null, + "requiredCapabilities": null, "routingName": "cdn", "serviceCategory": null, "signed": false, diff --git a/traffic_ops/testing/api/conf/traffic-ops-test.conf b/traffic_ops/testing/api/conf/traffic-ops-test.conf index 1143dc0979..6927cfd624 100644 --- a/traffic_ops/testing/api/conf/traffic-ops-test.conf +++ b/traffic_ops/testing/api/conf/traffic-ops-test.conf @@ -15,7 +15,7 @@ "use_ims": true, "role_based_permissions": false, "trafficOps": { - "URL": "https://localhost:6443", + "URL": "https://localhost:8443", "password": "twelve", "users": { "disallowed": "disallowed", @@ -28,10 +28,10 @@ } }, "trafficOpsDB": { - "dbname": "traffic_ops_development", + "dbname": "to_test", "description": "dev database traffic_ops_development", "hostname": "localhost", - "password": "twelve12", + "password": "", "port": "5432", "type": "Pg", "user": "traffic_ops" diff --git a/traffic_ops/traffic_ops_golang/api/shared_handlers.go b/traffic_ops/traffic_ops_golang/api/shared_handlers.go index 3fe6e786d7..8fb9815547 100644 --- a/traffic_ops/traffic_ops_golang/api/shared_handlers.go +++ b/traffic_ops/traffic_ops_golang/api/shared_handlers.go @@ -142,6 +142,7 @@ func SetLastModifiedHeader(r *http.Request, useIMS bool) bool { type errWriterFunc func(w http.ResponseWriter, r *http.Request, tx *sql.Tx, statusCode int, userErr error, sysErr error) type readSuccessWriterFunc func(w http.ResponseWriter, r *http.Request, statusCode int, results interface{}) type deleteSuccessWriterFunc func(w http.ResponseWriter, r *http.Request, message string) +type createSuccessWriterFunc func(w http.ResponseWriter, r *http.Request, statusCode int, alerts tc.Alerts, results interface{}) // ReadHandler creates a handler function from the pointer to a struct implementing the Reader interface // @@ -471,17 +472,33 @@ func deleteHandlerHelper(deleter Deleter, errHandler errWriterFunc, successHandl // *change log entry // *forming and writing the body over the wire func CreateHandler(creator Creator) http.HandlerFunc { + return createHandlerHelper( + creator, + HandleErr, + func(w http.ResponseWriter, r *http.Request, statusCode int, alerts tc.Alerts, results interface{}) { + w.WriteHeader(statusCode) + if len(alerts.Alerts) > 0 { + WriteAlertsObj(w, r, statusCode, alerts, results) + } else { + WriteResp(w, r, results) + } + + }, + ) +} + +func createHandlerHelper(creator Creator, errHandler errWriterFunc, successHandler createSuccessWriterFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { inf, userErr, sysErr, errCode := NewInfo(r, nil, nil) if userErr != nil || sysErr != nil { - HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr) + errHandler(w, r, inf.Tx.Tx, errCode, userErr, sysErr) return } defer inf.Close() interfacePtr := reflect.ValueOf(creator) if interfacePtr.Kind() != reflect.Ptr { - HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("reflect: can only indirect from a pointer")) + errHandler(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("reflect: can only indirect from a pointer")) return } objectType := reflect.Indirect(interfacePtr).Type() @@ -491,13 +508,13 @@ func CreateHandler(creator Creator) http.HandlerFunc { if c, ok := obj.(MultipleCreator); ok && c.AllowMultipleCreates() { data, err := ioutil.ReadAll(r.Body) if err != nil { - HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, err, nil) + errHandler(w, r, inf.Tx.Tx, http.StatusBadRequest, err, nil) return } objSlice, err := parseMultipleCreates(data, objectType, inf) if err != nil { - HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err) + errHandler(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, err) return } @@ -510,30 +527,30 @@ func CreateHandler(creator Creator) http.HandlerFunc { if sysErr != nil { code = http.StatusInternalServerError } - HandleErr(w, r, inf.Tx.Tx, code, userErr, sysErr) + errHandler(w, r, inf.Tx.Tx, code, userErr, sysErr) return } if t, ok := objElem.(Tenantable); ok { authorized, err := t.IsTenantAuthorized(inf.User) if err != nil { - HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("checking tenant authorized: "+err.Error())) + errHandler(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("checking tenant authorized: "+err.Error())) return } if !authorized { - HandleErr(w, r, inf.Tx.Tx, http.StatusForbidden, errors.New("not authorized on this tenant"), nil) + errHandler(w, r, inf.Tx.Tx, http.StatusForbidden, errors.New("not authorized on this tenant"), nil) return } } userErr, sysErr, errCode = objElem.Create() if userErr != nil || sysErr != nil { - HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr) + errHandler(w, r, inf.Tx.Tx, errCode, userErr, sysErr) return } if err = CreateChangeLog(ApiChange, Created, objElem, inf.User, inf.Tx.Tx); err != nil { - HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("inserting changelog: %w", err)) + errHandler(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("inserting changelog: %w", err)) return } } @@ -557,7 +574,7 @@ func CreateHandler(creator Creator) http.HandlerFunc { alerts.AddAlerts(objElem.(AlertsResponse).GetAlerts()) } } - WriteAlertsObj(w, r, http.StatusOK, alerts, responseObj) + successHandler(w, r, http.StatusOK, alerts, responseObj) } else { userErr, sysErr := decodeAndValidateRequestBody(r, obj) @@ -566,41 +583,59 @@ func CreateHandler(creator Creator) http.HandlerFunc { if sysErr != nil { code = http.StatusInternalServerError } - HandleErr(w, r, inf.Tx.Tx, code, userErr, sysErr) + errHandler(w, r, inf.Tx.Tx, code, userErr, sysErr) return } if t, ok := obj.(Tenantable); ok { authorized, err := t.IsTenantAuthorized(inf.User) if err != nil { - HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("checking tenant authorized: "+err.Error())) + errHandler(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("checking tenant authorized: "+err.Error())) return } if !authorized { - HandleErr(w, r, inf.Tx.Tx, http.StatusForbidden, errors.New("not authorized on this tenant"), nil) + errHandler(w, r, inf.Tx.Tx, http.StatusForbidden, errors.New("not authorized on this tenant"), nil) return } } userErr, sysErr, errCode = obj.Create() if userErr != nil || sysErr != nil { - HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr) + errHandler(w, r, inf.Tx.Tx, errCode, userErr, sysErr) return } if err := CreateChangeLog(ApiChange, Created, obj, inf.User, inf.Tx.Tx); err != nil { - HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("inserting changelog: %w", err)) + errHandler(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, fmt.Errorf("inserting changelog: %w", err)) return } alerts := tc.CreateAlerts(tc.SuccessLevel, obj.GetType()+" was created.") if alertsObj, hasAlerts := obj.(AlertsResponse); hasAlerts { alerts.AddAlerts(alertsObj.GetAlerts()) } - WriteAlertsObj(w, r, http.StatusOK, alerts, obj) + successHandler(w, r, http.StatusOK, alerts, obj) } } } +// DeprecatedCreateHandler creates a net/http.HandlerFunc for the passed Creator object, and adds a deprecation +// notice, optionally with a passed alternative route suggestion. +func DeprecatedCreateHandler(creator Creator, alternative *string) http.HandlerFunc { + return createHandlerHelper( + creator, + func(w http.ResponseWriter, r *http.Request, tx *sql.Tx, statusCode int, userErr error, sysErr error) { + HandleDeprecatedErr(w, r, tx, statusCode, userErr, sysErr, alternative) + }, + func(w http.ResponseWriter, r *http.Request, statusCode int, alerts tc.Alerts, results interface{}) { + depAlerts := CreateDeprecationAlerts(alternative) + for _, al := range alerts.Alerts { + depAlerts.Alerts = append(depAlerts.Alerts, al) + } + WriteAlertsObj(w, r, statusCode, depAlerts, results) + }, + ) +} + func parseMultipleCreates(data []byte, desiredType reflect.Type, inf *APIInfo) ([]Creator, error) { buf := ioutil.NopCloser(bytes.NewReader(data)) diff --git a/traffic_ops/traffic_ops_golang/routing/routes.go b/traffic_ops/traffic_ops_golang/routing/routes.go index 0b5796b39c..de9382737e 100644 --- a/traffic_ops/traffic_ops_golang/routing/routes.go +++ b/traffic_ops/traffic_ops_golang/routing/routes.go @@ -23,6 +23,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/apache/trafficcontrol/lib/go-util" "net/http" "runtime" "strconv" @@ -798,9 +799,9 @@ func Routes(d ServerData) ([]Route, http.Handler, error) { {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodDelete, Path: `deliveryservices/{xmlID}/urisignkeys$`, Handler: urisigning.RemoveDeliveryServiceURIKeysHandler, RequiredPrivLevel: auth.PrivLevelAdmin, RequiredPermissions: []string{"DS-SECURITY-KEY:DELETE", "DS-SECURITY-KEY:READ", "DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 4299254173}, //Delivery Service Required Capabilities: CRUD - {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodGet, Path: `deliveryservices_required_capabilities/?$`, Handler: api.ReadHandler(&deliveryservice.RequiredCapability{}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 41585222273}, - {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodPost, Path: `deliveryservices_required_capabilities/?$`, Handler: api.CreateHandler(&deliveryservice.RequiredCapability{}), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 40968739923}, - {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodDelete, Path: `deliveryservices_required_capabilities/?$`, Handler: api.DeleteHandler(&deliveryservice.RequiredCapability{}), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 44962893043}, + {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodGet, Path: `deliveryservices_required_capabilities/?$`, Handler: api.DeprecatedReadHandler(&deliveryservice.RequiredCapability{}, util.StrPtr("the deliveryservices endpoint")), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 41585222273}, + {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodPost, Path: `deliveryservices_required_capabilities/?$`, Handler: api.DeprecatedCreateHandler(&deliveryservice.RequiredCapability{}, util.StrPtr("the deliveryservices endpoint")), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 40968739923}, + {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodDelete, Path: `deliveryservices_required_capabilities/?$`, Handler: api.DeprecatedDeleteHandler(&deliveryservice.RequiredCapability{}, util.StrPtr("the deliveryservices endpoint")), RequiredPrivLevel: auth.PrivLevelOperations, RequiredPermissions: []string{"DELIVERY-SERVICE:READ", "DELIVERY-SERVICE:UPDATE"}, Authenticated: Authenticated, Middlewares: nil, ID: 44962893043}, // Federations by CDN (the actual table for federation) {Version: api.Version{Major: 4, Minor: 0}, Method: http.MethodGet, Path: `cdns/{name}/federations/?$`, Handler: api.ReadHandler(&cdnfederation.TOCDNFederation{}), RequiredPrivLevel: auth.PrivLevelReadOnly, RequiredPermissions: []string{"CDN:READ", "FEDERATION:READ", "DELIVERY-SERVICE:READ"}, Authenticated: Authenticated, Middlewares: nil, ID: 4892250323}, From 77aebfb265317cf1f35d23fcb670a8feefe3442f Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Wed, 16 Nov 2022 11:40:52 -0700 Subject: [PATCH 6/7] tests passing --- lib/go-tc/deliveryservice_requests.go | 121 ++++++++++++++++-- lib/go-tc/deliveryservice_requests_test.go | 4 +- .../testing/api/conf/traffic-ops-test.conf | 6 +- .../api/v4/deliveryservice_requests_test.go | 7 +- .../api/v5/deliveryservice_requests_test.go | 7 +- .../deliveryservice/request/assign.go | 18 ++- .../deliveryservice/request/requests.go | 97 +++++++++----- .../deliveryservice/request/status.go | 22 ++-- .../deliveryservice/request/validate.go | 2 +- 9 files changed, 216 insertions(+), 68 deletions(-) diff --git a/lib/go-tc/deliveryservice_requests.go b/lib/go-tc/deliveryservice_requests.go index 91ff66e454..83a4b24698 100644 --- a/lib/go-tc/deliveryservice_requests.go +++ b/lib/go-tc/deliveryservice_requests.go @@ -436,6 +436,54 @@ type DeliveryServiceRequestNullable struct { XMLID *string `json:"-" db:"xml_id"` } +// Downgrade will convert an instance of DeliveryServiceRequestV4 to DeliveryServiceRequestV40 +func (dsr DeliveryServiceRequestV4) Downgrade() DeliveryServiceRequestV40 { + var dsrV40 DeliveryServiceRequestV40 + dsrV40.Assignee = dsr.Assignee + dsrV40.AssigneeID = dsr.AssigneeID + dsrV40.Author = dsr.Author + dsrV40.AuthorID = dsr.AuthorID + dsrV40.ChangeType = dsr.ChangeType + dsrV40.CreatedAt = dsr.CreatedAt + dsrV40.ID = dsr.ID + dsrV40.LastEditedBy = dsr.LastEditedBy + dsrV40.LastEditedByID = dsr.LastEditedByID + dsrV40.LastUpdated = dsr.LastUpdated + if dsr.Original != nil { + dsrV40.Original = &dsr.Original.DeliveryServiceV40 + } + if dsr.Requested != nil { + dsrV40.Requested = &dsr.Requested.DeliveryServiceV40 + } + dsrV40.Status = dsr.Status + dsrV40.XMLID = dsr.XMLID + return dsrV40 +} + +// Upgrade will convert an instance of DeliveryServiceRequestV40 to DeliveryServiceRequestV4 +func (dsrV40 DeliveryServiceRequestV40) Upgrade() DeliveryServiceRequestV4 { + var dsrV4 DeliveryServiceRequestV41 + dsrV4.Assignee = dsrV40.Assignee + dsrV4.AssigneeID = dsrV40.AssigneeID + dsrV4.Author = dsrV40.Author + dsrV4.AuthorID = dsrV40.AuthorID + dsrV4.ChangeType = dsrV40.ChangeType + dsrV4.CreatedAt = dsrV40.CreatedAt + dsrV4.ID = dsrV40.ID + dsrV4.LastEditedBy = dsrV40.LastEditedBy + dsrV4.LastEditedByID = dsrV40.LastEditedByID + dsrV4.LastUpdated = dsrV40.LastUpdated + if dsrV40.Original != nil { + dsrV4.Original = &DeliveryServiceV4{DeliveryServiceV40: *dsrV40.Original} + } + if dsrV40.Requested != nil { + dsrV4.Requested = &DeliveryServiceV4{DeliveryServiceV40: *dsrV40.Requested} + } + dsrV4.Status = dsrV40.Status + dsrV4.XMLID = dsrV40.XMLID + return dsrV4 +} + // Upgrade coerces the DeliveryServiceRequestNullable to the newer // DeliveryServiceRequestV40 structure. // @@ -469,11 +517,13 @@ func (dsr DeliveryServiceRequestNullable) Upgrade() DeliveryServiceRequestV40 { } if dsr.DeliveryService != nil { if upgraded.ChangeType == DSRChangeTypeDelete { - upgraded.Original = new(DeliveryServiceV4) - *upgraded.Original = dsr.DeliveryService.UpgradeToV4() + upgraded.Original = new(DeliveryServiceV40) + orig := dsr.DeliveryService.UpgradeToV4().DeliveryServiceV40 + upgraded.Original = &orig } else { - upgraded.Requested = new(DeliveryServiceV4) - *upgraded.Requested = dsr.DeliveryService.UpgradeToV4() + upgraded.Requested = new(DeliveryServiceV40) + requested := dsr.DeliveryService.UpgradeToV4().DeliveryServiceV40 + upgraded.Requested = &requested } } if dsr.ID != nil { @@ -695,6 +745,53 @@ func (dsrct *DSRChangeType) UnmarshalJSON(b []byte) error { return nil } +// DeliveryServiceRequestV41 is the type of a Delivery Service Request in +// Traffic Ops API version 4.1. +type DeliveryServiceRequestV41 struct { + // Assignee is the username of the user assigned to the Delivery Service + // Request, if any. + Assignee *string `json:"assignee"` + // AssigneeID is the integral, unique identifier of the user assigned to the + // Delivery Service Request, if any. + AssigneeID *int `json:"-" db:"assignee_id"` + // Author is the username of the user who created the Delivery Service + // Request. + Author string `json:"author"` + // AuthorID is the integral, unique identifier of the user who created the + // Delivery Service Request, if/when it is known. + AuthorID *int `json:"-" db:"author_id"` + // ChangeType represents the type of change being made, must be one of + // "create", "change" or "delete". + ChangeType DSRChangeType `json:"changeType" db:"change_type"` + // CreatedAt is the date/time at which the Delivery Service Request was + // created. + CreatedAt time.Time `json:"createdAt" db:"created_at"` + // ID is the integral, unique identifier for the Delivery Service Request + // if/when it is known. + ID *int `json:"id" db:"id"` + // LastEditedBy is the username of the user by whom the Delivery Service + // Request was last edited. + LastEditedBy string `json:"lastEditedBy"` + // LastEditedByID is the integral, unique identifier of the user by whom the + // Delivery Service Request was last edited, if/when it is known. + LastEditedByID *int `json:"-" db:"last_edited_by_id"` + // LastUpdated is the date/time at which the Delivery Service was last + // modified. + LastUpdated time.Time `json:"lastUpdated" db:"last_updated"` + // Original is the original Delivery Service for which changes are + // requested. This is present in responses only for ChangeTypes 'change' and + // 'delete', and is only required in requests where ChangeType is 'delete'. + Original *DeliveryServiceV41 `json:"original,omitempty" db:"original"` + // Requested is the set of requested changes. This is present in responses + // only for ChangeTypes 'change' and 'create', and is only required in + // requests in those cases. + Requested *DeliveryServiceV41 `json:"requested,omitempty" db:"deliveryservice"` + // Status is the status of the Delivery Service Request. + Status RequestStatus `json:"status" db:"status"` + // Used internally to define the affected Delivery Service. + XMLID string `json:"-"` +} + // DeliveryServiceRequestV40 is the type of a Delivery Service Request in // Traffic Ops API version 4.0. type DeliveryServiceRequestV40 struct { @@ -731,11 +828,11 @@ type DeliveryServiceRequestV40 struct { // Original is the original Delivery Service for which changes are // requested. This is present in responses only for ChangeTypes 'change' and // 'delete', and is only required in requests where ChangeType is 'delete'. - Original *DeliveryServiceV4 `json:"original,omitempty" db:"original"` + Original *DeliveryServiceV40 `json:"original,omitempty" db:"original"` // Requested is the set of requested changes. This is present in responses // only for ChangeTypes 'change' and 'create', and is only required in // requests in those cases. - Requested *DeliveryServiceV4 `json:"requested,omitempty" db:"deliveryservice"` + Requested *DeliveryServiceV40 `json:"requested,omitempty" db:"deliveryservice"` // Status is the status of the Delivery Service Request. Status RequestStatus `json:"status" db:"status"` // Used internally to define the affected Delivery Service. @@ -744,7 +841,7 @@ type DeliveryServiceRequestV40 struct { // DeliveryServiceRequestV4 is the type of a Delivery Service Request as it // appears in API version 4. -type DeliveryServiceRequestV4 = DeliveryServiceRequestV40 +type DeliveryServiceRequestV4 = DeliveryServiceRequestV41 // IsOpen returns whether or not the Delivery Service Request is still "open" - // i.e. has not been rejected or completed. @@ -793,10 +890,16 @@ func (dsr DeliveryServiceRequestV40) Downgrade() DeliveryServiceRequestNullable downgraded.CreatedAt = TimeNoModFromTime(dsr.CreatedAt) if dsr.Requested != nil { downgraded.DeliveryService = new(DeliveryServiceNullableV30) - *downgraded.DeliveryService = dsr.Requested.DowngradeToV31() + if dsr.Requested != nil { + dsV4 := DeliveryServiceV4{DeliveryServiceV40: *dsr.Requested} + *downgraded.DeliveryService = dsV4.DowngradeToV31() + } } else if dsr.Original != nil { downgraded.DeliveryService = new(DeliveryServiceNullableV30) - *downgraded.DeliveryService = dsr.Original.DowngradeToV31() + if dsr.Original != nil { + dsV4 := DeliveryServiceV4{DeliveryServiceV40: *dsr.Original} + *downgraded.DeliveryService = dsV4.DowngradeToV31() + } } if dsr.ID != nil { downgraded.ID = new(int) diff --git a/lib/go-tc/deliveryservice_requests_test.go b/lib/go-tc/deliveryservice_requests_test.go index 2a6b00d07f..6c316d7de1 100644 --- a/lib/go-tc/deliveryservice_requests_test.go +++ b/lib/go-tc/deliveryservice_requests_test.go @@ -148,7 +148,7 @@ func TestDeliveryServiceRequestV40_Downgrade(t *testing.T) { LastEditedBy: "last edited by", LastEditedByID: nil, LastUpdated: time.Now(), - Requested: &DeliveryServiceV4{}, + Requested: &DeliveryServiceV40{}, Status: RequestStatusComplete, } dsr.Requested.XMLID = &xmlid @@ -213,7 +213,7 @@ func ExampleDeliveryServiceRequestV40_SetXMLID() { var dsr DeliveryServiceRequestV40 fmt.Println(dsr.XMLID == "") - dsr.Requested = new(DeliveryServiceV4) + dsr.Requested = new(DeliveryServiceV40) dsr.Requested.XMLID = new(string) *dsr.Requested.XMLID = "test" dsr.SetXMLID() diff --git a/traffic_ops/testing/api/conf/traffic-ops-test.conf b/traffic_ops/testing/api/conf/traffic-ops-test.conf index 6927cfd624..1143dc0979 100644 --- a/traffic_ops/testing/api/conf/traffic-ops-test.conf +++ b/traffic_ops/testing/api/conf/traffic-ops-test.conf @@ -15,7 +15,7 @@ "use_ims": true, "role_based_permissions": false, "trafficOps": { - "URL": "https://localhost:8443", + "URL": "https://localhost:6443", "password": "twelve", "users": { "disallowed": "disallowed", @@ -28,10 +28,10 @@ } }, "trafficOpsDB": { - "dbname": "to_test", + "dbname": "traffic_ops_development", "description": "dev database traffic_ops_development", "hostname": "localhost", - "password": "", + "password": "twelve12", "port": "5432", "type": "Pg", "user": "traffic_ops" diff --git a/traffic_ops/testing/api/v4/deliveryservice_requests_test.go b/traffic_ops/testing/api/v4/deliveryservice_requests_test.go index 07e2e12111..829a9c67d0 100644 --- a/traffic_ops/testing/api/v4/deliveryservice_requests_test.go +++ b/traffic_ops/testing/api/v4/deliveryservice_requests_test.go @@ -370,9 +370,10 @@ func validatePutDSRequestFields(expectedResp map[string]interface{}) utils.CkReq func CreateTestDeliveryServiceRequests(t *testing.T) { for _, dsr := range testData.DeliveryServiceRequests { - resetDS(dsr.Original) - resetDS(dsr.Requested) - respDSR, _, err := TOSession.CreateDeliveryServiceRequest(dsr, client.RequestOptions{}) + dsrV41 := dsr.Upgrade() + resetDS(dsrV41.Original) + resetDS(dsrV41.Requested) + respDSR, _, err := TOSession.CreateDeliveryServiceRequest(dsrV41, client.RequestOptions{}) assert.NoError(t, err, "Could not create Delivery Service Requests: %v - alerts: %+v", err, respDSR.Alerts) } } diff --git a/traffic_ops/testing/api/v5/deliveryservice_requests_test.go b/traffic_ops/testing/api/v5/deliveryservice_requests_test.go index c6e478a152..4ef28b48f7 100644 --- a/traffic_ops/testing/api/v5/deliveryservice_requests_test.go +++ b/traffic_ops/testing/api/v5/deliveryservice_requests_test.go @@ -370,9 +370,10 @@ func validatePutDSRequestFields(expectedResp map[string]interface{}) utils.CkReq func CreateTestDeliveryServiceRequests(t *testing.T) { for _, dsr := range testData.DeliveryServiceRequests { - resetDS(dsr.Original) - resetDS(dsr.Requested) - respDSR, _, err := TOSession.CreateDeliveryServiceRequest(dsr, client.RequestOptions{}) + dsrV41 := dsr.Upgrade() + resetDS(dsrV41.Original) + resetDS(dsrV41.Requested) + respDSR, _, err := TOSession.CreateDeliveryServiceRequest(dsrV41, client.RequestOptions{}) assert.NoError(t, err, "Could not create Delivery Service Requests: %v - alerts: %+v", err, respDSR.Alerts) } } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/assign.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/assign.go index 3f96a28950..fe2a8d40da 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/request/assign.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/assign.go @@ -57,7 +57,7 @@ func GetAssignment(w http.ResponseWriter, r *http.Request) { return } - var dsr tc.DeliveryServiceRequestV40 + var dsr tc.DeliveryServiceRequestV4 if err := inf.Tx.QueryRowx(selectQuery+"WHERE r.id=$1", inf.IntParams["id"]).StructScan(&dsr); err != nil { if err == sql.ErrNoRows { errCode = http.StatusNotFound @@ -72,7 +72,7 @@ func GetAssignment(w http.ResponseWriter, r *http.Request) { return } - authorized, err := isTenantAuthorized(dsr, inf) + authorized, err := isTenantAuthorized(dsr.Downgrade(), inf) if err != nil { api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err) return @@ -162,7 +162,7 @@ func PutAssignment(w http.ResponseWriter, r *http.Request) { req.Assignee = nil } - var dsr tc.DeliveryServiceRequestV40 + var dsr tc.DeliveryServiceRequestV4 if err := inf.Tx.QueryRowx(selectQuery+"WHERE r.id=$1", inf.IntParams["id"]).StructScan(&dsr); err != nil { if err == sql.ErrNoRows { errCode = http.StatusNotFound @@ -176,9 +176,11 @@ func PutAssignment(w http.ResponseWriter, r *http.Request) { api.HandleErr(w, r, tx, errCode, userErr, sysErr) return } - dsr.SetXMLID() + dsrV40 := dsr.Downgrade() + dsrV40.SetXMLID() + dsr.XMLID = dsrV40.XMLID - authorized, err := isTenantAuthorized(dsr, inf) + authorized, err := isTenantAuthorized(dsrV40, inf) if err != nil { api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err) return @@ -234,7 +236,11 @@ func PutAssignment(w http.ResponseWriter, r *http.Request) { if dsr.Requested != nil { *dsr.Requested = dsr.Requested.RemoveLD1AndLD2() } - resp = dsr + if inf.Version.Major == 4 && inf.Version.Minor == 0 { + resp = dsr.Downgrade() + } else { + resp = dsr + } } else { resp = dsr.Downgrade() } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go index 3a927903b5..50a920a652 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/requests.go @@ -251,22 +251,22 @@ func Get(w http.ResponseWriter, r *http.Request) { } defer rows.Close() - dsrs := []tc.DeliveryServiceRequestV40{} - needOriginals := map[int][]*tc.DeliveryServiceRequestV40{} + dsrs := []tc.DeliveryServiceRequestV4{} + needOriginals := map[int][]*tc.DeliveryServiceRequestV4{} var originalIDs []int for rows.Next() { - var dsr tc.DeliveryServiceRequestV40 + var dsr tc.DeliveryServiceRequestV4 if err = rows.StructScan(&dsr); err != nil { api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, fmt.Errorf("dsr scanning: %v", err)) return } dsrs = append(dsrs, dsr) - if dsr.IsOpen() && dsr.ChangeType != tc.DSRChangeTypeCreate { + if dsr.Downgrade().IsOpen() && dsr.ChangeType != tc.DSRChangeTypeCreate { if dsr.ChangeType == tc.DSRChangeTypeUpdate && dsr.Requested != nil && dsr.Requested.ID != nil { id := *dsr.Requested.ID if _, ok := needOriginals[id]; !ok { - needOriginals[id] = []*tc.DeliveryServiceRequestV40{&dsrs[len(dsrs)-1]} + needOriginals[id] = []*tc.DeliveryServiceRequestV4{&dsrs[len(dsrs)-1]} } else { needOriginals[id] = append(needOriginals[id], &dsrs[len(dsrs)-1]) } @@ -274,7 +274,7 @@ func Get(w http.ResponseWriter, r *http.Request) { } else if dsr.ChangeType == tc.DSRChangeTypeDelete && dsr.Original != nil && dsr.Original.ID != nil { id := *dsr.Original.ID if _, ok := needOriginals[id]; !ok { - needOriginals[id] = []*tc.DeliveryServiceRequestV40{&dsrs[len(dsrs)-1]} + needOriginals[id] = []*tc.DeliveryServiceRequestV4{&dsrs[len(dsrs)-1]} } else { needOriginals[id] = append(needOriginals[id], &dsrs[len(dsrs)-1]) } @@ -293,13 +293,22 @@ func Get(w http.ResponseWriter, r *http.Request) { api.HandleErr(w, r, tx, errCode, userErr, sysErr) return } + if version.Major == 4 && version.Minor == 0 { + fmt.Println("Srijeet here") + dsrsV40 := []tc.DeliveryServiceRequestV40{} + for _, d := range dsrs { + dsrsV40 = append(dsrsV40, d.Downgrade()) + } + api.WriteResp(w, r, dsrsV40) + return + } api.WriteResp(w, r, dsrs) return } downgraded := make([]tc.DeliveryServiceRequestNullable, 0, len(dsrs)) for _, dsr := range dsrs { - downgraded = append(downgraded, dsr.Downgrade()) + downgraded = append(downgraded, dsr.Downgrade().Downgrade()) } api.WriteResp(w, r, downgraded) @@ -340,7 +349,7 @@ func isTenantAuthorized(dsr tc.DeliveryServiceRequestV40, inf *api.APIInfo) (boo } // Warning: this assumes inf isn't nil, and neither is dsr, inf.Tx or inf.User or inf.Tx.Tx. -func insert(dsr *tc.DeliveryServiceRequestV40, inf *api.APIInfo) (int, error, error) { +func insert(dsr *tc.DeliveryServiceRequestV4, inf *api.APIInfo) (int, error, error) { dsr.Author = inf.User.UserName dsr.LastEditedBy = inf.User.UserName if dsr.ChangeType != tc.DSRChangeTypeDelete { @@ -422,7 +431,7 @@ func (d dsrManipulationResult) String() string { func createV4(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result dsrManipulationResult) { tx := inf.Tx.Tx - var dsr tc.DeliveryServiceRequestV40 + var dsr tc.DeliveryServiceRequestV4 if err := json.NewDecoder(r.Body).Decode(&dsr); err != nil { api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("decoding: %v", err), nil) return @@ -438,7 +447,8 @@ func createV4(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result return } - ok, err := isTenantAuthorized(dsr, inf) + dsrV40 := dsr.Downgrade() + ok, err := isTenantAuthorized(dsrV40, inf) if err != nil { api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err) return @@ -448,7 +458,8 @@ func createV4(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result return } - dsr.SetXMLID() + dsrV40.SetXMLID() + dsr.XMLID = dsrV40.XMLID if ok, err = dbhelpers.DSRExistsWithXMLID(dsr.XMLID, tx); err != nil { err = fmt.Errorf("checking for existence of DSR with xmlid '%s'", dsr.XMLID) api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err) @@ -482,9 +493,14 @@ func createV4(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result return } + dsrV40 = dsr.Downgrade() w.Header().Set("Location", fmt.Sprintf("/api/%d.%d/deliveryservice_requests/%d", inf.Version.Major, inf.Version.Minor, *dsr.ID)) w.WriteHeader(http.StatusCreated) - api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Delivery Service request created", dsr) + if inf.Version.Minor > 0 { + api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Delivery Service request created", dsr) + } else { + api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Delivery Service request created", dsrV40) + } result.Successful = true result.Assignee = dsr.Assignee @@ -552,7 +568,8 @@ func createLegacy(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (res return } - errCode, userErr, sysErr := insert(&upgraded, inf) + dsrV4 := upgraded.Upgrade() + errCode, userErr, sysErr := insert(&dsrV4, inf) if userErr != nil || sysErr != nil { api.HandleErr(w, r, tx, errCode, userErr, sysErr) return @@ -627,7 +644,7 @@ func Delete(w http.ResponseWriter, r *http.Request) { if version.Major >= 4 && version.Minor >= 0 { omitExtraLongDescFields = true } - var dsr tc.DeliveryServiceRequestV40 + var dsr tc.DeliveryServiceRequestV4 if err := inf.Tx.QueryRowx(selectQuery+"WHERE r.id=$1", inf.IntParams["id"]).StructScan(&dsr); err != nil { if err == sql.ErrNoRows { errCode = http.StatusNotFound @@ -639,9 +656,11 @@ func Delete(w http.ResponseWriter, r *http.Request) { api.HandleErr(w, r, tx, errCode, userErr, sysErr) return } - dsr.SetXMLID() + dsrV40 := dsr.Downgrade() + dsrV40.SetXMLID() + dsr.XMLID = dsrV40.XMLID - authorized, err := isTenantAuthorized(dsr, inf) + authorized, err := isTenantAuthorized(dsrV40, inf) if err != nil { api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err) return @@ -667,7 +686,7 @@ func Delete(w http.ResponseWriter, r *http.Request) { return } - if dsr.IsOpen() && dsr.ChangeType != tc.DSRChangeTypeCreate { + if dsrV40.IsOpen() && dsr.ChangeType != tc.DSRChangeTypeCreate { if dsr.ChangeType == tc.DSRChangeTypeDelete && dsr.Original != nil && dsr.Original.ID != nil { errCode, userErr, sysErr = getOriginals([]int{*dsr.Original.ID}, inf.Tx, map[int][]*tc.DeliveryServiceRequestV4{*dsr.Original.ID: {&dsr}}, omitExtraLongDescFields) } else if dsr.ChangeType == tc.DSRChangeTypeUpdate && dsr.Requested != nil && dsr.Requested.ID != nil { @@ -699,12 +718,22 @@ func Delete(w http.ResponseWriter, r *http.Request) { inf.CreateChangeLog(res.String()) } -func putV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result dsrManipulationResult) { +func putV4(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result dsrManipulationResult) { tx := inf.Tx.Tx - var dsr tc.DeliveryServiceRequestV40 - if err := json.NewDecoder(r.Body).Decode(&dsr); err != nil { - api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("decoding: %v", err), nil) - return + var dsr tc.DeliveryServiceRequestV4 + var dsrV40 tc.DeliveryServiceRequestV40 + if inf.Version.Major == 4 && inf.Version.Minor == 0 { + if err := json.NewDecoder(r.Body).Decode(&dsrV40); err != nil { + api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("decoding: %v", err), nil) + return + } + dsr = dsrV40.Upgrade() + } else { + if err := json.NewDecoder(r.Body).Decode(&dsr); err != nil { + api.HandleErr(w, r, tx, http.StatusBadRequest, fmt.Errorf("decoding: %v", err), nil) + return + } + dsrV40 = dsr.Downgrade() } if userErr, sysErr := validateV4(dsr, tx); userErr != nil || sysErr != nil { api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, sysErr) @@ -723,7 +752,7 @@ func putV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result ds dsr.Requested = nil } - authorized, err := isTenantAuthorized(dsr, inf) + authorized, err := isTenantAuthorized(dsrV40, inf) if err != nil { api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err) return @@ -778,7 +807,8 @@ func putV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result ds api.HandleErr(w, r, tx, errCode, userErr, sysErr) return } - dsr.SetXMLID() + dsrV40.SetXMLID() + dsr.XMLID = dsrV40.XMLID if dsr.ChangeType == tc.DSRChangeTypeUpdate { query := deliveryservice.SelectDeliveryServicesQuery + `WHERE xml_id=:XMLID` @@ -839,6 +869,7 @@ func putLegacy(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result *dsr.LastEditedByID = tc.IDNoMod(inf.User.ID) upgraded := dsr.Upgrade() + upgradedV41 := upgraded.Upgrade() authorized, err := isTenantAuthorized(upgraded, inf) if err != nil { @@ -851,12 +882,12 @@ func putLegacy(w http.ResponseWriter, r *http.Request, inf *api.APIInfo) (result } args := []interface{}{ - upgraded.AssigneeID, - upgraded.ChangeType, + upgradedV41.AssigneeID, + upgradedV41.ChangeType, inf.User.ID, - upgraded.Requested, - upgraded.Original, - upgraded.Status, + upgradedV41.Requested, + upgradedV41.Original, + upgradedV41.Status, inf.IntParams["id"], } if err := tx.QueryRow(updateQuery, args...).Scan(&dsr.CreatedAt, &dsr.LastUpdated); err != nil { @@ -913,7 +944,7 @@ func Put(w http.ResponseWriter, r *http.Request) { return } - var current tc.DeliveryServiceRequestV40 + var current tc.DeliveryServiceRequestV4 if err := inf.Tx.QueryRowx(selectQuery+"WHERE r.id=$1", id).StructScan(¤t); err != nil { if err == sql.ErrNoRows { errCode = http.StatusNotFound @@ -926,7 +957,7 @@ func Put(w http.ResponseWriter, r *http.Request) { return } - authorized, err := isTenantAuthorized(current, inf) + authorized, err := isTenantAuthorized(current.Downgrade(), inf) if err != nil { api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err) return @@ -936,7 +967,7 @@ func Put(w http.ResponseWriter, r *http.Request) { return } - if !current.IsOpen() { + if !current.Downgrade().IsOpen() { userErr = fmt.Errorf("cannot change DeliveryServiceRequest in '%s' status", current.Status) api.HandleErr(w, r, tx, http.StatusBadRequest, userErr, nil) return @@ -944,7 +975,7 @@ func Put(w http.ResponseWriter, r *http.Request) { var result dsrManipulationResult if inf.Version.Major >= 4 { - result = putV40(w, r, inf) + result = putV4(w, r, inf) } else { result = putLegacy(w, r, inf) } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/status.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/status.go index cbbcbe5f0d..afdd0f5f17 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/request/status.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/status.go @@ -57,7 +57,7 @@ func GetStatus(w http.ResponseWriter, r *http.Request) { return } - var dsr tc.DeliveryServiceRequestV40 + var dsr tc.DeliveryServiceRequestV4 if err := inf.Tx.QueryRowx(selectQuery+"WHERE r.id=$1", inf.IntParams["id"]).StructScan(&dsr); err != nil { if err == sql.ErrNoRows { errCode = http.StatusNotFound @@ -72,7 +72,7 @@ func GetStatus(w http.ResponseWriter, r *http.Request) { return } - authorized, err := isTenantAuthorized(dsr, inf) + authorized, err := isTenantAuthorized(dsr.Downgrade(), inf) if err != nil { api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err) return @@ -135,7 +135,7 @@ func PutStatus(w http.ResponseWriter, r *http.Request) { dsrID := inf.IntParams["id"] - var dsr tc.DeliveryServiceRequestV40 + var dsr tc.DeliveryServiceRequestV4 if err := inf.Tx.QueryRowx(selectQuery+"WHERE r.id=$1", dsrID).StructScan(&dsr); err != nil { if err == sql.ErrNoRows { errCode = http.StatusNotFound @@ -149,14 +149,16 @@ func PutStatus(w http.ResponseWriter, r *http.Request) { api.HandleErr(w, r, tx, errCode, userErr, sysErr) return } - dsr.SetXMLID() + dsrV40 := dsr.Downgrade() + dsrV40.SetXMLID() + dsr.XMLID = dsrV40.XMLID if err := dsr.Status.ValidTransition(req.Status); err != nil { api.HandleErr(w, r, tx, http.StatusBadRequest, err, nil) return } - authorized, err := isTenantAuthorized(dsr, inf) + authorized, err := isTenantAuthorized(dsrV40, inf) if err != nil { api.HandleErr(w, r, tx, http.StatusInternalServerError, nil, err) return @@ -172,7 +174,7 @@ func PutStatus(w http.ResponseWriter, r *http.Request) { // store the current original DS if the DSR is being closed // (and isn't a "create" request) - if dsr.IsOpen() && req.Status != tc.RequestStatusDraft && req.Status != tc.RequestStatusSubmitted && dsr.ChangeType != tc.DSRChangeTypeCreate { + if dsrV40.IsOpen() && req.Status != tc.RequestStatusDraft && req.Status != tc.RequestStatusSubmitted && dsr.ChangeType != tc.DSRChangeTypeCreate { if dsr.ChangeType == tc.DSRChangeTypeUpdate && dsr.Requested != nil && dsr.Requested.ID != nil { errCode, userErr, sysErr = getOriginals([]int{*dsr.Requested.ID}, inf.Tx, map[int][]*tc.DeliveryServiceRequestV4{*dsr.Requested.ID: {&dsr}}, omitExtraLongDescFields) if userErr != nil || sysErr != nil { @@ -205,7 +207,7 @@ func PutStatus(w http.ResponseWriter, r *http.Request) { return } } else if err := tx.QueryRow(updateStatusQuery, req.Status, dsr.LastEditedByID, *dsr.ID).Scan(&dsr.LastUpdated); err == nil { - if dsr.IsOpen() && dsr.ChangeType != tc.DSRChangeTypeCreate { + if dsrV40.IsOpen() && dsr.ChangeType != tc.DSRChangeTypeCreate { query := deliveryservice.SelectDeliveryServicesQuery + " WHERE ds.xml_id = :xmlid" original, userErr, sysErr, errCode := deliveryservice.GetDeliveryServices(query, map[string]interface{}{"xmlid": dsr.XMLID}, inf.Tx) if userErr != nil || sysErr != nil { @@ -236,7 +238,11 @@ func PutStatus(w http.ResponseWriter, r *http.Request) { if dsr.Requested != nil { *dsr.Requested = dsr.Requested.RemoveLD1AndLD2() } - resp = dsr + if inf.Version.Major == 4 && inf.Version.Minor == 0 { + resp = dsr.Downgrade() + } else { + resp = dsr + } } else { resp = dsr.Downgrade() } diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/request/validate.go b/traffic_ops/traffic_ops_golang/deliveryservice/request/validate.go index 8bf4dc604e..68b8fc6ed3 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/request/validate.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/request/validate.go @@ -78,7 +78,7 @@ func validateLegacy(dsr tc.DeliveryServiceRequestNullable, tx *sql.Tx) error { // validateV4 validates a DSR, returning - in order - a user-facing error that // should be shown to the client, and a system error. -func validateV4(dsr tc.DeliveryServiceRequestV40, tx *sql.Tx) (error, error) { +func validateV4(dsr tc.DeliveryServiceRequestV4, tx *sql.Tx) (error, error) { if tx == nil { return nil, errors.New("nil transaction") } From 2e415c753c87f8db3ce339efe5953021539dae0b Mon Sep 17 00:00:00 2001 From: Srijeet Chatterjee Date: Wed, 16 Nov 2022 12:24:34 -0700 Subject: [PATCH 7/7] fix tp test --- traffic_portal/test/integration/Data/deliveryservices.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/traffic_portal/test/integration/Data/deliveryservices.ts b/traffic_portal/test/integration/Data/deliveryservices.ts index 06bc4da2f6..4514acb7ed 100644 --- a/traffic_portal/test/integration/Data/deliveryservices.ts +++ b/traffic_portal/test/integration/Data/deliveryservices.ts @@ -361,7 +361,7 @@ export const deliveryservices = { { rcName: "DSTestCap", xmlID: "tpdservice2", - validationMessage: "deliveryservice.RequiredCapability was created." + validationMessage: "This endpoint is deprecated, please use the deliveryservices endpoint instead" } ], remove: [ @@ -469,7 +469,7 @@ export const deliveryservices = { { rcName: "DSTestCap", xmlID: "optpdservice2", - validationMessage: "deliveryservice.RequiredCapability was created." + validationMessage: "This endpoint is deprecated, please use the deliveryservices endpoint instead" } ], remove: [