-
Notifications
You must be signed in to change notification settings - Fork 706
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
Add Kubeapps setup for headless (API-only mode) #5543
Conversation
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
✅ Deploy Preview for kubeapps-dev canceled.
|
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
Should we keep any change to the chart like this to a
I'm not sure, but do these need to be two separate ingresses rather than one ingress with multiple rules? (not sure).
I think the strategy here should be thought through or discussed a bit more. Here are a few questions just looking at the outline here:
Generally, I'd recommend:
so that you can then continue developing and exploring behind that feature flag, without (1) affecting existing functionality and (2) accidentally releasing functionality before it's ready? |
+1. Totally agree with @absoludity comment. |
This change is caused by multicluster, but it doesn't have to be always associated to it necessarily. If someone wants to deploy Kubeapps in API mode only, unrelated to multicluster, this is the way.
Yes, for Nginx we need two separate ingresses. In order to correctly handle GRPC requests, Nginx requires the annotation
We are dialing the connection with TLS transport, so I assume we have server-side TLS. See here. We are also supporting custom CAs.
Why not? Ingress in second cluster is only accessed internally through kind network from the first cluster, so it can go to 443 as well. This is, the 443 opened port on the second cluster.
As mentioned above, this PR comes first and is totally independent from the multicluster feature. It just sets up Kubeapps in API only mode. There might be use cases that only want to invoke some APIs on Kubeapps. After enabling the flag, only GRPC and REST can be used, no UI. Why blocking this PR? |
Yeah, sure - as long as it's under
Cool - I'll be interested to hear how the nginx grpc config works. I thought from memory it required http 2 only or some slight difference which might make it tricky - hopefully not (there may be more details on the issue where we investigated running using envoy as a proxy, as I tried a few things with nginx for that too, from memory).
Yes - I just meant that with the current setup (where Kubeapps api server communicates with the k8s api server of the other clusters), we already have TLS setup and so just need to include the CAcert in our clusters config. That configuration part will be similar, but setting it up will mean we now have to create the certs or have cert-manager installed on the other clusters as well as some ingress support etc. All do-able, just quite complex to setup.
No - I mean, I don't see a need for kubeapps-apis on the worker to need to identify the client, unless we want to ensure no one else can use the API. I'd leave that for the future, if at all, personally.
If you've tested this PR by installing it in a cluster with nginx-ingress and gRPC setup and verified that you can use gRPCurl or similar to ensure it can be used the way you intend - great, I just thought it'd be easier to do that by verifying it works with our other PR, but I don't mind if you verify the external gRPC requests independently and land it. What I'd be keen to avoid is landing things on the assumption that it'll work before we've actually tested it in the way it's intended to work, that's all. |
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
Flag is moved from
Based on the annotation You can see an example of how it works in Create the Kubernetes Ingress resource for the gRPC app, setting up an Nginx ingress for GRPC is pretty straight forward.
I don't think Nginx is the one introducing the requirement for http2. As it is mentioned here:
That belongs to the other PR, I believe. This one does not touch clusters config. It just sets an ingress setup for Kubeapps APIs only. I like to think about this forgetting about multiclustering. This feature could live without multicluster.
Agree, no need for mutual TLS.
Of course I have verified it. It works just as any other ingress. Added a You can try yourself with:
Then the installation can be checked from a pod in the other cluster:
|
Perfect - thanks for the instructions for verifying it, that's what I was missing (or any indication that that had been done - glad it's obvious to you that it would have been, but it's not always the case).
Great. Yes the issue I'm remembering was related to gRPC-Web I think, so should be fine.
Sorry - miscommunication: that sentence wasn't meaning that you need to add the CAcert to the clusters config in this PR, but rather that that part will remain similar to what it is today, but the extra manual certs will be extra complexity, that's all:
Anyway, +1 for this one, thanks for the details of the setup and testing.
I think we currently have |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks Rafa!
@@ -0,0 +1,122 @@ | |||
{{- if and .Values.ingress.enabled .Values.featureFlags.apiOnly.enabled -}} | |||
{{- if and .Values.dashboard.enabled -}} | |||
{{ fail "Dashboard is enabled but will NOT work with Ingress in API mode. Please set \"dashboard.enabled\" to false." }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great - thanks for catching that even if we don't have a single value flag. Do we need a similar check for the condition on line 1, in that, does it make sense for people to deploy with apiOnly.enabled: true
while ingress itself is disabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feature is basically about Ingresses. Initially was under .ingress
chart value, and it is documented with Enable ingress for API operations only
, but I'll add a check for that.
enabled: false | ||
grpc: | ||
annotations: | ||
nginx.ingress.kubernetes.io/backend-protocol: GRPC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering why this one is configurable - won't our own clients need this specific annotation, even if others are optional/configurable? I expected this one to be hard-coded into the annotations of the ingress. Ah, or perhaps you're just separating that dependency here (ie. other clients may want other annotations). Related question, why does the default in the README show up as {}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're just separating that dependency here
Exactly. There is no trace of Nginx in the templates of ingresses. It is only through annotations that it is customized, hence this value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does the default in the README show up as {} ?
No idea, it is generated by the readme generator, so changing it manually is not an option :S
@@ -53,6 +53,15 @@ ${ADDITIONAL_CLUSTER_CONFIG}: devel/dex.crt | |||
|
|||
additional-cluster-kind: ${ADDITIONAL_CLUSTER_CONFIG} | |||
|
|||
devel/additional-nginx: | |||
kubectl apply --kubeconfig=${ADDITIONAL_CLUSTER_CONFIG} -f https://raw.githubusercontent.com/kubernetes/ingress-nginx/main/deploy/static/provider/kind/deploy.yaml | |||
# TODO: need to add wait for condition=exists or similar - https://github.com/kubernetes/kubernetes/issues/83242 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we do that in our cluster-kind
target which installs ingress-nginx:
kubeapps/script/makefiles/cluster-kind.mk
Lines 28 to 31 in e571216
kubectl wait --kubeconfig=${CLUSTER_CONFIG} --namespace ingress-nginx \ | |
--for=condition=ready pod \ | |
--selector=app.kubernetes.io/component=controller \ | |
--timeout=120s |
Signed-off-by: Rafa Castelblanque <rcastelblanq@vmware.com>
Description of the change
Adds a new chart flag
ingress.apiOnly.enabled
that allows to deploy Kubeapps with an ingress setup that only allows API calls to:/apis
usinghttp
/
usinggrpc
When the mode is enabled, the default ingress used so far is not deployed.
Instead deploys two ingresses:
kubeapps
ingress with path/
and protocolgrpc
kubeapps-http-api
ingress with path/apis
There is a requirement and enforcement message to disable the dashboard, as
/
path will not be available to serve it.Benefits
Kubeapps can be used in API-only mode:
It might also be beneficial for development, as there won't be the need to do port-forward to use GRPC.
Does not change default behavior
Possible drawbacks
This forces the user to disable the dashboard, as it will be unusable when the API mode is on. We could remove the requirement, but users might complain when trying to access dashboard.
Applicable issues