Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Fix reserved consistentHashQueryParams entries causing internal server error instead of client error #7405

Merged
zrhoffman merged 3 commits into from
Apr 5, 2023
Merged

Fix reserved consistentHashQueryParams entries causing internal server error instead of client error #7405

zrhoffman merged 3 commits into from
Apr 5, 2023

Conversation

@ocket8888
Copy link
Contributor

@ocket8888 ocket8888 commented Mar 15, 2023

This PR fixes #6385 by causing Traffic Ops to return the correct error code (400 Bad Request) and an alert describing what went wrong when a client attempts to create or modify a Delivery Service such that it would an entry in its Consistent Hashing Query string Parameters that is reserved for use by Traffic Router.

Which Traffic Control components are affected by this PR?

  • Traffic Ops

What is the best way to verify this PR?

topost -kp deliveryservices '{       
        "active": false,
        "cdnId": 2,
        "consistentHashQueryParams": ["format"],
        "displayName": "test",
        "dnsBypassCname": "test",
        "dscp": 1,
        "geoLimit": 1,
        "geoLimitCountries": "US,CA",
        "geoProvider": -7,
        "initialDispersion": 1,
        "ipv6RoutingEnabled": true,
        "logsEnabled": true,
        "missLat": 0,
        "missLong": 0,
        "multiSiteOrigin": false,
        "orgServerFqdn": "test.quest",
        "protocol": 7,
        "qstringIgnore": 5,
        "rangeRequestHandling": 0,
        "regionalGeoBlocking": false,
        "tenantId": 1,
        "typeId": 1,
        "xmlId": "test"
}'

If this is a bugfix, which Traffic Control versions contained the bug

  • not sure

PR submission checklist

  • This PR has tests
  • This PR has documentation
  • This PR has a CHANGELOG.md entry
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@ocket8888 ocket8888 added Traffic Ops related to Traffic Ops low impact affects only a small portion of a CDN, and cannot itself break one regression bug a bug in existing functionality introduced by a new version labels Mar 15, 2023
@codecov
Copy link

codecov bot commented Mar 15, 2023 •
edited
Loading

Codecov Report

Merging #7405 (03197b7) into master (904e240) will decrease coverage by 0.01%.
The diff coverage is 11.11%.

@@             Coverage Diff              @@
##             master    #7405      +/-   ##
============================================
- Coverage     27.12%   27.12%   -0.01%     
  Complexity       98       98              
============================================
  Files           685      685              
  Lines         77414    77421       +7     
  Branches         90       90              
============================================
- Hits          20999    20998       -1     
- Misses        54423    54431       +8     
  Partials       1992     1992
Flag Coverage Δ
golib_unit 48.40% <ø> (-0.01%) ⬇️
grove_unit 4.60% <ø> (ø)
t3c_unit 5.32% <ø> (ø)
traffic_monitor_unit 21.28% <ø> (ø)
traffic_ops_integration 69.42% <ø> (ø)
traffic_ops_unit 22.18% <11.11%> (-0.01%) ⬇️
traffic_stats_unit 10.14% <ø> (ø)
unit_tests 23.69% <11.11%> (-0.01%) ⬇️
v3 57.79% <ø> (ø)
v4 79.18% <ø> (ø)
v5 78.63% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/go-tc/deliveryservices.go 86.94% <ø> (ø)
...fic_ops_golang/deliveryservice/deliveryservices.go 17.67% <11.11%> (-0.06%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

zrhoffman
zrhoffman suggested changes Mar 29, 2023
View reviewed changes
@zrhoffman zrhoffman merged commit 94d9850 into apache:master Apr 5, 2023
@ocket8888 ocket8888 deleted the to/fix-chqp-ise branch April 5, 2023 21:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@zrhoffman zrhoffman zrhoffman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

low impact affects only a small portion of a CDN, and cannot itself break one regression bug a bug in existing functionality introduced by a new version Traffic Ops related to Traffic Ops

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Reserved consistentHashQueryParameters cause internal server error

2 participants

@ocket8888 @zrhoffman