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

SDK - Removed the build_image parameter from build_python_component function #1657

Conversation

Ark-kun
Copy link
Contributor

@Ark-kun Ark-kun commented Jul 23, 2019

Fixes #1661
This parameter seems to be unused and it's not clear what's the use case for it.


This change is Reviewable

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Jul 24, 2019

/test kubeflow-pipeline-sample-test

@Ark-kun Ark-kun changed the title SDK - Removed the broken build_image parameter from build_python_component function SDK - Removed the build_image parameter from build_python_component function Jul 24, 2019
…onent function

This parameter has never worked and has no useful purpose.
@Ark-kun Ark-kun force-pushed the SDK---Removed-the-broken-build_image-parameter-from-build_python_component-function branch from 96da012 to c67d9d4 Compare August 3, 2019 01:36
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Aug 6, 2019

/retest

1 similar comment
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Aug 7, 2019

/retest

@SinaChavoshi
Copy link
Contributor

/lgtm

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Aug 7, 2019

/approve

@k8s-ci-robot
Copy link
Contributor

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@SinaChavoshi
Copy link
Contributor

/lgtm

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Aug 8, 2019

/retest

2 similar comments
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Aug 9, 2019

/retest

@Ark-kun
Copy link
Contributor Author

Ark-kun commented Aug 9, 2019

/retest

@k8s-ci-robot k8s-ci-robot removed the lgtm label Aug 11, 2019
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Aug 12, 2019

/retest

1 similar comment
@Ark-kun
Copy link
Contributor Author

Ark-kun commented Aug 12, 2019

/retest

@Jeffwan
Copy link
Member

Jeffwan commented Aug 13, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot merged commit 980b4c7 into kubeflow:master Aug 13, 2019
@Ark-kun Ark-kun deleted the SDK---Removed-the-broken-build_image-parameter-from-build_python_component-function branch August 13, 2019 23:23
magdalenakuhn17 pushed a commit to magdalenakuhn17/pipelines that referenced this pull request Oct 22, 2023
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request to deprecate compiler.build_python_component(build_image=,..) flag
5 participants