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

tests added and enable self signed certs #69 #78

Merged
merged 6 commits into from
Jun 18, 2024

Conversation

alanbraz
Copy link
Collaborator

@alanbraz alanbraz commented Jun 13, 2024

using a single bundle cert was already implemented at

if let Some(Tls::Config(tls_config)) = &service_config.tls {

I tested running the local PII with mTLS and self signed certificated.

config/config.yaml Outdated Show resolved Hide resolved
src/clients.rs Outdated Show resolved Hide resolved
@evaline-ju
Copy link
Collaborator

evaline-ju commented Jun 13, 2024

using a single bundle cert was already implemented at

if let Some(Tls::Config(tls_config)) = &service_config.tls {

I tested running the local PII with mTLS and self signed certificated.

I'm a bit confused how this shows mTLS is already implemented here - what is in this "bundle cert" used that has all the components needed to do mutual authentication?

For the grpc clients there are also currently 3 fields expected (cert, key, ca) and for users just deploying this and using config, I feel we shouldn't need to make it obvious that "detectors" are using http clients for now and thus configure this way, vs "chunkers" and "generation" use grpc clients for now - and thus configure with non-bundled fields. As much as possible the expectations on "what fields need to be provided" should be as consistent as much as possible.

@alanbraz alanbraz mentioned this pull request Jun 14, 2024
4 tasks
@alanbraz
Copy link
Collaborator Author

All requested changes made and finished implementation.

Copy link
Collaborator

@evaline-ju evaline-ju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please check the linting failures in the build?

src/clients.rs Outdated Show resolved Hide resolved
src/clients.rs Outdated Show resolved Hide resolved
alanbraz and others added 3 commits June 14, 2024 14:13
Co-authored-by: Evaline Ju <69598118+evaline-ju@users.noreply.github.com>
@alanbraz
Copy link
Collaborator Author

@evaline-ju I think it is good for review now!

@evaline-ju evaline-ju linked an issue Jun 14, 2024 that may be closed by this pull request
4 tasks
src/clients.rs Outdated Show resolved Hide resolved
src/config.rs Show resolved Hide resolved
@gkumbhat gkumbhat merged commit ab1bc23 into foundation-model-stack:main Jun 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update HTTP client creation for mTLS
4 participants