From 8515dbca9c7f1f10a514201d0fbdfb59ff1421d2 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Tue, 7 Mar 2023 14:58:25 -0700 Subject: [PATCH 1/4] Fix ISE when orgserverFQDN is empty in certain situations --- .../deliveryservice/deliveryservices.go | 42 +++++++++++-------- .../deliveryservice/deliveryservices_test.go | 22 ++++++++++ 2 files changed, 46 insertions(+), 18 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go index 1ead836a1c..acb8b67787 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go @@ -1513,7 +1513,9 @@ func requiredIfMatchesTypeName(patterns []string, typeName string) func(interfac return nil case *string: if v != nil { - return nil + if *v != "" { + return nil + } } case *float64: if v != nil { @@ -1690,15 +1692,19 @@ func validateOrgServerFQDN(orgServerFQDN string) bool { return true } -func validateTypeFields(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { - // Validate the TypeName related fields below - err := error(nil) - DNSRegexType := "^DNS.*$" - HTTPRegexType := "^HTTP.*$" - SteeringRegexType := "^STEERING.*$" - latitudeErr := "Must be a floating point number within the range +-90" - longitudeErr := "Must be a floating point number within the range +-180" +const ( + dnsTypeRegexp = "^DNS.*$" + httpTypeRegexp = "^HTTP.*$" + steeringTypeRegexp = "^STEERING.*$" +) +const ( + latitudeErr = "Must be a floating point number within the range +-90" + longitudeErr = "Must be a floating point number within the range +-180" +) + +// validateTypeFields validates the TypeName-related field. +func validateTypeFields(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { typeName, err := tc.ValidateTypeID(tx, &ds.TypeID, "deliveryservice") if err != nil { return err @@ -1714,22 +1720,22 @@ func validateTypeFields(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { return fmt.Errorf("consistentHashQueryParams not allowed for '%s' deliveryservice type", typeName) })), "initialDispersion": validation.Validate(ds.InitialDispersion, - validation.By(requiredIfMatchesTypeName([]string{HTTPRegexType}, typeName)), + validation.By(requiredIfMatchesTypeName([]string{httpTypeRegexp}, typeName)), validation.By(tovalidate.IsGreaterThanZero)), "ipv6RoutingEnabled": validation.Validate(ds.IPV6RoutingEnabled, - validation.By(requiredIfMatchesTypeName([]string{SteeringRegexType, DNSRegexType, HTTPRegexType}, typeName))), + validation.By(requiredIfMatchesTypeName([]string{steeringTypeRegexp, dnsTypeRegexp, httpTypeRegexp}, typeName))), "missLat": validation.Validate(ds.MissLat, - validation.By(requiredIfMatchesTypeName([]string{DNSRegexType, HTTPRegexType}, typeName)), + validation.By(requiredIfMatchesTypeName([]string{dnsTypeRegexp, httpTypeRegexp}, typeName)), validation.Min(-90.0).Error(latitudeErr), validation.Max(90.0).Error(latitudeErr)), "missLong": validation.Validate(ds.MissLong, - validation.By(requiredIfMatchesTypeName([]string{DNSRegexType, HTTPRegexType}, typeName)), + validation.By(requiredIfMatchesTypeName([]string{dnsTypeRegexp, httpTypeRegexp}, typeName)), validation.Min(-180.0).Error(longitudeErr), validation.Max(180.0).Error(longitudeErr)), "multiSiteOrigin": validation.Validate(ds.MultiSiteOrigin, - validation.By(requiredIfMatchesTypeName([]string{DNSRegexType, HTTPRegexType}, typeName))), + validation.By(requiredIfMatchesTypeName([]string{dnsTypeRegexp, httpTypeRegexp}, typeName))), "orgServerFqdn": validation.Validate(ds.OrgServerFQDN, - validation.By(requiredIfMatchesTypeName([]string{DNSRegexType, HTTPRegexType}, typeName)), + validation.By(requiredIfMatchesTypeName([]string{dnsTypeRegexp, httpTypeRegexp}, typeName)), validation.NewStringRule(validateOrgServerFQDN, "must start with http:// or https:// and be followed by a valid hostname with an optional port (no trailing slash)")), "rangeSliceBlockSize": validation.Validate(ds, validation.By(func(dsi interface{}) error { @@ -1749,11 +1755,11 @@ func validateTypeFields(tx *sql.Tx, ds *tc.DeliveryServiceV5) error { return nil })), "protocol": validation.Validate(ds.Protocol, - validation.By(requiredIfMatchesTypeName([]string{SteeringRegexType, DNSRegexType, HTTPRegexType}, typeName))), + validation.By(requiredIfMatchesTypeName([]string{steeringTypeRegexp, dnsTypeRegexp, httpTypeRegexp}, typeName))), "qstringIgnore": validation.Validate(ds.QStringIgnore, - validation.By(requiredIfMatchesTypeName([]string{DNSRegexType, HTTPRegexType}, typeName))), + validation.By(requiredIfMatchesTypeName([]string{dnsTypeRegexp, httpTypeRegexp}, typeName))), "rangeRequestHandling": validation.Validate(ds.RangeRequestHandling, - validation.By(requiredIfMatchesTypeName([]string{DNSRegexType, HTTPRegexType}, typeName))), + validation.By(requiredIfMatchesTypeName([]string{dnsTypeRegexp, httpTypeRegexp}, typeName))), "tlsVersions": validation.Validate( &ds.TLSVersions, validation.By( diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go index b397c1c53d..c5b93e0133 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices_test.go @@ -364,3 +364,25 @@ func TestReadGetDeliveryServices(t *testing.T) { t.Errorf("Unexpected system error reading Delivery Services: %v", sysErr) } } + +func TestRequiredIfTypeMatchesName(t *testing.T) { + ds := &tc.DeliveryServiceV50{ + OrgServerFQDN: new(string), + Protocol: new(int), + InitialDispersion: new(int), + MissLat: new(float64), + MissLong: new(float64), + RangeRequestHandling: new(int), + QStringIgnore: new(int), + MaxRequestHeaderBytes: new(int), + IPV6RoutingEnabled: new(bool), + } + *ds.InitialDispersion = 1 + fn := requiredIfMatchesTypeName([]string{httpTypeRegexp, dnsTypeRegexp}, "HTTP") + err := fn(ds.OrgServerFQDN) + if err == nil { + t.Error("Failed to raise an error when the orgserver fqdn is empty") + } else { + t.Logf("Got expected error: %v", err) + } +} From ac2281fa2b20c52787a9c23571c2be7b6992e753 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Tue, 7 Mar 2023 15:01:24 -0700 Subject: [PATCH 2/4] Update CHANGELOG --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1552ce989a..be1bca2597 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,7 +86,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#7277](https://github.com/apache/trafficcontrol/pull/7277) *Traffic Control Cache Config (t3c)* remapdotconfig: remove skip check at mids for nocache/live - [#7282](https://github.com/apache/trafficcontrol/pull/7282) *Traffic Ops* Fixed issue with user getting correctly logged when using an access or bearer token authentication. - [#7346](https://github.com/apache/trafficcontrol/pull/7346) *Traffic Control Cache Config (t3c)* Fixed issue with stale lock file when using git to track changes. -- [#7352](https://github.com/apache/trafficcontrol/pull/7352) *Traffic Control Cache Config (t3c)* Fixed issue with application locking which would allow multiple instances of *t3c apply* to run concurrently. +- [#7352](https://github.com/apache/trafficcontrol/pull/7352) *Traffic Control Cache Config (t3c)* Fixed issue with application locking which would allow multiple instances of `t3c apply` to run concurrently. +- [#6775](https://github.com/apache/trafficcontrol/issues/6775) *Traffic Ops* Invalid "orgServerFqdn" in Delivery Service creation/update causes Internal Server Error ## [7.0.0] - 2022-07-19 ### Added From f99aab94e0913ce99f9a1b28d43b12b778e9086c Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Thu, 9 Mar 2023 08:22:37 -0700 Subject: [PATCH 3/4] Fix fixture DSRs missing orgserverFqdn --- traffic_ops/testing/api/v3/tc-fixtures.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/traffic_ops/testing/api/v3/tc-fixtures.json b/traffic_ops/testing/api/v3/tc-fixtures.json index f392b2d5f1..1cdece320b 100644 --- a/traffic_ops/testing/api/v3/tc-fixtures.json +++ b/traffic_ops/testing/api/v3/tc-fixtures.json @@ -312,6 +312,7 @@ "initialDispersion": 1, "logsEnabled": true, "longDesc": "long desc", + "orgServerFqdn": "http://example.test", "regionalGeoBlocking": true, "routingName": "goodroute", "tenant": "tenant1", @@ -334,6 +335,7 @@ "initialDispersion": 1, "logsEnabled": true, "longDesc": "long desc", + "orgServerFqdn": "http://example2.test", "regionalGeoBlocking": true, "routingName": "goodroute", "tenant": "tenant1", From d357c0d00e7b3e4f2e0cbbc9f191a6d97ac4b852 Mon Sep 17 00:00:00 2001 From: ocket8888 Date: Mon, 13 Mar 2023 14:05:19 -0600 Subject: [PATCH 4/4] Changed indentation to be consistently incorrect --- traffic_ops/testing/api/v3/tc-fixtures.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/traffic_ops/testing/api/v3/tc-fixtures.json b/traffic_ops/testing/api/v3/tc-fixtures.json index 1cdece320b..9d9a5b7802 100644 --- a/traffic_ops/testing/api/v3/tc-fixtures.json +++ b/traffic_ops/testing/api/v3/tc-fixtures.json @@ -312,7 +312,7 @@ "initialDispersion": 1, "logsEnabled": true, "longDesc": "long desc", - "orgServerFqdn": "http://example.test", + "orgServerFqdn": "http://example.test", "regionalGeoBlocking": true, "routingName": "goodroute", "tenant": "tenant1", @@ -335,7 +335,7 @@ "initialDispersion": 1, "logsEnabled": true, "longDesc": "long desc", - "orgServerFqdn": "http://example2.test", + "orgServerFqdn": "http://example2.test", "regionalGeoBlocking": true, "routingName": "goodroute", "tenant": "tenant1",