-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
SDK - Removed the build_image parameter from build_python_component function #1657
SDK - Removed the build_image parameter from build_python_component function #1657
Conversation
/test kubeflow-pipeline-sample-test |
…onent function This parameter has never worked and has no useful purpose.
96da012
to
c67d9d4
Compare
/retest |
1 similar comment
/retest |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Ark-kun The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
/retest |
2 similar comments
/retest |
/retest |
…eter-from-build_python_component-function
/retest |
1 similar comment
/retest |
/lgtm |
* Add SANs to server.crt On my testing environment (debian 11, openssl 1.1.1k, minikube with k8s 1.20, go 1.15, kfserving 0.5) I get some TLS errors when testing kfserving, namely "x509: certificate relies on legacy Common Name field, use SANs or temporarily enable Common Name matching" when trying to deploy a simple InferenceService resource. After some digging I found this problem: ``` openssl req -noout -text -in server.csr | grep "Subject Alternative Name" -A 1 X509v3 Subject Alternative Name: DNS:kfserving-webhook-server-service, DNS:kfserving-webhook-server-service.kfserving-system, DNS:kfserving-webhook-server-service.kfserving-system.svc, DNS:kfserving-webhook-server-service.kfserving-system.svc.cluster, DNS:kfserving-webhook-server-service.kfserving-system.svc.cluster.local openssl x509 -in server.crt -text -noout | grep "Subject Alternative Name" -A 1 ``` Meanwhile if I add this patch: ``` openssl x509 -in server.crt -text -noout | grep "Subject Alternative Name" -A 1 X509v3 Subject Alternative Name: DNS:kfserving-webhook-server-service, DNS:kfserving-webhook-server-service.kfserving-system, DNS:kfserving-webhook-server-service.kfserving-system.svc, DNS:kfserving-webhook-server-service.kfserving-system.svc.cluster, DNS:kfserving-webhook-server-service.kfserving-system.svc.cluster.local ``` As far as I got go 1.15 introduces a stricter policy for TLS certs, and openssl by default doesn't copy SANs from a .csr to .crt if not explicitly told. gh: kubeflow#1657 * Add patch for conversion webhook in self-signed-ca.sh Co-authored-by: haijohn <haijohnzju@gmail.com> gh: kubeflow#1657 Co-authored-by: haijohn <haijohnzju@gmail.com>
Fixes #1661
This parameter seems to be unused and it's not clear what's the use case for it.
This change is