Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with persisting proxy-defaults #20481

Merged
merged 2 commits into from
Feb 5, 2024
Merged

Fix issue with persisting proxy-defaults #20481

merged 2 commits into from
Feb 5, 2024

Conversation

hashi-derek
Copy link
Member

@hashi-derek hashi-derek commented Feb 5, 2024

This resolves an issue introduced in #19829 where the proxy-defaults configuration entry with an HTTP protocol cannot be updated after it has been persisted once and a router exists. This occurs because the protocol field is not properly pre-computed before being passed into validation functions and will result in a protocol mis-match error message (the new proxy-defaults will have an assumed protocol of tcp which will conflict with the router's need for http).

This PR hoists the protocol computing logic into the Normalize function of the config entry, which will ensure that it is properly populated prior to the validation function executing.

This resolves an issue introduced in #19829
where the proxy-defaults configuration entry with an HTTP protocol
cannot be updated after it has been persisted once and a router
exists. This occurs because the protocol field is not properly
pre-computed before being passed into validation functions.
@hashi-derek hashi-derek added backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. backport/1.16 This release series is no longer active on CE. Use backport/ent/1.16. backport/1.17 This release series is no longer active on CE. Use backport/ent/1.17. backport/1.18 labels Feb 5, 2024
@hashi-derek hashi-derek marked this pull request as ready for review February 5, 2024 20:55
@hashi-derek hashi-derek requested a review from a team as a code owner February 5, 2024 20:55
Copy link
Contributor

@curtbushko curtbushko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@david-yu david-yu linked an issue Feb 5, 2024 that may be closed by this pull request
@hashi-derek hashi-derek merged commit 922844b into main Feb 5, 2024
90 checks passed
@hashi-derek hashi-derek deleted the derekm/NET-7652 branch February 5, 2024 22:00
@hc-github-team-consul-core
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. backport/1.16 This release series is no longer active on CE. Use backport/ent/1.16. backport/1.17 This release series is no longer active on CE. Use backport/ent/1.17.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect validation of proxy-defaults update
4 participants