-
Notifications
You must be signed in to change notification settings - Fork 129
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
Simplify version detection #329
Conversation
Codecov Report
@@ Coverage Diff @@
## main #329 +/- ##
==========================================
+ Coverage 52.58% 52.72% +0.14%
==========================================
Files 60 60
Lines 4874 4874
==========================================
+ Hits 2563 2570 +7
+ Misses 2013 2002 -11
- Partials 298 302 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
The access to the version of kong shall be moved to the go-kong but there is a need for the client to be aware of the use of a workspace. There could also be a service to expose every versioned capabilities of kong like tags for credentials in deck or mtls authentication in kubernetes ingress controller. This way the semver dependency would only be required in go-kong |
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.
Thanks @mmorel-35 for looking into this.
There could also be a service to expose every versioned capabilities of kong
I think that would be the best approach for future maintainability; @rainest @shaneutt what do you think?
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.
See the comments/suggestions below, and please ensure that this works with Kong OSS and Enterprise (both with workspaces used with RBAC preventing access to the non-workspaced root and not). Unfortunately we don't have automated regression tests capturing that, but we need to ensure that we haven't broken anything in those scenarios before merging.
… community edition integration tests
@mflendrich Did you mean to imply that we should add that in here before merging in? |
Following my suggestion from last week I created a PR on go-kong. So that the It doesn't mean that this pr shouln'd be merged but if the go-Kong's one is accepted then a part of this will be replaced in the next version of go-kong. |
@mmorel-35, with the Kong/go-kong#65 PR moving the API machinery into go-kong, which parts of this PR are still relevant? Should we close it and follow up on Kong/go-kong#65 by using that API when that PR gets merged in? On integration testing: we're tracking the direction for integration testing in deck in #63. I'll post this PR there as prior work, but I don't think we're ready to merge the test yet. That being said, I think the best way forward would be to close this PR and follow up with:
|
I agree, this PR can be closed ! I don't think there is much left to keep with what's coming in go-kong. |
Fixes #324