You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2021/06/08 00:49:31 UTC
[GitHub] [trafficcontrol] ocket8888 opened a new pull request #5922: Per-Delivery Service TLS versions
ocket8888 opened a new pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922
## What does this PR (Pull Request) do?
- [x] This PR fixes #5891 and fixes #5894
This PR adds a new field to Delivery Services: `tlsVersions`. This allows users to specify the versions of TLS which should be allowable for clients retrieving Delivery Service content.
## Which Traffic Control components are affected by this PR?
- Documentation
- Traffic Ops Client (Go)
- Traffic Ops
## What is the best way to verify this PR?
Make sure the new tests pass, and that none of the old ones are broken.
## The following criteria are ALL met by this PR
- [x] This PR includes tests
- [x] This PR includes documentation
- [x] This PR includes an update to CHANGELOG.md
- [x] This PR includes any and all required license headers
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY**
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651911793
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -110,6 +110,42 @@ func (ds *TODeliveryService) IsTenantAuthorized(user *auth.CurrentUser) (bool, e
return isTenantAuthorized(ds.ReqInfo, &ds.DeliveryServiceV4)
}
+const getTLSVersionsQuery = `
Review comment:
In that case, I would split out the "base" query string for reuse, then just have the specific `WHERE` clauses added on in each place it's used.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651800265
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
Review comment:
I did that because I copy/pasted it in from the "DS Active Flag" changes, where that was actually necessary because it's changing the type of something that was in the lowest-level struct in that "inheritance" tree. I can change it to use nesting, but as soon as we make a breaking change it'll need to do this anyway - for example this PR is still changing a couple of things: the DS match list in API < v4 is a pointer to a slice, which is an unnecessary double-null-check headache because slices are already "nullable", and the `lastUpdated` field is currently in the special ATC format but this changes it to use RFC3339 by switching from `TimeNoMod` to `time.Time`. Those changes seemed too small to warrant their own PR to me, so I left them in, but I'll take them out if you want. I don't think switching time formats should be done across all objects in a single PR, because for things that use the CRUDer that'll mean either doubling the amount of code per endpoint to allow backwar
d compatibility, or rewriting them to not use the CRUDer. I think. Depending on the "flavor" of CRUDer they use.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651322551
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
Review comment:
Why change this embedded struct pattern? If we add `DeliveryServiceV41`, we have to copy/paste all the pre-existing fields again and will end up with mostly-duplicated structs that proliferate with every new API version. The original version seems like it would help cut down on duplicated fields at least.
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -231,12 +292,17 @@ func CreateV31(w http.ResponseWriter, r *http.Request) {
return
}
- res, status, userErr, sysErr := createV31(w, r, inf, ds)
+ res, alerts, status, userErr, sysErr := createV31(w, r, inf, ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice creation was successful.", []tc.DeliveryServiceV31{*res})
+ if res == nil {
Review comment:
ditto prior comment
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -1070,87 +1259,103 @@ func updateV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 *
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
defer resultRows.Close()
if !resultRows.Next() {
- return nil, http.StatusNotFound, errors.New("no delivery service found with this id"), nil
+ return nil, alerts, http.StatusNotFound, errors.New("no delivery service found with this id"), nil
}
- lastUpdated := tc.TimeNoMod{}
+ var lastUpdated time.Time
if err := resultRows.Scan(&lastUpdated); err != nil {
- return nil, http.StatusInternalServerError, nil, errors.New("scan updating delivery service: " + err.Error())
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("scan updating delivery service: " + err.Error())
}
if resultRows.Next() {
xmlID := ""
if ds.XMLID != nil {
xmlID = *ds.XMLID
}
- return nil, http.StatusInternalServerError, nil, errors.New("updating delivery service " + xmlID + ": " + "this update affected too many rows: > 1")
- }
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("updating delivery service " + xmlID + ": " + "this update affected too many rows: > 1")
+ }
if ds.ID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing id after update")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("missing id after update")
}
if ds.XMLID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing xml_id after update")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("missing XMLID after update")
}
if ds.TypeID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing type after update")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("missing type id after update")
}
if ds.RoutingName == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing routing name after update")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("missing routing name after update")
+ }
+
+ if resultRows.Next() {
Review comment:
Why do we need to call this and check a 3rd time?
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -339,40 +739,177 @@ type DeliveryServiceRemovedFieldsV11 struct {
CacheURL *string `json:"cacheurl" db:"cacheurl"`
}
-// DowngradeToV3 converts the 4.x DS to a 3.x DS
+// DowngradeToV3 converts the 4.x DS to a 3.x DS.
func (ds *DeliveryServiceV4) DowngradeToV3() DeliveryServiceNullableV30 {
- return DeliveryServiceNullableV30{
- DeliveryServiceV30: DeliveryServiceV30{
- DeliveryServiceNullableV15: DeliveryServiceNullableV15{
- DeliveryServiceNullableV14: DeliveryServiceNullableV14{
- DeliveryServiceNullableV13: DeliveryServiceNullableV13{
- DeliveryServiceNullableV12: DeliveryServiceNullableV12{
- DeliveryServiceNullableV11: DeliveryServiceNullableV11{
- DeliveryServiceNullableFieldsV11: ds.DeliveryServiceNullableFieldsV11,
- },
- },
- DeliveryServiceFieldsV13: ds.DeliveryServiceFieldsV13,
- },
- DeliveryServiceFieldsV14: ds.DeliveryServiceFieldsV14,
- },
- DeliveryServiceFieldsV15: ds.DeliveryServiceFieldsV15,
- },
- DeliveryServiceFieldsV30: ds.DeliveryServiceFieldsV30,
- },
- DeliveryServiceFieldsV31: ds.DeliveryServiceFieldsV31,
+ var ret DeliveryServiceNullableV30
+ ret.Active = ds.Active
+ ret.AnonymousBlockingEnabled = ds.AnonymousBlockingEnabled
+ ret.CCRDNSTTL = ds.CCRDNSTTL
+ ret.CDNID = ds.CDNID
+ ret.CDNName = ds.CDNName
+ ret.CheckPath = ds.CheckPath
+ ret.ConsistentHashRegex = ds.ConsistentHashRegex
+ ret.ConsistentHashQueryParams = make([]string, len(ds.ConsistentHashQueryParams))
+ copy(ret.ConsistentHashQueryParams, ds.ConsistentHashQueryParams)
+ ret.DeepCachingType = ds.DeepCachingType
+ ret.DisplayName = ds.DisplayName
+ ret.DNSBypassCNAME = ds.DNSBypassCNAME
+ ret.DNSBypassIP = ds.DNSBypassIP
+ ret.DNSBypassIP6 = ds.DNSBypassIP6
+ ret.DNSBypassTTL = ds.DNSBypassTTL
+ ret.DSCP = ds.DSCP
+ ret.EcsEnabled = ds.EcsEnabled
+ ret.EdgeHeaderRewrite = ds.EdgeHeaderRewrite
+ ret.ExampleURLs = make([]string, len(ds.ExampleURLs))
+ copy(ret.ExampleURLs, ds.ExampleURLs)
+ ret.FirstHeaderRewrite = ds.FirstHeaderRewrite
+ ret.FQPacingRate = ds.FQPacingRate
+ ret.GeoLimit = ds.GeoLimit
+ ret.GeoLimitCountries = ds.GeoLimitCountries
+ ret.GeoLimitRedirectURL = ds.GeoLimitRedirectURL
+ ret.GeoProvider = ds.GeoProvider
+ ret.GlobalMaxMBPS = ds.GlobalMaxMBPS
+ ret.GlobalMaxTPS = ds.GlobalMaxTPS
+ ret.HTTPBypassFQDN = ds.HTTPBypassFQDN
+ ret.ID = ds.ID
+ ret.InfoURL = ds.InfoURL
+ ret.InitialDispersion = ds.InitialDispersion
+ ret.InnerHeaderRewrite = ds.InnerHeaderRewrite
+ ret.IPV6RoutingEnabled = ds.IPV6RoutingEnabled
+ ret.LastHeaderRewrite = ds.LastHeaderRewrite
+ ret.LastUpdated = TimeNoModFromTime(ds.LastUpdated)
+ ret.LogsEnabled = ds.LogsEnabled
+ ret.LongDesc = ds.LongDesc
+ ret.LongDesc1 = ds.LongDesc1
+ ret.LongDesc2 = ds.LongDesc2
+ if ds.MatchList != nil {
+ ret.MatchList = new([]DeliveryServiceMatch)
+ *ret.MatchList = make([]DeliveryServiceMatch, len(ds.MatchList))
+ copy(*ret.MatchList, ds.MatchList)
+ } else {
+ ret.MatchList = nil
}
-}
-
-// UpgradeToV4 converts the 3.x DS to a 4.x DS
+ ret.MaxDNSAnswers = ds.MaxDNSAnswers
+ ret.MaxOriginConnections = ds.MaxOriginConnections
+ ret.MaxRequestHeaderBytes = ds.MaxRequestHeaderBytes
+ ret.MidHeaderRewrite = ds.MidHeaderRewrite
+ ret.MissLat = ds.MissLat
+ ret.MissLong = ds.MissLong
+ ret.MultiSiteOrigin = ds.MultiSiteOrigin
+ ret.OriginShield = ds.OriginShield
+ ret.OrgServerFQDN = ds.OrgServerFQDN
+ ret.ProfileDesc = ds.ProfileDesc
+ ret.ProfileID = ds.ProfileID
+ ret.ProfileName = ds.ProfileName
+ ret.Protocol = ds.Protocol
+ ret.QStringIgnore = ds.QStringIgnore
+ ret.RangeRequestHandling = ds.RangeRequestHandling
+ ret.RangeSliceBlockSize = ds.RangeSliceBlockSize
+ ret.RegexRemap = ds.RegexRemap
+ ret.RegionalGeoBlocking = ds.RegionalGeoBlocking
+ ret.RemapText = ds.RemapText
+ ret.RoutingName = ds.RoutingName
+ ret.ServiceCategory = ds.ServiceCategory
+ ret.Signed = ds.Signed
+ ret.SigningAlgorithm = ds.SigningAlgorithm
+ ret.SSLKeyVersion = ds.SSLKeyVersion
+ ret.Tenant = ds.Tenant
+ ret.TenantID = ds.TenantID
+ ret.Topology = ds.Topology
+ ret.TRResponseHeaders = ds.TRResponseHeaders
+ ret.TRRequestHeaders = ds.TRRequestHeaders
+ ret.Type = ds.Type
+ ret.TypeID = ds.TypeID
+ ret.XMLID = ds.XMLID
+
+ return ret
+}
+
+// UpgradeToV4 converts the 3.x DS to a 4.x DS.
func (ds *DeliveryServiceNullableV30) UpgradeToV4() DeliveryServiceV4 {
- return DeliveryServiceV4{
- DeliveryServiceFieldsV31: ds.DeliveryServiceFieldsV31,
- DeliveryServiceFieldsV30: ds.DeliveryServiceFieldsV30,
- DeliveryServiceFieldsV15: ds.DeliveryServiceFieldsV15,
- DeliveryServiceFieldsV14: ds.DeliveryServiceFieldsV14,
- DeliveryServiceFieldsV13: ds.DeliveryServiceFieldsV13,
- DeliveryServiceNullableFieldsV11: ds.DeliveryServiceNullableFieldsV11,
+ var ret DeliveryServiceV4
Review comment:
RE: the un-embedding, this change and the corresponding downgrade function change seem less-preferable than the original embedding
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -189,12 +240,17 @@ func CreateV15(w http.ResponseWriter, r *http.Request) {
return
}
- res, status, userErr, sysErr := createV15(w, r, inf, ds)
+ res, alerts, status, userErr, sysErr := createV15(w, r, inf, ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice creation was successful.", []tc.DeliveryServiceNullableV15{*res})
+ if res == nil {
Review comment:
ditto prior comment
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -124,12 +160,17 @@ func CreateV12(w http.ResponseWriter, r *http.Request) {
return
}
- res, status, userErr, sysErr := createV12(w, r, inf, ds)
+ res, alerts, status, userErr, sysErr := createV12(w, r, inf, ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice creation was successful.", []tc.DeliveryServiceNullableV12{*res})
+ if res == nil {
Review comment:
Honestly, I don't think this should ever happen. And if it does, the end result is mostly the same: the panic will be recovered by the middleware, we'll get a nice stacktrace in the logs, and the user still gets a 503.
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/safe.go
##########
@@ -94,17 +94,39 @@ func UpdateSafe(w http.ResponseWriter, r *http.Request) {
ds := dses[0]
alertMsg := "Delivery Service safe update successful."
- if inf.Version != nil && inf.Version.Major == 1 && inf.Version.Minor < 5 {
- switch inf.Version.Minor {
+ if inf.Version == nil {
+ log.Warnln("API version found to be null in DS safe update")
+ api.WriteRespAlertObj(w, r, tc.SuccessLevel, alertMsg, dses)
Review comment:
this should `return` after, right?
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -252,61 +318,67 @@ func CreateV40(w http.ResponseWriter, r *http.Request) {
return
}
- res, status, userErr, sysErr := createV40(w, r, inf, ds)
+ res, alerts, status, userErr, sysErr := createV40(w, r, inf, ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice creation was successful.", []tc.DeliveryServiceV40{*res})
+ if res == nil || res.ID == nil {
Review comment:
ditto prior comment
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -110,6 +110,42 @@ func (ds *TODeliveryService) IsTenantAuthorized(user *auth.CurrentUser) (bool, e
return isTenantAuthorized(ds.ReqInfo, &ds.DeliveryServiceV4)
}
+const getTLSVersionsQuery = `
+SELECT tls_version
+FROM public.deliveryservice_tls_version
+WHERE deliveryservice = $1
+ORDER BY tls_version
+`
+
+// GetDSTLSVersions retrieves the TLS versions explicitly supported by a
+// Delivery Service identified by dsID. This will panic if handed a nil
Review comment:
> This will panic if handed a nil transaction.
I think in general this is obvious -- do we really need to call it out in GoDoc comments?
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -210,12 +266,17 @@ func CreateV30(w http.ResponseWriter, r *http.Request) {
return
}
- res, status, userErr, sysErr := createV30(w, r, inf, ds)
+ res, alerts, status, userErr, sysErr := createV30(w, r, inf, ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice creation was successful.", []tc.DeliveryServiceV30{*res})
+ if res == nil {
Review comment:
ditto prior comment
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active *bool `json:"active" db:"active"`
+ // AnonymousBlockingEnabled sets whether or not anonymized IP addresses
+ // (e.g. Tor exit nodes) should be restricted from accessing the Delivery
+ // Service's content.
+ AnonymousBlockingEnabled *bool `json:"anonymousBlockingEnabled" db:"anonymous_blocking_enabled"`
+ // CCRDNSTTL sets the Time-to-Live - in seconds - for DNS responses for this
+ // Delivery Service from Traffic Router.
+ CCRDNSTTL *int `json:"ccrDnsTtl" db:"ccr_dns_ttl"`
+ // CDNID is the integral, unique identifier for the CDN to which the
+ // Delivery Service belongs.
+ CDNID *int `json:"cdnId" db:"cdn_id"`
+ // CDNName is the name of the CDN to which the Delivery Service belongs.
+ CDNName *string `json:"cdnName"`
+ // CheckPath is a path which may be requested of the Delivery Service's
+ // origin to ensure it's working properly.
+ CheckPath *string `json:"checkPath" db:"check_path"`
+ // ConsistentHashQueryParams is a list of al of the query string parameters
+ // which ought to be considered by Traffic Router in client request URIs for
+ // HTTP-routed Delivery Services in the hashing process.
+ ConsistentHashQueryParams []string `json:"consistentHashQueryParams"`
+ // ConsistentHashRegex is used by Traffic Router to extract meaningful parts
+ // of a client's request URI for HTTP-routed Delivery Services before
+ // hashing the request to find a cache server to which to direct the client.
+ ConsistentHashRegex *string `json:"consistentHashRegex"`
+ // DeepCachingType may only legally point to 'ALWAYS' or 'NEVER', which
+ // define whether "deep caching" may or may not be used for this Delivery
+ // Service, respectively.
+ DeepCachingType *DeepCachingType `json:"deepCachingType" db:"deep_caching_type"`
+ // DisplayName is a human-friendly name that might be used in some UIs
+ // somewhere.
+ DisplayName *string `json:"displayName" db:"display_name"`
+ // DNSBypassCNAME is a fully qualified domain name to be used in a CNAME
+ // record presented to clients in bypass scenarios.
+ DNSBypassCNAME *string `json:"dnsBypassCname" db:"dns_bypass_cname"`
+ // DNSBypassIP is an IPv4 address to be used in an A record presented to
+ // clients in bypass scenarios.
+ DNSBypassIP *string `json:"dnsBypassIp" db:"dns_bypass_ip"`
+ // DNSBypassIP6 is an IPv6 address to be used in an AAAA record presented to
+ // clients in bypass scenarios.
+ DNSBypassIP6 *string `json:"dnsBypassIp6" db:"dns_bypass_ip6"`
+ // DNSBypassTTL sets the Time-to-Live - in seconds - of DNS responses from
+ // the Traffic Router that contain records for bypass destinations.
+ DNSBypassTTL *int `json:"dnsBypassTtl" db:"dns_bypass_ttl"`
+ // DSCP sets the Differentiated Services Code Point for IP packets
+ // transferred between clients, origins, and cache servers when obtaining
+ // and serving content for this Delivery Service.
+ // See Also: https://en.wikipedia.org/wiki/Differentiated_services
+ DSCP *int `json:"dscp" db:"dscp"`
+ // EcsEnabled describes whether or not the Traffic Router's EDNS0 Client
+ // Subnet extensions should be enabled when serving DNS responses for this
+ // Delivery Service. Even if this is true, the Traffic Router may still
+ // have the extensions unilaterally disabled in its own configuration.
+ EcsEnabled bool `json:"ecsEnabled" db:"ecs_enabled"`
+ // EdgeHeaderRewrite is a "header rewrite rule" used by ATS at the Edge-tier
+ // of caching. This has no effect on Delivery Services that don't use a
+ // Topology.
+ EdgeHeaderRewrite *string `json:"edgeHeaderRewrite" db:"edge_header_rewrite"`
+ // ExampleURLs is a list of all of the URLs from which content may be
+ // requested from the Delivery Service.
+ ExampleURLs []string `json:"exampleURLs"`
+ // FirstHeaderRewrite is a "header rewrite rule" used by ATS at the first
+ // caching layer encountered in the Delivery Service's Topology, or nil if
+ // there is no such rule. This has no effect on Delivery Services that don't
+ // employ Topologies.
+ FirstHeaderRewrite *string `json:"firstHeaderRewrite" db:"first_header_rewrite"`
+ // FQPacingRate sets the maximum bytes per second a cache server will deliver
+ // on any single TCP connection for this Delivery Service. This may never
+ // legally point to a value less than zero.
+ FQPacingRate *int `json:"fqPacingRate" db:"fq_pacing_rate"`
+ // GeoLimit defines whether or not access to a Delivery Service's content
+ // should be limited based on the requesting client's geographic location.
+ // Despite that this is a pointer to an arbitrary integer, the only valid
+ // values are 0 (which indicates that content should not be limited
+ // geographically), 1 (which indicates that content should only be served to
+ // clients whose IP addresses can be found within a Coverage Zone File), and
+ // 2 (which indicates that content should be served to clients whose IP
+ // addresses can be found within a Coverage Zone File OR are allowed access
+ // according to the "array" in GeoLimitCountries).
+ GeoLimit *int `json:"geoLimit" db:"geo_limit"`
+ // GeoLimitCountries is an "array" of "country codes" that itemizes the
+ // countries within which the Delivery Service's content ought to be made
+ // available. This has no effect if GeoLimit is not a pointer to exactly the
+ // value 2.
+ GeoLimitCountries *string `json:"geoLimitCountries" db:"geo_limit_countries"`
+ // GeoLimitRedirectURL is a URL to which clients will be redirected if their
+ // access to the Delivery Service's content is blocked by GeoLimit rules.
+ GeoLimitRedirectURL *string `json:"geoLimitRedirectURL" db:"geolimit_redirect_url"`
+ // GeoProvider names the type of database to be used for providing IP
+ // address-to-geographic-location mapping for this Delivery Service. The
+ // only valid values to which it may point are 0 (which indicates the use of
+ // a MaxMind GeoIP2 database) and 1 (which indicates the use of a Neustar
+ // GeoPoint IP address database).
+ GeoProvider *int `json:"geoProvider" db:"geo_provider"`
+ // GlobalMaxMBPS defines a maximum number of MegaBytes Per Second which may
+ // be served for the Delivery Service before redirecting clients to bypass
+ // locations.
+ GlobalMaxMBPS *int `json:"globalMaxMbps" db:"global_max_mbps"`
+ // GlobalMaxTPS defines a maximum number of Transactions Per Second which
+ // may be served for the Delivery Service before redirecting clients to
+ // bypass locations.
+ GlobalMaxTPS *int `json:"globalMaxTps" db:"global_max_tps"`
+ // HTTPBypassFQDN is a network location to which clients will be redirected
+ // in bypass scenarios using HTTP "Location" headers and appropriate
+ // redirection response codes.
+ HTTPBypassFQDN *string `json:"httpBypassFqdn" db:"http_bypass_fqdn"`
+ // ID is an integral, unique identifier for the Delivery Service.
+ ID *int `json:"id" db:"id"`
+ // InfoURL is a URL to which operators or clients may be directed to obtain
+ // further information about a Delivery Service.
+ InfoURL *string `json:"infoUrl" db:"info_url"`
+ // InitialDispersion sets the number of cache servers within the first
+ // caching layer ("Edge-tier" in a non-Topology context) across which
+ // content will be dispersed per Cache Group.
+ InitialDispersion *int `json:"initialDispersion" db:"initial_dispersion"`
+ // InnerHeaderRewrite is a "header rewrite rule" used by ATS at all caching
+ // layers encountered in the Delivery Service's Topology except the first
+ // and last, or nil if there is no such rule. This has no effect on Delivery
+ // Services that don't employ Topologies.
+ InnerHeaderRewrite *string `json:"innerHeaderRewrite" db:"inner_header_rewrite"`
+ // IPV6RoutingEnabled controls whether or not routing over IPv6 should be
+ // done for this Delivery Service.
+ IPV6RoutingEnabled *bool `json:"ipv6RoutingEnabled" db:"ipv6_routing_enabled"`
+ // LastHeaderRewrite is a "header rewrite rule" used by ATS at the first
+ // caching layer encountered in the Delivery Service's Topology, or nil if
+ // there is no such rule. This has no effect on Delivery Services that don't
+ // employ Topologies.
+ LastHeaderRewrite *string `json:"lastHeaderRewrite" db:"last_header_rewrite"`
+ // LastUpdated is the time and date at which the Delivery Service was last
+ // updated.
+ LastUpdated time.Time `json:"lastUpdated" db:"last_updated"`
+ // LogsEnabled controls nothing. It is kept only for legacy compatibility.
+ LogsEnabled *bool `json:"logsEnabled" db:"logs_enabled"`
+ // LongDesc is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc *string `json:"longDesc" db:"long_desc"`
+ // LongDesc1 is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc1 *string `json:"longDesc1" db:"long_desc_1"`
+ // LongDesc2 is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc2 *string `json:"longDesc2" db:"long_desc_2"`
+ // MatchList is a list of Regular Expressions used for routing the Delivery
+ // Service. Order matters, and the array is not allowed to be sparse.
+ MatchList []DeliveryServiceMatch `json:"matchList"`
+ // MaxDNSAnswers sets the maximum number of records which should be returned
+ // by Traffic Router in DNS responses to requests for resolving names for
+ // this Delivery Service.
+ MaxDNSAnswers *int `json:"maxDnsAnswers" db:"max_dns_answers"`
+ // MaxOriginConnections defines the total maximum number of connections
+ // that the highest caching layer ("Mid-tier" in a non-Topology context) is
+ // allowed to have concurrently open to the Delivery Service's Origin. This
+ // may never legally point to a value less than 0.
+ MaxOriginConnections *int `json:"maxOriginConnections" db:"max_origin_connections"`
+ // MaxRequestHeaderBytes is the maximum size (in bytes) of the request
+ // header that is allowed for this Delivery Service.
+ MaxRequestHeaderBytes *int `json:"maxRequestHeaderBytes" db:"max_request_header_bytes"`
+ // MidHeaderRewrite is a "header rewrite rule" used by ATS at the Mid-tier
+ // of caching. This has no effect on Delivery Services that don't use a
+ // Topology.
+ MidHeaderRewrite *string `json:"midHeaderRewrite" db:"mid_header_rewrite"`
+ // MissLat is a latitude to default to for clients of this Delivery Service
+ // when geolocation attempts fail.
+ MissLat *float64 `json:"missLat" db:"miss_lat"`
+ // MissLong is a longitude to default to for clients of this Delivery
+ // Service when geolocation attempts fail.
+ MissLong *float64 `json:"missLong" db:"miss_long"`
+ // MultiSiteOrigin determines whether or not the Delivery Service makes use
+ // of "Multi-Site Origin".
+ MultiSiteOrigin *bool `json:"multiSiteOrigin" db:"multi_site_origin"`
+ // OriginShield is a field that does nothing. It is kept only for legacy
+ // compatibility reasons.
+ OriginShield *string `json:"originShield" db:"origin_shield"`
+ // OrgServerFQDN is the URL - NOT Fully Qualified Domain Name - of the
+ // origin of the Delivery Service's content.
+ OrgServerFQDN *string `json:"orgServerFqdn" db:"org_server_fqdn"`
+ // ProfileDesc is the Description of the Profile used by the Delivery
+ // Service, if any.
+ ProfileDesc *string `json:"profileDescription"`
+ // ProfileID is the integral, unique identifier of the Profile used by the
+ // Delivery Service, if any.
+ ProfileID *int `json:"profileId" db:"profile"`
+ // ProfileName is the Name of the Profile used by the Delivery Service, if
+ // any.
+ ProfileName *string `json:"profileName"`
+ // Protocol defines the protocols by which caching servers may communicate
+ // with clients. The valid values to which it may point are 0 (which implies
+ // that only HTTP may be used), 1 (which implies that only HTTPS may be
+ // used), 2 (which implies that either HTTP or HTTPS may be used), and 3
+ // (which implies that clients using HTTP must be redirected to use HTTPS,
+ // while communications over HTTPS may proceed as normal).
+ Protocol *int `json:"protocol" db:"protocol"`
+ // QStringIgnore sets how query strings in HTTP requests to cache servers
+ // from clients are treated. The only valid values to which it may point are
+ // 0 (which implies that all caching layers will pass query strings in
+ // upstream requests and use them in the cache key), 1 (which implies that
+ // all caching layers will pass query strings in upstream requests, but not
+ // use them in cache keys), and 2 (which implies that the first encountered
+ // caching layer - "Edge-tier" in a non-Topology context - will strip query
+ // strings, effectively preventing them from being passed in upstream
+ // requests, and not use them in the cache key).
+ QStringIgnore *int `json:"qstringIgnore" db:"qstring_ignore"`
+ // RangeRequestHandling defines how HTTP GET requests with a Range header
+ // will be handled by cache servers serving the Delivery Service's content.
+ // The only valid values to which it may point are 0 (which implies that
+ // Range requests will not be cached at all), 1 (which implies that the
+ // background_fetch plugin will be used to service the range request while
+ // caching the whole object), 2 (which implies that the cache_range_requests
+ // plugin will be used to cache ranges as unique objects), and 3 (which
+ // implies that the slice plugin will be used to slice range based requests
+ // into deterministic chunks.)
+ RangeRequestHandling *int `json:"rangeRequestHandling" db:"range_request_handling"`
+ // RangeSliceBlockSize defines the size of range request blocks - or
+ // "slices" - used by the "slice" plugin. This has no effect if
+ // RangeRequestHandling does not point to exactly 3. This may never legally
+ // point to a value less than zero.
+ RangeSliceBlockSize *int `json:"rangeSliceBlockSize" db:"range_slice_block_size"`
+ // Regex Remap is a raw line to be inserted into "regex_remap.config" on the
+ // cache server. Care is necessitated in its use, because the input is in no
+ // way restricted, validated, or limited in scope to the Delivery Service.
+ RegexRemap *string `json:"regexRemap" db:"regex_remap"`
+ // RegionalGeoBlocking defines whether or not whatever Regional Geo Blocking
+ // rules are configured on the Traffic Router serving content for this
+ // Delivery Service will have an effect on the traffic of this Delivery
+ // Service.
+ RegionalGeoBlocking *bool `json:"regionalGeoBlocking" db:"regional_geo_blocking"`
+ // RemapText is raw text to insert in "remap.config" on the cache servers
+ // serving content for this Delivery Service. Care is necessitated in its
+ // use, because the input is in no way restricted, validated, or limited in
+ // scope to the Delivery Service.
+ RemapText *string `json:"remapText" db:"remap_text"`
+ // RoutingName defines the lowest-level DNS label used by the Delivery
+ // Service, e.g. if trafficcontrol.apache.org were a Delivery Service, it
+ // would have a RoutingName of "trafficcontrol".
+ RoutingName *string `json:"routingName" db:"routing_name"`
+ // ServiceCategory defines a category to which a Delivery Service may
+ // belong, which will cause HTTP Responses containing content for the
+ // Delivery Service to have the "X-CDN-SVC" header with a value that is the
+ // XMLID of the Delivery Service.
+ ServiceCategory *string `json:"serviceCategory" db:"service_category"`
+ // Signed is a legacy field. It is allowed to be `true` if and only if
+ // SigningAlgorithm is not nil.
+ Signed bool `json:"signed"`
+ // SigningAlgorithm is the name of the algorithm used to sign CDN URIs for
+ // this Delivery Service's content, or nil if no URI signing is done for the
+ // Delivery Service. This may only point to the values "url_sig" or
+ // "uri_signing".
+ SigningAlgorithm *string `json:"signingAlgorithm" db:"signing_algorithm"`
+ // SSLKeyVersion incremented whenever Traffic Portal generates new SSL keys
+ // for the Delivery Service, effectively making it a "generational" marker.
+ SSLKeyVersion *int `json:"sslKeyVersion" db:"ssl_key_version"`
+ // Tenant is the Tenant to which the Delivery Service belongs.
+ Tenant *string `json:"tenant"`
+ // TenantID is the integral, unique identifier for the Tenant to which the
+ // Delivery Service belongs.
+ TenantID *int `json:"tenantId" db:"tenant_id"`
+ // TLSVersions is the list of explicitly supported TLS versions for cache
+ // servers serving the Delivery Service's content.
+ TLSVersions []string `json:"tlsVersions" db:"tls_versions"`
+ // Topology is the name of the Topology used by the Delivery Service, or nil
+ // if no Topology is used.
+ Topology *string `json:"topology" db:"topology"`
+ // TRResponseHeaders is a set of headers (separated by CRLF pairs as per the
+ // HTTP spec) and their values (separated by a colon as per the HTTP spec)
+ // which will be sent by Traffic Router in HTTP responses to client requests
+ // for this Delivery Service's content. This has no effect on DNS-routed or
+ // un-routed Delivery Service Types.
+ TRResponseHeaders *string `json:"trResponseHeaders"`
+ // TRRequestHeaders is an "array" of HTTP headers which should be logged
+ // from client HTTP requests for this Delivery Service's content by Traffic
+ // Router, separated by newlines. This has no effect on DNS-routed or
+ // un-routed Delivery Service Types.
+ TRRequestHeaders *string `json:"trRequestHeaders"`
+ // Type describes how content is routed and cached for this Delivery Service
+ // as well as what other properties have any meaning.
+ Type *DSType `json:"type"`
+ // TypeID is an integral, unique identifier for the Tenant to which the
+ // Delivery Service belongs.
+ TypeID *int `json:"typeId" db:"type"`
+ // XMLID is a unique identifier that is also the second lowest-level DNS
+ // label used by the Delivery Service. For example, if a Delivery Service's
+ // content may be requested from video.demo1.mycdn.ciab.test, it may be
+ // inferred that the Delivery Service's XMLID is demo1.
+ XMLID *string `json:"xmlId" db:"xml_id"`
}
// DeliveryServiceV4 is a Delivery Service as it appears in version 4 of the
// Traffic Ops API - it always points to the highest minor version in APIv4.
type DeliveryServiceV4 = DeliveryServiceV40
+// These are the TLS Versions known by Apache Traffic Control to exist.
+const (
+ // Deprecated: TLS version 1.0 is known to be insecure.
+ TLSVersion10 = "1.0"
+ // Deprecated: TLS version 1.1 is known to be insecure.
+ TLSVersion11 = "1.1"
+ TLSVersion12 = "1.2"
+ TLSVersion13 = "1.3"
+)
+
+func newerDisallowedMessage(old string, newer []string) string {
+ l := len(newer)
+ if l < 1 {
+ return ""
+ }
+
+ var msg strings.Builder
+ msg.WriteString("old TLS version ")
+ msg.WriteString(old)
+ msg.WriteString(" is allowed, but newer version")
+ if l > 1 {
+ msg.WriteRune('s')
+ }
+ msg.WriteRune(' ')
+ msg.WriteString(newer[0])
+ if l > 1 {
+ msg.WriteString(", ")
+ if l > 2 {
+ msg.WriteString(newer[1])
+ msg.WriteString(", and ")
+ msg.WriteString(newer[2])
+ } else {
+ msg.WriteString("and ")
+ msg.WriteString(newer[1])
+ }
+ msg.WriteString(" are ")
+ } else {
+ msg.WriteString(" is ")
+ }
+ msg.WriteString("disallowed; this configuration may be insecure")
+
+ return msg.String()
+}
+
+// TLSVersionsAlerts generates warning-level alerts for the given TLS versions
+// array. It will warn if newer versions are disallowed while older, less
+// secure versions are allowed, or if there are unrecognized versions present.
+//
+// This does NOT verify that the passed TLS versions are _valid_, it ONLY
+// creates warnings based on conditions that are possibly detrimental to CDN
+// operation, but can, in fact, work.
+func TLSVersionsAlerts(vers []string) Alerts {
Review comment:
Does it make sense for this function (and `newerDisallowedMessage`) to live in `lib/go-tc`? Do we envision this being used anywhere other than within TO?
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active *bool `json:"active" db:"active"`
+ // AnonymousBlockingEnabled sets whether or not anonymized IP addresses
+ // (e.g. Tor exit nodes) should be restricted from accessing the Delivery
+ // Service's content.
+ AnonymousBlockingEnabled *bool `json:"anonymousBlockingEnabled" db:"anonymous_blocking_enabled"`
+ // CCRDNSTTL sets the Time-to-Live - in seconds - for DNS responses for this
+ // Delivery Service from Traffic Router.
+ CCRDNSTTL *int `json:"ccrDnsTtl" db:"ccr_dns_ttl"`
+ // CDNID is the integral, unique identifier for the CDN to which the
+ // Delivery Service belongs.
+ CDNID *int `json:"cdnId" db:"cdn_id"`
+ // CDNName is the name of the CDN to which the Delivery Service belongs.
+ CDNName *string `json:"cdnName"`
+ // CheckPath is a path which may be requested of the Delivery Service's
+ // origin to ensure it's working properly.
+ CheckPath *string `json:"checkPath" db:"check_path"`
+ // ConsistentHashQueryParams is a list of al of the query string parameters
+ // which ought to be considered by Traffic Router in client request URIs for
+ // HTTP-routed Delivery Services in the hashing process.
+ ConsistentHashQueryParams []string `json:"consistentHashQueryParams"`
+ // ConsistentHashRegex is used by Traffic Router to extract meaningful parts
+ // of a client's request URI for HTTP-routed Delivery Services before
+ // hashing the request to find a cache server to which to direct the client.
+ ConsistentHashRegex *string `json:"consistentHashRegex"`
+ // DeepCachingType may only legally point to 'ALWAYS' or 'NEVER', which
+ // define whether "deep caching" may or may not be used for this Delivery
+ // Service, respectively.
+ DeepCachingType *DeepCachingType `json:"deepCachingType" db:"deep_caching_type"`
+ // DisplayName is a human-friendly name that might be used in some UIs
+ // somewhere.
+ DisplayName *string `json:"displayName" db:"display_name"`
+ // DNSBypassCNAME is a fully qualified domain name to be used in a CNAME
+ // record presented to clients in bypass scenarios.
+ DNSBypassCNAME *string `json:"dnsBypassCname" db:"dns_bypass_cname"`
+ // DNSBypassIP is an IPv4 address to be used in an A record presented to
+ // clients in bypass scenarios.
+ DNSBypassIP *string `json:"dnsBypassIp" db:"dns_bypass_ip"`
+ // DNSBypassIP6 is an IPv6 address to be used in an AAAA record presented to
+ // clients in bypass scenarios.
+ DNSBypassIP6 *string `json:"dnsBypassIp6" db:"dns_bypass_ip6"`
+ // DNSBypassTTL sets the Time-to-Live - in seconds - of DNS responses from
+ // the Traffic Router that contain records for bypass destinations.
+ DNSBypassTTL *int `json:"dnsBypassTtl" db:"dns_bypass_ttl"`
+ // DSCP sets the Differentiated Services Code Point for IP packets
+ // transferred between clients, origins, and cache servers when obtaining
+ // and serving content for this Delivery Service.
+ // See Also: https://en.wikipedia.org/wiki/Differentiated_services
+ DSCP *int `json:"dscp" db:"dscp"`
+ // EcsEnabled describes whether or not the Traffic Router's EDNS0 Client
+ // Subnet extensions should be enabled when serving DNS responses for this
+ // Delivery Service. Even if this is true, the Traffic Router may still
+ // have the extensions unilaterally disabled in its own configuration.
+ EcsEnabled bool `json:"ecsEnabled" db:"ecs_enabled"`
+ // EdgeHeaderRewrite is a "header rewrite rule" used by ATS at the Edge-tier
+ // of caching. This has no effect on Delivery Services that don't use a
+ // Topology.
+ EdgeHeaderRewrite *string `json:"edgeHeaderRewrite" db:"edge_header_rewrite"`
+ // ExampleURLs is a list of all of the URLs from which content may be
+ // requested from the Delivery Service.
+ ExampleURLs []string `json:"exampleURLs"`
+ // FirstHeaderRewrite is a "header rewrite rule" used by ATS at the first
+ // caching layer encountered in the Delivery Service's Topology, or nil if
+ // there is no such rule. This has no effect on Delivery Services that don't
+ // employ Topologies.
+ FirstHeaderRewrite *string `json:"firstHeaderRewrite" db:"first_header_rewrite"`
+ // FQPacingRate sets the maximum bytes per second a cache server will deliver
+ // on any single TCP connection for this Delivery Service. This may never
+ // legally point to a value less than zero.
+ FQPacingRate *int `json:"fqPacingRate" db:"fq_pacing_rate"`
+ // GeoLimit defines whether or not access to a Delivery Service's content
+ // should be limited based on the requesting client's geographic location.
+ // Despite that this is a pointer to an arbitrary integer, the only valid
+ // values are 0 (which indicates that content should not be limited
+ // geographically), 1 (which indicates that content should only be served to
+ // clients whose IP addresses can be found within a Coverage Zone File), and
+ // 2 (which indicates that content should be served to clients whose IP
+ // addresses can be found within a Coverage Zone File OR are allowed access
+ // according to the "array" in GeoLimitCountries).
+ GeoLimit *int `json:"geoLimit" db:"geo_limit"`
+ // GeoLimitCountries is an "array" of "country codes" that itemizes the
+ // countries within which the Delivery Service's content ought to be made
+ // available. This has no effect if GeoLimit is not a pointer to exactly the
+ // value 2.
+ GeoLimitCountries *string `json:"geoLimitCountries" db:"geo_limit_countries"`
+ // GeoLimitRedirectURL is a URL to which clients will be redirected if their
+ // access to the Delivery Service's content is blocked by GeoLimit rules.
+ GeoLimitRedirectURL *string `json:"geoLimitRedirectURL" db:"geolimit_redirect_url"`
+ // GeoProvider names the type of database to be used for providing IP
+ // address-to-geographic-location mapping for this Delivery Service. The
+ // only valid values to which it may point are 0 (which indicates the use of
+ // a MaxMind GeoIP2 database) and 1 (which indicates the use of a Neustar
+ // GeoPoint IP address database).
+ GeoProvider *int `json:"geoProvider" db:"geo_provider"`
+ // GlobalMaxMBPS defines a maximum number of MegaBytes Per Second which may
+ // be served for the Delivery Service before redirecting clients to bypass
+ // locations.
+ GlobalMaxMBPS *int `json:"globalMaxMbps" db:"global_max_mbps"`
+ // GlobalMaxTPS defines a maximum number of Transactions Per Second which
+ // may be served for the Delivery Service before redirecting clients to
+ // bypass locations.
+ GlobalMaxTPS *int `json:"globalMaxTps" db:"global_max_tps"`
+ // HTTPBypassFQDN is a network location to which clients will be redirected
+ // in bypass scenarios using HTTP "Location" headers and appropriate
+ // redirection response codes.
+ HTTPBypassFQDN *string `json:"httpBypassFqdn" db:"http_bypass_fqdn"`
+ // ID is an integral, unique identifier for the Delivery Service.
+ ID *int `json:"id" db:"id"`
+ // InfoURL is a URL to which operators or clients may be directed to obtain
+ // further information about a Delivery Service.
+ InfoURL *string `json:"infoUrl" db:"info_url"`
+ // InitialDispersion sets the number of cache servers within the first
+ // caching layer ("Edge-tier" in a non-Topology context) across which
+ // content will be dispersed per Cache Group.
+ InitialDispersion *int `json:"initialDispersion" db:"initial_dispersion"`
+ // InnerHeaderRewrite is a "header rewrite rule" used by ATS at all caching
+ // layers encountered in the Delivery Service's Topology except the first
+ // and last, or nil if there is no such rule. This has no effect on Delivery
+ // Services that don't employ Topologies.
+ InnerHeaderRewrite *string `json:"innerHeaderRewrite" db:"inner_header_rewrite"`
+ // IPV6RoutingEnabled controls whether or not routing over IPv6 should be
+ // done for this Delivery Service.
+ IPV6RoutingEnabled *bool `json:"ipv6RoutingEnabled" db:"ipv6_routing_enabled"`
+ // LastHeaderRewrite is a "header rewrite rule" used by ATS at the first
+ // caching layer encountered in the Delivery Service's Topology, or nil if
+ // there is no such rule. This has no effect on Delivery Services that don't
+ // employ Topologies.
+ LastHeaderRewrite *string `json:"lastHeaderRewrite" db:"last_header_rewrite"`
+ // LastUpdated is the time and date at which the Delivery Service was last
+ // updated.
+ LastUpdated time.Time `json:"lastUpdated" db:"last_updated"`
+ // LogsEnabled controls nothing. It is kept only for legacy compatibility.
+ LogsEnabled *bool `json:"logsEnabled" db:"logs_enabled"`
+ // LongDesc is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc *string `json:"longDesc" db:"long_desc"`
+ // LongDesc1 is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc1 *string `json:"longDesc1" db:"long_desc_1"`
+ // LongDesc2 is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc2 *string `json:"longDesc2" db:"long_desc_2"`
+ // MatchList is a list of Regular Expressions used for routing the Delivery
+ // Service. Order matters, and the array is not allowed to be sparse.
+ MatchList []DeliveryServiceMatch `json:"matchList"`
+ // MaxDNSAnswers sets the maximum number of records which should be returned
+ // by Traffic Router in DNS responses to requests for resolving names for
+ // this Delivery Service.
+ MaxDNSAnswers *int `json:"maxDnsAnswers" db:"max_dns_answers"`
+ // MaxOriginConnections defines the total maximum number of connections
+ // that the highest caching layer ("Mid-tier" in a non-Topology context) is
+ // allowed to have concurrently open to the Delivery Service's Origin. This
+ // may never legally point to a value less than 0.
+ MaxOriginConnections *int `json:"maxOriginConnections" db:"max_origin_connections"`
+ // MaxRequestHeaderBytes is the maximum size (in bytes) of the request
+ // header that is allowed for this Delivery Service.
+ MaxRequestHeaderBytes *int `json:"maxRequestHeaderBytes" db:"max_request_header_bytes"`
+ // MidHeaderRewrite is a "header rewrite rule" used by ATS at the Mid-tier
+ // of caching. This has no effect on Delivery Services that don't use a
+ // Topology.
+ MidHeaderRewrite *string `json:"midHeaderRewrite" db:"mid_header_rewrite"`
+ // MissLat is a latitude to default to for clients of this Delivery Service
+ // when geolocation attempts fail.
+ MissLat *float64 `json:"missLat" db:"miss_lat"`
+ // MissLong is a longitude to default to for clients of this Delivery
+ // Service when geolocation attempts fail.
+ MissLong *float64 `json:"missLong" db:"miss_long"`
+ // MultiSiteOrigin determines whether or not the Delivery Service makes use
+ // of "Multi-Site Origin".
+ MultiSiteOrigin *bool `json:"multiSiteOrigin" db:"multi_site_origin"`
+ // OriginShield is a field that does nothing. It is kept only for legacy
+ // compatibility reasons.
+ OriginShield *string `json:"originShield" db:"origin_shield"`
+ // OrgServerFQDN is the URL - NOT Fully Qualified Domain Name - of the
+ // origin of the Delivery Service's content.
+ OrgServerFQDN *string `json:"orgServerFqdn" db:"org_server_fqdn"`
+ // ProfileDesc is the Description of the Profile used by the Delivery
+ // Service, if any.
+ ProfileDesc *string `json:"profileDescription"`
+ // ProfileID is the integral, unique identifier of the Profile used by the
+ // Delivery Service, if any.
+ ProfileID *int `json:"profileId" db:"profile"`
+ // ProfileName is the Name of the Profile used by the Delivery Service, if
+ // any.
+ ProfileName *string `json:"profileName"`
+ // Protocol defines the protocols by which caching servers may communicate
+ // with clients. The valid values to which it may point are 0 (which implies
+ // that only HTTP may be used), 1 (which implies that only HTTPS may be
+ // used), 2 (which implies that either HTTP or HTTPS may be used), and 3
+ // (which implies that clients using HTTP must be redirected to use HTTPS,
+ // while communications over HTTPS may proceed as normal).
+ Protocol *int `json:"protocol" db:"protocol"`
+ // QStringIgnore sets how query strings in HTTP requests to cache servers
+ // from clients are treated. The only valid values to which it may point are
+ // 0 (which implies that all caching layers will pass query strings in
+ // upstream requests and use them in the cache key), 1 (which implies that
+ // all caching layers will pass query strings in upstream requests, but not
+ // use them in cache keys), and 2 (which implies that the first encountered
+ // caching layer - "Edge-tier" in a non-Topology context - will strip query
+ // strings, effectively preventing them from being passed in upstream
+ // requests, and not use them in the cache key).
+ QStringIgnore *int `json:"qstringIgnore" db:"qstring_ignore"`
+ // RangeRequestHandling defines how HTTP GET requests with a Range header
+ // will be handled by cache servers serving the Delivery Service's content.
+ // The only valid values to which it may point are 0 (which implies that
+ // Range requests will not be cached at all), 1 (which implies that the
+ // background_fetch plugin will be used to service the range request while
+ // caching the whole object), 2 (which implies that the cache_range_requests
+ // plugin will be used to cache ranges as unique objects), and 3 (which
+ // implies that the slice plugin will be used to slice range based requests
+ // into deterministic chunks.)
+ RangeRequestHandling *int `json:"rangeRequestHandling" db:"range_request_handling"`
+ // RangeSliceBlockSize defines the size of range request blocks - or
+ // "slices" - used by the "slice" plugin. This has no effect if
+ // RangeRequestHandling does not point to exactly 3. This may never legally
+ // point to a value less than zero.
+ RangeSliceBlockSize *int `json:"rangeSliceBlockSize" db:"range_slice_block_size"`
+ // Regex Remap is a raw line to be inserted into "regex_remap.config" on the
+ // cache server. Care is necessitated in its use, because the input is in no
+ // way restricted, validated, or limited in scope to the Delivery Service.
+ RegexRemap *string `json:"regexRemap" db:"regex_remap"`
+ // RegionalGeoBlocking defines whether or not whatever Regional Geo Blocking
+ // rules are configured on the Traffic Router serving content for this
+ // Delivery Service will have an effect on the traffic of this Delivery
+ // Service.
+ RegionalGeoBlocking *bool `json:"regionalGeoBlocking" db:"regional_geo_blocking"`
+ // RemapText is raw text to insert in "remap.config" on the cache servers
+ // serving content for this Delivery Service. Care is necessitated in its
+ // use, because the input is in no way restricted, validated, or limited in
+ // scope to the Delivery Service.
+ RemapText *string `json:"remapText" db:"remap_text"`
+ // RoutingName defines the lowest-level DNS label used by the Delivery
+ // Service, e.g. if trafficcontrol.apache.org were a Delivery Service, it
+ // would have a RoutingName of "trafficcontrol".
+ RoutingName *string `json:"routingName" db:"routing_name"`
+ // ServiceCategory defines a category to which a Delivery Service may
+ // belong, which will cause HTTP Responses containing content for the
+ // Delivery Service to have the "X-CDN-SVC" header with a value that is the
+ // XMLID of the Delivery Service.
+ ServiceCategory *string `json:"serviceCategory" db:"service_category"`
+ // Signed is a legacy field. It is allowed to be `true` if and only if
+ // SigningAlgorithm is not nil.
+ Signed bool `json:"signed"`
+ // SigningAlgorithm is the name of the algorithm used to sign CDN URIs for
+ // this Delivery Service's content, or nil if no URI signing is done for the
+ // Delivery Service. This may only point to the values "url_sig" or
+ // "uri_signing".
+ SigningAlgorithm *string `json:"signingAlgorithm" db:"signing_algorithm"`
+ // SSLKeyVersion incremented whenever Traffic Portal generates new SSL keys
+ // for the Delivery Service, effectively making it a "generational" marker.
+ SSLKeyVersion *int `json:"sslKeyVersion" db:"ssl_key_version"`
+ // Tenant is the Tenant to which the Delivery Service belongs.
+ Tenant *string `json:"tenant"`
+ // TenantID is the integral, unique identifier for the Tenant to which the
+ // Delivery Service belongs.
+ TenantID *int `json:"tenantId" db:"tenant_id"`
+ // TLSVersions is the list of explicitly supported TLS versions for cache
+ // servers serving the Delivery Service's content.
+ TLSVersions []string `json:"tlsVersions" db:"tls_versions"`
+ // Topology is the name of the Topology used by the Delivery Service, or nil
+ // if no Topology is used.
+ Topology *string `json:"topology" db:"topology"`
+ // TRResponseHeaders is a set of headers (separated by CRLF pairs as per the
+ // HTTP spec) and their values (separated by a colon as per the HTTP spec)
+ // which will be sent by Traffic Router in HTTP responses to client requests
+ // for this Delivery Service's content. This has no effect on DNS-routed or
+ // un-routed Delivery Service Types.
+ TRResponseHeaders *string `json:"trResponseHeaders"`
+ // TRRequestHeaders is an "array" of HTTP headers which should be logged
+ // from client HTTP requests for this Delivery Service's content by Traffic
+ // Router, separated by newlines. This has no effect on DNS-routed or
+ // un-routed Delivery Service Types.
+ TRRequestHeaders *string `json:"trRequestHeaders"`
+ // Type describes how content is routed and cached for this Delivery Service
+ // as well as what other properties have any meaning.
+ Type *DSType `json:"type"`
+ // TypeID is an integral, unique identifier for the Tenant to which the
+ // Delivery Service belongs.
+ TypeID *int `json:"typeId" db:"type"`
+ // XMLID is a unique identifier that is also the second lowest-level DNS
+ // label used by the Delivery Service. For example, if a Delivery Service's
+ // content may be requested from video.demo1.mycdn.ciab.test, it may be
+ // inferred that the Delivery Service's XMLID is demo1.
+ XMLID *string `json:"xmlId" db:"xml_id"`
}
// DeliveryServiceV4 is a Delivery Service as it appears in version 4 of the
// Traffic Ops API - it always points to the highest minor version in APIv4.
type DeliveryServiceV4 = DeliveryServiceV40
+// These are the TLS Versions known by Apache Traffic Control to exist.
+const (
+ // Deprecated: TLS version 1.0 is known to be insecure.
+ TLSVersion10 = "1.0"
+ // Deprecated: TLS version 1.1 is known to be insecure.
+ TLSVersion11 = "1.1"
+ TLSVersion12 = "1.2"
+ TLSVersion13 = "1.3"
+)
+
+func newerDisallowedMessage(old string, newer []string) string {
Review comment:
This function name should probably include something about TLS -- otherwise, it's unclear that this has anything to do w/ TLS versions without reading it.
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -145,12 +186,17 @@ func CreateV13(w http.ResponseWriter, r *http.Request) {
return
}
- res, status, userErr, sysErr := createV13(w, r, inf, ds)
+ res, alerts, status, userErr, sysErr := createV13(w, r, inf, ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice creation was successful.", []tc.DeliveryServiceNullableV13{*res})
+ if res == nil {
Review comment:
ditto prior comment
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -316,31 +388,89 @@ func createV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 t
&ds.ID)
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
}
if err := EnsureCacheURLParams(tx, *ds.ID, *ds.XMLID, dsV31.CacheURL); err != nil {
- return nil, http.StatusInternalServerError, nil, err
+ return nil, alerts, http.StatusInternalServerError, nil, err
}
oldRes := tc.DeliveryServiceV31(ds.DowngradeToV3())
- return &oldRes, status, userErr, sysErr
+ return &oldRes, alerts, status, userErr, sysErr
+}
+
+// 'ON CONFLICT DO NOTHING' should be unnecessary because all data should be
+// dumped from the table before re-insertion, but it's also harmless because
+// the only conflict that could occur is a fully duplicate row, which is fine
+// since we're intending to create that data anyway. Although it is weird.
+const insertTLSVersionsQuery = `
+INSERT INTO public.deliveryservice_tls_version (deliveryservice, tls_version)
+ SELECT
+ $1 AS deliveryservice,
+ UNNEST($2::text[]) AS tls_version
+ON CONFLICT DO NOTHING
+`
+
+func recreateTLSVersions(versions []string, dsid int, tx *sql.Tx) error {
+ err := tx.QueryRow(`DELETE FROM public.deliveryservice_tls_version WHERE deliveryservice = $1`, dsid).Scan()
Review comment:
Should we use `tx.Exec` here instead since this query doesn't return a row? Then we don't have to have a blank `Scan`.
##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -891,8 +891,9 @@ func CheckTopology(tx *sqlx.Tx, ds tc.DeliveryServiceV4) (int, error, error) {
return http.StatusBadRequest, fmt.Errorf("no such Topology '%s'", *ds.Topology), nil
}
+ // note that this segfaults if the ds doesn't have a properly set CDNID
Review comment:
this comment seems unnecessary -- _any_ nil pointer dereference is going to segfault
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -316,31 +388,89 @@ func createV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 t
&ds.ID)
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
}
if err := EnsureCacheURLParams(tx, *ds.ID, *ds.XMLID, dsV31.CacheURL); err != nil {
- return nil, http.StatusInternalServerError, nil, err
+ return nil, alerts, http.StatusInternalServerError, nil, err
}
oldRes := tc.DeliveryServiceV31(ds.DowngradeToV3())
- return &oldRes, status, userErr, sysErr
+ return &oldRes, alerts, status, userErr, sysErr
+}
+
+// 'ON CONFLICT DO NOTHING' should be unnecessary because all data should be
+// dumped from the table before re-insertion, but it's also harmless because
+// the only conflict that could occur is a fully duplicate row, which is fine
+// since we're intending to create that data anyway. Although it is weird.
+const insertTLSVersionsQuery = `
+INSERT INTO public.deliveryservice_tls_version (deliveryservice, tls_version)
+ SELECT
+ $1 AS deliveryservice,
+ UNNEST($2::text[]) AS tls_version
+ON CONFLICT DO NOTHING
+`
+
+func recreateTLSVersions(versions []string, dsid int, tx *sql.Tx) error {
+ err := tx.QueryRow(`DELETE FROM public.deliveryservice_tls_version WHERE deliveryservice = $1`, dsid).Scan()
+ if err != nil && !errors.Is(err, sql.ErrNoRows) {
+ return fmt.Errorf("cleaning up existing TLS version for DS #%d: %w", dsid, err)
+ }
+
+ if len(versions) < 1 {
+ return nil
+ }
+
+ rows, err := tx.Query(insertTLSVersionsQuery, dsid, pq.Array(versions))
Review comment:
Similar to prior comment, why `Query` over `Exec` since this query doesn't return rows?
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -587,12 +729,17 @@ func UpdateV12(w http.ResponseWriter, r *http.Request) {
}
ds.ID = &id
- res, status, userErr, sysErr := updateV12(w, r, inf, &ds)
+ res, alerts, status, userErr, sysErr := updateV12(w, r, inf, &ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice update was successful.", []tc.DeliveryServiceNullableV12{*res})
+ if res == nil {
Review comment:
ditto earlier comments on the `CreateVXX` functions
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -166,12 +212,17 @@ func CreateV14(w http.ResponseWriter, r *http.Request) {
return
}
- res, status, userErr, sysErr := createV14(w, r, inf, ds)
+ res, alerts, status, userErr, sysErr := createV14(w, r, inf, ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice creation was successful.", []tc.DeliveryServiceNullableV14{*res})
+ if res == nil {
Review comment:
ditto prior comment
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -416,88 +546,100 @@ func createV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 t
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
defer resultRows.Close()
id := 0
- lastUpdated := tc.TimeNoMod{}
+ var lastUpdated time.Time
Review comment:
What does this change fix?
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -611,12 +758,17 @@ func UpdateV13(w http.ResponseWriter, r *http.Request) {
}
ds.ID = &id
- res, status, userErr, sysErr := updateV13(w, r, inf, &ds)
+ res, alerts, status, userErr, sysErr := updateV13(w, r, inf, &ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice update was successful.", []tc.DeliveryServiceNullableV13{*res})
+ if res == nil {
Review comment:
ditto earlier comments on the `CreateVXX` functions
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -110,6 +110,42 @@ func (ds *TODeliveryService) IsTenantAuthorized(user *auth.CurrentUser) (bool, e
return isTenantAuthorized(ds.ReqInfo, &ds.DeliveryServiceV4)
}
+const getTLSVersionsQuery = `
Review comment:
Why not reuse the subquery from the main `SelectDeliveryServicesQuery`? Then we can also use `pq.Array` in `GetDSTLSVersions` rather than the manual array scanning.
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -316,31 +388,89 @@ func createV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 t
&ds.ID)
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
}
if err := EnsureCacheURLParams(tx, *ds.ID, *ds.XMLID, dsV31.CacheURL); err != nil {
- return nil, http.StatusInternalServerError, nil, err
+ return nil, alerts, http.StatusInternalServerError, nil, err
}
oldRes := tc.DeliveryServiceV31(ds.DowngradeToV3())
- return &oldRes, status, userErr, sysErr
+ return &oldRes, alerts, status, userErr, sysErr
+}
+
+// 'ON CONFLICT DO NOTHING' should be unnecessary because all data should be
+// dumped from the table before re-insertion, but it's also harmless because
+// the only conflict that could occur is a fully duplicate row, which is fine
+// since we're intending to create that data anyway. Although it is weird.
+const insertTLSVersionsQuery = `
+INSERT INTO public.deliveryservice_tls_version (deliveryservice, tls_version)
+ SELECT
+ $1 AS deliveryservice,
+ UNNEST($2::text[]) AS tls_version
+ON CONFLICT DO NOTHING
+`
+
+func recreateTLSVersions(versions []string, dsid int, tx *sql.Tx) error {
+ err := tx.QueryRow(`DELETE FROM public.deliveryservice_tls_version WHERE deliveryservice = $1`, dsid).Scan()
+ if err != nil && !errors.Is(err, sql.ErrNoRows) {
+ return fmt.Errorf("cleaning up existing TLS version for DS #%d: %w", dsid, err)
+ }
+
+ if len(versions) < 1 {
+ return nil
+ }
+
+ rows, err := tx.Query(insertTLSVersionsQuery, dsid, pq.Array(versions))
+ if err != nil {
+ return fmt.Errorf("inserting new TLS versions: %w", err)
+ }
+ err = rows.Close()
+ if err != nil {
+ log.Errorln("closing TLS versions insert rows: ", err.Error())
+ }
+ if err = rows.Err(); err != nil {
+ log.Errorln("an error occurred at some point in the TLS insertion query: ", err.Error())
+ }
+
+ return nil
+}
+
+// generates warning-level alerts (if any are necessary) for a Delivery Service
+// based on its TLS Versions. This will panic if handed a nil transaction, or
+// if the given Delivery Service has a nil TypeID.
+func generateTLSVersionWarnings(ds tc.DeliveryServiceV4, tx *sql.Tx) tc.Alerts {
+ alerts := tc.TLSVersionsAlerts(ds.TLSVersions)
+
+ var httpType int
+ if err := tx.QueryRow(`SELECT id FROM public.type WHERE name = 'HTTP'`).Scan(&httpType); err != nil {
+ log.Errorln("Failed to get 'HTTP' type: ", err.Error())
+ } else if httpType == *ds.TypeID {
+ if len(ds.TLSVersions) >= 1 {
+ alerts.AddNewAlert(tc.WarnLevel, "tlsVersions has no effect on 'HTTP' Delivery Services")
Review comment:
I think you meant to check the DS protocol here rather than the DS type, e.g. `protocol == 0`
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -316,31 +388,89 @@ func createV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 t
&ds.ID)
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
}
if err := EnsureCacheURLParams(tx, *ds.ID, *ds.XMLID, dsV31.CacheURL); err != nil {
- return nil, http.StatusInternalServerError, nil, err
+ return nil, alerts, http.StatusInternalServerError, nil, err
}
oldRes := tc.DeliveryServiceV31(ds.DowngradeToV3())
- return &oldRes, status, userErr, sysErr
+ return &oldRes, alerts, status, userErr, sysErr
+}
+
+// 'ON CONFLICT DO NOTHING' should be unnecessary because all data should be
+// dumped from the table before re-insertion, but it's also harmless because
+// the only conflict that could occur is a fully duplicate row, which is fine
+// since we're intending to create that data anyway. Although it is weird.
+const insertTLSVersionsQuery = `
+INSERT INTO public.deliveryservice_tls_version (deliveryservice, tls_version)
+ SELECT
+ $1 AS deliveryservice,
+ UNNEST($2::text[]) AS tls_version
+ON CONFLICT DO NOTHING
+`
+
+func recreateTLSVersions(versions []string, dsid int, tx *sql.Tx) error {
+ err := tx.QueryRow(`DELETE FROM public.deliveryservice_tls_version WHERE deliveryservice = $1`, dsid).Scan()
+ if err != nil && !errors.Is(err, sql.ErrNoRows) {
+ return fmt.Errorf("cleaning up existing TLS version for DS #%d: %w", dsid, err)
+ }
+
+ if len(versions) < 1 {
+ return nil
+ }
+
+ rows, err := tx.Query(insertTLSVersionsQuery, dsid, pq.Array(versions))
+ if err != nil {
+ return fmt.Errorf("inserting new TLS versions: %w", err)
+ }
+ err = rows.Close()
+ if err != nil {
+ log.Errorln("closing TLS versions insert rows: ", err.Error())
+ }
+ if err = rows.Err(); err != nil {
+ log.Errorln("an error occurred at some point in the TLS insertion query: ", err.Error())
+ }
+
+ return nil
+}
+
+// generates warning-level alerts (if any are necessary) for a Delivery Service
+// based on its TLS Versions. This will panic if handed a nil transaction, or
Review comment:
> This will panic...
ditto earlier comment about this
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -635,12 +787,17 @@ func UpdateV14(w http.ResponseWriter, r *http.Request) {
}
ds.ID = &id
- res, status, userErr, sysErr := updateV14(w, r, inf, &ds)
+ res, alerts, status, userErr, sysErr := updateV14(w, r, inf, &ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice update was successful.", []tc.DeliveryServiceNullableV14{*res})
+ if res == nil {
Review comment:
ditto earlier comments on the `CreateVXX` functions
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -875,70 +1052,82 @@ WHERE
&dsV31.MaxRequestHeaderBytes,
); err != nil {
if err == sql.ErrNoRows {
- return nil, http.StatusNotFound, fmt.Errorf("delivery service ID %d not found", *dsV31.ID), nil
+ return nil, tc.Alerts{}, http.StatusNotFound, fmt.Errorf("delivery service ID %d not found", *dsV31.ID), nil
}
- return nil, http.StatusInternalServerError, nil, fmt.Errorf("querying delivery service ID %d: %s", *dsV31.ID, err.Error())
+ return nil, tc.Alerts{}, http.StatusInternalServerError, nil, fmt.Errorf("querying delivery service ID %d: %s", *dsV31.ID, err.Error())
}
- res, status, userErr, sysErr := updateV31(w, r, inf, &dsV31)
+ res, alerts, status, userErr, sysErr := updateV31(w, r, inf, &dsV31)
if res != nil {
- return &res.DeliveryServiceV30, status, userErr, sysErr
+ return &res.DeliveryServiceV30, alerts, status, userErr, sysErr
}
- return nil, status, userErr, sysErr
+ return nil, alerts, status, userErr, sysErr
}
-func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 *tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, int, error, error) {
+func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 *tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, tc.Alerts, int, error, error) {
+ var alerts tc.Alerts
+
dsNull := tc.DeliveryServiceNullableV30(*dsV31)
ds := dsNull.UpgradeToV4()
- dsV40 := tc.DeliveryServiceV40(ds)
+ dsV40 := ds
+ if dsV40.ID == nil {
Review comment:
this nil check is unnecessary as all the `UpdateVXX` functions explicitly set the DS ID to a non-nil value before calling the `updateVXX` functions.
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -659,12 +816,17 @@ func UpdateV15(w http.ResponseWriter, r *http.Request) {
}
ds.ID = &id
- res, status, userErr, sysErr := updateV15(w, r, inf, &ds)
+ res, alerts, status, userErr, sysErr := updateV15(w, r, inf, &ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice update was successful.", []tc.DeliveryServiceNullableV15{*res})
+ if res == nil {
Review comment:
ditto earlier comments on the `CreateVXX` functions
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -706,12 +873,17 @@ func UpdateV31(w http.ResponseWriter, r *http.Request) {
return
}
ds.ID = &id
- res, status, userErr, sysErr := updateV31(w, r, inf, &ds)
+ res, alerts, status, userErr, sysErr := updateV31(w, r, inf, &ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice update was successful.", []tc.DeliveryServiceV31{*res})
+ if res == nil {
Review comment:
ditto earlier comments on the `CreateVXX` functions
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -683,12 +845,17 @@ func UpdateV30(w http.ResponseWriter, r *http.Request) {
}
ds.ID = &id
- res, status, userErr, sysErr := updateV30(w, r, inf, &ds)
+ res, alerts, status, userErr, sysErr := updateV30(w, r, inf, &ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice update was successful.", []tc.DeliveryServiceV30{*res})
+ if res == nil {
Review comment:
ditto earlier comments on the `CreateVXX` functions
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -1070,87 +1259,103 @@ func updateV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 *
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
defer resultRows.Close()
if !resultRows.Next() {
- return nil, http.StatusNotFound, errors.New("no delivery service found with this id"), nil
+ return nil, alerts, http.StatusNotFound, errors.New("no delivery service found with this id"), nil
}
- lastUpdated := tc.TimeNoMod{}
+ var lastUpdated time.Time
Review comment:
ditto earlier comment about this
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -729,15 +901,20 @@ func UpdateV40(w http.ResponseWriter, r *http.Request) {
return
}
ds.ID = &id
- res, status, userErr, sysErr := updateV40(w, r, inf, &ds)
+ res, alerts, status, userErr, sysErr := updateV40(w, r, inf, &ds)
if userErr != nil || sysErr != nil {
api.HandleErr(w, r, inf.Tx.Tx, status, userErr, sysErr)
return
}
- api.WriteRespAlertObj(w, r, tc.SuccessLevel, "Deliveryservice update was successful.", []tc.DeliveryServiceV40{*res})
+ if res == nil {
Review comment:
ditto earlier comments on the `CreateVXX` functions
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r652081126
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
Review comment:
I was hoping #4996 would get added to the 6.x milestone, but it isn't so far. But still, I'd like to keep the timestamps as RFC3339, even if that would mean duplication.
Unfortunately, I think mostly duplicated structs are just a fact of life with a versioned API in Go; even with the nesting of previous versions we have compatibility problems such that clients of the library are forbidden from constructing the objects with direct field initialization like
```go
f := FooVX{
Property: "value,
}
```
(which has broken in the past and will break in the future for as long as we use nesting, but I don't think that's widely known and so it's probably done in thousands of places outside of `lib/go-tc`)
Using nesting saves code changes for non-breaking changes, but makes it harder to make breaking changes.
If you want me to take out all the breaking structure changes I absolutely will, to be clear, since they aren't really necessary to accomplish the PR's goal. But then I intend to open a PR to re-introduce them, and I just hoped they could be small enough to fit into this, since I was touching the struct anyway.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r647783806
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active bool `json:"active" db:"active"`
Review comment:
Because those fields are mandatory, and then you don't have to add three lines to check for `nil` everywhere they're used
It probably doesn't belong in this PR though.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r648740923
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active bool `json:"active" db:"active"`
Review comment:
in a handler in TO you can, but then if it's a PUT or POST you need to check again after reading from the database. And utility functions that don't know the contexts in which they might be used need to check, and client methods need to check before access, and code using the clients needs to check before access on the responses, and it needs to get checked ad nauseum in testing code. It's a giant pain that Go won't let you just say "if it's missing this property, throw an error". That would solve so many problems.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r647658309
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active bool `json:"active" db:"active"`
Review comment:
Why are you making fields non-nullable? How else will we be able to distinguish between values that are empty vs null?
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -1070,87 +1189,88 @@ func updateV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 *
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
defer resultRows.Close()
if !resultRows.Next() {
- return nil, http.StatusNotFound, errors.New("no delivery service found with this id"), nil
+ return nil, alerts, http.StatusNotFound, errors.New("no delivery service found with this id"), nil
}
- lastUpdated := tc.TimeNoMod{}
+ var lastUpdated time.Time
if err := resultRows.Scan(&lastUpdated); err != nil {
- return nil, http.StatusInternalServerError, nil, errors.New("scan updating delivery service: " + err.Error())
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("scan updating delivery service: " + err.Error())
}
if resultRows.Next() {
- xmlID := ""
- if ds.XMLID != nil {
- xmlID = *ds.XMLID
- }
- return nil, http.StatusInternalServerError, nil, errors.New("updating delivery service " + xmlID + ": " + "this update affected too many rows: > 1")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("updating delivery service " + ds.XMLID + ": " + "this update affected too many rows: > 1")
}
if ds.ID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing id after update")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("missing id after update")
}
- if ds.XMLID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing xml_id after update")
- }
- if ds.TypeID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing type after update")
- }
- if ds.RoutingName == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing routing name after update")
+
+ if inf.Version != nil && inf.Version.Major >= 4 {
Review comment:
This kind of goes against the underlying theme of the DS update/create functions, which is that the _latest_ function in the chain (in this case `createV40`/`updateV40`) doesn't have any version-specific logic like this. Rather, the `createV31`/`updateV31` functions are responsible for _upgrading_ the 3.1 request into a 4.0 request, passing it to the 4.0 handler for processing, then downgrading the result back to a 3.1 response.
For update requests, that generally means reading the 4.0 fields from the DB to populate the 4.0 struct before passing it to `updateV40`.
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -416,88 +503,93 @@ func createV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 t
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
defer resultRows.Close()
id := 0
- lastUpdated := tc.TimeNoMod{}
+ var lastUpdated time.Time
if !resultRows.Next() {
- return nil, http.StatusInternalServerError, nil, errors.New("no deliveryservice request inserted, no id was returned")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("no deliveryservice request inserted, no id was returned")
}
if err := resultRows.Scan(&id, &lastUpdated); err != nil {
- return nil, http.StatusInternalServerError, nil, errors.New("could not scan id from insert: " + err.Error())
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("could not scan id from insert: " + err.Error())
}
if resultRows.Next() {
- return nil, http.StatusInternalServerError, nil, errors.New("too many ids returned from deliveryservice request insert")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("too many ids returned from deliveryservice request insert")
}
ds.ID = &id
if ds.ID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing id after insert")
- }
- if ds.XMLID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing xml_id after insert")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("missing id after insert")
}
- if ds.TypeID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing type after insert")
- }
- dsType, err := getTypeFromID(*ds.TypeID, tx)
+ dsType, err := getTypeFromID(ds.TypeID, tx)
if err != nil {
- return nil, http.StatusInternalServerError, nil, errors.New("getting delivery service type: " + err.Error())
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("getting delivery service type: " + err.Error())
}
ds.Type = &dsType
- if err := createDefaultRegex(tx, *ds.ID, *ds.XMLID); err != nil {
- return nil, http.StatusInternalServerError, nil, errors.New("creating default regex: " + err.Error())
+ // Don't touch TLSVersions if using old API versions
Review comment:
This kind of goes against the underlying theme of the DS update/create functions, which is that the _latest_ function in the chain (in this case `createV40`/`updateV40`) doesn't have any version-specific logic like this. Rather, the `createV31`/`updateV31` functions are responsible for _upgrading_ the 3.1 request into a 4.0 request, passing it to the 4.0 handler for processing, then downgrading the result back to a 3.1 response.
Since the `createV31` function doesn't need to do anything special other than embedding the 3.1 request fields into a new 4.0 struct (which it currently does), this version-specific logic can just be deleted.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651790889
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -110,6 +110,42 @@ func (ds *TODeliveryService) IsTenantAuthorized(user *auth.CurrentUser) (bool, e
return isTenantAuthorized(ds.ReqInfo, &ds.DeliveryServiceV4)
}
+const getTLSVersionsQuery = `
+SELECT tls_version
+FROM public.deliveryservice_tls_version
+WHERE deliveryservice = $1
+ORDER BY tls_version
+`
+
+// GetDSTLSVersions retrieves the TLS versions explicitly supported by a
+// Delivery Service identified by dsID. This will panic if handed a nil
Review comment:
I don't know that's obvious without reading the function. It might be obvious to you, being familiar with how the TO codebase most often has functions that panic when given nil transactions, but that doesn't mean it's obvious to everyone. You could reasonably think that it would return an error instead of crashing the whole thread it's running on.
I don't think it hurts anything to make it clear in the GoDoc.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r661789912
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -496,7 +556,6 @@ func createV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 t
return nil, http.StatusInternalServerError, nil, errors.New("error writing to audit log: " + err.Error())
}
- dsV40 = tc.DeliveryServiceV40(ds)
Review comment:
I think we actually need this line still, because `ds` has been mutated throughout this function, and we want to return those mutations in the response.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651884492
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/safe.go
##########
@@ -94,17 +94,39 @@ func UpdateSafe(w http.ResponseWriter, r *http.Request) {
ds := dses[0]
alertMsg := "Delivery Service safe update successful."
- if inf.Version != nil && inf.Version.Major == 1 && inf.Version.Minor < 5 {
- switch inf.Version.Minor {
+ if inf.Version == nil {
+ log.Warnln("API version found to be null in DS safe update")
+ api.WriteRespAlertObj(w, r, tc.SuccessLevel, alertMsg, dses)
Review comment:
Oh I see, I read `log.Warnln` and assumed this was not a success.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r652104969
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
Review comment:
> Unfortunately, I think mostly duplicated structs are just a fact of life with a versioned API in Go
I refuse to accept this statement. We need to be aggressively removing old API versions so that we can collapse the nested structs back into one struct over time.
Can you make the RFC3339 change without de-embedding the struct? Sure, that would change it for prior API versions, but that seems like a worthwhile breakage (if it even does break anything).
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651795083
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -416,88 +546,100 @@ func createV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 t
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
defer resultRows.Close()
id := 0
- lastUpdated := tc.TimeNoMod{}
+ var lastUpdated time.Time
Review comment:
Specifically, the change on this line fixes typing. A `tc.TimeNoMod` value is not assignable to `time.Time` - although you could extract the nested time instead, but I think this is clearer.
As to why/whether that's necessary, we're discussing that on your second comment in this review above.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651775848
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active *bool `json:"active" db:"active"`
+ // AnonymousBlockingEnabled sets whether or not anonymized IP addresses
+ // (e.g. Tor exit nodes) should be restricted from accessing the Delivery
+ // Service's content.
+ AnonymousBlockingEnabled *bool `json:"anonymousBlockingEnabled" db:"anonymous_blocking_enabled"`
+ // CCRDNSTTL sets the Time-to-Live - in seconds - for DNS responses for this
+ // Delivery Service from Traffic Router.
+ CCRDNSTTL *int `json:"ccrDnsTtl" db:"ccr_dns_ttl"`
+ // CDNID is the integral, unique identifier for the CDN to which the
+ // Delivery Service belongs.
+ CDNID *int `json:"cdnId" db:"cdn_id"`
+ // CDNName is the name of the CDN to which the Delivery Service belongs.
+ CDNName *string `json:"cdnName"`
+ // CheckPath is a path which may be requested of the Delivery Service's
+ // origin to ensure it's working properly.
+ CheckPath *string `json:"checkPath" db:"check_path"`
+ // ConsistentHashQueryParams is a list of al of the query string parameters
+ // which ought to be considered by Traffic Router in client request URIs for
+ // HTTP-routed Delivery Services in the hashing process.
+ ConsistentHashQueryParams []string `json:"consistentHashQueryParams"`
+ // ConsistentHashRegex is used by Traffic Router to extract meaningful parts
+ // of a client's request URI for HTTP-routed Delivery Services before
+ // hashing the request to find a cache server to which to direct the client.
+ ConsistentHashRegex *string `json:"consistentHashRegex"`
+ // DeepCachingType may only legally point to 'ALWAYS' or 'NEVER', which
+ // define whether "deep caching" may or may not be used for this Delivery
+ // Service, respectively.
+ DeepCachingType *DeepCachingType `json:"deepCachingType" db:"deep_caching_type"`
+ // DisplayName is a human-friendly name that might be used in some UIs
+ // somewhere.
+ DisplayName *string `json:"displayName" db:"display_name"`
+ // DNSBypassCNAME is a fully qualified domain name to be used in a CNAME
+ // record presented to clients in bypass scenarios.
+ DNSBypassCNAME *string `json:"dnsBypassCname" db:"dns_bypass_cname"`
+ // DNSBypassIP is an IPv4 address to be used in an A record presented to
+ // clients in bypass scenarios.
+ DNSBypassIP *string `json:"dnsBypassIp" db:"dns_bypass_ip"`
+ // DNSBypassIP6 is an IPv6 address to be used in an AAAA record presented to
+ // clients in bypass scenarios.
+ DNSBypassIP6 *string `json:"dnsBypassIp6" db:"dns_bypass_ip6"`
+ // DNSBypassTTL sets the Time-to-Live - in seconds - of DNS responses from
+ // the Traffic Router that contain records for bypass destinations.
+ DNSBypassTTL *int `json:"dnsBypassTtl" db:"dns_bypass_ttl"`
+ // DSCP sets the Differentiated Services Code Point for IP packets
+ // transferred between clients, origins, and cache servers when obtaining
+ // and serving content for this Delivery Service.
+ // See Also: https://en.wikipedia.org/wiki/Differentiated_services
+ DSCP *int `json:"dscp" db:"dscp"`
+ // EcsEnabled describes whether or not the Traffic Router's EDNS0 Client
+ // Subnet extensions should be enabled when serving DNS responses for this
+ // Delivery Service. Even if this is true, the Traffic Router may still
+ // have the extensions unilaterally disabled in its own configuration.
+ EcsEnabled bool `json:"ecsEnabled" db:"ecs_enabled"`
+ // EdgeHeaderRewrite is a "header rewrite rule" used by ATS at the Edge-tier
+ // of caching. This has no effect on Delivery Services that don't use a
+ // Topology.
+ EdgeHeaderRewrite *string `json:"edgeHeaderRewrite" db:"edge_header_rewrite"`
+ // ExampleURLs is a list of all of the URLs from which content may be
+ // requested from the Delivery Service.
+ ExampleURLs []string `json:"exampleURLs"`
+ // FirstHeaderRewrite is a "header rewrite rule" used by ATS at the first
+ // caching layer encountered in the Delivery Service's Topology, or nil if
+ // there is no such rule. This has no effect on Delivery Services that don't
+ // employ Topologies.
+ FirstHeaderRewrite *string `json:"firstHeaderRewrite" db:"first_header_rewrite"`
+ // FQPacingRate sets the maximum bytes per second a cache server will deliver
+ // on any single TCP connection for this Delivery Service. This may never
+ // legally point to a value less than zero.
+ FQPacingRate *int `json:"fqPacingRate" db:"fq_pacing_rate"`
+ // GeoLimit defines whether or not access to a Delivery Service's content
+ // should be limited based on the requesting client's geographic location.
+ // Despite that this is a pointer to an arbitrary integer, the only valid
+ // values are 0 (which indicates that content should not be limited
+ // geographically), 1 (which indicates that content should only be served to
+ // clients whose IP addresses can be found within a Coverage Zone File), and
+ // 2 (which indicates that content should be served to clients whose IP
+ // addresses can be found within a Coverage Zone File OR are allowed access
+ // according to the "array" in GeoLimitCountries).
+ GeoLimit *int `json:"geoLimit" db:"geo_limit"`
+ // GeoLimitCountries is an "array" of "country codes" that itemizes the
+ // countries within which the Delivery Service's content ought to be made
+ // available. This has no effect if GeoLimit is not a pointer to exactly the
+ // value 2.
+ GeoLimitCountries *string `json:"geoLimitCountries" db:"geo_limit_countries"`
+ // GeoLimitRedirectURL is a URL to which clients will be redirected if their
+ // access to the Delivery Service's content is blocked by GeoLimit rules.
+ GeoLimitRedirectURL *string `json:"geoLimitRedirectURL" db:"geolimit_redirect_url"`
+ // GeoProvider names the type of database to be used for providing IP
+ // address-to-geographic-location mapping for this Delivery Service. The
+ // only valid values to which it may point are 0 (which indicates the use of
+ // a MaxMind GeoIP2 database) and 1 (which indicates the use of a Neustar
+ // GeoPoint IP address database).
+ GeoProvider *int `json:"geoProvider" db:"geo_provider"`
+ // GlobalMaxMBPS defines a maximum number of MegaBytes Per Second which may
+ // be served for the Delivery Service before redirecting clients to bypass
+ // locations.
+ GlobalMaxMBPS *int `json:"globalMaxMbps" db:"global_max_mbps"`
+ // GlobalMaxTPS defines a maximum number of Transactions Per Second which
+ // may be served for the Delivery Service before redirecting clients to
+ // bypass locations.
+ GlobalMaxTPS *int `json:"globalMaxTps" db:"global_max_tps"`
+ // HTTPBypassFQDN is a network location to which clients will be redirected
+ // in bypass scenarios using HTTP "Location" headers and appropriate
+ // redirection response codes.
+ HTTPBypassFQDN *string `json:"httpBypassFqdn" db:"http_bypass_fqdn"`
+ // ID is an integral, unique identifier for the Delivery Service.
+ ID *int `json:"id" db:"id"`
+ // InfoURL is a URL to which operators or clients may be directed to obtain
+ // further information about a Delivery Service.
+ InfoURL *string `json:"infoUrl" db:"info_url"`
+ // InitialDispersion sets the number of cache servers within the first
+ // caching layer ("Edge-tier" in a non-Topology context) across which
+ // content will be dispersed per Cache Group.
+ InitialDispersion *int `json:"initialDispersion" db:"initial_dispersion"`
+ // InnerHeaderRewrite is a "header rewrite rule" used by ATS at all caching
+ // layers encountered in the Delivery Service's Topology except the first
+ // and last, or nil if there is no such rule. This has no effect on Delivery
+ // Services that don't employ Topologies.
+ InnerHeaderRewrite *string `json:"innerHeaderRewrite" db:"inner_header_rewrite"`
+ // IPV6RoutingEnabled controls whether or not routing over IPv6 should be
+ // done for this Delivery Service.
+ IPV6RoutingEnabled *bool `json:"ipv6RoutingEnabled" db:"ipv6_routing_enabled"`
+ // LastHeaderRewrite is a "header rewrite rule" used by ATS at the first
+ // caching layer encountered in the Delivery Service's Topology, or nil if
+ // there is no such rule. This has no effect on Delivery Services that don't
+ // employ Topologies.
+ LastHeaderRewrite *string `json:"lastHeaderRewrite" db:"last_header_rewrite"`
+ // LastUpdated is the time and date at which the Delivery Service was last
+ // updated.
+ LastUpdated time.Time `json:"lastUpdated" db:"last_updated"`
+ // LogsEnabled controls nothing. It is kept only for legacy compatibility.
+ LogsEnabled *bool `json:"logsEnabled" db:"logs_enabled"`
+ // LongDesc is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc *string `json:"longDesc" db:"long_desc"`
+ // LongDesc1 is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc1 *string `json:"longDesc1" db:"long_desc_1"`
+ // LongDesc2 is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc2 *string `json:"longDesc2" db:"long_desc_2"`
+ // MatchList is a list of Regular Expressions used for routing the Delivery
+ // Service. Order matters, and the array is not allowed to be sparse.
+ MatchList []DeliveryServiceMatch `json:"matchList"`
+ // MaxDNSAnswers sets the maximum number of records which should be returned
+ // by Traffic Router in DNS responses to requests for resolving names for
+ // this Delivery Service.
+ MaxDNSAnswers *int `json:"maxDnsAnswers" db:"max_dns_answers"`
+ // MaxOriginConnections defines the total maximum number of connections
+ // that the highest caching layer ("Mid-tier" in a non-Topology context) is
+ // allowed to have concurrently open to the Delivery Service's Origin. This
+ // may never legally point to a value less than 0.
+ MaxOriginConnections *int `json:"maxOriginConnections" db:"max_origin_connections"`
+ // MaxRequestHeaderBytes is the maximum size (in bytes) of the request
+ // header that is allowed for this Delivery Service.
+ MaxRequestHeaderBytes *int `json:"maxRequestHeaderBytes" db:"max_request_header_bytes"`
+ // MidHeaderRewrite is a "header rewrite rule" used by ATS at the Mid-tier
+ // of caching. This has no effect on Delivery Services that don't use a
+ // Topology.
+ MidHeaderRewrite *string `json:"midHeaderRewrite" db:"mid_header_rewrite"`
+ // MissLat is a latitude to default to for clients of this Delivery Service
+ // when geolocation attempts fail.
+ MissLat *float64 `json:"missLat" db:"miss_lat"`
+ // MissLong is a longitude to default to for clients of this Delivery
+ // Service when geolocation attempts fail.
+ MissLong *float64 `json:"missLong" db:"miss_long"`
+ // MultiSiteOrigin determines whether or not the Delivery Service makes use
+ // of "Multi-Site Origin".
+ MultiSiteOrigin *bool `json:"multiSiteOrigin" db:"multi_site_origin"`
+ // OriginShield is a field that does nothing. It is kept only for legacy
+ // compatibility reasons.
+ OriginShield *string `json:"originShield" db:"origin_shield"`
+ // OrgServerFQDN is the URL - NOT Fully Qualified Domain Name - of the
+ // origin of the Delivery Service's content.
+ OrgServerFQDN *string `json:"orgServerFqdn" db:"org_server_fqdn"`
+ // ProfileDesc is the Description of the Profile used by the Delivery
+ // Service, if any.
+ ProfileDesc *string `json:"profileDescription"`
+ // ProfileID is the integral, unique identifier of the Profile used by the
+ // Delivery Service, if any.
+ ProfileID *int `json:"profileId" db:"profile"`
+ // ProfileName is the Name of the Profile used by the Delivery Service, if
+ // any.
+ ProfileName *string `json:"profileName"`
+ // Protocol defines the protocols by which caching servers may communicate
+ // with clients. The valid values to which it may point are 0 (which implies
+ // that only HTTP may be used), 1 (which implies that only HTTPS may be
+ // used), 2 (which implies that either HTTP or HTTPS may be used), and 3
+ // (which implies that clients using HTTP must be redirected to use HTTPS,
+ // while communications over HTTPS may proceed as normal).
+ Protocol *int `json:"protocol" db:"protocol"`
+ // QStringIgnore sets how query strings in HTTP requests to cache servers
+ // from clients are treated. The only valid values to which it may point are
+ // 0 (which implies that all caching layers will pass query strings in
+ // upstream requests and use them in the cache key), 1 (which implies that
+ // all caching layers will pass query strings in upstream requests, but not
+ // use them in cache keys), and 2 (which implies that the first encountered
+ // caching layer - "Edge-tier" in a non-Topology context - will strip query
+ // strings, effectively preventing them from being passed in upstream
+ // requests, and not use them in the cache key).
+ QStringIgnore *int `json:"qstringIgnore" db:"qstring_ignore"`
+ // RangeRequestHandling defines how HTTP GET requests with a Range header
+ // will be handled by cache servers serving the Delivery Service's content.
+ // The only valid values to which it may point are 0 (which implies that
+ // Range requests will not be cached at all), 1 (which implies that the
+ // background_fetch plugin will be used to service the range request while
+ // caching the whole object), 2 (which implies that the cache_range_requests
+ // plugin will be used to cache ranges as unique objects), and 3 (which
+ // implies that the slice plugin will be used to slice range based requests
+ // into deterministic chunks.)
+ RangeRequestHandling *int `json:"rangeRequestHandling" db:"range_request_handling"`
+ // RangeSliceBlockSize defines the size of range request blocks - or
+ // "slices" - used by the "slice" plugin. This has no effect if
+ // RangeRequestHandling does not point to exactly 3. This may never legally
+ // point to a value less than zero.
+ RangeSliceBlockSize *int `json:"rangeSliceBlockSize" db:"range_slice_block_size"`
+ // Regex Remap is a raw line to be inserted into "regex_remap.config" on the
+ // cache server. Care is necessitated in its use, because the input is in no
+ // way restricted, validated, or limited in scope to the Delivery Service.
+ RegexRemap *string `json:"regexRemap" db:"regex_remap"`
+ // RegionalGeoBlocking defines whether or not whatever Regional Geo Blocking
+ // rules are configured on the Traffic Router serving content for this
+ // Delivery Service will have an effect on the traffic of this Delivery
+ // Service.
+ RegionalGeoBlocking *bool `json:"regionalGeoBlocking" db:"regional_geo_blocking"`
+ // RemapText is raw text to insert in "remap.config" on the cache servers
+ // serving content for this Delivery Service. Care is necessitated in its
+ // use, because the input is in no way restricted, validated, or limited in
+ // scope to the Delivery Service.
+ RemapText *string `json:"remapText" db:"remap_text"`
+ // RoutingName defines the lowest-level DNS label used by the Delivery
+ // Service, e.g. if trafficcontrol.apache.org were a Delivery Service, it
+ // would have a RoutingName of "trafficcontrol".
+ RoutingName *string `json:"routingName" db:"routing_name"`
+ // ServiceCategory defines a category to which a Delivery Service may
+ // belong, which will cause HTTP Responses containing content for the
+ // Delivery Service to have the "X-CDN-SVC" header with a value that is the
+ // XMLID of the Delivery Service.
+ ServiceCategory *string `json:"serviceCategory" db:"service_category"`
+ // Signed is a legacy field. It is allowed to be `true` if and only if
+ // SigningAlgorithm is not nil.
+ Signed bool `json:"signed"`
+ // SigningAlgorithm is the name of the algorithm used to sign CDN URIs for
+ // this Delivery Service's content, or nil if no URI signing is done for the
+ // Delivery Service. This may only point to the values "url_sig" or
+ // "uri_signing".
+ SigningAlgorithm *string `json:"signingAlgorithm" db:"signing_algorithm"`
+ // SSLKeyVersion incremented whenever Traffic Portal generates new SSL keys
+ // for the Delivery Service, effectively making it a "generational" marker.
+ SSLKeyVersion *int `json:"sslKeyVersion" db:"ssl_key_version"`
+ // Tenant is the Tenant to which the Delivery Service belongs.
+ Tenant *string `json:"tenant"`
+ // TenantID is the integral, unique identifier for the Tenant to which the
+ // Delivery Service belongs.
+ TenantID *int `json:"tenantId" db:"tenant_id"`
+ // TLSVersions is the list of explicitly supported TLS versions for cache
+ // servers serving the Delivery Service's content.
+ TLSVersions []string `json:"tlsVersions" db:"tls_versions"`
+ // Topology is the name of the Topology used by the Delivery Service, or nil
+ // if no Topology is used.
+ Topology *string `json:"topology" db:"topology"`
+ // TRResponseHeaders is a set of headers (separated by CRLF pairs as per the
+ // HTTP spec) and their values (separated by a colon as per the HTTP spec)
+ // which will be sent by Traffic Router in HTTP responses to client requests
+ // for this Delivery Service's content. This has no effect on DNS-routed or
+ // un-routed Delivery Service Types.
+ TRResponseHeaders *string `json:"trResponseHeaders"`
+ // TRRequestHeaders is an "array" of HTTP headers which should be logged
+ // from client HTTP requests for this Delivery Service's content by Traffic
+ // Router, separated by newlines. This has no effect on DNS-routed or
+ // un-routed Delivery Service Types.
+ TRRequestHeaders *string `json:"trRequestHeaders"`
+ // Type describes how content is routed and cached for this Delivery Service
+ // as well as what other properties have any meaning.
+ Type *DSType `json:"type"`
+ // TypeID is an integral, unique identifier for the Tenant to which the
+ // Delivery Service belongs.
+ TypeID *int `json:"typeId" db:"type"`
+ // XMLID is a unique identifier that is also the second lowest-level DNS
+ // label used by the Delivery Service. For example, if a Delivery Service's
+ // content may be requested from video.demo1.mycdn.ciab.test, it may be
+ // inferred that the Delivery Service's XMLID is demo1.
+ XMLID *string `json:"xmlId" db:"xml_id"`
}
// DeliveryServiceV4 is a Delivery Service as it appears in version 4 of the
// Traffic Ops API - it always points to the highest minor version in APIv4.
type DeliveryServiceV4 = DeliveryServiceV40
+// These are the TLS Versions known by Apache Traffic Control to exist.
+const (
+ // Deprecated: TLS version 1.0 is known to be insecure.
+ TLSVersion10 = "1.0"
+ // Deprecated: TLS version 1.1 is known to be insecure.
+ TLSVersion11 = "1.1"
+ TLSVersion12 = "1.2"
+ TLSVersion13 = "1.3"
+)
+
+func newerDisallowedMessage(old string, newer []string) string {
+ l := len(newer)
+ if l < 1 {
+ return ""
+ }
+
+ var msg strings.Builder
+ msg.WriteString("old TLS version ")
+ msg.WriteString(old)
+ msg.WriteString(" is allowed, but newer version")
+ if l > 1 {
+ msg.WriteRune('s')
+ }
+ msg.WriteRune(' ')
+ msg.WriteString(newer[0])
+ if l > 1 {
+ msg.WriteString(", ")
+ if l > 2 {
+ msg.WriteString(newer[1])
+ msg.WriteString(", and ")
+ msg.WriteString(newer[2])
+ } else {
+ msg.WriteString("and ")
+ msg.WriteString(newer[1])
+ }
+ msg.WriteString(" are ")
+ } else {
+ msg.WriteString(" is ")
+ }
+ msg.WriteString("disallowed; this configuration may be insecure")
+
+ return msg.String()
+}
+
+// TLSVersionsAlerts generates warning-level alerts for the given TLS versions
+// array. It will warn if newer versions are disallowed while older, less
+// secure versions are allowed, or if there are unrecognized versions present.
+//
+// This does NOT verify that the passed TLS versions are _valid_, it ONLY
+// creates warnings based on conditions that are possibly detrimental to CDN
+// operation, but can, in fact, work.
+func TLSVersionsAlerts(vers []string) Alerts {
Review comment:
Yeah, idk. It's not using any information specific to TO, like a database connection or something, so I stuck it in there. I'm not convinced it's the right place, it just seemed like it _could_ go there. Although maybe the notion of Alerts being associated with a set of TLS Versions is inherently TO-centric. Do you think it makes more sense to put this in the `github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/deliveryservice` package?
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r647786092
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -1070,87 +1189,88 @@ func updateV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 *
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
defer resultRows.Close()
if !resultRows.Next() {
- return nil, http.StatusNotFound, errors.New("no delivery service found with this id"), nil
+ return nil, alerts, http.StatusNotFound, errors.New("no delivery service found with this id"), nil
}
- lastUpdated := tc.TimeNoMod{}
+ var lastUpdated time.Time
if err := resultRows.Scan(&lastUpdated); err != nil {
- return nil, http.StatusInternalServerError, nil, errors.New("scan updating delivery service: " + err.Error())
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("scan updating delivery service: " + err.Error())
}
if resultRows.Next() {
- xmlID := ""
- if ds.XMLID != nil {
- xmlID = *ds.XMLID
- }
- return nil, http.StatusInternalServerError, nil, errors.New("updating delivery service " + xmlID + ": " + "this update affected too many rows: > 1")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("updating delivery service " + ds.XMLID + ": " + "this update affected too many rows: > 1")
}
if ds.ID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing id after update")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("missing id after update")
}
- if ds.XMLID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing xml_id after update")
- }
- if ds.TypeID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing type after update")
- }
- if ds.RoutingName == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing routing name after update")
+
+ if inf.Version != nil && inf.Version.Major >= 4 {
Review comment:
The way changing the TLS versions works is it makes two database calls: one to delete the old ones and another to insert the new ones (if any). So in the situation where a DS with tls versions is being manipulated at APIv3, that will add 3 more db calls, rather than a normal `int` or `string` field that would result in just one.
If that's not a big deal I can change it, but that's why it is the way it is right now
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651775848
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active *bool `json:"active" db:"active"`
+ // AnonymousBlockingEnabled sets whether or not anonymized IP addresses
+ // (e.g. Tor exit nodes) should be restricted from accessing the Delivery
+ // Service's content.
+ AnonymousBlockingEnabled *bool `json:"anonymousBlockingEnabled" db:"anonymous_blocking_enabled"`
+ // CCRDNSTTL sets the Time-to-Live - in seconds - for DNS responses for this
+ // Delivery Service from Traffic Router.
+ CCRDNSTTL *int `json:"ccrDnsTtl" db:"ccr_dns_ttl"`
+ // CDNID is the integral, unique identifier for the CDN to which the
+ // Delivery Service belongs.
+ CDNID *int `json:"cdnId" db:"cdn_id"`
+ // CDNName is the name of the CDN to which the Delivery Service belongs.
+ CDNName *string `json:"cdnName"`
+ // CheckPath is a path which may be requested of the Delivery Service's
+ // origin to ensure it's working properly.
+ CheckPath *string `json:"checkPath" db:"check_path"`
+ // ConsistentHashQueryParams is a list of al of the query string parameters
+ // which ought to be considered by Traffic Router in client request URIs for
+ // HTTP-routed Delivery Services in the hashing process.
+ ConsistentHashQueryParams []string `json:"consistentHashQueryParams"`
+ // ConsistentHashRegex is used by Traffic Router to extract meaningful parts
+ // of a client's request URI for HTTP-routed Delivery Services before
+ // hashing the request to find a cache server to which to direct the client.
+ ConsistentHashRegex *string `json:"consistentHashRegex"`
+ // DeepCachingType may only legally point to 'ALWAYS' or 'NEVER', which
+ // define whether "deep caching" may or may not be used for this Delivery
+ // Service, respectively.
+ DeepCachingType *DeepCachingType `json:"deepCachingType" db:"deep_caching_type"`
+ // DisplayName is a human-friendly name that might be used in some UIs
+ // somewhere.
+ DisplayName *string `json:"displayName" db:"display_name"`
+ // DNSBypassCNAME is a fully qualified domain name to be used in a CNAME
+ // record presented to clients in bypass scenarios.
+ DNSBypassCNAME *string `json:"dnsBypassCname" db:"dns_bypass_cname"`
+ // DNSBypassIP is an IPv4 address to be used in an A record presented to
+ // clients in bypass scenarios.
+ DNSBypassIP *string `json:"dnsBypassIp" db:"dns_bypass_ip"`
+ // DNSBypassIP6 is an IPv6 address to be used in an AAAA record presented to
+ // clients in bypass scenarios.
+ DNSBypassIP6 *string `json:"dnsBypassIp6" db:"dns_bypass_ip6"`
+ // DNSBypassTTL sets the Time-to-Live - in seconds - of DNS responses from
+ // the Traffic Router that contain records for bypass destinations.
+ DNSBypassTTL *int `json:"dnsBypassTtl" db:"dns_bypass_ttl"`
+ // DSCP sets the Differentiated Services Code Point for IP packets
+ // transferred between clients, origins, and cache servers when obtaining
+ // and serving content for this Delivery Service.
+ // See Also: https://en.wikipedia.org/wiki/Differentiated_services
+ DSCP *int `json:"dscp" db:"dscp"`
+ // EcsEnabled describes whether or not the Traffic Router's EDNS0 Client
+ // Subnet extensions should be enabled when serving DNS responses for this
+ // Delivery Service. Even if this is true, the Traffic Router may still
+ // have the extensions unilaterally disabled in its own configuration.
+ EcsEnabled bool `json:"ecsEnabled" db:"ecs_enabled"`
+ // EdgeHeaderRewrite is a "header rewrite rule" used by ATS at the Edge-tier
+ // of caching. This has no effect on Delivery Services that don't use a
+ // Topology.
+ EdgeHeaderRewrite *string `json:"edgeHeaderRewrite" db:"edge_header_rewrite"`
+ // ExampleURLs is a list of all of the URLs from which content may be
+ // requested from the Delivery Service.
+ ExampleURLs []string `json:"exampleURLs"`
+ // FirstHeaderRewrite is a "header rewrite rule" used by ATS at the first
+ // caching layer encountered in the Delivery Service's Topology, or nil if
+ // there is no such rule. This has no effect on Delivery Services that don't
+ // employ Topologies.
+ FirstHeaderRewrite *string `json:"firstHeaderRewrite" db:"first_header_rewrite"`
+ // FQPacingRate sets the maximum bytes per second a cache server will deliver
+ // on any single TCP connection for this Delivery Service. This may never
+ // legally point to a value less than zero.
+ FQPacingRate *int `json:"fqPacingRate" db:"fq_pacing_rate"`
+ // GeoLimit defines whether or not access to a Delivery Service's content
+ // should be limited based on the requesting client's geographic location.
+ // Despite that this is a pointer to an arbitrary integer, the only valid
+ // values are 0 (which indicates that content should not be limited
+ // geographically), 1 (which indicates that content should only be served to
+ // clients whose IP addresses can be found within a Coverage Zone File), and
+ // 2 (which indicates that content should be served to clients whose IP
+ // addresses can be found within a Coverage Zone File OR are allowed access
+ // according to the "array" in GeoLimitCountries).
+ GeoLimit *int `json:"geoLimit" db:"geo_limit"`
+ // GeoLimitCountries is an "array" of "country codes" that itemizes the
+ // countries within which the Delivery Service's content ought to be made
+ // available. This has no effect if GeoLimit is not a pointer to exactly the
+ // value 2.
+ GeoLimitCountries *string `json:"geoLimitCountries" db:"geo_limit_countries"`
+ // GeoLimitRedirectURL is a URL to which clients will be redirected if their
+ // access to the Delivery Service's content is blocked by GeoLimit rules.
+ GeoLimitRedirectURL *string `json:"geoLimitRedirectURL" db:"geolimit_redirect_url"`
+ // GeoProvider names the type of database to be used for providing IP
+ // address-to-geographic-location mapping for this Delivery Service. The
+ // only valid values to which it may point are 0 (which indicates the use of
+ // a MaxMind GeoIP2 database) and 1 (which indicates the use of a Neustar
+ // GeoPoint IP address database).
+ GeoProvider *int `json:"geoProvider" db:"geo_provider"`
+ // GlobalMaxMBPS defines a maximum number of MegaBytes Per Second which may
+ // be served for the Delivery Service before redirecting clients to bypass
+ // locations.
+ GlobalMaxMBPS *int `json:"globalMaxMbps" db:"global_max_mbps"`
+ // GlobalMaxTPS defines a maximum number of Transactions Per Second which
+ // may be served for the Delivery Service before redirecting clients to
+ // bypass locations.
+ GlobalMaxTPS *int `json:"globalMaxTps" db:"global_max_tps"`
+ // HTTPBypassFQDN is a network location to which clients will be redirected
+ // in bypass scenarios using HTTP "Location" headers and appropriate
+ // redirection response codes.
+ HTTPBypassFQDN *string `json:"httpBypassFqdn" db:"http_bypass_fqdn"`
+ // ID is an integral, unique identifier for the Delivery Service.
+ ID *int `json:"id" db:"id"`
+ // InfoURL is a URL to which operators or clients may be directed to obtain
+ // further information about a Delivery Service.
+ InfoURL *string `json:"infoUrl" db:"info_url"`
+ // InitialDispersion sets the number of cache servers within the first
+ // caching layer ("Edge-tier" in a non-Topology context) across which
+ // content will be dispersed per Cache Group.
+ InitialDispersion *int `json:"initialDispersion" db:"initial_dispersion"`
+ // InnerHeaderRewrite is a "header rewrite rule" used by ATS at all caching
+ // layers encountered in the Delivery Service's Topology except the first
+ // and last, or nil if there is no such rule. This has no effect on Delivery
+ // Services that don't employ Topologies.
+ InnerHeaderRewrite *string `json:"innerHeaderRewrite" db:"inner_header_rewrite"`
+ // IPV6RoutingEnabled controls whether or not routing over IPv6 should be
+ // done for this Delivery Service.
+ IPV6RoutingEnabled *bool `json:"ipv6RoutingEnabled" db:"ipv6_routing_enabled"`
+ // LastHeaderRewrite is a "header rewrite rule" used by ATS at the first
+ // caching layer encountered in the Delivery Service's Topology, or nil if
+ // there is no such rule. This has no effect on Delivery Services that don't
+ // employ Topologies.
+ LastHeaderRewrite *string `json:"lastHeaderRewrite" db:"last_header_rewrite"`
+ // LastUpdated is the time and date at which the Delivery Service was last
+ // updated.
+ LastUpdated time.Time `json:"lastUpdated" db:"last_updated"`
+ // LogsEnabled controls nothing. It is kept only for legacy compatibility.
+ LogsEnabled *bool `json:"logsEnabled" db:"logs_enabled"`
+ // LongDesc is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc *string `json:"longDesc" db:"long_desc"`
+ // LongDesc1 is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc1 *string `json:"longDesc1" db:"long_desc_1"`
+ // LongDesc2 is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc2 *string `json:"longDesc2" db:"long_desc_2"`
+ // MatchList is a list of Regular Expressions used for routing the Delivery
+ // Service. Order matters, and the array is not allowed to be sparse.
+ MatchList []DeliveryServiceMatch `json:"matchList"`
+ // MaxDNSAnswers sets the maximum number of records which should be returned
+ // by Traffic Router in DNS responses to requests for resolving names for
+ // this Delivery Service.
+ MaxDNSAnswers *int `json:"maxDnsAnswers" db:"max_dns_answers"`
+ // MaxOriginConnections defines the total maximum number of connections
+ // that the highest caching layer ("Mid-tier" in a non-Topology context) is
+ // allowed to have concurrently open to the Delivery Service's Origin. This
+ // may never legally point to a value less than 0.
+ MaxOriginConnections *int `json:"maxOriginConnections" db:"max_origin_connections"`
+ // MaxRequestHeaderBytes is the maximum size (in bytes) of the request
+ // header that is allowed for this Delivery Service.
+ MaxRequestHeaderBytes *int `json:"maxRequestHeaderBytes" db:"max_request_header_bytes"`
+ // MidHeaderRewrite is a "header rewrite rule" used by ATS at the Mid-tier
+ // of caching. This has no effect on Delivery Services that don't use a
+ // Topology.
+ MidHeaderRewrite *string `json:"midHeaderRewrite" db:"mid_header_rewrite"`
+ // MissLat is a latitude to default to for clients of this Delivery Service
+ // when geolocation attempts fail.
+ MissLat *float64 `json:"missLat" db:"miss_lat"`
+ // MissLong is a longitude to default to for clients of this Delivery
+ // Service when geolocation attempts fail.
+ MissLong *float64 `json:"missLong" db:"miss_long"`
+ // MultiSiteOrigin determines whether or not the Delivery Service makes use
+ // of "Multi-Site Origin".
+ MultiSiteOrigin *bool `json:"multiSiteOrigin" db:"multi_site_origin"`
+ // OriginShield is a field that does nothing. It is kept only for legacy
+ // compatibility reasons.
+ OriginShield *string `json:"originShield" db:"origin_shield"`
+ // OrgServerFQDN is the URL - NOT Fully Qualified Domain Name - of the
+ // origin of the Delivery Service's content.
+ OrgServerFQDN *string `json:"orgServerFqdn" db:"org_server_fqdn"`
+ // ProfileDesc is the Description of the Profile used by the Delivery
+ // Service, if any.
+ ProfileDesc *string `json:"profileDescription"`
+ // ProfileID is the integral, unique identifier of the Profile used by the
+ // Delivery Service, if any.
+ ProfileID *int `json:"profileId" db:"profile"`
+ // ProfileName is the Name of the Profile used by the Delivery Service, if
+ // any.
+ ProfileName *string `json:"profileName"`
+ // Protocol defines the protocols by which caching servers may communicate
+ // with clients. The valid values to which it may point are 0 (which implies
+ // that only HTTP may be used), 1 (which implies that only HTTPS may be
+ // used), 2 (which implies that either HTTP or HTTPS may be used), and 3
+ // (which implies that clients using HTTP must be redirected to use HTTPS,
+ // while communications over HTTPS may proceed as normal).
+ Protocol *int `json:"protocol" db:"protocol"`
+ // QStringIgnore sets how query strings in HTTP requests to cache servers
+ // from clients are treated. The only valid values to which it may point are
+ // 0 (which implies that all caching layers will pass query strings in
+ // upstream requests and use them in the cache key), 1 (which implies that
+ // all caching layers will pass query strings in upstream requests, but not
+ // use them in cache keys), and 2 (which implies that the first encountered
+ // caching layer - "Edge-tier" in a non-Topology context - will strip query
+ // strings, effectively preventing them from being passed in upstream
+ // requests, and not use them in the cache key).
+ QStringIgnore *int `json:"qstringIgnore" db:"qstring_ignore"`
+ // RangeRequestHandling defines how HTTP GET requests with a Range header
+ // will be handled by cache servers serving the Delivery Service's content.
+ // The only valid values to which it may point are 0 (which implies that
+ // Range requests will not be cached at all), 1 (which implies that the
+ // background_fetch plugin will be used to service the range request while
+ // caching the whole object), 2 (which implies that the cache_range_requests
+ // plugin will be used to cache ranges as unique objects), and 3 (which
+ // implies that the slice plugin will be used to slice range based requests
+ // into deterministic chunks.)
+ RangeRequestHandling *int `json:"rangeRequestHandling" db:"range_request_handling"`
+ // RangeSliceBlockSize defines the size of range request blocks - or
+ // "slices" - used by the "slice" plugin. This has no effect if
+ // RangeRequestHandling does not point to exactly 3. This may never legally
+ // point to a value less than zero.
+ RangeSliceBlockSize *int `json:"rangeSliceBlockSize" db:"range_slice_block_size"`
+ // Regex Remap is a raw line to be inserted into "regex_remap.config" on the
+ // cache server. Care is necessitated in its use, because the input is in no
+ // way restricted, validated, or limited in scope to the Delivery Service.
+ RegexRemap *string `json:"regexRemap" db:"regex_remap"`
+ // RegionalGeoBlocking defines whether or not whatever Regional Geo Blocking
+ // rules are configured on the Traffic Router serving content for this
+ // Delivery Service will have an effect on the traffic of this Delivery
+ // Service.
+ RegionalGeoBlocking *bool `json:"regionalGeoBlocking" db:"regional_geo_blocking"`
+ // RemapText is raw text to insert in "remap.config" on the cache servers
+ // serving content for this Delivery Service. Care is necessitated in its
+ // use, because the input is in no way restricted, validated, or limited in
+ // scope to the Delivery Service.
+ RemapText *string `json:"remapText" db:"remap_text"`
+ // RoutingName defines the lowest-level DNS label used by the Delivery
+ // Service, e.g. if trafficcontrol.apache.org were a Delivery Service, it
+ // would have a RoutingName of "trafficcontrol".
+ RoutingName *string `json:"routingName" db:"routing_name"`
+ // ServiceCategory defines a category to which a Delivery Service may
+ // belong, which will cause HTTP Responses containing content for the
+ // Delivery Service to have the "X-CDN-SVC" header with a value that is the
+ // XMLID of the Delivery Service.
+ ServiceCategory *string `json:"serviceCategory" db:"service_category"`
+ // Signed is a legacy field. It is allowed to be `true` if and only if
+ // SigningAlgorithm is not nil.
+ Signed bool `json:"signed"`
+ // SigningAlgorithm is the name of the algorithm used to sign CDN URIs for
+ // this Delivery Service's content, or nil if no URI signing is done for the
+ // Delivery Service. This may only point to the values "url_sig" or
+ // "uri_signing".
+ SigningAlgorithm *string `json:"signingAlgorithm" db:"signing_algorithm"`
+ // SSLKeyVersion incremented whenever Traffic Portal generates new SSL keys
+ // for the Delivery Service, effectively making it a "generational" marker.
+ SSLKeyVersion *int `json:"sslKeyVersion" db:"ssl_key_version"`
+ // Tenant is the Tenant to which the Delivery Service belongs.
+ Tenant *string `json:"tenant"`
+ // TenantID is the integral, unique identifier for the Tenant to which the
+ // Delivery Service belongs.
+ TenantID *int `json:"tenantId" db:"tenant_id"`
+ // TLSVersions is the list of explicitly supported TLS versions for cache
+ // servers serving the Delivery Service's content.
+ TLSVersions []string `json:"tlsVersions" db:"tls_versions"`
+ // Topology is the name of the Topology used by the Delivery Service, or nil
+ // if no Topology is used.
+ Topology *string `json:"topology" db:"topology"`
+ // TRResponseHeaders is a set of headers (separated by CRLF pairs as per the
+ // HTTP spec) and their values (separated by a colon as per the HTTP spec)
+ // which will be sent by Traffic Router in HTTP responses to client requests
+ // for this Delivery Service's content. This has no effect on DNS-routed or
+ // un-routed Delivery Service Types.
+ TRResponseHeaders *string `json:"trResponseHeaders"`
+ // TRRequestHeaders is an "array" of HTTP headers which should be logged
+ // from client HTTP requests for this Delivery Service's content by Traffic
+ // Router, separated by newlines. This has no effect on DNS-routed or
+ // un-routed Delivery Service Types.
+ TRRequestHeaders *string `json:"trRequestHeaders"`
+ // Type describes how content is routed and cached for this Delivery Service
+ // as well as what other properties have any meaning.
+ Type *DSType `json:"type"`
+ // TypeID is an integral, unique identifier for the Tenant to which the
+ // Delivery Service belongs.
+ TypeID *int `json:"typeId" db:"type"`
+ // XMLID is a unique identifier that is also the second lowest-level DNS
+ // label used by the Delivery Service. For example, if a Delivery Service's
+ // content may be requested from video.demo1.mycdn.ciab.test, it may be
+ // inferred that the Delivery Service's XMLID is demo1.
+ XMLID *string `json:"xmlId" db:"xml_id"`
}
// DeliveryServiceV4 is a Delivery Service as it appears in version 4 of the
// Traffic Ops API - it always points to the highest minor version in APIv4.
type DeliveryServiceV4 = DeliveryServiceV40
+// These are the TLS Versions known by Apache Traffic Control to exist.
+const (
+ // Deprecated: TLS version 1.0 is known to be insecure.
+ TLSVersion10 = "1.0"
+ // Deprecated: TLS version 1.1 is known to be insecure.
+ TLSVersion11 = "1.1"
+ TLSVersion12 = "1.2"
+ TLSVersion13 = "1.3"
+)
+
+func newerDisallowedMessage(old string, newer []string) string {
+ l := len(newer)
+ if l < 1 {
+ return ""
+ }
+
+ var msg strings.Builder
+ msg.WriteString("old TLS version ")
+ msg.WriteString(old)
+ msg.WriteString(" is allowed, but newer version")
+ if l > 1 {
+ msg.WriteRune('s')
+ }
+ msg.WriteRune(' ')
+ msg.WriteString(newer[0])
+ if l > 1 {
+ msg.WriteString(", ")
+ if l > 2 {
+ msg.WriteString(newer[1])
+ msg.WriteString(", and ")
+ msg.WriteString(newer[2])
+ } else {
+ msg.WriteString("and ")
+ msg.WriteString(newer[1])
+ }
+ msg.WriteString(" are ")
+ } else {
+ msg.WriteString(" is ")
+ }
+ msg.WriteString("disallowed; this configuration may be insecure")
+
+ return msg.String()
+}
+
+// TLSVersionsAlerts generates warning-level alerts for the given TLS versions
+// array. It will warn if newer versions are disallowed while older, less
+// secure versions are allowed, or if there are unrecognized versions present.
+//
+// This does NOT verify that the passed TLS versions are _valid_, it ONLY
+// creates warnings based on conditions that are possibly detrimental to CDN
+// operation, but can, in fact, work.
+func TLSVersionsAlerts(vers []string) Alerts {
Review comment:
Yeah, idk. It's not using any information specific to TO, like a database connection or something, so I stuck it in there. I'm not convinced it's the right place, it just seemed like it _could_ go there. Although maybe the notion of Alerts being associated with a set of TLS Versions is inherently TO-centric.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651807493
##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -891,8 +891,9 @@ func CheckTopology(tx *sqlx.Tx, ds tc.DeliveryServiceV4) (int, error, error) {
return http.StatusBadRequest, fmt.Errorf("no such Topology '%s'", *ds.Topology), nil
}
+ // note that this segfaults if the ds doesn't have a properly set CDNID
Review comment:
Should also say "panic", not "segfault", since this is Go.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp merged pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp merged pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922
--
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.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r650116333
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active bool `json:"active" db:"active"`
Review comment:
Do you have a link to the list conversation?
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active bool `json:"active" db:"active"`
Review comment:
~~Do you have a link to the list conversation?~~ answered offline
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active bool `json:"active" db:"active"`
Review comment:
That's sort of what I was thinking. Either `map[string]interface{}` or you could make "shadow structs" used internally in TO that mirror the API structs, except every property is `json.RawMessage`. Idk which would be easier/smaller.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r650159206
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active bool `json:"active" db:"active"`
Review comment:
I'd be more open to having a custom `UnmarshalJSON` method that basically checks the raw JSON as `map[string]interface{}` and ensures that the required fields are present. After that validation, it could just call `json.Unmarshal` like normal.
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active bool `json:"active" db:"active"`
Review comment:
Ideally we'd only have 1 and only 1 struct per resource. Once we have more than 1 struct (like we do today w/ most nullable vs non-nullable structs), they _always_ get out of sync.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r652045198
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active *bool `json:"active" db:"active"`
+ // AnonymousBlockingEnabled sets whether or not anonymized IP addresses
+ // (e.g. Tor exit nodes) should be restricted from accessing the Delivery
+ // Service's content.
+ AnonymousBlockingEnabled *bool `json:"anonymousBlockingEnabled" db:"anonymous_blocking_enabled"`
+ // CCRDNSTTL sets the Time-to-Live - in seconds - for DNS responses for this
+ // Delivery Service from Traffic Router.
+ CCRDNSTTL *int `json:"ccrDnsTtl" db:"ccr_dns_ttl"`
+ // CDNID is the integral, unique identifier for the CDN to which the
+ // Delivery Service belongs.
+ CDNID *int `json:"cdnId" db:"cdn_id"`
+ // CDNName is the name of the CDN to which the Delivery Service belongs.
+ CDNName *string `json:"cdnName"`
+ // CheckPath is a path which may be requested of the Delivery Service's
+ // origin to ensure it's working properly.
+ CheckPath *string `json:"checkPath" db:"check_path"`
+ // ConsistentHashQueryParams is a list of al of the query string parameters
+ // which ought to be considered by Traffic Router in client request URIs for
+ // HTTP-routed Delivery Services in the hashing process.
+ ConsistentHashQueryParams []string `json:"consistentHashQueryParams"`
+ // ConsistentHashRegex is used by Traffic Router to extract meaningful parts
+ // of a client's request URI for HTTP-routed Delivery Services before
+ // hashing the request to find a cache server to which to direct the client.
+ ConsistentHashRegex *string `json:"consistentHashRegex"`
+ // DeepCachingType may only legally point to 'ALWAYS' or 'NEVER', which
+ // define whether "deep caching" may or may not be used for this Delivery
+ // Service, respectively.
+ DeepCachingType *DeepCachingType `json:"deepCachingType" db:"deep_caching_type"`
+ // DisplayName is a human-friendly name that might be used in some UIs
+ // somewhere.
+ DisplayName *string `json:"displayName" db:"display_name"`
+ // DNSBypassCNAME is a fully qualified domain name to be used in a CNAME
+ // record presented to clients in bypass scenarios.
+ DNSBypassCNAME *string `json:"dnsBypassCname" db:"dns_bypass_cname"`
+ // DNSBypassIP is an IPv4 address to be used in an A record presented to
+ // clients in bypass scenarios.
+ DNSBypassIP *string `json:"dnsBypassIp" db:"dns_bypass_ip"`
+ // DNSBypassIP6 is an IPv6 address to be used in an AAAA record presented to
+ // clients in bypass scenarios.
+ DNSBypassIP6 *string `json:"dnsBypassIp6" db:"dns_bypass_ip6"`
+ // DNSBypassTTL sets the Time-to-Live - in seconds - of DNS responses from
+ // the Traffic Router that contain records for bypass destinations.
+ DNSBypassTTL *int `json:"dnsBypassTtl" db:"dns_bypass_ttl"`
+ // DSCP sets the Differentiated Services Code Point for IP packets
+ // transferred between clients, origins, and cache servers when obtaining
+ // and serving content for this Delivery Service.
+ // See Also: https://en.wikipedia.org/wiki/Differentiated_services
+ DSCP *int `json:"dscp" db:"dscp"`
+ // EcsEnabled describes whether or not the Traffic Router's EDNS0 Client
+ // Subnet extensions should be enabled when serving DNS responses for this
+ // Delivery Service. Even if this is true, the Traffic Router may still
+ // have the extensions unilaterally disabled in its own configuration.
+ EcsEnabled bool `json:"ecsEnabled" db:"ecs_enabled"`
+ // EdgeHeaderRewrite is a "header rewrite rule" used by ATS at the Edge-tier
+ // of caching. This has no effect on Delivery Services that don't use a
+ // Topology.
+ EdgeHeaderRewrite *string `json:"edgeHeaderRewrite" db:"edge_header_rewrite"`
+ // ExampleURLs is a list of all of the URLs from which content may be
+ // requested from the Delivery Service.
+ ExampleURLs []string `json:"exampleURLs"`
+ // FirstHeaderRewrite is a "header rewrite rule" used by ATS at the first
+ // caching layer encountered in the Delivery Service's Topology, or nil if
+ // there is no such rule. This has no effect on Delivery Services that don't
+ // employ Topologies.
+ FirstHeaderRewrite *string `json:"firstHeaderRewrite" db:"first_header_rewrite"`
+ // FQPacingRate sets the maximum bytes per second a cache server will deliver
+ // on any single TCP connection for this Delivery Service. This may never
+ // legally point to a value less than zero.
+ FQPacingRate *int `json:"fqPacingRate" db:"fq_pacing_rate"`
+ // GeoLimit defines whether or not access to a Delivery Service's content
+ // should be limited based on the requesting client's geographic location.
+ // Despite that this is a pointer to an arbitrary integer, the only valid
+ // values are 0 (which indicates that content should not be limited
+ // geographically), 1 (which indicates that content should only be served to
+ // clients whose IP addresses can be found within a Coverage Zone File), and
+ // 2 (which indicates that content should be served to clients whose IP
+ // addresses can be found within a Coverage Zone File OR are allowed access
+ // according to the "array" in GeoLimitCountries).
+ GeoLimit *int `json:"geoLimit" db:"geo_limit"`
+ // GeoLimitCountries is an "array" of "country codes" that itemizes the
+ // countries within which the Delivery Service's content ought to be made
+ // available. This has no effect if GeoLimit is not a pointer to exactly the
+ // value 2.
+ GeoLimitCountries *string `json:"geoLimitCountries" db:"geo_limit_countries"`
+ // GeoLimitRedirectURL is a URL to which clients will be redirected if their
+ // access to the Delivery Service's content is blocked by GeoLimit rules.
+ GeoLimitRedirectURL *string `json:"geoLimitRedirectURL" db:"geolimit_redirect_url"`
+ // GeoProvider names the type of database to be used for providing IP
+ // address-to-geographic-location mapping for this Delivery Service. The
+ // only valid values to which it may point are 0 (which indicates the use of
+ // a MaxMind GeoIP2 database) and 1 (which indicates the use of a Neustar
+ // GeoPoint IP address database).
+ GeoProvider *int `json:"geoProvider" db:"geo_provider"`
+ // GlobalMaxMBPS defines a maximum number of MegaBytes Per Second which may
+ // be served for the Delivery Service before redirecting clients to bypass
+ // locations.
+ GlobalMaxMBPS *int `json:"globalMaxMbps" db:"global_max_mbps"`
+ // GlobalMaxTPS defines a maximum number of Transactions Per Second which
+ // may be served for the Delivery Service before redirecting clients to
+ // bypass locations.
+ GlobalMaxTPS *int `json:"globalMaxTps" db:"global_max_tps"`
+ // HTTPBypassFQDN is a network location to which clients will be redirected
+ // in bypass scenarios using HTTP "Location" headers and appropriate
+ // redirection response codes.
+ HTTPBypassFQDN *string `json:"httpBypassFqdn" db:"http_bypass_fqdn"`
+ // ID is an integral, unique identifier for the Delivery Service.
+ ID *int `json:"id" db:"id"`
+ // InfoURL is a URL to which operators or clients may be directed to obtain
+ // further information about a Delivery Service.
+ InfoURL *string `json:"infoUrl" db:"info_url"`
+ // InitialDispersion sets the number of cache servers within the first
+ // caching layer ("Edge-tier" in a non-Topology context) across which
+ // content will be dispersed per Cache Group.
+ InitialDispersion *int `json:"initialDispersion" db:"initial_dispersion"`
+ // InnerHeaderRewrite is a "header rewrite rule" used by ATS at all caching
+ // layers encountered in the Delivery Service's Topology except the first
+ // and last, or nil if there is no such rule. This has no effect on Delivery
+ // Services that don't employ Topologies.
+ InnerHeaderRewrite *string `json:"innerHeaderRewrite" db:"inner_header_rewrite"`
+ // IPV6RoutingEnabled controls whether or not routing over IPv6 should be
+ // done for this Delivery Service.
+ IPV6RoutingEnabled *bool `json:"ipv6RoutingEnabled" db:"ipv6_routing_enabled"`
+ // LastHeaderRewrite is a "header rewrite rule" used by ATS at the first
+ // caching layer encountered in the Delivery Service's Topology, or nil if
+ // there is no such rule. This has no effect on Delivery Services that don't
+ // employ Topologies.
+ LastHeaderRewrite *string `json:"lastHeaderRewrite" db:"last_header_rewrite"`
+ // LastUpdated is the time and date at which the Delivery Service was last
+ // updated.
+ LastUpdated time.Time `json:"lastUpdated" db:"last_updated"`
+ // LogsEnabled controls nothing. It is kept only for legacy compatibility.
+ LogsEnabled *bool `json:"logsEnabled" db:"logs_enabled"`
+ // LongDesc is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc *string `json:"longDesc" db:"long_desc"`
+ // LongDesc1 is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc1 *string `json:"longDesc1" db:"long_desc_1"`
+ // LongDesc2 is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc2 *string `json:"longDesc2" db:"long_desc_2"`
+ // MatchList is a list of Regular Expressions used for routing the Delivery
+ // Service. Order matters, and the array is not allowed to be sparse.
+ MatchList []DeliveryServiceMatch `json:"matchList"`
+ // MaxDNSAnswers sets the maximum number of records which should be returned
+ // by Traffic Router in DNS responses to requests for resolving names for
+ // this Delivery Service.
+ MaxDNSAnswers *int `json:"maxDnsAnswers" db:"max_dns_answers"`
+ // MaxOriginConnections defines the total maximum number of connections
+ // that the highest caching layer ("Mid-tier" in a non-Topology context) is
+ // allowed to have concurrently open to the Delivery Service's Origin. This
+ // may never legally point to a value less than 0.
+ MaxOriginConnections *int `json:"maxOriginConnections" db:"max_origin_connections"`
+ // MaxRequestHeaderBytes is the maximum size (in bytes) of the request
+ // header that is allowed for this Delivery Service.
+ MaxRequestHeaderBytes *int `json:"maxRequestHeaderBytes" db:"max_request_header_bytes"`
+ // MidHeaderRewrite is a "header rewrite rule" used by ATS at the Mid-tier
+ // of caching. This has no effect on Delivery Services that don't use a
+ // Topology.
+ MidHeaderRewrite *string `json:"midHeaderRewrite" db:"mid_header_rewrite"`
+ // MissLat is a latitude to default to for clients of this Delivery Service
+ // when geolocation attempts fail.
+ MissLat *float64 `json:"missLat" db:"miss_lat"`
+ // MissLong is a longitude to default to for clients of this Delivery
+ // Service when geolocation attempts fail.
+ MissLong *float64 `json:"missLong" db:"miss_long"`
+ // MultiSiteOrigin determines whether or not the Delivery Service makes use
+ // of "Multi-Site Origin".
+ MultiSiteOrigin *bool `json:"multiSiteOrigin" db:"multi_site_origin"`
+ // OriginShield is a field that does nothing. It is kept only for legacy
+ // compatibility reasons.
+ OriginShield *string `json:"originShield" db:"origin_shield"`
+ // OrgServerFQDN is the URL - NOT Fully Qualified Domain Name - of the
+ // origin of the Delivery Service's content.
+ OrgServerFQDN *string `json:"orgServerFqdn" db:"org_server_fqdn"`
+ // ProfileDesc is the Description of the Profile used by the Delivery
+ // Service, if any.
+ ProfileDesc *string `json:"profileDescription"`
+ // ProfileID is the integral, unique identifier of the Profile used by the
+ // Delivery Service, if any.
+ ProfileID *int `json:"profileId" db:"profile"`
+ // ProfileName is the Name of the Profile used by the Delivery Service, if
+ // any.
+ ProfileName *string `json:"profileName"`
+ // Protocol defines the protocols by which caching servers may communicate
+ // with clients. The valid values to which it may point are 0 (which implies
+ // that only HTTP may be used), 1 (which implies that only HTTPS may be
+ // used), 2 (which implies that either HTTP or HTTPS may be used), and 3
+ // (which implies that clients using HTTP must be redirected to use HTTPS,
+ // while communications over HTTPS may proceed as normal).
+ Protocol *int `json:"protocol" db:"protocol"`
+ // QStringIgnore sets how query strings in HTTP requests to cache servers
+ // from clients are treated. The only valid values to which it may point are
+ // 0 (which implies that all caching layers will pass query strings in
+ // upstream requests and use them in the cache key), 1 (which implies that
+ // all caching layers will pass query strings in upstream requests, but not
+ // use them in cache keys), and 2 (which implies that the first encountered
+ // caching layer - "Edge-tier" in a non-Topology context - will strip query
+ // strings, effectively preventing them from being passed in upstream
+ // requests, and not use them in the cache key).
+ QStringIgnore *int `json:"qstringIgnore" db:"qstring_ignore"`
+ // RangeRequestHandling defines how HTTP GET requests with a Range header
+ // will be handled by cache servers serving the Delivery Service's content.
+ // The only valid values to which it may point are 0 (which implies that
+ // Range requests will not be cached at all), 1 (which implies that the
+ // background_fetch plugin will be used to service the range request while
+ // caching the whole object), 2 (which implies that the cache_range_requests
+ // plugin will be used to cache ranges as unique objects), and 3 (which
+ // implies that the slice plugin will be used to slice range based requests
+ // into deterministic chunks.)
+ RangeRequestHandling *int `json:"rangeRequestHandling" db:"range_request_handling"`
+ // RangeSliceBlockSize defines the size of range request blocks - or
+ // "slices" - used by the "slice" plugin. This has no effect if
+ // RangeRequestHandling does not point to exactly 3. This may never legally
+ // point to a value less than zero.
+ RangeSliceBlockSize *int `json:"rangeSliceBlockSize" db:"range_slice_block_size"`
+ // Regex Remap is a raw line to be inserted into "regex_remap.config" on the
+ // cache server. Care is necessitated in its use, because the input is in no
+ // way restricted, validated, or limited in scope to the Delivery Service.
+ RegexRemap *string `json:"regexRemap" db:"regex_remap"`
+ // RegionalGeoBlocking defines whether or not whatever Regional Geo Blocking
+ // rules are configured on the Traffic Router serving content for this
+ // Delivery Service will have an effect on the traffic of this Delivery
+ // Service.
+ RegionalGeoBlocking *bool `json:"regionalGeoBlocking" db:"regional_geo_blocking"`
+ // RemapText is raw text to insert in "remap.config" on the cache servers
+ // serving content for this Delivery Service. Care is necessitated in its
+ // use, because the input is in no way restricted, validated, or limited in
+ // scope to the Delivery Service.
+ RemapText *string `json:"remapText" db:"remap_text"`
+ // RoutingName defines the lowest-level DNS label used by the Delivery
+ // Service, e.g. if trafficcontrol.apache.org were a Delivery Service, it
+ // would have a RoutingName of "trafficcontrol".
+ RoutingName *string `json:"routingName" db:"routing_name"`
+ // ServiceCategory defines a category to which a Delivery Service may
+ // belong, which will cause HTTP Responses containing content for the
+ // Delivery Service to have the "X-CDN-SVC" header with a value that is the
+ // XMLID of the Delivery Service.
+ ServiceCategory *string `json:"serviceCategory" db:"service_category"`
+ // Signed is a legacy field. It is allowed to be `true` if and only if
+ // SigningAlgorithm is not nil.
+ Signed bool `json:"signed"`
+ // SigningAlgorithm is the name of the algorithm used to sign CDN URIs for
+ // this Delivery Service's content, or nil if no URI signing is done for the
+ // Delivery Service. This may only point to the values "url_sig" or
+ // "uri_signing".
+ SigningAlgorithm *string `json:"signingAlgorithm" db:"signing_algorithm"`
+ // SSLKeyVersion incremented whenever Traffic Portal generates new SSL keys
+ // for the Delivery Service, effectively making it a "generational" marker.
+ SSLKeyVersion *int `json:"sslKeyVersion" db:"ssl_key_version"`
+ // Tenant is the Tenant to which the Delivery Service belongs.
+ Tenant *string `json:"tenant"`
+ // TenantID is the integral, unique identifier for the Tenant to which the
+ // Delivery Service belongs.
+ TenantID *int `json:"tenantId" db:"tenant_id"`
+ // TLSVersions is the list of explicitly supported TLS versions for cache
+ // servers serving the Delivery Service's content.
+ TLSVersions []string `json:"tlsVersions" db:"tls_versions"`
+ // Topology is the name of the Topology used by the Delivery Service, or nil
+ // if no Topology is used.
+ Topology *string `json:"topology" db:"topology"`
+ // TRResponseHeaders is a set of headers (separated by CRLF pairs as per the
+ // HTTP spec) and their values (separated by a colon as per the HTTP spec)
+ // which will be sent by Traffic Router in HTTP responses to client requests
+ // for this Delivery Service's content. This has no effect on DNS-routed or
+ // un-routed Delivery Service Types.
+ TRResponseHeaders *string `json:"trResponseHeaders"`
+ // TRRequestHeaders is an "array" of HTTP headers which should be logged
+ // from client HTTP requests for this Delivery Service's content by Traffic
+ // Router, separated by newlines. This has no effect on DNS-routed or
+ // un-routed Delivery Service Types.
+ TRRequestHeaders *string `json:"trRequestHeaders"`
+ // Type describes how content is routed and cached for this Delivery Service
+ // as well as what other properties have any meaning.
+ Type *DSType `json:"type"`
+ // TypeID is an integral, unique identifier for the Tenant to which the
+ // Delivery Service belongs.
+ TypeID *int `json:"typeId" db:"type"`
+ // XMLID is a unique identifier that is also the second lowest-level DNS
+ // label used by the Delivery Service. For example, if a Delivery Service's
+ // content may be requested from video.demo1.mycdn.ciab.test, it may be
+ // inferred that the Delivery Service's XMLID is demo1.
+ XMLID *string `json:"xmlId" db:"xml_id"`
}
// DeliveryServiceV4 is a Delivery Service as it appears in version 4 of the
// Traffic Ops API - it always points to the highest minor version in APIv4.
type DeliveryServiceV4 = DeliveryServiceV40
+// These are the TLS Versions known by Apache Traffic Control to exist.
+const (
+ // Deprecated: TLS version 1.0 is known to be insecure.
+ TLSVersion10 = "1.0"
+ // Deprecated: TLS version 1.1 is known to be insecure.
+ TLSVersion11 = "1.1"
+ TLSVersion12 = "1.2"
+ TLSVersion13 = "1.3"
+)
+
+func newerDisallowedMessage(old string, newer []string) string {
+ l := len(newer)
+ if l < 1 {
+ return ""
+ }
+
+ var msg strings.Builder
+ msg.WriteString("old TLS version ")
+ msg.WriteString(old)
+ msg.WriteString(" is allowed, but newer version")
+ if l > 1 {
+ msg.WriteRune('s')
+ }
+ msg.WriteRune(' ')
+ msg.WriteString(newer[0])
+ if l > 1 {
+ msg.WriteString(", ")
+ if l > 2 {
+ msg.WriteString(newer[1])
+ msg.WriteString(", and ")
+ msg.WriteString(newer[2])
+ } else {
+ msg.WriteString("and ")
+ msg.WriteString(newer[1])
+ }
+ msg.WriteString(" are ")
+ } else {
+ msg.WriteString(" is ")
+ }
+ msg.WriteString("disallowed; this configuration may be insecure")
+
+ return msg.String()
+}
+
+// TLSVersionsAlerts generates warning-level alerts for the given TLS versions
+// array. It will warn if newer versions are disallowed while older, less
+// secure versions are allowed, or if there are unrecognized versions present.
+//
+// This does NOT verify that the passed TLS versions are _valid_, it ONLY
+// creates warnings based on conditions that are possibly detrimental to CDN
+// operation, but can, in fact, work.
+func TLSVersionsAlerts(vers []string) Alerts {
Review comment:
That's fine -- I'll mark this resolved
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r647789667
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -1070,87 +1189,88 @@ func updateV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 *
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
defer resultRows.Close()
if !resultRows.Next() {
- return nil, http.StatusNotFound, errors.New("no delivery service found with this id"), nil
+ return nil, alerts, http.StatusNotFound, errors.New("no delivery service found with this id"), nil
}
- lastUpdated := tc.TimeNoMod{}
+ var lastUpdated time.Time
if err := resultRows.Scan(&lastUpdated); err != nil {
- return nil, http.StatusInternalServerError, nil, errors.New("scan updating delivery service: " + err.Error())
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("scan updating delivery service: " + err.Error())
}
if resultRows.Next() {
- xmlID := ""
- if ds.XMLID != nil {
- xmlID = *ds.XMLID
- }
- return nil, http.StatusInternalServerError, nil, errors.New("updating delivery service " + xmlID + ": " + "this update affected too many rows: > 1")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("updating delivery service " + ds.XMLID + ": " + "this update affected too many rows: > 1")
}
if ds.ID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing id after update")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("missing id after update")
}
- if ds.XMLID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing xml_id after update")
- }
- if ds.TypeID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing type after update")
- }
- if ds.RoutingName == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing routing name after update")
+
+ if inf.Version != nil && inf.Version.Major >= 4 {
Review comment:
That's ok, if the extra DB calls for prior API versions is a big deal to clients, they are free to use the latest API version 😄 . However, I think the overhead of 1 extra DB query is super negligible.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651786548
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/safe.go
##########
@@ -94,17 +94,39 @@ func UpdateSafe(w http.ResponseWriter, r *http.Request) {
ds := dses[0]
alertMsg := "Delivery Service safe update successful."
- if inf.Version != nil && inf.Version.Major == 1 && inf.Version.Minor < 5 {
- switch inf.Version.Minor {
+ if inf.Version == nil {
+ log.Warnln("API version found to be null in DS safe update")
+ api.WriteRespAlertObj(w, r, tc.SuccessLevel, alertMsg, dses)
Review comment:
no, that would skip writing the changelog message below
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651802698
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -875,70 +1052,82 @@ WHERE
&dsV31.MaxRequestHeaderBytes,
); err != nil {
if err == sql.ErrNoRows {
- return nil, http.StatusNotFound, fmt.Errorf("delivery service ID %d not found", *dsV31.ID), nil
+ return nil, tc.Alerts{}, http.StatusNotFound, fmt.Errorf("delivery service ID %d not found", *dsV31.ID), nil
}
- return nil, http.StatusInternalServerError, nil, fmt.Errorf("querying delivery service ID %d: %s", *dsV31.ID, err.Error())
+ return nil, tc.Alerts{}, http.StatusInternalServerError, nil, fmt.Errorf("querying delivery service ID %d: %s", *dsV31.ID, err.Error())
}
- res, status, userErr, sysErr := updateV31(w, r, inf, &dsV31)
+ res, alerts, status, userErr, sysErr := updateV31(w, r, inf, &dsV31)
if res != nil {
- return &res.DeliveryServiceV30, status, userErr, sysErr
+ return &res.DeliveryServiceV30, alerts, status, userErr, sysErr
}
- return nil, status, userErr, sysErr
+ return nil, alerts, status, userErr, sysErr
}
-func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 *tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, int, error, error) {
+func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 *tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, tc.Alerts, int, error, error) {
+ var alerts tc.Alerts
+
dsNull := tc.DeliveryServiceNullableV30(*dsV31)
ds := dsNull.UpgradeToV4()
- dsV40 := tc.DeliveryServiceV40(ds)
+ dsV40 := ds
+ if dsV40.ID == nil {
Review comment:
Got it. Also, though, it seems like the only reason `dsV40` exists is as an unnecessary typecast of `ds` to `tc.DeliveryServiceV40` - do you think it's safe to just get rid of it and just use `ds` instead? I wasn't totally sure.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rob05c commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r648746378
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active bool `json:"active" db:"active"`
Review comment:
> It's a giant pain that Go won't let you just say "if it's missing this property, throw an error". That would solve so many problems.
I agree. In fact, I wrote a library to do exactly that: https://github.com/rob05c/apiver - optional things are pointers, non-pointers that are missing throw decode errors. It was rejected on the list.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r652081126
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
Review comment:
I was hoping #4996 would get added to the 6.x milestone, but it isn't so far. But still, I'd like to keep the timestamps as RFC3339, even if that would mean duplication.
Unfortunately, I think mostly duplicated structs are just a fact of life with a versioned API in Go; even with the nesting of previous versions we have compatibility problems such that clients of the library are forbidden from constructing the objects with direct field initialization like
```go
f := FooVX{
Property: "value,
}
```
(which has broken in the past and will break in the future for as long as we use nesting, but I don't think is widely known and so is probably done in thousands of places outside of `lib/go-tc`)
Using nesting saves code changes for non-breaking changes, but makes it harder to make breaking changes.
If you want me to take out all the breaking structure changes I absolutely will, to be clear, since they aren't really necessary to accomplish the PR's goal. But then I intend to open a PR to re-introduce them, and I just hoped they could be small enough to fit into this, since I was touching the struct anyway.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r652115957
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
Review comment:
> Can you make the RFC3339 change without de-embedding the struct? Sure, that would change it for prior API versions, but that seems like a worthwhile breakage (if it even does break anything).
I actually think that might break Traffic Portal and possibly any other web-based client of the API used heavily for Delivery Service interactions that may or may not exist. It's possible it wouldn't - one of those is much easier to test than the other.
>> Unfortunately, I think mostly duplicated structs are just a fact of life with a versioned API in Go
> I refuse to accept this statement.
Well, in truth, it's more about what we're willing to give up. not duplicating things is going to mean that you can write code today that won't compile tomorrow. It's just that I'd rather duplicate things than deal with that, personally.
> We need to be aggressively removing old API versions
Agree. I think we can deprecate v2 and v3 with ATCv6 as soon as APIv1 is actually removed.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r647786297
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active bool `json:"active" db:"active"`
Review comment:
They're mandatory, yes, but how can TO know if you really asked for CDN ID 0 or simply didn't send a CDN ID in the request?
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r647802585
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active bool `json:"active" db:"active"`
Review comment:
It can't, and it also can't tell the difference between `null` and a missing value - the way Go's JSON parser works by default is highly annoying, and the only real way around it is to write custom marshalling and unmarshalling methods.
I'm going to take those changes out of the PR, they shouldn't have been there in the first place. It just bothers me that assignment and reading always take extra steps for data you know has to exist, is all.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r652081126
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
Review comment:
I was hoping #4996 would get added to the 6.x milestone, but it isn't so far. But still, I'd like to keep the timestamps as RFC3339, even if that would mean duplication.
Unfortunately, I think mostly duplicated structs are just a fact of life with a versioned API in Go; even with the nesting of previous versions we have compatibility problems such that clients of the library are forbidden from constructing the objects with direct field initialization like
```go
f := FooVX{
Property: "value,
}
```
Using nesting saves code changes for non-breaking changes, but makes it harder to make breaking changes.
If you want me to take out all the breaking structure changes I absolutely will, to be clear, since they aren't really necessary to accomplish the PR's goal. But then I intend to open a PR to re-introduce them, and I just hoped they could be small enough to fit into this, since I was touching the struct anyway.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r652122130
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
Review comment:
Ok, there might be another option. If you add a new `LastUpdated time.Time` field with the same JSON tags (and probably db tags for good measure) to `DeliveryServiceV40`, when doing JSON marshalling at V40, it will take precedence over the _embedded_ `LastUpdated`. We just have to make sure to set both fields before marshalling the response.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651896706
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -110,6 +110,42 @@ func (ds *TODeliveryService) IsTenantAuthorized(user *auth.CurrentUser) (bool, e
return isTenantAuthorized(ds.ReqInfo, &ds.DeliveryServiceV4)
}
+const getTLSVersionsQuery = `
+SELECT tls_version
+FROM public.deliveryservice_tls_version
+WHERE deliveryservice = $1
+ORDER BY tls_version
+`
+
+// GetDSTLSVersions retrieves the TLS versions explicitly supported by a
+// Delivery Service identified by dsID. This will panic if handed a nil
Review comment:
> It might be obvious to you, being familiar with how the TO codebase most often has functions that panic when given nil transactions
The audience for these comments is TO developers, who will surely learn quickly that you can't get anywhere with a nil DB transaction when you need to access the DB. I don't care strongly enough to -1 this PR for these few instances, but I would -1 a PR that added these comments to every function of this kind.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r652054126
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
Review comment:
I see. It seems like the DS active flag changes are on the back-burner still, so if we make this change now but have to cut 6.0.x before that is implemented, we'll end up with yet another mostly-duplicated struct.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651785362
##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -891,8 +891,9 @@ func CheckTopology(tx *sqlx.Tx, ds tc.DeliveryServiceV4) (int, error, error) {
return http.StatusBadRequest, fmt.Errorf("no such Topology '%s'", *ds.Topology), nil
}
+ // note that this segfaults if the ds doesn't have a properly set CDNID
Review comment:
I think I meant for that to go in the GoDoc for that function. It is pretty worthless there.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651803739
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -1070,87 +1259,103 @@ func updateV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 *
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
defer resultRows.Close()
if !resultRows.Next() {
- return nil, http.StatusNotFound, errors.New("no delivery service found with this id"), nil
+ return nil, alerts, http.StatusNotFound, errors.New("no delivery service found with this id"), nil
}
- lastUpdated := tc.TimeNoMod{}
+ var lastUpdated time.Time
if err := resultRows.Scan(&lastUpdated); err != nil {
- return nil, http.StatusInternalServerError, nil, errors.New("scan updating delivery service: " + err.Error())
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("scan updating delivery service: " + err.Error())
}
if resultRows.Next() {
xmlID := ""
if ds.XMLID != nil {
xmlID = *ds.XMLID
}
- return nil, http.StatusInternalServerError, nil, errors.New("updating delivery service " + xmlID + ": " + "this update affected too many rows: > 1")
- }
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("updating delivery service " + xmlID + ": " + "this update affected too many rows: > 1")
+ }
if ds.ID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing id after update")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("missing id after update")
}
if ds.XMLID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing xml_id after update")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("missing XMLID after update")
}
if ds.TypeID == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing type after update")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("missing type id after update")
}
if ds.RoutingName == nil {
- return nil, http.StatusInternalServerError, nil, errors.New("missing routing name after update")
+ return nil, alerts, http.StatusInternalServerError, nil, errors.New("missing routing name after update")
+ }
+
+ if resultRows.Next() {
Review comment:
We don't, looks like a bad rebase.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651820649
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -110,6 +110,42 @@ func (ds *TODeliveryService) IsTenantAuthorized(user *auth.CurrentUser) (bool, e
return isTenantAuthorized(ds.ReqInfo, &ds.DeliveryServiceV4)
}
+const getTLSVersionsQuery = `
Review comment:
well I can switch to using `ARRAY_AGG` if you think that's betetr, but the two can't be literally the same string because they have different `WHERE` clauses that can't be consolidated.
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -110,6 +110,42 @@ func (ds *TODeliveryService) IsTenantAuthorized(user *auth.CurrentUser) (bool, e
return isTenantAuthorized(ds.ReqInfo, &ds.DeliveryServiceV4)
}
+const getTLSVersionsQuery = `
Review comment:
well I can switch to using `ARRAY_AGG` if you think that's better, but the two can't be literally the same string because they have different `WHERE` clauses that can't be consolidated.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651802698
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -875,70 +1052,82 @@ WHERE
&dsV31.MaxRequestHeaderBytes,
); err != nil {
if err == sql.ErrNoRows {
- return nil, http.StatusNotFound, fmt.Errorf("delivery service ID %d not found", *dsV31.ID), nil
+ return nil, tc.Alerts{}, http.StatusNotFound, fmt.Errorf("delivery service ID %d not found", *dsV31.ID), nil
}
- return nil, http.StatusInternalServerError, nil, fmt.Errorf("querying delivery service ID %d: %s", *dsV31.ID, err.Error())
+ return nil, tc.Alerts{}, http.StatusInternalServerError, nil, fmt.Errorf("querying delivery service ID %d: %s", *dsV31.ID, err.Error())
}
- res, status, userErr, sysErr := updateV31(w, r, inf, &dsV31)
+ res, alerts, status, userErr, sysErr := updateV31(w, r, inf, &dsV31)
if res != nil {
- return &res.DeliveryServiceV30, status, userErr, sysErr
+ return &res.DeliveryServiceV30, alerts, status, userErr, sysErr
}
- return nil, status, userErr, sysErr
+ return nil, alerts, status, userErr, sysErr
}
-func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 *tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, int, error, error) {
+func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 *tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, tc.Alerts, int, error, error) {
+ var alerts tc.Alerts
+
dsNull := tc.DeliveryServiceNullableV30(*dsV31)
ds := dsNull.UpgradeToV4()
- dsV40 := tc.DeliveryServiceV40(ds)
+ dsV40 := ds
+ if dsV40.ID == nil {
Review comment:
Got it. Also, though, it seems like the only reason `dsV40` exists is as a typecast of `ds` to `tc.DeliveryServiceV40` - do you think it's safe to just get rid of it and just use `ds` instead? I wasn't totally sure.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651908234
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -875,70 +1052,82 @@ WHERE
&dsV31.MaxRequestHeaderBytes,
); err != nil {
if err == sql.ErrNoRows {
- return nil, http.StatusNotFound, fmt.Errorf("delivery service ID %d not found", *dsV31.ID), nil
+ return nil, tc.Alerts{}, http.StatusNotFound, fmt.Errorf("delivery service ID %d not found", *dsV31.ID), nil
}
- return nil, http.StatusInternalServerError, nil, fmt.Errorf("querying delivery service ID %d: %s", *dsV31.ID, err.Error())
+ return nil, tc.Alerts{}, http.StatusInternalServerError, nil, fmt.Errorf("querying delivery service ID %d: %s", *dsV31.ID, err.Error())
}
- res, status, userErr, sysErr := updateV31(w, r, inf, &dsV31)
+ res, alerts, status, userErr, sysErr := updateV31(w, r, inf, &dsV31)
if res != nil {
- return &res.DeliveryServiceV30, status, userErr, sysErr
+ return &res.DeliveryServiceV30, alerts, status, userErr, sysErr
}
- return nil, status, userErr, sysErr
+ return nil, alerts, status, userErr, sysErr
}
-func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 *tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, int, error, error) {
+func updateV31(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV31 *tc.DeliveryServiceV31) (*tc.DeliveryServiceV31, tc.Alerts, int, error, error) {
+ var alerts tc.Alerts
+
dsNull := tc.DeliveryServiceNullableV30(*dsV31)
ds := dsNull.UpgradeToV4()
- dsV40 := tc.DeliveryServiceV40(ds)
+ dsV40 := ds
+ if dsV40.ID == nil {
Review comment:
I believe the original intent was that the latest handler (i.e. `updateV40/createV40`), would take a V4X struct as input then immediately cast it to the major version alias, e.g. `DeliveryServiceV4`. That way, all the other functions take the major version alias as input and don't need updated with every single new minor version. `UpgradeToV4` upgrades to the major version struct, but it should probably actually upgrade to the `V40` struct instead, since it's used in the `updateV31` function. We wouldn't want the `updateV31` function to upgrade its input to `V41` because that would skip a link (4.0) in the version upgrade chain.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651790889
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -110,6 +110,42 @@ func (ds *TODeliveryService) IsTenantAuthorized(user *auth.CurrentUser) (bool, e
return isTenantAuthorized(ds.ReqInfo, &ds.DeliveryServiceV4)
}
+const getTLSVersionsQuery = `
+SELECT tls_version
+FROM public.deliveryservice_tls_version
+WHERE deliveryservice = $1
+ORDER BY tls_version
+`
+
+// GetDSTLSVersions retrieves the TLS versions explicitly supported by a
+// Delivery Service identified by dsID. This will panic if handed a nil
Review comment:
I don't know that that's obvious without reading the function. It might be obvious to you, being familiar with how the TO codebase most often has functions that panic when given nil transactions, but that doesn't mean it's obvious to everyone. You could reasonably think that it would return an error instead of crashing the whole thread it's running on.
I don't think it hurts anything to make it clear in the GoDoc.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r651795083
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -416,88 +546,100 @@ func createV40(w http.ResponseWriter, r *http.Request, inf *api.APIInfo, dsV40 t
if err != nil {
usrErr, sysErr, code := api.ParseDBError(err)
- return nil, code, usrErr, sysErr
+ return nil, alerts, code, usrErr, sysErr
}
defer resultRows.Close()
id := 0
- lastUpdated := tc.TimeNoMod{}
+ var lastUpdated time.Time
Review comment:
Specifically, the change on this line fixes typing. A `tc.TimeNoMod` value is not assignable to `time.Time` - although you could extract the nested time instead, I think this is clearer.
As to why/whether that's necessary, we're discussing that on your second comment in this review above.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] rawlinp commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
rawlinp commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r647806332
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active bool `json:"active" db:"active"`
Review comment:
Not all pointer-based reads have to be preceded by a nil-check. If it's a required field, generally just the request validation needs to do the nil-check, and the rest of the code can reasonably assume that the field is non-nil.
--
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:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #5922: Per-Delivery Service TLS versions
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #5922:
URL: https://github.com/apache/trafficcontrol/pull/5922#discussion_r652042451
##########
File path: lib/go-tc/deliveryservices.go
##########
@@ -192,18 +194,416 @@ type DeliveryServiceFieldsV31 struct {
// DeliveryServiceV40 is a Delivery Service as it appears in version 4.0 of the
// Traffic Ops API.
type DeliveryServiceV40 struct {
- DeliveryServiceFieldsV31
- DeliveryServiceFieldsV30
- DeliveryServiceFieldsV15
- DeliveryServiceFieldsV14
- DeliveryServiceFieldsV13
- DeliveryServiceNullableFieldsV11
+ // Active dictates whether the Delivery Service is routed by Traffic Router.
+ Active *bool `json:"active" db:"active"`
+ // AnonymousBlockingEnabled sets whether or not anonymized IP addresses
+ // (e.g. Tor exit nodes) should be restricted from accessing the Delivery
+ // Service's content.
+ AnonymousBlockingEnabled *bool `json:"anonymousBlockingEnabled" db:"anonymous_blocking_enabled"`
+ // CCRDNSTTL sets the Time-to-Live - in seconds - for DNS responses for this
+ // Delivery Service from Traffic Router.
+ CCRDNSTTL *int `json:"ccrDnsTtl" db:"ccr_dns_ttl"`
+ // CDNID is the integral, unique identifier for the CDN to which the
+ // Delivery Service belongs.
+ CDNID *int `json:"cdnId" db:"cdn_id"`
+ // CDNName is the name of the CDN to which the Delivery Service belongs.
+ CDNName *string `json:"cdnName"`
+ // CheckPath is a path which may be requested of the Delivery Service's
+ // origin to ensure it's working properly.
+ CheckPath *string `json:"checkPath" db:"check_path"`
+ // ConsistentHashQueryParams is a list of al of the query string parameters
+ // which ought to be considered by Traffic Router in client request URIs for
+ // HTTP-routed Delivery Services in the hashing process.
+ ConsistentHashQueryParams []string `json:"consistentHashQueryParams"`
+ // ConsistentHashRegex is used by Traffic Router to extract meaningful parts
+ // of a client's request URI for HTTP-routed Delivery Services before
+ // hashing the request to find a cache server to which to direct the client.
+ ConsistentHashRegex *string `json:"consistentHashRegex"`
+ // DeepCachingType may only legally point to 'ALWAYS' or 'NEVER', which
+ // define whether "deep caching" may or may not be used for this Delivery
+ // Service, respectively.
+ DeepCachingType *DeepCachingType `json:"deepCachingType" db:"deep_caching_type"`
+ // DisplayName is a human-friendly name that might be used in some UIs
+ // somewhere.
+ DisplayName *string `json:"displayName" db:"display_name"`
+ // DNSBypassCNAME is a fully qualified domain name to be used in a CNAME
+ // record presented to clients in bypass scenarios.
+ DNSBypassCNAME *string `json:"dnsBypassCname" db:"dns_bypass_cname"`
+ // DNSBypassIP is an IPv4 address to be used in an A record presented to
+ // clients in bypass scenarios.
+ DNSBypassIP *string `json:"dnsBypassIp" db:"dns_bypass_ip"`
+ // DNSBypassIP6 is an IPv6 address to be used in an AAAA record presented to
+ // clients in bypass scenarios.
+ DNSBypassIP6 *string `json:"dnsBypassIp6" db:"dns_bypass_ip6"`
+ // DNSBypassTTL sets the Time-to-Live - in seconds - of DNS responses from
+ // the Traffic Router that contain records for bypass destinations.
+ DNSBypassTTL *int `json:"dnsBypassTtl" db:"dns_bypass_ttl"`
+ // DSCP sets the Differentiated Services Code Point for IP packets
+ // transferred between clients, origins, and cache servers when obtaining
+ // and serving content for this Delivery Service.
+ // See Also: https://en.wikipedia.org/wiki/Differentiated_services
+ DSCP *int `json:"dscp" db:"dscp"`
+ // EcsEnabled describes whether or not the Traffic Router's EDNS0 Client
+ // Subnet extensions should be enabled when serving DNS responses for this
+ // Delivery Service. Even if this is true, the Traffic Router may still
+ // have the extensions unilaterally disabled in its own configuration.
+ EcsEnabled bool `json:"ecsEnabled" db:"ecs_enabled"`
+ // EdgeHeaderRewrite is a "header rewrite rule" used by ATS at the Edge-tier
+ // of caching. This has no effect on Delivery Services that don't use a
+ // Topology.
+ EdgeHeaderRewrite *string `json:"edgeHeaderRewrite" db:"edge_header_rewrite"`
+ // ExampleURLs is a list of all of the URLs from which content may be
+ // requested from the Delivery Service.
+ ExampleURLs []string `json:"exampleURLs"`
+ // FirstHeaderRewrite is a "header rewrite rule" used by ATS at the first
+ // caching layer encountered in the Delivery Service's Topology, or nil if
+ // there is no such rule. This has no effect on Delivery Services that don't
+ // employ Topologies.
+ FirstHeaderRewrite *string `json:"firstHeaderRewrite" db:"first_header_rewrite"`
+ // FQPacingRate sets the maximum bytes per second a cache server will deliver
+ // on any single TCP connection for this Delivery Service. This may never
+ // legally point to a value less than zero.
+ FQPacingRate *int `json:"fqPacingRate" db:"fq_pacing_rate"`
+ // GeoLimit defines whether or not access to a Delivery Service's content
+ // should be limited based on the requesting client's geographic location.
+ // Despite that this is a pointer to an arbitrary integer, the only valid
+ // values are 0 (which indicates that content should not be limited
+ // geographically), 1 (which indicates that content should only be served to
+ // clients whose IP addresses can be found within a Coverage Zone File), and
+ // 2 (which indicates that content should be served to clients whose IP
+ // addresses can be found within a Coverage Zone File OR are allowed access
+ // according to the "array" in GeoLimitCountries).
+ GeoLimit *int `json:"geoLimit" db:"geo_limit"`
+ // GeoLimitCountries is an "array" of "country codes" that itemizes the
+ // countries within which the Delivery Service's content ought to be made
+ // available. This has no effect if GeoLimit is not a pointer to exactly the
+ // value 2.
+ GeoLimitCountries *string `json:"geoLimitCountries" db:"geo_limit_countries"`
+ // GeoLimitRedirectURL is a URL to which clients will be redirected if their
+ // access to the Delivery Service's content is blocked by GeoLimit rules.
+ GeoLimitRedirectURL *string `json:"geoLimitRedirectURL" db:"geolimit_redirect_url"`
+ // GeoProvider names the type of database to be used for providing IP
+ // address-to-geographic-location mapping for this Delivery Service. The
+ // only valid values to which it may point are 0 (which indicates the use of
+ // a MaxMind GeoIP2 database) and 1 (which indicates the use of a Neustar
+ // GeoPoint IP address database).
+ GeoProvider *int `json:"geoProvider" db:"geo_provider"`
+ // GlobalMaxMBPS defines a maximum number of MegaBytes Per Second which may
+ // be served for the Delivery Service before redirecting clients to bypass
+ // locations.
+ GlobalMaxMBPS *int `json:"globalMaxMbps" db:"global_max_mbps"`
+ // GlobalMaxTPS defines a maximum number of Transactions Per Second which
+ // may be served for the Delivery Service before redirecting clients to
+ // bypass locations.
+ GlobalMaxTPS *int `json:"globalMaxTps" db:"global_max_tps"`
+ // HTTPBypassFQDN is a network location to which clients will be redirected
+ // in bypass scenarios using HTTP "Location" headers and appropriate
+ // redirection response codes.
+ HTTPBypassFQDN *string `json:"httpBypassFqdn" db:"http_bypass_fqdn"`
+ // ID is an integral, unique identifier for the Delivery Service.
+ ID *int `json:"id" db:"id"`
+ // InfoURL is a URL to which operators or clients may be directed to obtain
+ // further information about a Delivery Service.
+ InfoURL *string `json:"infoUrl" db:"info_url"`
+ // InitialDispersion sets the number of cache servers within the first
+ // caching layer ("Edge-tier" in a non-Topology context) across which
+ // content will be dispersed per Cache Group.
+ InitialDispersion *int `json:"initialDispersion" db:"initial_dispersion"`
+ // InnerHeaderRewrite is a "header rewrite rule" used by ATS at all caching
+ // layers encountered in the Delivery Service's Topology except the first
+ // and last, or nil if there is no such rule. This has no effect on Delivery
+ // Services that don't employ Topologies.
+ InnerHeaderRewrite *string `json:"innerHeaderRewrite" db:"inner_header_rewrite"`
+ // IPV6RoutingEnabled controls whether or not routing over IPv6 should be
+ // done for this Delivery Service.
+ IPV6RoutingEnabled *bool `json:"ipv6RoutingEnabled" db:"ipv6_routing_enabled"`
+ // LastHeaderRewrite is a "header rewrite rule" used by ATS at the first
+ // caching layer encountered in the Delivery Service's Topology, or nil if
+ // there is no such rule. This has no effect on Delivery Services that don't
+ // employ Topologies.
+ LastHeaderRewrite *string `json:"lastHeaderRewrite" db:"last_header_rewrite"`
+ // LastUpdated is the time and date at which the Delivery Service was last
+ // updated.
+ LastUpdated time.Time `json:"lastUpdated" db:"last_updated"`
+ // LogsEnabled controls nothing. It is kept only for legacy compatibility.
+ LogsEnabled *bool `json:"logsEnabled" db:"logs_enabled"`
+ // LongDesc is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc *string `json:"longDesc" db:"long_desc"`
+ // LongDesc1 is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc1 *string `json:"longDesc1" db:"long_desc_1"`
+ // LongDesc2 is a description of the Delivery Service, having arbitrary
+ // length.
+ LongDesc2 *string `json:"longDesc2" db:"long_desc_2"`
+ // MatchList is a list of Regular Expressions used for routing the Delivery
+ // Service. Order matters, and the array is not allowed to be sparse.
+ MatchList []DeliveryServiceMatch `json:"matchList"`
+ // MaxDNSAnswers sets the maximum number of records which should be returned
+ // by Traffic Router in DNS responses to requests for resolving names for
+ // this Delivery Service.
+ MaxDNSAnswers *int `json:"maxDnsAnswers" db:"max_dns_answers"`
+ // MaxOriginConnections defines the total maximum number of connections
+ // that the highest caching layer ("Mid-tier" in a non-Topology context) is
+ // allowed to have concurrently open to the Delivery Service's Origin. This
+ // may never legally point to a value less than 0.
+ MaxOriginConnections *int `json:"maxOriginConnections" db:"max_origin_connections"`
+ // MaxRequestHeaderBytes is the maximum size (in bytes) of the request
+ // header that is allowed for this Delivery Service.
+ MaxRequestHeaderBytes *int `json:"maxRequestHeaderBytes" db:"max_request_header_bytes"`
+ // MidHeaderRewrite is a "header rewrite rule" used by ATS at the Mid-tier
+ // of caching. This has no effect on Delivery Services that don't use a
+ // Topology.
+ MidHeaderRewrite *string `json:"midHeaderRewrite" db:"mid_header_rewrite"`
+ // MissLat is a latitude to default to for clients of this Delivery Service
+ // when geolocation attempts fail.
+ MissLat *float64 `json:"missLat" db:"miss_lat"`
+ // MissLong is a longitude to default to for clients of this Delivery
+ // Service when geolocation attempts fail.
+ MissLong *float64 `json:"missLong" db:"miss_long"`
+ // MultiSiteOrigin determines whether or not the Delivery Service makes use
+ // of "Multi-Site Origin".
+ MultiSiteOrigin *bool `json:"multiSiteOrigin" db:"multi_site_origin"`
+ // OriginShield is a field that does nothing. It is kept only for legacy
+ // compatibility reasons.
+ OriginShield *string `json:"originShield" db:"origin_shield"`
+ // OrgServerFQDN is the URL - NOT Fully Qualified Domain Name - of the
+ // origin of the Delivery Service's content.
+ OrgServerFQDN *string `json:"orgServerFqdn" db:"org_server_fqdn"`
+ // ProfileDesc is the Description of the Profile used by the Delivery
+ // Service, if any.
+ ProfileDesc *string `json:"profileDescription"`
+ // ProfileID is the integral, unique identifier of the Profile used by the
+ // Delivery Service, if any.
+ ProfileID *int `json:"profileId" db:"profile"`
+ // ProfileName is the Name of the Profile used by the Delivery Service, if
+ // any.
+ ProfileName *string `json:"profileName"`
+ // Protocol defines the protocols by which caching servers may communicate
+ // with clients. The valid values to which it may point are 0 (which implies
+ // that only HTTP may be used), 1 (which implies that only HTTPS may be
+ // used), 2 (which implies that either HTTP or HTTPS may be used), and 3
+ // (which implies that clients using HTTP must be redirected to use HTTPS,
+ // while communications over HTTPS may proceed as normal).
+ Protocol *int `json:"protocol" db:"protocol"`
+ // QStringIgnore sets how query strings in HTTP requests to cache servers
+ // from clients are treated. The only valid values to which it may point are
+ // 0 (which implies that all caching layers will pass query strings in
+ // upstream requests and use them in the cache key), 1 (which implies that
+ // all caching layers will pass query strings in upstream requests, but not
+ // use them in cache keys), and 2 (which implies that the first encountered
+ // caching layer - "Edge-tier" in a non-Topology context - will strip query
+ // strings, effectively preventing them from being passed in upstream
+ // requests, and not use them in the cache key).
+ QStringIgnore *int `json:"qstringIgnore" db:"qstring_ignore"`
+ // RangeRequestHandling defines how HTTP GET requests with a Range header
+ // will be handled by cache servers serving the Delivery Service's content.
+ // The only valid values to which it may point are 0 (which implies that
+ // Range requests will not be cached at all), 1 (which implies that the
+ // background_fetch plugin will be used to service the range request while
+ // caching the whole object), 2 (which implies that the cache_range_requests
+ // plugin will be used to cache ranges as unique objects), and 3 (which
+ // implies that the slice plugin will be used to slice range based requests
+ // into deterministic chunks.)
+ RangeRequestHandling *int `json:"rangeRequestHandling" db:"range_request_handling"`
+ // RangeSliceBlockSize defines the size of range request blocks - or
+ // "slices" - used by the "slice" plugin. This has no effect if
+ // RangeRequestHandling does not point to exactly 3. This may never legally
+ // point to a value less than zero.
+ RangeSliceBlockSize *int `json:"rangeSliceBlockSize" db:"range_slice_block_size"`
+ // Regex Remap is a raw line to be inserted into "regex_remap.config" on the
+ // cache server. Care is necessitated in its use, because the input is in no
+ // way restricted, validated, or limited in scope to the Delivery Service.
+ RegexRemap *string `json:"regexRemap" db:"regex_remap"`
+ // RegionalGeoBlocking defines whether or not whatever Regional Geo Blocking
+ // rules are configured on the Traffic Router serving content for this
+ // Delivery Service will have an effect on the traffic of this Delivery
+ // Service.
+ RegionalGeoBlocking *bool `json:"regionalGeoBlocking" db:"regional_geo_blocking"`
+ // RemapText is raw text to insert in "remap.config" on the cache servers
+ // serving content for this Delivery Service. Care is necessitated in its
+ // use, because the input is in no way restricted, validated, or limited in
+ // scope to the Delivery Service.
+ RemapText *string `json:"remapText" db:"remap_text"`
+ // RoutingName defines the lowest-level DNS label used by the Delivery
+ // Service, e.g. if trafficcontrol.apache.org were a Delivery Service, it
+ // would have a RoutingName of "trafficcontrol".
+ RoutingName *string `json:"routingName" db:"routing_name"`
+ // ServiceCategory defines a category to which a Delivery Service may
+ // belong, which will cause HTTP Responses containing content for the
+ // Delivery Service to have the "X-CDN-SVC" header with a value that is the
+ // XMLID of the Delivery Service.
+ ServiceCategory *string `json:"serviceCategory" db:"service_category"`
+ // Signed is a legacy field. It is allowed to be `true` if and only if
+ // SigningAlgorithm is not nil.
+ Signed bool `json:"signed"`
+ // SigningAlgorithm is the name of the algorithm used to sign CDN URIs for
+ // this Delivery Service's content, or nil if no URI signing is done for the
+ // Delivery Service. This may only point to the values "url_sig" or
+ // "uri_signing".
+ SigningAlgorithm *string `json:"signingAlgorithm" db:"signing_algorithm"`
+ // SSLKeyVersion incremented whenever Traffic Portal generates new SSL keys
+ // for the Delivery Service, effectively making it a "generational" marker.
+ SSLKeyVersion *int `json:"sslKeyVersion" db:"ssl_key_version"`
+ // Tenant is the Tenant to which the Delivery Service belongs.
+ Tenant *string `json:"tenant"`
+ // TenantID is the integral, unique identifier for the Tenant to which the
+ // Delivery Service belongs.
+ TenantID *int `json:"tenantId" db:"tenant_id"`
+ // TLSVersions is the list of explicitly supported TLS versions for cache
+ // servers serving the Delivery Service's content.
+ TLSVersions []string `json:"tlsVersions" db:"tls_versions"`
+ // Topology is the name of the Topology used by the Delivery Service, or nil
+ // if no Topology is used.
+ Topology *string `json:"topology" db:"topology"`
+ // TRResponseHeaders is a set of headers (separated by CRLF pairs as per the
+ // HTTP spec) and their values (separated by a colon as per the HTTP spec)
+ // which will be sent by Traffic Router in HTTP responses to client requests
+ // for this Delivery Service's content. This has no effect on DNS-routed or
+ // un-routed Delivery Service Types.
+ TRResponseHeaders *string `json:"trResponseHeaders"`
+ // TRRequestHeaders is an "array" of HTTP headers which should be logged
+ // from client HTTP requests for this Delivery Service's content by Traffic
+ // Router, separated by newlines. This has no effect on DNS-routed or
+ // un-routed Delivery Service Types.
+ TRRequestHeaders *string `json:"trRequestHeaders"`
+ // Type describes how content is routed and cached for this Delivery Service
+ // as well as what other properties have any meaning.
+ Type *DSType `json:"type"`
+ // TypeID is an integral, unique identifier for the Tenant to which the
+ // Delivery Service belongs.
+ TypeID *int `json:"typeId" db:"type"`
+ // XMLID is a unique identifier that is also the second lowest-level DNS
+ // label used by the Delivery Service. For example, if a Delivery Service's
+ // content may be requested from video.demo1.mycdn.ciab.test, it may be
+ // inferred that the Delivery Service's XMLID is demo1.
+ XMLID *string `json:"xmlId" db:"xml_id"`
}
// DeliveryServiceV4 is a Delivery Service as it appears in version 4 of the
// Traffic Ops API - it always points to the highest minor version in APIv4.
type DeliveryServiceV4 = DeliveryServiceV40
+// These are the TLS Versions known by Apache Traffic Control to exist.
+const (
+ // Deprecated: TLS version 1.0 is known to be insecure.
+ TLSVersion10 = "1.0"
+ // Deprecated: TLS version 1.1 is known to be insecure.
+ TLSVersion11 = "1.1"
+ TLSVersion12 = "1.2"
+ TLSVersion13 = "1.3"
+)
+
+func newerDisallowedMessage(old string, newer []string) string {
+ l := len(newer)
+ if l < 1 {
+ return ""
+ }
+
+ var msg strings.Builder
+ msg.WriteString("old TLS version ")
+ msg.WriteString(old)
+ msg.WriteString(" is allowed, but newer version")
+ if l > 1 {
+ msg.WriteRune('s')
+ }
+ msg.WriteRune(' ')
+ msg.WriteString(newer[0])
+ if l > 1 {
+ msg.WriteString(", ")
+ if l > 2 {
+ msg.WriteString(newer[1])
+ msg.WriteString(", and ")
+ msg.WriteString(newer[2])
+ } else {
+ msg.WriteString("and ")
+ msg.WriteString(newer[1])
+ }
+ msg.WriteString(" are ")
+ } else {
+ msg.WriteString(" is ")
+ }
+ msg.WriteString("disallowed; this configuration may be insecure")
+
+ return msg.String()
+}
+
+// TLSVersionsAlerts generates warning-level alerts for the given TLS versions
+// array. It will warn if newer versions are disallowed while older, less
+// secure versions are allowed, or if there are unrecognized versions present.
+//
+// This does NOT verify that the passed TLS versions are _valid_, it ONLY
+// creates warnings based on conditions that are possibly detrimental to CDN
+// operation, but can, in fact, work.
+func TLSVersionsAlerts(vers []string) Alerts {
Review comment:
Update: this function (now method) is now fully responsible for generating warnings, since we're now properly checking protocol instead of type there's no database dependency at all. So it made sense to me to make it a method of the DS, but it can still easily go in TO if you think that makes more sense.
--
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:
users@infra.apache.org