TaylorCFrey commented on a change in pull request #5345:
URL: https://github.com/apache/trafficcontrol/pull/5345#discussion_r540428065
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -335,6 +349,46 @@ func ParseOrgServerFQDN(orgServerFQDN string) (*string,
*string, *string, error)
return &protocol, &FQDN, port, nil
}
+func (ds *DeliveryServiceV31) Sanitize() {
+ if ds.GeoLimitCountries != nil {
+ *ds.GeoLimitCountries =
strings.ToUpper(strings.Replace(*ds.GeoLimitCountries, " ", "", -1))
+ }
+ if ds.ProfileID != nil && *ds.ProfileID == -1 {
+ ds.ProfileID = nil
+ }
+ setNilIfEmpty(
+ &ds.EdgeHeaderRewrite,
+ &ds.MidHeaderRewrite,
+ &ds.FirstHeaderRewrite,
+ &ds.InnerHeaderRewrite,
+ &ds.LastHeaderRewrite,
+ )
+ if ds.RoutingName == nil || *ds.RoutingName == "" {
+ ds.RoutingName = util.StrPtr(DefaultRoutingName)
+ }
+ if ds.AnonymousBlockingEnabled == nil {
+ ds.AnonymousBlockingEnabled = util.BoolPtr(false)
+ }
+ signedAlgorithm := SigningAlgorithmURLSig
+ if ds.Signed && (ds.SigningAlgorithm == nil || *ds.SigningAlgorithm ==
"") {
+ ds.SigningAlgorithm = &signedAlgorithm
+ }
+ if !ds.Signed && ds.SigningAlgorithm != nil && *ds.SigningAlgorithm ==
signedAlgorithm {
+ ds.Signed = true
+ }
+ if ds.MaxOriginConnections == nil || *ds.MaxOriginConnections < 0 {
+ ds.MaxOriginConnections = util.IntPtr(0)
+ }
+ if ds.DeepCachingType == nil {
+ s := DeepCachingType("")
+ ds.DeepCachingType = &s
+ }
+ *ds.DeepCachingType =
DeepCachingTypeFromString(string(*ds.DeepCachingType))
+ if ds.MaxRequestHeaderSize == nil {
+ ds.MaxRequestHeaderSize = util.IntPtr(131072)
Review comment:
Is there something special about this value (131072)? Should we asign it
as a variable to avoid magic number confusion in the future?
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -179,6 +188,11 @@ type DeliveryServiceNullableV30 struct {
ServiceCategory *string `json:"serviceCategory"
db:"service_category"`
}
+type DeliveryServiceV31 struct {
Review comment:
Do we need to signal if this, too, is `nullable` or not, like the nested
struct?
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -32,6 +32,8 @@ import (
*/
const DefaultRoutingName = "cdn"
+const MaxUint = ^uint(0)
Review comment:
Are these max values used?
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -532,6 +586,121 @@ func (ds *DeliveryServiceNullableV30)
validateTypeFields(tx *sql.Tx) error {
return nil
}
+func (ds *DeliveryServiceV31) validateTypeFields(tx *sql.Tx) 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"
+
+ typeName, err := ValidateTypeID(tx, ds.TypeID, "deliveryservice")
+ if err != nil {
+ return err
+ }
+
+ errs := validation.Errors{
+ "consistentHashQueryParams": validation.Validate(ds,
+ validation.By(func(dsi interface{}) error {
+ ds := dsi.(*DeliveryServiceV31)
+ if len(ds.ConsistentHashQueryParams) == 0 ||
DSType(typeName).IsHTTP() {
+ return nil
+ }
+ return fmt.Errorf("consistentHashQueryParams
not allowed for '%s' deliveryservice type", typeName)
+ })),
+ "initialDispersion": validation.Validate(ds.InitialDispersion,
+
validation.By(requiredIfMatchesTypeName([]string{HTTPRegexType}, typeName)),
+ validation.By(tovalidate.IsGreaterThanZero)),
+ "ipv6RoutingEnabled": validation.Validate(ds.IPV6RoutingEnabled,
+
validation.By(requiredIfMatchesTypeName([]string{SteeringRegexType,
DNSRegexType, HTTPRegexType}, typeName))),
+ "missLat": validation.Validate(ds.MissLat,
+
validation.By(requiredIfMatchesTypeName([]string{DNSRegexType, HTTPRegexType},
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.Min(-180.0).Error(longitudeErr),
+ validation.Max(180.0).Error(longitudeErr)),
+ "multiSiteOrigin": validation.Validate(ds.MultiSiteOrigin,
+
validation.By(requiredIfMatchesTypeName([]string{DNSRegexType, HTTPRegexType},
typeName))),
+ "orgServerFqdn": validation.Validate(ds.OrgServerFQDN,
+
validation.By(requiredIfMatchesTypeName([]string{DNSRegexType, HTTPRegexType},
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 {
+ ds := dsi.(*DeliveryServiceV31)
+ if ds.RangeRequestHandling != nil {
+ if *ds.RangeRequestHandling == 3 {
+ return
validation.Validate(ds.RangeSliceBlockSize, validation.Required,
+ // Per Slice Plugin
implementation
+ validation.Min(262144),
// 256KiB
+
validation.Max(33554432), // 32MiB
Review comment:
Would these also be considered _"magic numbers"_ even though they are
explained in comments?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
