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

HTTPS with Helm values (GKE) #153

Open
eostis opened this issue Feb 25, 2023 · 12 comments
Open

HTTPS with Helm values (GKE) #153

eostis opened this issue Feb 25, 2023 · 12 comments

Comments

@eostis
Copy link

eostis commented Feb 25, 2023

Is there a way to add GKE ManagedCertificates to the Helm values?

@etiennedi
Copy link
Member

Let me loop in @iamcaleberic here to comment on best practices, as I'm not sure what the way to go would be here.

Since ManagedCertificates are specific to GKE, I'm not sure if they should be part of the general Helm chart. But maybe there would be a simple way to wrap our Helm chart and make it GKE-specific. Then you could easily add managed certs there? Or maybe there is a better way.

@eostis
Copy link
Author

eostis commented Feb 27, 2023

There are already GCP values :)

Strating vithout HTTPS is very unsettling.

@iamcaleberic
Copy link

We need to add the ManagedCertificate object to the helm chart

apiVersion: networking.gke.io/v1
kind: ManagedCertificate
metadata:
  name: managed-cert
spec:
  domains:
    - DOMAIN_NAME1
    - DOMAIN_NAME2

And feature gate them in the values to apply to gke specific setups in the values, would also require use of an ingress or gateway to make use of it.

@stale
Copy link

stale bot commented May 21, 2023

Thank you for your contribution to Weaviate. This issue has not received any activity in a while and has therefore been marked as stale. Stale issues will eventually be autoclosed. This does not mean that we are ruling out to work on this issue, but it most likely has not been prioritized high enough in the last months.
If you believe that this issue should remain open, please leave a short reply. This lets us know that the issue is not abandoned and acts as a reminder for our team to consider prioritizing this again.
Please also consider if you can make a contribution to help with the solution of this issue. If you are willing to contribute, but don't know where to start, please leave a quick message and we'll try to help you.
Thank you, The Weaviate Team

@eostis
Copy link
Author

eostis commented May 30, 2023

Reopening. This is an important subject.

The documentation now shows several methods of authentication, but all useless without SSL.

@StefanBogdan StefanBogdan self-assigned this Jun 20, 2023
@jschwanz
Copy link

I just started looking into this, but I'm on Azure and typically using cert-manager + Let's Encrypt. I'm definitely interested to see progress on HTTPS/ingress capabilities.

@StefanBogdan
Copy link
Member

Hi @eostis @jschwanz, the reason we do not have ManagedCertificate resource in the Helm Chart is because it is GCP specific. The current helm chart is meant to deploy the Weaviate database in any K8s cluster, no Cloud vendor specific. Also we do not provision an Ingress because some users want to have a private Weaviate instance and they have an application in the same cluster that talks to Weaviate. Our concept of the Helm chart is to have a single repo that deploys Weaviate and any additional stuff that is vendor specific to leave it to the user.

For example in GKE if you want HTTPS protocol for your instance it means that you need an Ingress resource, a ManagedCertificate and a global IP to associate with the Ingress AND to have a domain name. The last 2 are not possible (as far as I know) to provision using k8s resources and this could lead to users having bad experience because they do not understand why it is not working.

In the case of Azure, it means we have to have a dependency on the cert-manager which means we have to make sure it is always up to date, but this could because very difficult to maintain once we add more vendors.

On AWS you do not even need an Ingress you can add a TLS cert on a LoadBalancer by adding the correct annotations to the k8s LoadBalancer Service.

It does not mean we are not willing to add this, it is more that we never had a need for this so far. We assumed that the users who want to have HTTPS will be the ones that understand what it is and why it is needed and how to do this. That is why we never added it.

My question would be: Why not to create the ManagedCertificate and the Ingress outside the weaviate helm chart?

Please let me know if you disagree and the reasons you want it to be part of the Helm chart. After this we will evaluate it and we might add it to the official helm release.

@StefanBogdan StefanBogdan transferred this issue from weaviate/weaviate Jul 3, 2023
@eostis
Copy link
Author

eostis commented Jul 3, 2023

Hi @StefanBogdan,

The main reason is to lower the tech barrier to using Weaviate, especially for small businesses. SSL is difficult to configure, and most/all businesses will refuse to start without.

There are already cloud-specific files, like gcsSecret.yaml for backups. Some more for SSL would not be inconceivable.

A mix of templates and "gcloud" scripts could be a very strong help at setting the SLL for the cluster namespaces. As far as I know, everything can be scripted with K8S: https://cert-manager.io/docs/tutorials/getting-started-with-cert-manager-on-google-kubernetes-engine-using-lets-encrypt-for-ingress-ssl/

@jschwanz
Copy link

jschwanz commented Jul 3, 2023

