This is an automated email from the ASF dual-hosted git repository.
zrhoffman 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 e890c04 Validating pattern for POST regex call in DS (#5017)
e890c04 is described below
commit e890c0473767f562becd3668cc14911d0ade0e83
Author: rimashah25 <[email protected]>
AuthorDate: Fri Sep 18 13:47:29 2020 -0600
Validating pattern for POST regex call in DS (#5017)
* Validating pattern for POST regex call in DS
* Added test case to validate pattern for DS regex
* Updated Changelog
* Combined DS regex validation functions
* Combined DS regex validation functions-1
* Added test case for v3-client
* Resolved conflict in CHANGELOG.md
* Added v2-client test case and updated unit test.
* Fixed unit test.
* Removed fmt statement.
* Updated per review comments.
---
CHANGELOG.md | 1 +
.../testing/api/v2/deliveryservicesregexes_test.go | 30 ++++++++-
traffic_ops/testing/api/v2/tc-fixtures.json | 6 ++
.../testing/api/v3/deliveryservicesregexes_test.go | 32 +++++++++-
traffic_ops/testing/api/v3/tc-fixtures.json | 6 ++
.../deliveryservicesregexes.go | 45 +++++++------
.../deliveryservicesregexes_test.go | 73 ++++++++++++++--------
traffic_ops/v2-client/deliveryservice_regexes.go | 21 +++++++
traffic_ops/v3-client/deliveryservice_regexes.go | 25 ++++++++
9 files changed, 186 insertions(+), 53 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d019414..bf1f94e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -46,6 +46,7 @@ The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).
### Fixed
- Fixed Reference urls for Cache Config on Delivery service pages (HTTP, DNS)
in Traffic Portal.
+- Fixed #4981 - Cannot create routing regular expression with a blank pattern
param in Delivery Service [Related github
issues](https://github.com/apache/trafficcontrol/issues/4981)
- Fixed #4979 - Returns a Bad Request error during server creation with
missing profileId [Related github
issue](https://github.com/apache/trafficcontrol/issues/4979)
- Fixed #3400 - Allow "0" as a TTL value for Static DNS entries
- Fixed #4743 - Validate absolute DNS name requirement on Static DNS entry for
CNAME type [Related github
issue](https://github.com/apache/trafficcontrol/issues/4743)
diff --git a/traffic_ops/testing/api/v2/deliveryservicesregexes_test.go
b/traffic_ops/testing/api/v2/deliveryservicesregexes_test.go
index d074043..c123bbf 100644
--- a/traffic_ops/testing/api/v2/deliveryservicesregexes_test.go
+++ b/traffic_ops/testing/api/v2/deliveryservicesregexes_test.go
@@ -26,6 +26,7 @@ import (
func TestDeliveryServicesRegexes(t *testing.T) {
WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles,
Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers,
DeliveryServices, DeliveryServicesRegexes}, func() {
QueryDSRegexTest(t)
+ CreateTestDSRegexWithMissingPattern(t)
})
}
@@ -93,6 +94,31 @@ func DeleteTestDeliveryServicesRegexes(t *testing.T) {
}
}
+func CreateTestDSRegexWithMissingPattern(t *testing.T) {
+ var regex = testData.DeliveryServicesRegexes[3]
+ ds, _, err := TOSession.GetDeliveryServiceByXMLIDNullable(regex.DSName)
+ if err != nil {
+ t.Fatalf("unable to get ds %v: %v", regex.DSName, err)
+ }
+ if len(ds) == 0 {
+ t.Fatalf("unable to get ds %v", regex.DSName)
+ }
+
+ var dsID int
+ if ds[0].ID == nil {
+ t.Fatal("ds has a nil id")
+ } else {
+ dsID = *ds[0].ID
+ }
+
+ regexPost := tc.DeliveryServiceRegexPost{Type: regex.Type, SetNumber:
regex.SetNumber, Pattern: regex.Pattern}
+
+ _, _, err = TOSession.PostDeliveryServiceRegexesByDSID(dsID, regexPost)
+ if err == nil {
+ t.Fatalf("Expected: 400 Bad Request but got: %v", err)
+ }
+}
+
func loadDSRegexIDs(t *testing.T, test *tc.DeliveryServiceRegexesTest) {
dsTypes, _, err := TOSession.GetTypeByName(test.TypeName)
if err != nil {
@@ -132,8 +158,8 @@ func QueryDSRegexTest(t *testing.T) {
if err != nil {
t.Fatal("unable to get ds_regex by id " + strconv.Itoa(dsID))
}
- if len(dsRegexes) != 3 {
- t.Fatal("expected to get 3 ds_regex, got " +
strconv.Itoa(len(dsRegexes)))
+ if len(dsRegexes) != 4 {
+ t.Fatal("expected to get 4 ds_regex, got " +
strconv.Itoa(len(dsRegexes)))
}
params := make(map[string]string)
diff --git a/traffic_ops/testing/api/v2/tc-fixtures.json
b/traffic_ops/testing/api/v2/tc-fixtures.json
index a30a338..4856985 100644
--- a/traffic_ops/testing/api/v2/tc-fixtures.json
+++ b/traffic_ops/testing/api/v2/tc-fixtures.json
@@ -717,6 +717,12 @@
"typeName": "HOST_REGEXP",
"setNumber": 1,
"pattern" : ".*"
+ },
+ {
+ "dsName": "ds1",
+ "typeName": "HOST_REGEXP",
+ "setNumber": 3,
+ "pattern" : ""
}
],
"deliveryservicesRequiredCapabilities": [
diff --git a/traffic_ops/testing/api/v3/deliveryservicesregexes_test.go
b/traffic_ops/testing/api/v3/deliveryservicesregexes_test.go
index fc58337..02c0ecd 100644
--- a/traffic_ops/testing/api/v3/deliveryservicesregexes_test.go
+++ b/traffic_ops/testing/api/v3/deliveryservicesregexes_test.go
@@ -17,12 +17,12 @@ package v3
import (
"fmt"
- "github.com/apache/trafficcontrol/lib/go-rfc"
"net/http"
"strconv"
"testing"
"time"
+ "github.com/apache/trafficcontrol/lib/go-rfc"
"github.com/apache/trafficcontrol/lib/go-tc"
)
@@ -30,6 +30,7 @@ func TestDeliveryServicesRegexes(t *testing.T) {
WithObjs(t, []TCObj{CDNs, Types, Tenants, Users, Parameters, Profiles,
Statuses, Divisions, Regions, PhysLocations, CacheGroups, Servers, Topologies,
DeliveryServices, DeliveryServicesRegexes}, func() {
QueryDSRegexTest(t)
QueryDSRegexTestIMS(t)
+ CreateTestDSRegexWithMissingPattern(t)
})
}
@@ -97,6 +98,31 @@ func DeleteTestDeliveryServicesRegexes(t *testing.T) {
}
}
+func CreateTestDSRegexWithMissingPattern(t *testing.T) {
+ var regex = testData.DeliveryServicesRegexes[3]
+ ds, _, err :=
TOSession.GetDeliveryServiceByXMLIDNullableWithHdr(regex.DSName, nil)
+ if err != nil {
+ t.Fatalf("unable to get ds %v: %v", regex.DSName, err)
+ }
+ if len(ds) == 0 {
+ t.Fatalf("unable to get ds %v", regex.DSName)
+ }
+
+ var dsID int
+ if ds[0].ID == nil {
+ t.Fatal("ds has a nil id")
+ } else {
+ dsID = *ds[0].ID
+ }
+
+ regexPost := tc.DeliveryServiceRegexPost{Type: regex.Type, SetNumber:
regex.SetNumber, Pattern: regex.Pattern}
+
+ _, reqInfo, _ := TOSession.PostDeliveryServiceRegexesByDSID(dsID,
regexPost)
+ if reqInfo.StatusCode != http.StatusBadRequest {
+ t.Errorf("Expected: %v, but got: %v", http.StatusBadRequest,
reqInfo.StatusCode)
+ }
+}
+
func loadDSRegexIDs(t *testing.T, test *tc.DeliveryServiceRegexesTest) {
dsTypes, _, err := TOSession.GetTypeByName(test.TypeName)
if err != nil {
@@ -151,8 +177,8 @@ func QueryDSRegexTest(t *testing.T) {
if err != nil {
t.Fatal("unable to get ds_regex by id " + strconv.Itoa(dsID))
}
- if len(dsRegexes) != 3 {
- t.Fatal("expected to get 3 ds_regex, got " +
strconv.Itoa(len(dsRegexes)))
+ if len(dsRegexes) != 4 {
+ t.Fatal("expected to get 4 ds_regex, got " +
strconv.Itoa(len(dsRegexes)))
}
params := make(map[string]string)
diff --git a/traffic_ops/testing/api/v3/tc-fixtures.json
b/traffic_ops/testing/api/v3/tc-fixtures.json
index ce5b78c..ad28c14 100644
--- a/traffic_ops/testing/api/v3/tc-fixtures.json
+++ b/traffic_ops/testing/api/v3/tc-fixtures.json
@@ -932,6 +932,12 @@
"typeName": "HOST_REGEXP",
"setNumber": 1,
"pattern" : ".*"
+ },
+ {
+ "dsName": "ds1",
+ "typeName": "HOST_REGEXP",
+ "setNumber": 3,
+ "pattern" : ""
}
],
"deliveryservicesRequiredCapabilities": [
diff --git
a/traffic_ops/traffic_ops_golang/deliveryservicesregexes/deliveryservicesregexes.go
b/traffic_ops/traffic_ops_golang/deliveryservicesregexes/deliveryservicesregexes.go
index 75fbb65..a441aa0 100644
---
a/traffic_ops/traffic_ops_golang/deliveryservicesregexes/deliveryservicesregexes.go
+++
b/traffic_ops/traffic_ops_golang/deliveryservicesregexes/deliveryservicesregexes.go
@@ -28,10 +28,12 @@ import (
"strconv"
"github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/apache/trafficcontrol/lib/go-tc/tovalidate"
"github.com/apache/trafficcontrol/lib/go-util"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant"
+ validation "github.com/go-ozzo/ozzo-validation"
"github.com/lib/pq"
)
@@ -221,12 +223,7 @@ func Post(w http.ResponseWriter, r *http.Request) {
return
}
- if err := validateDSRegexType(tx, dsr.Type); err != nil {
- api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, err, nil)
- return
- }
-
- if err := validateDSRegexOrder(tx, inf.IntParams["dsid"],
dsr.SetNumber); err != nil {
+ if err := validateDSRegex(tx, dsr, inf.IntParams["dsid"]); err != nil {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, err, nil)
return
}
@@ -303,12 +300,7 @@ func Put(w http.ResponseWriter, r *http.Request) {
return
}
- if err := validateDSRegexOrder(tx, inf.IntParams["dsid"],
dsr.SetNumber); err != nil {
- api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, err, nil)
- return
- }
-
- if err := validateDSRegexType(tx, dsr.Type); err != nil {
+ if err := validateDSRegex(tx, dsr, inf.IntParams["dsid"]); err != nil {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, err, nil)
return
}
@@ -337,24 +329,31 @@ func Put(w http.ResponseWriter, r *http.Request) {
api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Delivery service regex
creation was successful.", respObj)
}
-func validateDSRegexType(tx *sql.Tx, typeID int) error {
- _, err := tc.ValidateTypeID(tx, &typeID, "regex")
- return err
-}
-
-func validateDSRegexOrder(tx *sql.Tx, dsID int, order int) error {
+// Validate POST/PUT regex struct
+func validateDSRegex(tx *sql.Tx, dsr tc.DeliveryServiceRegexPost, dsID int)
error {
var ds int
- if order < 0 {
+ var setNumberErr error
+ if dsr.SetNumber < 0 {
return errors.New("cannot add regex with order < 0")
}
err := tx.QueryRow(`
-select deliveryservice from deliveryservice_regex
+select deliveryservice from deliveryservice_regex
where deliveryservice = $1 and set_number = $2`,
- dsID, order).Scan(&ds)
+ dsID, dsr.SetNumber).Scan(&ds)
if err == nil {
- return errors.New("cannot add regex, another regex with the
same order exists")
+ setNumberErr = errors.New("cannot add regex, another regex with
the same order exists")
+ } else {
+ setNumberErr = nil
}
- return nil
+
+ _, typeErr := tc.ValidateTypeID(tx, &dsr.Type, "regex")
+
+ errs := validation.Errors{
+ "type": typeErr,
+ "setNumber": setNumberErr,
+ "pattern": validation.Validate(dsr.Pattern,
validation.Required)}
+
+ return util.JoinErrs(tovalidate.ToErrors(errs))
}
func Delete(w http.ResponseWriter, r *http.Request) {
diff --git
a/traffic_ops/traffic_ops_golang/deliveryservicesregexes/deliveryservicesregexes_test.go
b/traffic_ops/traffic_ops_golang/deliveryservicesregexes/deliveryservicesregexes_test.go
index 2a9bf5e..11e8f6b 100644
---
a/traffic_ops/traffic_ops_golang/deliveryservicesregexes/deliveryservicesregexes_test.go
+++
b/traffic_ops/traffic_ops_golang/deliveryservicesregexes/deliveryservicesregexes_test.go
@@ -1,11 +1,5 @@
package deliveryservicesregexes
-import (
- "github.com/jmoiron/sqlx"
- "gopkg.in/DATA-DOG/go-sqlmock.v1"
- "testing"
-)
-
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
@@ -25,42 +19,71 @@ import (
* under the License.
*/
-func TestValidateDSRegexOrder(t *testing.T) {
- expected := `cannot add regex, another regex with the same order exists`
+import (
+ "github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/jmoiron/sqlx"
+ "gopkg.in/DATA-DOG/go-sqlmock.v1"
+ "testing"
+)
+
+func TestValidateDSRegexOrderExisting(t *testing.T) {
+ expected := `'setNumber' cannot add regex, another regex with the same
order exists`
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")
defer db.Close()
- cols := []string{"deliveryservice"}
+ cols := []string{"name", "use_in_table"}
rows := sqlmock.NewRows(cols)
rows = rows.AddRow(
+ "HTTP",
+ "regex",
+ )
+ cols2 := []string{"deliveryservice"}
+ rows2 := sqlmock.NewRows(cols2)
+ rows2 = rows2.AddRow(
1,
)
+
+ regex := tc.DeliveryServiceRegexPost{Type: 33, SetNumber: 3, Pattern:
".*"}
mock.ExpectBegin()
- mock.ExpectQuery("select").WithArgs(1, 3).WillReturnRows(rows)
+ mock.ExpectQuery("select").WithArgs(1,
regex.SetNumber).WillReturnRows(rows2)
+ mock.ExpectQuery("SELECT").WithArgs(regex.Type).WillReturnRows(rows)
+ mock.ExpectCommit()
tx := db.MustBegin().Tx
- err = validateDSRegexOrder(tx, 1, 3)
+ err = validateDSRegex(tx, regex, 1)
if err == nil {
- t.Fatal("Expected error but got nil")
+ t.Fatalf("Expected error '%v' but got none", expected)
}
- if err.Error() != expected {
- t.Fatalf("Expected error was %v, got %v", expected, err.Error())
+}
+
+func TestValidateDSRegex(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)
}
- mock.ExpectQuery("select").WithArgs(1, 4).WillReturnRows(nil)
+ defer mockDB.Close()
+ db := sqlx.NewDb(mockDB, "sqlmock")
+ defer db.Close()
+ cols := []string{"name", "use_in_table"}
+ rows := sqlmock.NewRows(cols)
+ rows = rows.AddRow(
+ "HTTP",
+ "regex",
+ )
+ cols2 := []string{"deliveryservice"}
+ rows2 := sqlmock.NewRows(cols2)
+
+ regex := tc.DeliveryServiceRegexPost{Type: 33, SetNumber: 3, Pattern:
".*"}
+ mock.ExpectBegin()
+ mock.ExpectQuery("select").WithArgs(1,
regex.SetNumber).WillReturnRows(rows2)
+ mock.ExpectQuery("SELECT").WithArgs(regex.Type).WillReturnRows(rows)
mock.ExpectCommit()
- err = validateDSRegexOrder(tx, 1, 3)
+ tx := db.MustBegin().Tx
+ err = validateDSRegex(tx, regex, 1)
if err != nil {
- t.Fatalf("Expect no error, got %v", err.Error())
- }
- err = validateDSRegexOrder(tx, 1, -1)
- if err == nil {
- t.Fatal("Expect error saying cannot add regex with order < 0,
got nothing")
- }
- if err.Error() != "cannot add regex with order < 0" {
- t.Errorf("Expected error detail to be 'cannot add regex with
order <0', got %v", err.Error())
+ t.Fatalf("Expected no error but got %v", err.Error())
}
}
diff --git a/traffic_ops/v2-client/deliveryservice_regexes.go
b/traffic_ops/v2-client/deliveryservice_regexes.go
index e2bbd70..85901e2 100644
--- a/traffic_ops/v2-client/deliveryservice_regexes.go
+++ b/traffic_ops/v2-client/deliveryservice_regexes.go
@@ -16,7 +16,10 @@
package client
import (
+ "encoding/json"
"fmt"
+ "net"
+ "net/http"
"github.com/apache/trafficcontrol/lib/go-tc"
)
@@ -39,3 +42,21 @@ func (to *Session) GetDeliveryServiceRegexesByDSID(dsID int,
params map[string]s
}
return response.Response, reqInf, nil
}
+
+func (to *Session) PostDeliveryServiceRegexesByDSID(dsID int, regex
tc.DeliveryServiceRegexPost) (tc.Alerts, ReqInf, error) {
+ var alerts tc.Alerts
+ var remoteAddr net.Addr
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr:
remoteAddr}
+ reqBody, err := json.Marshal(regex)
+ if err != nil {
+ return alerts, reqInf, err
+ }
+
+ _, remoteAddr, err = to.request(http.MethodPost,
fmt.Sprintf(API_DS_REGEXES, dsID), reqBody)
+ reqInf.RemoteAddr = remoteAddr
+ if err != nil {
+ return alerts, reqInf, err
+ }
+
+ return alerts, reqInf, nil
+}
diff --git a/traffic_ops/v3-client/deliveryservice_regexes.go
b/traffic_ops/v3-client/deliveryservice_regexes.go
index d9dbd42..3e412df 100644
--- a/traffic_ops/v3-client/deliveryservice_regexes.go
+++ b/traffic_ops/v3-client/deliveryservice_regexes.go
@@ -16,7 +16,10 @@
package client
import (
+ "encoding/json"
"fmt"
+ "net"
+ "net/http"
"github.com/apache/trafficcontrol/lib/go-tc"
)
@@ -39,3 +42,25 @@ func (to *Session) GetDeliveryServiceRegexesByDSID(dsID int,
params map[string]s
}
return response.Response, reqInf, nil
}
+
+func (to *Session) PostDeliveryServiceRegexesByDSID(dsID int, regex
tc.DeliveryServiceRegexPost) (tc.Alerts, ReqInf, error) {
+ var alerts tc.Alerts
+ var remoteAddr net.Addr
+ reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr:
remoteAddr}
+ reqBody, err := json.Marshal(regex)
+ if err != nil {
+ return alerts, reqInf, err
+ }
+
+ resp, remoteAddr, err := to.request(http.MethodPost,
fmt.Sprintf(API_DS_REGEXES, dsID), reqBody, nil)
+ reqInf.RemoteAddr = remoteAddr
+ if resp != nil {
+ reqInf.StatusCode = resp.StatusCode
+ }
+ if err != nil {
+ return alerts, reqInf, err
+ }
+ defer resp.Body.Close()
+
+ return alerts, reqInf, nil
+}
