verdaccio / charts

☸️🐳 Verdaccio Helm chart repository
https://charts.verdaccio.org
MIT License
60 stars 58 forks source link

Documentation for Ingress HTTPS options #63

Open Splaktar opened 3 years ago

Splaktar commented 3 years ago

In https://github.com/verdaccio/charts#configuration, there are no entries for these options in values.yaml:

ingress:
  enabled: false
  # Set to true if you are on an old cluster where apiVersion extensions/v1beta1 is required
  useExtensionsApi: false
  paths:
    - /
  # Use this to define, ALB ingress's actions annotation based routing. Ex: for ssl-redirect
  # Ref: https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/guide/tasks/ssl_redirect/
  extraPaths: []
  hosts:
    - npm.blah.com
# annotations:
#   kubernetes.io/ingress.class: nginx
  tls:
    - secretName: secret
      hosts:
        - npm.blah.com

I'd like to set up TLS, but it's not clear to me how to do this. I provisioned a certificate for my load balancer in Google Cloud, but I'm not sure how to get that hooked up into this config.

Splaktar commented 3 years ago

Is the ManagedCertificate kind (as mentioned here) supported with this chart?

Splaktar commented 3 years ago

I was able to create a ManagedCertificate separately via

registry-staging-certificate.yaml

apiVersion: networking.gke.io/v1
kind: ManagedCertificate
metadata:
  name: registry-staging
spec:
  domains:
    - registry-staging.mydomain.com

And then kubectl apply -f registry-staging-certificate.yaml.

Then I used this ingress definition:

ingress:
  enabled: true
  # Set to true if you are on an old cluster where apiVersion extensions/v1beta1 is required
  useExtensionsApi: false
  # This (/*) is due to gce ingress needing a glob where nginx ingress doesn't (/).
  paths:
    - /*
  extraPaths: [ ]
  hosts:
    - registry-staging.mydomain.com
  annotations:
    kubernetes.io/ingress.global-static-ip-name: "xxx.xxx.xxx.xxx"
    networking.gke.io/managed-certificates: "registry-staging"
    kubernetes.io/ingress.class: "gce"
#  tls:
#    - secretName: registry-staging
#      hosts:
#        - registry-staging.mydomain.com
todpunk commented 3 years ago

This seems what I would expect. Ingress annotations are how you would tell your ingress controller (in this case something in GCP) what to do. Verdaccio would not be responsible for terminating TLS and ultimately you wouldn't want your services (verdaccio included) to terminate TLS. The ingress controller should via Ingress object metadata.

Or a shorter version: This is an Ingress controller concern, not a concern with pods/services/ingresses, and the documentation would live in either ingress controller or kubernetes core concept docs.

Does that satisfy your needs on this issue?

Splaktar commented 3 years ago

It looks like one of the things that I was doing wrong was

kubernetes.io/ingress.global-static-ip-name: "xxx.xxx.xxx.xxx"

should be

kubernetes.io/ingress.global-static-ip-name: registry-staging-ip

It needs to use the static IP's name and not the IP itself. Strange that it just failed silently on that.

todpunk commented 3 years ago

This is one of those fun things about Kubernetes. There's things that Kubernetes itself handles, and it will provide "events" that are errors that you can troubleshoot with, but then there's all sorts of stuff like this where it's just another "tag" or extra field or different API version that is valid yaml, but doesn't actually tell anything in Kubernetes to do anything, and isn't correct for some other thing (like in this case I think this is for GCP's Load Balancer glue maybe?), but that other thing can't guess what's wrong or if it does, the error is somewhere totally different.

It gets better over time as you learn the nuances of what gets passed to what for a given thing, but I admit it's confusing for a while. Welcome to the k8s fold =cP

Feel free to close this issue if you wish, or if you run into an issue with Verdaccio's chart itself we're happy to help of course. We'll probably close the issue in a few days if nothing further comes up on this one just to keep things clean. (If I'm missing something and you still need a Verdaccio question answered, feel free to point out my error as well.)

Splaktar commented 3 years ago

Thank you for that explanation. I certainly appreciate it.

That said, I do think that there is some benefit to improving the docs for this chart on GKE. There are a number of custom links and docs for AWS, but it's lacking the GKE side.

For instance, the tls: configuration section in the default values is misleading if using a ManagedCertificate since it can only by used for self-managed certificates.

I understand not wanting to repeat all of the GKE docs, but it would be nice to mention or point to the docs for things like this

    kubernetes.io/ingress.global-static-ip-name: registry-ip-name
    networking.gke.io/managed-certificates: "registry-staging"
    kubernetes.io/ingress.class: "gce"

I guess just linking to https://cloud.google.com/kubernetes-engine/docs/concepts/ingress would send people in the right direction. I had to read through so much unrelated stuff to find what I needed.

The issue in https://github.com/verdaccio/charts/issues/62 was also a big bump in this road since every update I had to go and reset the service's targetPort.