jkirschner-hashicorp
commented
3 years ago Hi @chrisjohnson,
Thank you for the detailed, thoughtful feedback on this and many other Github issues - we really appreciate it.
We'll look into this more and consider what should be improved (e.g., docs/examples, handling of PrefixRewrite).
In the meantime, as a workaround, are you able to achieve what you want by appending a / to the end of your PathPrefix and setting PrefixRewrite to /? Example:
{
"Match": {
"HTTP": {
"PathPrefix": "/sp/"
}
},
"Destination": {
"Service": "sp-api",
"Namespace": "testing-rx8",
"PrefixRewrite": "/"
}
},This won't work if you need the exact path /sp to map to /, but it seems like it would map any path of the form /sp/* to /*.
Regarding your points above:
-
I'm also not sure why the blog post you linked includes
PrefixRewriteinRoutes.Match.HTTP. A docs PR from a month after that blog entry was posted showsPrefixRewritein the same location is it now:Routes.Destination. I may try to see if that was ever accurate. Regardless, a corrected version of that example seems like a useful addition to the service-router page's sample config entries section. -
My initial expectation would have also been that
/sp => /would work as expected, though I can also see why that results in/sp/* => //*. Going forward, we can think about it there's a better way to handle this case (e.g., perhaps ignore the leading/for rewrite purposes, so there's no//results ifPrefixRewrite = '/'). -
I haven't dug into this, but my guess is: because
PrefixRewritehas a default value of"", the code doesn't make a distinction between an explicitly set value of""and an unset value. Thus, theservice-routerconfig is seen as valid with an "unset"PrefixRewrite, which is consistent with what you observed.
chrisjohnson
Overview of the Issue
The docs for using PrefixRewrite are not great and it seems like there's a bug that won't allow me to use empty string as a value
1: I don't actually see where in the docs PrefixRewrite is actually clearly explained. I found the doc entry for it but it doesn't explain how it's actually used. This blog post is the only example for PrefixRewrite that I can find, and it does not have the structure that is currently in use (I'm assuming the structure changed since then?)
I did reverse engineer the logic by looking at the golang tests that involve PrefixRewrite and I was able to deduce that the PrefixRewrite value is used as the direct replacement value for whatever PathPrefix matched
2: That said, the example on that blog post as well as the golang tests all seem to use an example that works like this:
However this results in a request like this:
curl localhost:21001/sp/bar->//barThis is confusing. I understand that technically it is valid from consul's POV but it produces strange behavior where the request ends up with an extra slash. The tests should be changed and an example should be added to the documentation page
3: Finally, I think there may actually be a bug, because when I try to produce the expected result by using the following configuration:
It won't let me use an empty string for PrefixRewrite -- when I read the config back it shows that the PrefixRewrite was simply eliminated:
Consul info for both Client and Server
Client info
``` agent: check_monitors = 0 check_ttls = 0 checks = 87 services = 58 build: prerelease = revision = cf701c09 version = 1.9.7+ent consul: acl = enabled known_servers = 10 server = false license: customer = 480e18da-6001-8ce2-7b8e-8642e1cb3225 expiration_time = 2023-03-25 00:00:00 +0000 UTC features = Automated Backups, Automated Upgrades, Enhanced Read Scalability, Network Segments, Redundancy Zone, Advanced Network Federation, Namespaces, SSO, Audit Logging id = eda50725-0f08-ccaf-f3e3-b26cae0812d4 install_id = * issue_time = 2021-03-25 04:56:39.925162515 +0000 UTC modules = Global Visibility, Routing and Scale, Governance and Policy product = consul start_time = 2021-03-24 00:00:00 +0000 UTC runtime: arch = amd64 cpu_count = 4 goroutines = 1017 max_procs = 4 os = linux version = go1.15.13 serf_lan: coordinate_resets = 0 encrypted = true event_queue = 0 event_time = 108 failed = 0 health_score = 0 intent_queue = 0 left = 0 member_time = 34097 members = 12 query_queue = 0 query_time = 862 ```Server info
``` agent: check_monitors = 0 check_ttls = 1 checks = 1 services = 1 build: prerelease = revision = cf701c09 version = 1.9.7+ent consul: acl = enabled bootstrap = false known_datacenters = 4 leader = false leader_addr = 10.3.52.53:8300 server = true license: customer = 480e18da-6001-8ce2-7b8e-8642e1cb3225 expiration_time = 2023-03-25 00:00:00 +0000 UTC features = Automated Backups, Automated Upgrades, Enhanced Read Scalability, Network Segments, Redundancy Zone, Advanced Network Federation, Namespaces, SSO, Audit Logging id = eda50725-0f08-ccaf-f3e3-b26cae0812d4 install_id = * issue_time = 2021-03-25 04:56:39.925162515 +0000 UTC modules = Global Visibility, Routing and Scale, Governance and Policy product = consul start_time = 2021-03-24 00:00:00 +0000 UTC raft: applied_index = 646355 commit_index = 646355 fsm_pending = 0 last_contact = 41.753905ms last_log_index = 646355 last_log_term = 7804 last_snapshot_index = 629975 last_snapshot_term = 7796 latest_configuration = [{Suffrage:Nonvoter ID:53fca35b-cd1a-0800-b6ee-c60527ca7313 Address:10.3.52.41:8300} {Suffrage:Voter ID:0ac1c505-c157-220a-8d8a-4af254d3d4f0 Address:10.3.52.42:8300} {Suffrage:Voter ID:c2498167-5c9c-18e0-be21-4595f7a20ae4 Address:10.3.52.55:8300} {Suffrage:Nonvoter ID:4a1ae97c-4e4f-be48-19b7-bc92b3742e20 Address:10.3.52.56:8300} {Suffrage:Voter ID:64e0f24d-a51f-c82e-17aa-2eb20c165617 Address:10.3.52.14:8300} {Suffrage:Nonvoter ID:c3809b2c-e13a-662f-03da-7f7eccd04edb Address:10.3.52.52:8300} {Suffrage:Nonvoter ID:769747f8-e741-25e1-f5f3-4714706db717 Address:10.3.52.35:8300} {Suffrage:Voter ID:107fc5ce-dbf6-1b5f-2cfa-263608ec691b Address:10.3.52.53:8300} {Suffrage:Nonvoter ID:f5ef8ba0-a971-2aa6-1eb8-d0e0e034f5ac Address:10.3.52.54:8300} {Suffrage:Voter ID:0ee8fe98-540f-b67b-878c-1b50f91a9d09 Address:10.3.52.36:8300}] latest_configuration_index = 0 num_peers = 0 protocol_version = 3 protocol_version_max = 3 protocol_version_min = 0 snapshot_version_max = 1 snapshot_version_min = 0 state = Follower term = 7804 runtime: arch = amd64 cpu_count = 2 goroutines = 285 max_procs = 2 os = linux version = go1.15.13 serf_lan: coordinate_resets = 0 encrypted = true event_queue = 0 event_time = 24 failed = 0 health_score = 0 intent_queue = 0 left = 0 member_time = 1098 members = 10 query_queue = 0 query_time = 1 serf_wan: coordinate_resets = 0 encrypted = true event_queue = 0 event_time = 1 failed = 0 health_score = 0 intent_queue = 0 left = 0 member_time = 3766 members = 33 query_queue = 0 query_time = 862 ```