rstudio / helm

Helm Resources for RStudio Products
MIT License
32 stars 28 forks source link

Change Connect default service.type and other service generation items #478

Closed tnederlof closed 5 months ago

tnederlof commented 5 months ago

This PR contains the following changes, following the changes made to RSPM by @atheriel in 2021 (PR here: https://github.com/rstudio/helm/pull/122) This change to RSPM was made over 2 years ago and went pretty smoothly, now bringing it to Connect.

Since the first of these is a breaking change, I bumped the minor version for the chart.

Moving to ClusterIP

ClusterIP is the Kubernetes default, exposing the service in-cluster only. This is a secure, well-understood default. For external users, there are a few options:

It seems like the last route (which is the current default) is much more uncommon than the first two, so I'd advocate for this change.

In addition:

dbkegley commented 5 months ago

I installed the helm chart from this branch using our local development setup for Connect and everything worked as expected 👍

david@Davids-MacBook-Pro:connect [main] » k get svc
NAME                      TYPE        CLUSTER-IP     EXTERNAL-IP   PORT(S)           AGE
connect-rstudio-connect   ClusterIP   10.109.240.7   <none>        80/TCP,9108/TCP   6s

Before:

david@Davids-MacBook-Pro:connect [main] » k get svc
NAME                      TYPE        CLUSTER-IP     EXTERNAL-IP   PORT(S)                       AGE
connect-rstudio-connect   NodePort    10.103.146.3   <none>        80:30590/TCP,9108:32590/TCP   10s
tnederlof commented 5 months ago

@dbkegley thanks, works on my side and I verified annotations still make it through on all types. I am unable to merge bc I think CI doesn't like the latest commit being an auto one. If you are able to when the appropriate people have reviewed please do!