-
Notifications
You must be signed in to change notification settings - Fork 108
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
HTTPSO-based Routing Table #669
Conversation
@JorTurFer @tomkerkhove, sorry for the delay! Here we have the new Routing Table implementation. I'll be adding new tests and fixing existing ones in the following days. I would appreciate reviews in the meantime. I left the support for path-based routing out of this PR because it alredy changes a lot of stuff, but I architected the new RT with that feature in mind. I'll be working on that next. |
df71c09
to
dcee452
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking fantastic!
Some comments inline
I'll review it deeper during next days
Hi @t0rr3sp3dr0 Thanks and well done! |
@similark, it is WIP and I should have a PR by Monday. On this very first version we will only allow one path per HTTPScaledObject, every URL that matches such prefix will be routed to the target specified on the manifest. Is that enough for your use case? The 2nd version will allow multiple paths to be directed to a single target. But this would be better implemented by having a new API Version of HTTPScaledObject and that needs to be discussed an may take longer to be implemented. |
b9e465b
to
1487432
Compare
2bb2d49
to
d402fba
Compare
Signed-off-by: Pedro Tôrres <[email protected]>
d402fba
to
4c81994
Compare
Signed-off-by: Pedro Tôrres <[email protected]>
Signed-off-by: Pedro Tôrres <[email protected]>
Signed-off-by: Pedro Tôrres <[email protected]>
Signed-off-by: Pedro Tôrres <[email protected]>
Signed-off-by: Pedro Tôrres <[email protected]>
Signed-off-by: Pedro Tôrres <[email protected]>
Signed-off-by: Pedro Tôrres <[email protected]>
Hi! I've just started using this addon and this feature is really crucial as well as #677. |
@andrey-bondar, this patch is currently feature-complete and I’m adding more tests while I wait for an in-depth review. The review may take some time, given the size of the PR and that we currently have a small number of maintainers, but it is being worked on. In the meantime, feel free to leave a review as well. |
I have checked the PR and looks good from coding pov, but I'd like to review it in depth with calm to understand correctly what is happening |
Signed-off-by: Pedro Tôrres <[email protected]>
Signed-off-by: Pedro Tôrres <[email protected]>
Signed-off-by: Pedro Tôrres <[email protected]>
Signed-off-by: Pedro Tôrres <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have done an awesome job @t0rr3sp3dr0, I have left some small comments inline
Signed-off-by: Pedro Tôrres <[email protected]>
LGTM! |
Signed-off-by: Pedro Tôrres <[email protected]>
Reimplements routing table of the HTTP Add-On to make it using HTTPSO objects as the source of truth instead of relying on a ConfigMap that is updated periodicaly by the operator.
Highlights:
Checklist
README.md
docs/
directoryFixes #605