-
Notifications
You must be signed in to change notification settings - Fork 5
feat(ingress): automatic host and path discovery #49
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
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
3c63e48
feat(ingress): iterate over hosts and paths
akosveres 8ddd7fb
docs(ingress): Update docs and sample
akosveres c2c73b1
build(trivy): use ECR repository
akosveres b138673
build(gha): add smenatic-validate PR step
akosveres 05c21d4
fix(annotations): handle the resource disable state
akosveres 0f4309c
chore(format): quality of code improvements
akosveres 65c2f64
chore(labels): remove unused label definition
akosveres a86f2b3
fix(annotations): change path logic
akosveres e9da870
fix: append variable
akosveres e2175eb
fix: remove reduntant unused line
akosveres File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
With that it seems we can add only once custom
endpoint+path+status. I'd suggest to have one annotation that can define multiple targets. E.g.:Even better, maybe include JSON in the value that is directly tied to API.
JSON for flexibility, simple syntax for UX. Should be decided, but I'd move from ability to set only one endpoint.
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.
100% agree that we need something, not sure what the best path forward is, I'd hate to manage JSON inside YAML..
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.
Then we need to see if we can find best simple abstraction of what we want to achieve here and not be slapped in the face because it does not support some basic functionality. I'd still have API reference in mind here to make sure we cover basics.
What do you propose?
Also, what would be the minimum (original) API JSON request for hostname+path? Just to see verbosity. Benefit here is the ability to support those kind of annotations even in Service resources and documentation is already available.
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.
I suggest here we write some requirement and improve it.
We can also end up with a new CRD where we can specify specific options for each resource, including auth and etc.