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

feat: allow using custom certificates for serving #128

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

leseb
Copy link
Collaborator

@leseb leseb commented Oct 29, 2024

b6f70d4 feat: allow using custom certificates for serving

commit b6f70d4
Author: Sébastien Han [email protected]
Date: Tue Oct 29 11:51:59 2024 +0100

feat: allow using custom certificates for serving

The model serving endpoint for the judge model does not always have
verified certificates, sometimes they are self-signed. The communication
will be encrypted but the certificate security chain won't be valid. Now
we have a new `--judge-serving-model-ca-cert` flag that allows use
to use custom certificates when interacting with the judge model serving
endpoint.
The secret that holds the judge model serving details can be amended
with a new property: `JUDGE_CA_CERT: "cm-ca-cert"` to point to the
ConfigMap that contains the custom certificates bundle.

Signed-off-by: Sébastien Han <[email protected]>

@leseb leseb force-pushed the insecure-tls branch 2 times, most recently from 5262eb0 to c70b6ec Compare October 29, 2024 11:03
@leseb
Copy link
Collaborator Author

leseb commented Oct 29, 2024

@HumairAK FYI

@leseb leseb marked this pull request as draft October 29, 2024 14:04
@leseb leseb changed the title feat: allow using insecure TLS for judge endpoint feat: allow using custom certificates for serving Oct 29, 2024
@leseb leseb marked this pull request as ready for review October 29, 2024 15:47
Copy link
Contributor

@szaher szaher left a comment

Choose a reason for hiding this comment

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

we are fetching the ca-bundle from a configmap not a secret.

a small nit issue, let's name the key for the configmap either ca.crt or ca-bundle.crt or service-ca.crt instead of tls.crt not to confuse the user.

standalone/standalone.py Outdated Show resolved Hide resolved
standalone/standalone.tpl Outdated Show resolved Hide resolved
Copy link
Contributor

@szaher szaher left a comment

Choose a reason for hiding this comment

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

lgtm

@leseb
Copy link
Collaborator Author

leseb commented Oct 29, 2024

Corresponding eval PR instructlab/eval#163

standalone/standalone.py Outdated Show resolved Hide resolved
standalone/standalone.tpl Outdated Show resolved Hide resolved
standalone/standalone.py Outdated Show resolved Hide resolved
standalone/standalone.py Outdated Show resolved Hide resolved
@leseb leseb requested review from HumairAK and gmfrasca October 30, 2024 08:44
@leseb leseb force-pushed the insecure-tls branch 2 times, most recently from 911e0fa to c0342ec Compare October 30, 2024 09:34
Copy link
Contributor

@szaher szaher left a comment

Choose a reason for hiding this comment

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

lgtm

The model serving endpoint for the judge model does not always have
verified certificates, sometimes they are self-signed. The communication
will be encrypted but the certificate security chain won't be valid. Now
we have a new `--judge-serving-model-ca-cert` flag that allows use
to use custom certificates when interacting with the judge model serving
endpoint.
The secret that holds the judge model serving details can be amended
with a new property: `JUDGE_CA_CERT: "cm-ca-cert"` to point to the
ConfigMap that contains the custom certificates bundle.

Signed-off-by: Sébastien Han <[email protected]>
Copy link
Collaborator

@MichaelClifford MichaelClifford left a comment

Choose a reason for hiding this comment

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

LGTM

@MichaelClifford MichaelClifford merged commit 0fc76f3 into opendatahub-io:main Oct 30, 2024
1 check passed
@leseb leseb deleted the insecure-tls branch October 30, 2024 13:45
MichaelClifford added a commit that referenced this pull request Oct 30, 2024
feat: allow using custom certificates for serving (backport #128)
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.

5 participants