vmware-archive / helm-crd

Experimental CRD controller for managing Helm releases
Apache License 2.0
100 stars 13 forks source link

adds support for fetching charts with http authentication #23

Closed sameersbn closed 6 years ago

sameersbn commented 6 years ago

This PR enables users to specify the HTTP authorization token (in a secret) for downloading charts from a private helm repository. Following is an example manifest,

apiVersion: kubeapps.com/v1alpha1
kind: AppRepository
metadata:
  annotations: {}
  labels:
    created-by: kubeapps
    name: fuloi
  name: fuloi
  namespace: kubeapps
spec:
  type: helm
  url: http://artifactory.fuloi.com/artifactory/helm
  auth:
    secretKeyRef:
      name: apprepo-fuloi-secrets
      key: authorizationHeader

Here is a sneekpeek of the changes in action.

sneekpeek

Fixes https://github.com/kubeapps/kubeapps/issues/269

sameersbn commented 6 years ago

cc @anguslees could you please take a look

sameersbn commented 6 years ago

I've made the suggested changes. For the tests, I've currently added tests for the resolveURL function. Will work on additional tests in a follow up PR

sameersbn commented 6 years ago

Thanks for the review @anguslees .. I've added support for the case where the base url ends with index.yaml and expanded on the tests.

prydonius commented 6 years ago

I've added support for the case where the base url ends with index.yaml and expanded on the tests.

IMO we shouldn't really be supporting this usecase, what's the motivation? To me, it seems like defensive programming. A repo URL with an index.yaml is not a correct repo URL:

$ helm repo add test https://bitnami.com/repo/index.yaml                                                                                                  
Error: Looks like "https://bitnami.com/repo/index.yaml" is not a valid chart repository or cannot be reached: Failed to fetch https://bitnami.com/repo/index.yaml/index.yaml : 404 Not Found

It would be better to error out in these cases and get the user to fix the URL.

EDIT: ah I just saw @anguslees's comment about the confusion over repo URLs, sorry.