/kind feature
Currently, a kfserving-gateway is used for kfserving inference endpoints. It uses LoadBalancer and all endpoints are publicly available without protection.
This doesn't meet requirement for production grade cluster. We need a way to project these endpoints. Seems currently probe is a blocker to inference behind authn layer.
Describe the solution you'd like
Long term, I think there're two options. (assume we can probe issue fixed)
Remove kfserving-gateway, reuse istio-ingressgateway which means requests from external need to get authenticated. I think both IAP (GCP), Coginito/OIDC (AWS) supports programmatic authentication. I am not sure about Dex. The advantage is this solution reuse AuthN and AuthZ from existing infra.
Still have separate gateway for kfserving. Leave implementation to different vendors. User could build authentication on top of it. For example, AWS can replace service with ingress and LB level authentication for it. The reason we don't reuse istio-gateway is because we can have other authentication strategy for kfserving. For example, each user can request different APIKeys for different models. etc.
Have a middleware for kfserving to manage API Keys. Not sure if there's existing solution on Kubernetes. This sounds like very common use case.
Anything else you would like to add:
Pipeline SDK will have similar issue, we can consider this together.
The user experience should be simple enough, client can get clientId and secret to refresh token or just use an assigned token to make call directly.
Solution needs to be latency optimized.
Related Issue:
https://github.com/kubeflow/kfctl/issues/140
https://github.com/kubeflow/kubeflow/issues/4912
@yuzisun @ellis-bigelow @animeshsingh @jlewi @cliveseldon
Issue-Label Bot is automatically applying the labels:
| Label | Probability |
| ------------- | ------------- |
| feature | 0.88 |
Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback!
Links: app homepage, dashboard and code for this bot.
I would definitely lean option 1. Our default install should provide a single gateway that is configured correctly for kubeflow. Multiple gateways creates confusion and potential security holes. Option 3 seems like a non-starter. I'd prefer that we aren't in the business of authentication. Curious to hear what others thing.
+1 to option 1.
We should try to follow the pattern of "being secure" by default; i.e. we should not be exposing services publicly without authentication by default.
/cc @krishnadurai
KFServing ingress gateway is a temporary walk around for the knative probing issue https://github.com/knative/serving/pull/6962, now they changed to the best effort probe instead of hard fail when getting HTTP 401/403 since probes are not backed up by credentials.
In order to use Kubeflow ingress gateway(should be simple config change), we first need to wait for above knative bug fix released and also we should find a solution to support programming credentials which would be the real use cases for KFServing, for some scenarios we may need api keys for different inference services.
Adding to @yuzisun 's explanation, I'll link the history of issues to provide some context around our current implementation and how it is linked to Knative's probing mechanism.
Since Istio ingressgateway is auth enabled, it was leading to the following issue:
kubeflow/kfserving#668
This was due to the probing mechanism of KNative istio-networking probe requests through the gateway which didn't have credentials for it.
Highlighted here:
knative/serving#6829
This lead to a workaround implementation with a separate gateway for 1.0 Kubeflow release.
Marked by issue: kubeflow/manifests#924
As soon as KNative releases its next version with a non-request based probing mechanism, we plan to change the gateway back to istio-ingressgateway.
@krishnadurai Are we exposing in cluster services on a public IP with no auth? I don't think this is an ok work around. Are only the health probes only allowed or is it all requests?
It looks like the change on master was kubeflow/manifests#949.
It looks like this was cherry-picked onto v1.0 branch in kubeflow/manifests#1027. It looks like this was reverted in kubeflow/manifests#1037 but then re-cherry-picked in kubeflow/manifests#1072
@animeshsingh @ellis-bigelow @krishnadurai I'm a little unclear about what is going on here.
Has KFServing never worked with Kubeflow 1.0.0 with auth secured ingress?
Which version of KFServing shipped with Kubeflow 1.0.0?
To summarize:
/cc @richardsliu @animeshsingh @yanniszark
@jlewi The story is a bit complicated but I think there is no behavior change as far as I know for Istio/Dex. I do not think KFServing ever worked in Kubeflow with Auth due to various issues we found along the way.
KFServing was initially shipped in Kubeflow 0.7 with KFServing 0.2.0(which is later patched with 0.2.2 for pod mutator issue in 0.7.1) and Knative 0.8. In Knative default release Knative Gateway is installed and only port 80 is exposed, so KFServing are available through Knative gateway without security protection, also there was gateway conflicts issue https://github.com/kubeflow/kfserving/issues/629 with Kubeflow Gateway in some of the Kubeflow setup. You can check @ryandawsonuk's issue https://github.com/kubeflow/kfserving/issues/371, we were thinking about to change Knative configuration to point to Kubeflow Gateway but due to the bug https://github.com/knative/serving/issues/5700 in Knative 0.8, we are not able to set to Kubeflow Gateway because it is in a different namespace than knative-serving.
I then attempted in https://github.com/kubeflow/manifests/pull/651 to upgrade to knative 0.11 which contains fix for above issue, changed Knative to use Kubeflow gateway instead and deleted Knative Gateway, so that we can use the auth with Kubeflow Gateway. But this caused #668 due to the Knative probe issue which then leads to @krishnadurai's walk around, although this does not solve the auth issue, it does address https://github.com/kubeflow/kfserving/issues/629.
So in summary currently there isn't a way to use Kubeflow Gateway for the auth unfortunately. We are in hope of Knative's new release with the probe fix so we can start to use Kubeflow Gateway with protection. Maybe we can ask Knative Team to back patch the fix to 0.11, let us know what action we should take here.
@jlewi to answer the security concerns:
If we are opening up ports publicly by default without auth this is a P0 bug and needs to be fixed even if that means disabling KFServing
Totally agreed!
We should not open ports publicly by default.
The current workaround of adding a Gateway with no auth is valid, but should be opt-in and the admin must know what they're doing (maybe they have a layer of auth in front of the gw).
Looking at the configs for the kfserving-gateway application, I see:
kfctl_istio_dex config uses Istio 1.3.1 and does NOT use this workaround.kfctl_k8s_istio, kfctl_gcp_basic_auth, kfctl_gcp_iap, kfctl_ibm configs use Istio 1.6.1 and USE this workaround. This is a potential security concern.Thanks @yuzisun
@krishnadurai, as @yanniszark suggests we need to avoid exposing things publicly by default. How do we go about doing that? Do we just need to revert kubeflow/manifests#1072 on the v1.0-branch and kubeflow/manifests#949 on master?
Do we just need to revert kubeflow/manifests#1072 on the v1.0-branch and kubeflow/manifests#949 on master?
We will need to revert both of these PRs to disable the no-auth behaviour for GCP IAP. The rest of the configs do not get affected by this revert.
This revert would mean KFServing wouldn't be available by default on KF 1.0 versions on configs which have auth enabled. Whereas for the configs which don't have auth enabled on istio-ingressgateway, it's the same except that the gateway for KFServing changes back to istio-ingressgateway.
We could provide an opt-in mechanism (with an unauthenticated alternate gateway) for GCP IAP as suggested by @yanniszark for KFServing until the time a final solution for KFServing is reached.
/cc @yuzisun @animeshsingh @ellis-bigelow
Thanks @krishnadurai
With respect to mitigation which KFDefs are affected?
* Are there any other possible mitigations?
There are two options to mitigate:
istio-ingressgateway which is auth enabled along with the patched release. @yuzisun and @animeshsingh can help us co-ordinate and get there. This option seems to be the best way forward as it accomodates the functioning of KFServing with v1.0 release.kfserving-gateway an opt-in mechanism to enable KFServing after reverting the PRs: kubeflow/manifests#1072 on the v1.0-branch and kubeflow/manifests#949 on master.* I think not exposing things publicly should be the default behavior. But if a particular platform/kfdef OWNERs wants to explicitly LGTM opening up public ports by default on their platform then I'm ok with that.
This would apply to auth based KFDefs: GCP IAP and AWS (auth enabled, @Jeffwan to confirm if it affects AWS auth enabled set-up). Since all other configs are no-auth based setups even for istio-ingressgateway.
* I would like to request a post-mortem to * Figure out the long term solution * Figure out what happened and how we can prevent similar issues in the future * I'd like to understand why and when this was cherry-picked on the 1.0 branch
I agree. We could have a post-mortem to detail what happened and led to this situation.
With respect to mitigation which KFDefs are affected?
* Looking at GCP I only see kfserving-gateway in v1.0.1 and v1.0.2 not v1.0.0
kfserving-gateway enabled requests to KFServing. I suspect that auth based setups in AWS will also be affected. @Jeffwan could you confirm?istio-ingressgateway and for v1.0.1 and v1.0.2 it is kfserving-gateway.* If we remove kfserving-gateway what happens? Will Kubeflow still deploy?
Kubeflow will still deploy. The behaviour will be the same as v1.0.0 configs.
* Do we want to update the v1.0.1 and v1.0.2 configs or do we just want to create a new v1.0.3 config? * I think I generally prefer not changing releases once we've pinned them.
I suggest not to change tagged releases. Instead, we could specify the risk in release notes for the affected versions and point to the postmortem document.
We should release a new patch v1.0.3 with the discussed fix.
Thanks @krishnadurai. In general, I think we should follow the philosophy of revert first rather than trying to forward fix.
It looks like the v1.0 branch has some important fixes
Can we revert any KFServing changes to what shipped with 1.0.0? Then figure out the forward fix?
I would suggest we revert to 1.0.0 and then figure out exactly what state we are in. Based on this thread, its unclear to me whether KFServing was working in which configurations in 1.0.0.
@krishnadurai I can confirm AWS will be affected as well. Richard and I are working on v1.0.2. If there's quick change we can make, we can rollout in this version.
@Jeffwan, we would need to make these reverts and that would change the KFDefs too. Hopefully, we may not need to change the KFDefs.
The knative probe fix has been released in Knative 0.14 and the fix is getting back ported to Knative 0.11 https://github.com/knative/serving/pull/7597 but seems blocked on broken CI.
Whoop!
On Thu, Apr 16, 2020 at 5:20 PM Dan Sun notifications@github.com wrote:
The knative probe fix https://github.com/knative/serving/pull/6962 has
been released in Knative 0.14
https://github.com/knative/serving/releases/tag/v0.14.0 and fix is
getting back ported to Knative 0.11 knative/serving#6962
https://github.com/knative/serving/pull/6962 but seems blocked on
broken CI.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubeflow/kfserving/issues/760#issuecomment-614964961,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AATXLCF3MWWKYBUVLKQYI4TRM6OEJANCNFSM4L3E5HUA
.
--
We have to wait for the fix getting to 0.11 and this is the highest Knative version Kubeflow installation can get to because after 0.12 Knative has bumped minimal Kubernetes version to 1.15 while Kubeflow is still on 1.14.
Can we move KFServing to K8s 1.15 (for an eventual 0.4 release) right now?
I expect that next time we catch a Kubeflow release, we will need to be on
1.15.
On Thu, Apr 16, 2020 at 5:42 PM Dan Sun notifications@github.com wrote:
We have to wait for the fix getting to 0.11 and this is the highest
Knative version Kubeflow installation can goes to because after 0.12
Knative has bumped minimal Kubernetes version to 1.15 while Kubeflow is
still on 1.14.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubeflow/kfserving/issues/760#issuecomment-614970381,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AATXLCHJAZYJFIDJSGX6CHTRM6QVVANCNFSM4L3E5HUA
.
--
Can we move KFServing to K8s 1.15 (for an eventual 0.4 release) right now? I expect that next time we catch a Kubeflow release, we will need to be on 1.15.
…
KFServing 0.3 is already moved to 1.15.
@jlewi @ellis-bigelow @animeshsingh @Jeffwan @cliveseldon @krishnadurai
I have tested the knative probe fix with GCP/IAP, I am able to get KFServing working e2e with https://github.com/kubeflow/manifests/pull/1137, however KFServing as is does not work out of the box for following issues.
1) currently KFServing is using host based routing and seems like GCP/IAP only support path based routing from what I have experimented, I could be wrong. If that is the case we need additional virtual service configuration to map the url path to correct service which requires work in KFServing.
apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
name: kfserving-kubeflow-yuzi-dan-kfserving-test
namespace: kubeflow-yuzi-dan
spec:
gateways:
- kubeflow/kubeflow-gateway
hosts:
- '*'
http:
- match:
- uri:
prefix: /kfserving/kubeflow-yuzi-dan/sklearn-iris
rewrite:
uri: /v1/models/sklearn-iris:predict
route:
- destination:
host: cluster-local-gateway.istio-system.svc.cluster.local
headers:
request:
set:
Host: sklearn-iris-predictor-default.kubeflow-yuzi-dan.svc.cluster.local
weight: 100
timeout: 300s
2) Istio sidecar is automatically injected in kubeflow user namespace, so we need additional authorization policy to allow sending the request to inference service after getting authenticated, we are getting 403 currently. Also there is open issue that knative autoscaling does not work with deprecated istio rbac rules. A walk around is that user can add annotation sidecar.istio.io/inject: false on KFServing inference service, seems kubeflow tfserving example does the same trick.
As discussed in last WG meeting, @cliveseldon will help test this probe fix with Istio/Dex kfdef and @Jeffwan can help on aws kfdef. I am unsure how Istio/Dex will work with KFServing since it does not support programatic token, @krishnadurai @yanniszark might have some ideas.
@yuzisun Thanks for the update. Is it just the probes that are going through the gateway?
@yuzisun Thanks for the update. Is it just the probes that are going through the gateway?
Probes are fine now, I am testing the actual inference requests to the ingress gateway with protected endpoint according to tfserving iap doc , looks like GCP/IAP requires path based routing so KFServing needs to setup additional Istio virtual service configuration to map the path to inference service host names.
python iap_request.py https://kfserving.endpoints.kfserving-248310.cloud.goog/
kfserving/kubeflow-yuzi-dan/sklearn-iris
165686043417-vq3k1nd60m4svttrj8f1r749augem3fl.apps.googleusercontent.com --input=./docs/samples/sklearn/iris-input.json
{"predictions": [1, 1]}
Sounds good. My assumption would be that most requests to models would be coming from within the service mesh. So if models are accessible i) within the mesh and ii) not exposed publicly without authorization then I think we have addressed the critical issue. Making KFServing work behind IAP could be a lower priority, non blocking issue.
/reopen
@yuzisun: Reopened this issue.
In response to this:
/reopen
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
* @jlewi @ellistarn I have created #824 to follow up the KFServing and GCP/IAP integration to see if anyone can help out. * @sachua has helped to confirm KFServing is now working with Istio/Dex setup in auth environment and here is the [great doc](https://github.com/kubeflow/kfserving/tree/master/docs/samples/istio-dex) he wrote * @Jeffwan @animeshsingh Can you help confirm KFServing works with kubeflow manifest master in AWS and IBM setup ?
@Jeffwan @animeshsingh Is KFServing working with Istio/Dex in AWS. I followed @sachua doc.
@jagadeeshi2i You need to bring cookie to client to make it work. I would suggest you open a new issue to talk about your problems
Most helpful comment
I would definitely lean option 1. Our default install should provide a single gateway that is configured correctly for kubeflow. Multiple gateways creates confusion and potential security holes. Option 3 seems like a non-starter. I'd prefer that we aren't in the business of authentication. Curious to hear what others thing.