-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
enable postgres filter extension #4328
Conversation
Signed-off-by: Faseela K <[email protected]>
😊 Welcome @kfaseela! This is either your first contribution to the Istio proxy repo, or it's been You can learn more about the Istio working groups, code of conduct, and contributing guidelines Thanks for contributing! Courtesy of your friendly welcome wagon. |
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.
approved after accepting DLB and other contrib accelerators.
I thought we agreed to deploy this in prod using contrib build istio/istio#42380 (comment) and then based on that we want to move forward. Has that changed? @howardjohn @hzxuzhonghu WDYT? |
@ramaraochavali This was tested by adding in ISTIO_ENABLED_CONTRIB_EXTENSIONS as in this PR, and is working fine, I will add the details on how the testing was done to the original issue. Once this is done, probably we have to work on Istio standard APIs to use postgres, but that can be a different PR and a bigger activity? Until then we can have in CONTRIB_EXTENSIONS which we already have for many other extensions? If I have to add atleast a task in istio documentation showing how this works, I would need this PR. |
It does not really matter whether it is in contrib extensions or not - it will be part of main Istio proxy build. All I am saying is we need a process around brining Envoy contrib extensions to Istio (they are there in contrib in Envoy for a reason). Some of the extensions already have an API (for example cryptoMB) and IMO whatever contrib extensions we bring here, we should have a plan to work with Envoy community to move it main so that they have proper test coverage etc.etc. |
/retest |
Signed-off-by: Faseela K [email protected]
Fixes #42380