This is an automated email from the ASF dual-hosted git repository.
rawlin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new 621f654 Type for a steering target should match only applicable
values (#4734)
621f654 is described below
commit 621f6547bbf75634c630467c4d77b1b059cf1e04
Author: Srijeet Chatterjee <[email protected]>
AuthorDate: Tue Jun 2 16:48:44 2020 -0600
Type for a steering target should match only applicable values (#4734)
* Type for a steering target should match only applicable values
* Adding changes for documentation and PUT path
* Formatting
* go fmt
* Code review fixes
* Code review fixes
* Code review fixes
* Removing extra call to Validate
---
CHANGELOG.md | 3 +-
docs/source/api/v3/steering_id_targets.rst | 22 +++---
.../source/api/v3/steering_id_targets_targetID.rst | 10 +--
docs/source/overview/delivery_services.rst | 2 +
lib/go-tc/steeringtarget.go | 4 +
.../steeringtargets/steeringtargets_test.go | 92 ++++++++++++++++++++++
6 files changed, 116 insertions(+), 17 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a0d129e..18234e8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,7 +23,8 @@ The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).
- Removed audit logging from the `POST /api/x/serverchecks` Traffic Ops API
endpoint in order to reduce audit log spam
- Fixed audit logging from the `/jobs` APIs to bring them back to the same
level of information provided by TO-Perl
- Fixed `maxRevalDurationDays` validation for `POST
/api/1.x/user/current/jobs` and added that validation to the `/api/x/jobs`
endpoints
-- Fixed update procedure of servers, so that if a server is linked to one or
more delivery services, you cannot change its "cdn".
+- Fixed update procedure of servers, so that if a server is linked to one or
more delivery services, you cannot change its "cdn". [Related github
issue](https://github.com/apache/trafficcontrol/issues/4116)
+- Fixed `POST /api/x/steering` and `PUT /api/x/steering` so that a steering
target with an invalid `type` is no longer accepted. [Related github
issue](https://github.com/apache/trafficcontrol/issues/3531)
- Fixed `cachegroups` READ endpoint, so that if a request is made with the
`type` specified as a non integer value, you get back a `400` with error
details, instead of a `500`. [Related github
issue](https://github.com/apache/trafficcontrol/issues/4703)
### Changed
diff --git a/docs/source/api/v3/steering_id_targets.rst
b/docs/source/api/v3/steering_id_targets.rst
index 33be8f3..237d50e 100644
--- a/docs/source/api/v3/steering_id_targets.rst
+++ b/docs/source/api/v3/steering_id_targets.rst
@@ -72,9 +72,9 @@ Response Structure
:deliveryServiceId: An integral, unique identifier for the steering
:term:`Delivery Service`
:target: A string that is the :ref:`ds-xmlid` of this target
:term:`Delivery Service`
:targetId: An integral, unique identifier for this target
:term:`Delivery Service`
-:type: The routing type of this target :term:`Delivery Service`
-:typeId: An integral, unique identifier for the :ref:`routing type
<ds-types>` of this target :term:`Delivery Service`
-:value: The 'weight' attributed to this steering target as an
integer
+:type: The steering type of this target :term:`Delivery Service`.
This should be one of ``STEERING_WEIGHT``, ``STEERING_ORDER``,
``STEERING_GEO_ORDER`` or ``STEERING_GEO_WEIGHT``
+:typeId: An integral, unique identifier for the :ref:`steering type
<ds-steering>` of this target :term:`Delivery Service`
+:value: The 'weight', 'order', 'geo_order' or 'geo_weight'
attributed to this steering target as an integer
.. code-block:: http
:caption: Response Example
@@ -97,8 +97,8 @@ Response Structure
"deliveryServiceId": 2,
"target": "demo1",
"targetId": 1,
- "type": "HTTP",
- "typeId": 1,
+ "type": "STEERING_ORDER",
+ "typeId": 44,
"value": 100
}
]}
@@ -122,8 +122,8 @@ Request Structure
+------+---------------------------------------------------------------------------------------------------------+
:targetId: The integral, unique identifier of a :term:`Delivery Service` which
shall be a new steering target for the :term:`Delivery Service` identified by
the ``ID`` path parameter
-:typeId: The integral, unique identifier of the routing type of the new
target :term:`Delivery Service`
-:value: The 'weight' which shall be attributed to the new target
:term:`Delivery Service`
+:typeId: The integral, unique identifier of the steering type of the new
target :term:`Delivery Service`. This should be corresponding to one of
``STEERING_WEIGHT``, ``STEERING_ORDER``, ``STEERING_GEO_ORDER`` or
``STEERING_GEO_WEIGHT``
+:value: The 'weight', 'order', 'geo_order' or 'geo_weight' which shall be
attributed to the new target :term:`Delivery Service`
.. code-block:: http
:caption: Request Example
@@ -139,7 +139,7 @@ Request Structure
{
"targetId": 1,
"value": 100,
- "typeId": 1
+ "typeId": 43
}
Response Structure
@@ -148,9 +148,9 @@ Response Structure
:deliveryServiceId: An integral, unique identifier for the steering
:term:`Delivery Service`
:target: A string that is the :ref:`ds-xmlid` of this target
:term:`Delivery Service`
:targetId: An integral, unique identifier for this target
:term:`Delivery Service`
-:type: The routing type of this target :term:`Delivery Service`
-:typeId: An integral, unique identifier for the :ref:`routing type
<ds-types>` of this target :term:`Delivery Service`
-:value: The 'weight' attributed to this steering target as an
integer
+:type: The steering type of this target :term:`Delivery Service`.
This should be one of ``STEERING_WEIGHT``, ``STEERING_ORDER``,
``STEERING_GEO_ORDER`` or ``STEERING_GEO_WEIGHT``
+:typeId: An integral, unique identifier for the :ref:`steering type
<ds-steering>` of this target :term:`Delivery Service`
+:value: The 'weight', 'order', 'geo_order' or 'geo_weight'
attributed to this steering target as an integer
.. code-block:: http
:caption: Response Example
diff --git a/docs/source/api/v3/steering_id_targets_targetID.rst
b/docs/source/api/v3/steering_id_targets_targetID.rst
index 0f0ffd1..a1758f2 100644
--- a/docs/source/api/v3/steering_id_targets_targetID.rst
+++ b/docs/source/api/v3/steering_id_targets_targetID.rst
@@ -39,8 +39,8 @@ Request Structure
| targetID | The integral, unique identifier of a :term:`Delivery
Service` which is a target of the :term:`Delivery Service` identified by ``ID``
|
+----------+--------------------------------------------------------------------------------------------------------------------------------------+
-:typeId: The integral, unique identifier of the :ref:`routing type <ds-types>`
of the target :term:`Delivery Service`
-:value: The 'weight' which shall be attributed to the target :term:`Delivery
Service`
+:typeId: The integral, unique identifier of the :ref:`steering type
<ds-steering>` of the target :term:`Delivery Service`. This should be
corresponding to one of ``STEERING_WEIGHT``, ``STEERING_ORDER``,
``STEERING_GEO_ORDER`` or ``STEERING_GEO_WEIGHT``
+:value: The 'weight', 'order', 'geo_order' or 'geo_weight' which shall be
attributed to the target :term:`Delivery Service`
.. code-block:: http
:caption: Request Example
@@ -55,7 +55,7 @@ Request Structure
{
"value": 1,
- "typeId": 1
+ "typeId": 43
}
Response Structure
@@ -64,8 +64,8 @@ Response Structure
:deliveryServiceId: An integral, unique identifier for the steering
:term:`Delivery Service`
:target: A string that is the :ref:`ds-xmlid` of this target
:term:`Delivery Service`
:targetId: An integral, unique identifier for this target
:term:`Delivery Service`
-:type: The routing type of this target :term:`Delivery Service`
-:typeId: An integral, unique identifier for the :ref:`routing type
<ds-types>` of this target :term:`Delivery Service`
+:type: The steering type of this target :term:`Delivery Service`
+:typeId: An integral, unique identifier for the :ref:`steering type
<ds-steering>` of this target :term:`Delivery Service`
:value: The 'weight' attributed to this steering target as an
integer
.. code-block:: http
diff --git a/docs/source/overview/delivery_services.rst
b/docs/source/overview/delivery_services.rst
index 7d5d217..88164d4 100644
--- a/docs/source/overview/delivery_services.rst
+++ b/docs/source/overview/delivery_services.rst
@@ -829,6 +829,8 @@ HTTP_LIVE_NATNL
HTTP_NO_CACHE\ [#dupOrigin]_
Uses HTTP Content Routing, but :term:`cache servers` will not actually
cache the delivered content - they act as just proxies. This will bypass any
existing :term:`Mid-tier` entirely (as it's totally useless when content is not
being cached).
+.. _ds-steering:
+
STEERING
This is a sort of "meta" Delivery Service. It is used for directing
clients to one of a set of Delivery Services, rather than delivering content
directly itself. The Delivery Services to which a STEERING Delivery Service
routes clients are referred to as "targets". Targets in general have an
associated "value" and can be of several :term:`Types` that define the meaning
of the value - these being:
diff --git a/lib/go-tc/steeringtarget.go b/lib/go-tc/steeringtarget.go
index 2588b66..ea973a8 100644
--- a/lib/go-tc/steeringtarget.go
+++ b/lib/go-tc/steeringtarget.go
@@ -52,6 +52,10 @@ func (st SteeringTargetNullable) Validate(tx *sql.Tx) error {
if st.TypeID == nil {
errs = append(errs, "missing typeId")
}
+ valid, err := ValidateTypeID(tx, st.TypeID, "steering_target")
+ if valid == "" {
+ errs = append(errs, err.Error())
+ }
if st.Value == nil {
errs = append(errs, "missing value")
}
diff --git
a/traffic_ops/traffic_ops_golang/steeringtargets/steeringtargets_test.go
b/traffic_ops/traffic_ops_golang/steeringtargets/steeringtargets_test.go
new file mode 100644
index 0000000..2a79a23
--- /dev/null
+++ b/traffic_ops/traffic_ops_golang/steeringtargets/steeringtargets_test.go
@@ -0,0 +1,92 @@
+package steeringtargets
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+ "github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/apache/trafficcontrol/lib/go-util"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+ "github.com/jmoiron/sqlx"
+ "gopkg.in/DATA-DOG/go-sqlmock.v1"
+ "testing"
+)
+
+func TestInvalidSteeringTargetType(t *testing.T) {
+ mockDB, mock, err := sqlmock.New()
+ if err != nil {
+ t.Fatalf("an error '%s' was not expected when opening a stub
database connection", err)
+ }
+ defer mockDB.Close()
+
+ db := sqlx.NewDb(mockDB, "sqlmock")
+ rows := sqlmock.NewRows([]string{
+ "name",
+ "use_in_table",
+ })
+ rows = rows.AddRow("HTTP", "server")
+ defer db.Close()
+ mock.ExpectBegin()
+ mock.ExpectQuery("SELECT").WithArgs(1).WillReturnRows(rows)
+ tx := db.MustBegin()
+
+ expected := `type is not a valid steering_target type`
+ var dsID, targetID uint64
+ var val util.JSONIntStr
+ dsID = 0
+ targetID = 1
+ typeID := 1
+ val = 100
+
+ st := tc.SteeringTargetNullable{
+ DeliveryService: nil,
+ DeliveryServiceID: &dsID,
+ Target: nil,
+ TargetID: &targetID,
+ Type: nil,
+ TypeID: &typeID,
+ Value: &val,
+ }
+ m := make(map[string]string)
+ m["deliveryservice"] = "3"
+ stObj := &TOSteeringTargetV11{
+ APIInfoImpl: api.APIInfoImpl{
+ ReqInfo: &api.APIInfo{
+ Params: m,
+ IntParams: nil,
+ User: nil,
+ ReqID: 0,
+ Version: nil,
+ Tx: tx,
+ Config: nil,
+ },
+ },
+ SteeringTargetNullable: st,
+ DSTenantID: nil,
+ LastUpdated: nil,
+ }
+
+ err = stObj.Validate()
+ if err == nil {
+ t.Fatal("expected user error to say that type is invalid, got
no error instead")
+ }
+ if err.Error() != expected {
+ t.Errorf("Expected error details %v, got %v", expected,
err.Error())
+ }
+}