For me I don't think I'm looking for anything vendor specific. What I would need would be the ability to enable a standard kind: Ingress with metadata.annotations, spec.ingressClassName, spec.rules, and spec.tls. Yeah I recognize I can do all of this outside of Helm, but it'd be more convenient to have it all defined and versioned in one configuration.

@StefanBogdan
Copy link
Member

We are going to explore the option of adding the Ingress AND the cert-manager to the helm chart. Thanks for the feedback.

@bryantbiggs
Copy link

I would recommend not adding cert-manager to the chart - if you expose Ingress and/or Gateway resources, this should be enough for users to utilize with other resources such as cert-manager

@heresandyboy
Copy link

heresandyboy commented Jul 27, 2023

I had also had trouble when deploying to a KOPS managed cluster in AWS. Without an ingress record provided in the templates I was unable to access Weaviate in our cluster.

To workaround it I had cloned the weaviate-helm repo and copied in an ingress template into the templates folder, and added values for ingress. This is all working nicely for me, but having the ingress built into the chart will help with maintaining this.

In case it offers any assistance to this issue, here is my working ingress template and example values:

ingress.yml

{{- if .Values.ingress.enabled -}}
{{- $fullName := .Values.service.name -}}
{{- $svcPort := "" -}}
{{- range .Values.service.ports }}
  {{- if eq .name "http" }}
    {{- $svcPort = .port -}}
  {{- end }}
{{- end }}
{{- if and .Values.ingress.className (not (semverCompare ">=1.18-0" .Capabilities.KubeVersion.GitVersion)) }}
  {{- if not (hasKey .Values.ingress.annotations "kubernetes.io/ingress.class") }}
  {{- $_ := set .Values.ingress.annotations "kubernetes.io/ingress.class" .Values.ingress.className}}
  {{- end }}
{{- end }}
{{- if semverCompare ">=1.19-0" .Capabilities.KubeVersion.GitVersion -}}
apiVersion: networking.k8s.io/v1
{{- else if semverCompare ">=1.14-0" .Capabilities.KubeVersion.GitVersion -}}
apiVersion: networking.k8s.io/v1beta1
{{- else -}}
apiVersion: extensions/v1beta1
{{- end }}
kind: Ingress
metadata:
  name: {{ $fullName }}
  labels:
    app.kubernetes.io/name: weaviate
    app.kubernetes.io/managed-by: helm
  {{- with .Values.ingress.annotations }}
  annotations:
    {{- toYaml . | nindent 4 }}
  {{- end }}
spec:
  {{- if and .Values.ingress.className (semverCompare ">=1.18-0" .Capabilities.KubeVersion.GitVersion) }}
  ingressClassName: {{ .Values.ingress.className }}
  {{- end }}
  {{- if .Values.ingress.tls }}
  tls:
    {{- range .Values.ingress.tls }}
    - hosts:
        {{- range .hosts }}
        - {{ . | quote }}
        {{- end }}
      secretName: {{ .secretName }}
    {{- end }}
  {{- end }}
  rules:
    {{- range .Values.ingress.hosts }}
    - host: {{ .host | quote }}
      http:
        paths:
          {{- range .paths }}
          - path: {{ .path }}
            {{- if and .pathType (semverCompare ">=1.18-0" $.Capabilities.KubeVersion.GitVersion) }}
            pathType: {{ .pathType }}
            {{- end }}
            backend:
              {{- if semverCompare ">=1.19-0" $.Capabilities.KubeVersion.GitVersion }}
              service:
                name: {{ $fullName }}
                port:
                  number: {{ $svcPort }}
              {{- else }}
              serviceName: {{ $fullName }}
              servicePort: {{ $svcPort }}
              {{- end }}
          {{- end }}
    {{- end }}
{{- end }}

values passed into helm

            image:
              tag: 1.20.0
            service:
              name: weaviate
              ports:
                - name: http
                  protocol: TCP
                  port: 80
              type: NodePort
              
            ingress:
              enabled: true
              tls: []
              hosts:
                - host: weaviate.systems.kubernetes.testing.aws.zen.co.uk
                  paths:
                    - path: '/*'
                      pathType: ImplementationSpecific
              annotations:
                kubernetes.io/ingress.class: alb
                alb.ingress.kubernetes.io/listen-ports: '[{"HTTPS":443}]'
                alb.ingress.kubernetes.io/scheme: internal
                alb.ingress.kubernetes.io/backend-protocol: HTTP
                alb.ingress.kubernetes.io/healthcheck-path: /ping
                alb.ingress.kubernetes.io/group.name: weaviate
                external-dns.alpha.kubernetes.io/hostname: weaviate.systems.kubernetes.testing.aws.zen.co.uk

@StefanBogdan StefanBogdan removed their assignment Sep 5, 2023
@StefanBogdan StefanBogdan self-assigned this Sep 15, 2023
@StefanBogdan StefanBogdan removed their assignment Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants