Skip to content

Commit

Permalink
Fix APIv4.0 returning APIv4.1 DS structures (#7156)
Browse files Browse the repository at this point in the history
* Fix APIv4.0 returning APIv4.1 DS structures

* Fix compilation issue in TRUTH

* Fix version matching in read handler

* Fix improper struct version used in client tests
  • Loading branch information
ocket8888 committed Oct 26, 2022
1 parent 33b86c5 commit 56b9d3c
Show file tree
Hide file tree
Showing 13 changed files with 454 additions and 304 deletions.
4 changes: 2 additions & 2 deletions cache-config/t3cutil/toreq/conversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ func serverToLatest(oldSv *tc.ServerV40) (*atscfg.Server, error) {
return &asv, nil
}

func dsesToLatest(dses []tc.DeliveryServiceV40) []atscfg.DeliveryService {
return atscfg.V40ToDeliveryServices(dses)
func dsesToLatest(dses []tc.DeliveryServiceV4) []atscfg.DeliveryService {
return atscfg.V4ToDeliveryServices(dses)
}

func jobsToLatest(jobs []tc.InvalidationJobV4) []atscfg.InvalidationJob {
Expand Down
2 changes: 1 addition & 1 deletion cache-config/t3cutil/toreq/toreqold/conversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func serverToLatest(oldSv *tc.ServerV30) (*atscfg.Server, error) {
}

func dsesToLatest(dses []tc.DeliveryServiceNullableV30) []atscfg.DeliveryService {
newDSes := []tc.DeliveryServiceV40{}
newDSes := []tc.DeliveryServiceV4{}
for _, ds := range dses {
newDSes = append(newDSes, ds.UpgradeToV4())
}
Expand Down
6 changes: 3 additions & 3 deletions lib/go-atscfg/atscfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type Server tc.ServerV40
// DeliveryService is a tc.DeliveryService for the latest lib/go-tc and traffic_ops/vx-client type.
// This allows atscfg to not have to change the type everywhere it's used, every time ATC changes the base type,
// but to only have to change it here, and the places where breaking symbol changes were made.
type DeliveryService tc.DeliveryServiceV40
type DeliveryService tc.DeliveryServiceV4

// InvalidationJob is a tc.InvalidationJob for the latest lib/go-tc and traffic_ops/vx-client type.
// This allows atscfg to not have to change the type everywhere it's used, every time ATC changes the base type,
Expand All @@ -72,7 +72,7 @@ type InvalidationJob tc.InvalidationJobV4
type ServerUpdateStatus tc.ServerUpdateStatusV4

// ToDeliveryServices converts a slice of the latest lib/go-tc and traffic_ops/vx-client type to the local alias.
func ToDeliveryServices(dses []tc.DeliveryServiceV40) []DeliveryService {
func ToDeliveryServices(dses []tc.DeliveryServiceV4) []DeliveryService {
ad := make([]DeliveryService, 0, len(dses))
for _, ds := range dses {
ad = append(ad, DeliveryService(ds))
Expand All @@ -81,7 +81,7 @@ func ToDeliveryServices(dses []tc.DeliveryServiceV40) []DeliveryService {
}

// V40ToDeliveryServices converts a slice of the old traffic_ops/v4-client type to the local alias.
func V40ToDeliveryServices(dses []tc.DeliveryServiceV40) []DeliveryService {
func V4ToDeliveryServices(dses []tc.DeliveryServiceV4) []DeliveryService {
ad := make([]DeliveryService, 0, len(dses))
for _, ds := range dses {
ad = append(ad, DeliveryService(ds))
Expand Down
99 changes: 75 additions & 24 deletions lib/go-tc/deliveryservices.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,17 @@ type DeliveryServicesResponseV40 struct {
Alerts
}

// DeliveryServicesResponseV41 is the type of a response from the
// /api/4.1/deliveryservices Traffic Ops endpoint.
type DeliveryServicesResponseV41 struct {
Response []DeliveryServiceV41 `json:"response"`
Alerts
}

// DeliveryServicesResponseV4 is the type of a response from the
// /api/4.x/deliveryservices Traffic Ops endpoint.
// It always points to the type for the latest minor version of APIv4.
type DeliveryServicesResponseV4 = DeliveryServicesResponseV40
type DeliveryServicesResponseV4 = DeliveryServicesResponseV41

// DeliveryServicesNullableResponse roughly models the structure of responses
// from Traffic Ops to GET requests made to its
Expand Down Expand Up @@ -253,9 +260,20 @@ type DeliveryServiceV40 struct {
GeoLimitCountries GeoLimitCountriesType `json:"geoLimitCountries"`
}

// DeliveryServiceV41 is a Delivery Service as it appears in version 4.1 of the
// Traffic Ops API.
type DeliveryServiceV41 struct {
DeliveryServiceV40

// Regional indicates whether the Delivery Service's MaxOriginConnections is
// only per Cache Group, rather than divided over all Cache Servers in child
// Cache Groups of the Origin.
Regional bool `json:"regional" db:"regional"`
}

// 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
type DeliveryServiceV4 = DeliveryServiceV41

// These are the TLS Versions known by Apache Traffic Control to exist.
const (
Expand Down Expand Up @@ -301,17 +319,7 @@ func newerTLSVersionsDisallowedMessage(old string, newer []string) string {
return msg.String()
}

// TLSVersionsAlerts generates warning-level alerts for the Delivery Service's
// TLS versions array. It will warn if newer versions are disallowed while
// older, less secure versions are allowed, if there are unrecognized versions
// present, if the Delivery Service's Protocol does not make use of TLS
// Versions, and whenever TLSVersions are explicitly set at all.
//
// This does NOT verify that the Delivery Service's TLS versions are _valid_,
// it ONLY creates warnings based on conditions that are possibly detrimental
// to CDN operation, but can, in fact, work.
func (ds DeliveryServiceV4) TLSVersionsAlerts() Alerts {
vers := ds.TLSVersions
func tlsVersionsAlerts(vers []string, protocol int) Alerts {
messages := []string{}

if len(vers) > 0 {
Expand Down Expand Up @@ -380,13 +388,46 @@ func (ds DeliveryServiceV4) TLSVersionsAlerts() Alerts {
}
}

if ds.Protocol != nil && *ds.Protocol == DSProtocolHTTP {
if protocol == DSProtocolHTTP {
messages = append(messages, "tlsVersions has no effect on Delivery Services with Protocol '0' (HTTP_ONLY)")
}

return CreateAlerts(WarnLevel, messages...)
}

// TLSVersionsAlerts generates warning-level alerts for the Delivery Service's
// TLS versions array. It will warn if newer versions are disallowed while
// older, less secure versions are allowed, if there are unrecognized versions
// present, if the Delivery Service's Protocol does not make use of TLS
// Versions, and whenever TLSVersions are explicitly set at all.
//
// This does NOT verify that the Delivery Service's TLS versions are _valid_,
// it ONLY creates warnings based on conditions that are possibly detrimental
// to CDN operation, but can, in fact, work.
func (ds DeliveryServiceV40) TLSVersionsAlerts() Alerts {
vers := ds.TLSVersions
var protocol int
if ds.Protocol != nil {
protocol = *ds.Protocol
} else {
protocol = -1
}
return tlsVersionsAlerts(vers, protocol)
}

// TLSVersionsAlerts generates warning-level alerts for the Delivery Service's
// TLS versions array. It will warn if newer versions are disallowed while
// older, less secure versions are allowed, if there are unrecognized versions
// present, if the Delivery Service's Protocol does not make use of TLS
// Versions, and whenever TLSVersions are explicitly set at all.
//
// This does NOT verify that the Delivery Service's TLS versions are _valid_,
// it ONLY creates warnings based on conditions that are possibly detrimental
// to CDN operation, but can, in fact, work.
func (ds DeliveryServiceV41) TLSVersionsAlerts() Alerts {
return ds.DeliveryServiceV40.TLSVersionsAlerts()
}

// DeliveryServiceV30 represents a Delivery Service as they appear in version
// 3.0 of the Traffic Ops API.
//
Expand Down Expand Up @@ -843,7 +884,16 @@ type DeliveryServiceRemovedFieldsV11 struct {
CacheURL *string `json:"cacheurl" db:"cacheurl"`
}

// RemoveLD1AndLD2 removes the Long Description 1 and Long Description 2 fields from a V 4.x DS, and returns the resulting struct.
// RemoveLD1AndLD2 removes the Long Description 1 and Long Description 2 fields
// from a V4.0 Delivery Service, and returns the resulting struct.
func (ds *DeliveryServiceV40) RemoveLD1AndLD2() DeliveryServiceV40 {
ds.LongDesc1 = nil
ds.LongDesc2 = nil
return *ds
}

// RemoveLD1AndLD2 removes the Long Description 1 and Long Description 2 fields
// from a V 4.x DS, and returns the resulting struct.
func (ds *DeliveryServiceV4) RemoveLD1AndLD2() DeliveryServiceV4 {
ds.LongDesc1 = nil
ds.LongDesc2 = nil
Expand Down Expand Up @@ -883,18 +933,19 @@ func (ds *DeliveryServiceNullableV30) UpgradeToV4() DeliveryServiceV4 {
var geo GeoLimitCountriesType
if ds.GeoLimitCountries != nil {
str := *ds.GeoLimitCountries
geo = make([]string, 0)
geo = strings.Split(str, ",")
}
return DeliveryServiceV4{
DeliveryServiceFieldsV31: ds.DeliveryServiceFieldsV31,
DeliveryServiceFieldsV30: ds.DeliveryServiceFieldsV30,
DeliveryServiceFieldsV15: ds.DeliveryServiceFieldsV15,
DeliveryServiceFieldsV14: ds.DeliveryServiceFieldsV14,
DeliveryServiceFieldsV13: ds.DeliveryServiceFieldsV13,
DeliveryServiceNullableFieldsV11: ds.DeliveryServiceNullableFieldsV11,
TLSVersions: nil,
GeoLimitCountries: geo,
DeliveryServiceV40: DeliveryServiceV40{
DeliveryServiceFieldsV31: ds.DeliveryServiceFieldsV31,
DeliveryServiceFieldsV30: ds.DeliveryServiceFieldsV30,
DeliveryServiceFieldsV15: ds.DeliveryServiceFieldsV15,
DeliveryServiceFieldsV14: ds.DeliveryServiceFieldsV14,
DeliveryServiceFieldsV13: ds.DeliveryServiceFieldsV13,
DeliveryServiceNullableFieldsV11: ds.DeliveryServiceNullableFieldsV11,
TLSVersions: nil,
GeoLimitCountries: geo,
},
}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/go-tc/deliveryservices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ func BenchmarkTLSVersionsAlerts(b *testing.B) {
versions = append(versions, fmt.Sprintf("%d.%d", major, minor))
}
}
ds := DeliveryServiceV4{TLSVersions: versions}
ds := DeliveryServiceV40{TLSVersions: versions}

b.ReportAllocs()
b.ResetTimer()
Expand Down
6 changes: 3 additions & 3 deletions traffic_ops/testing/api/v4/deliveryservices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ func TestDeliveryServices(t *testing.T) {

func validateDSExpectedFields(expectedResp map[string]interface{}) utils.CkReqFunc {
return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
dsResp := resp.([]tc.DeliveryServiceV40)
dsResp := resp.([]tc.DeliveryServiceV4)
for field, expected := range expectedResp {
for _, ds := range dsResp {
switch field {
Expand Down Expand Up @@ -658,7 +658,7 @@ func validateDSExpectedFields(expectedResp map[string]interface{}) utils.CkReqFu

func validatePagination(paginationParam string) utils.CkReqFunc {
return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
paginationResp := resp.([]tc.DeliveryServiceV40)
paginationResp := resp.([]tc.DeliveryServiceV4)

opts := client.NewRequestOptions()
opts.QueryParameters.Set("orderby", "id")
Expand All @@ -680,7 +680,7 @@ func validatePagination(paginationParam string) utils.CkReqFunc {

func validateDescSort() utils.CkReqFunc {
return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, alerts tc.Alerts, _ error) {
dsDescResp := resp.([]tc.DeliveryServiceV40)
dsDescResp := resp.([]tc.DeliveryServiceV4)
var descSortedList []string
var ascSortedList []string
assert.GreaterOrEqual(t, len(dsDescResp), 2, "Need at least 2 XMLIDs in Traffic Ops to test desc sort, found: %d", len(dsDescResp))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func validateServersDeliveryServices(expectedDSID int) utils.CkReqFunc {
return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
assert.RequireNotNil(t, resp, "Expected Server Delivery Service response to not be nil.")
var found bool
deliveryServices := resp.([]tc.DeliveryServiceV40)
deliveryServices := resp.([]tc.DeliveryServiceV4)
for _, ds := range deliveryServices {
if ds.ID != nil && *ds.ID == expectedDSID {
found = true
Expand Down
6 changes: 3 additions & 3 deletions traffic_ops/testing/api/v5/deliveryservices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ func TestDeliveryServices(t *testing.T) {

func validateDSExpectedFields(expectedResp map[string]interface{}) utils.CkReqFunc {
return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
dsResp := resp.([]tc.DeliveryServiceV40)
dsResp := resp.([]tc.DeliveryServiceV4)
for field, expected := range expectedResp {
for _, ds := range dsResp {
switch field {
Expand Down Expand Up @@ -658,7 +658,7 @@ func validateDSExpectedFields(expectedResp map[string]interface{}) utils.CkReqFu

func validatePagination(paginationParam string) utils.CkReqFunc {
return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
paginationResp := resp.([]tc.DeliveryServiceV40)
paginationResp := resp.([]tc.DeliveryServiceV4)

opts := client.NewRequestOptions()
opts.QueryParameters.Set("orderby", "id")
Expand All @@ -680,7 +680,7 @@ func validatePagination(paginationParam string) utils.CkReqFunc {

func validateDescSort() utils.CkReqFunc {
return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, alerts tc.Alerts, _ error) {
dsDescResp := resp.([]tc.DeliveryServiceV40)
dsDescResp := resp.([]tc.DeliveryServiceV4)
var descSortedList []string
var ascSortedList []string
assert.GreaterOrEqual(t, len(dsDescResp), 2, "Need at least 2 XMLIDs in Traffic Ops to test desc sort, found: %d", len(dsDescResp))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func validateServersDeliveryServices(expectedDSID int) utils.CkReqFunc {
return func(t *testing.T, _ toclientlib.ReqInf, resp interface{}, _ tc.Alerts, _ error) {
assert.RequireNotNil(t, resp, "Expected Server Delivery Service response to not be nil.")
var found bool
deliveryServices := resp.([]tc.DeliveryServiceV40)
deliveryServices := resp.([]tc.DeliveryServiceV4)
for _, ds := range deliveryServices {
if ds.ID != nil && *ds.ID == expectedDSID {
found = true
Expand Down

0 comments on commit 56b9d3c

Please sign in to comment.