Skip to content
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

Merged
merged 1 commit into from
Jan 10, 2023
Merged

Conversation

kfaseela
Copy link
Member

@kfaseela kfaseela commented Jan 10, 2023

Signed-off-by: Faseela K [email protected]

Fixes #42380

@kfaseela kfaseela requested a review from a team as a code owner January 10, 2023 09:02
@istio-policy-bot
Copy link

😊 Welcome @kfaseela! This is either your first contribution to the Istio proxy repo, or it's been
awhile since you've been here.

You can learn more about the Istio working groups, code of conduct, and contributing guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 10, 2023
Copy link
Member

@zirain zirain left a 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.

@ramaraochavali
Copy link

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?

@kfaseela
Copy link
Member Author

kfaseela commented Jan 10, 2023

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.

@ramaraochavali
Copy link

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.

@kfaseela
Copy link
Member Author

/retest

@istio-testing istio-testing merged commit 727b243 into istio:master Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Register postgres proxy filter API in Istio
5 participants