diff --git a/cmd/helm/dependency.go b/cmd/helm/dependency.go index d6799a4e4..76a8ed846 100644 --- a/cmd/helm/dependency.go +++ b/cmd/helm/dependency.go @@ -73,20 +73,20 @@ repository added to helm by "helm add repo". Version matching is also supported for this case. A repository can be defined as a git URL. The path must start with a prefix of -"git:" followed by a valid git repository URL. +"git://" followed by a valid git repository URL. # Chart.yaml dependencies: - - name: nginx - version: "master" - repository: "git:https://github.com/helm/helm-chart.git" + - name: helm-chart + version: "main" + repository: "git://https://github.com/helm/helm-chart.git" The 'repository' can be the https or ssh URL that you would use to clone a git repo or add as a git remote, prefixed with 'git:'. -For example 'git:git@github.com:helm/helm-chart.git' or -'git:https://github.com/helm/helm-chart.git' +For example 'git://git@github.com:helm/helm-chart.git' or +'git://https://github.com/helm/helm-chart.git' -When using a 'git:' repository, the 'version' must be a valid tag or branch +When using a 'git://' repository, the 'version' must be a valid semantic tag or branch name for the git repo. For example 'master'. Limitations when working with git repositories: diff --git a/go.mod b/go.mod index e7da0b8aa..5f4eaa2f3 100644 --- a/go.mod +++ b/go.mod @@ -30,6 +30,7 @@ require ( github.com/spf13/cobra v1.5.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.0 + github.com/whilp/git-urls v1.0.0 github.com/xeipuuv/gojsonschema v1.2.0 golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 diff --git a/go.sum b/go.sum index 0a8e911c4..0dde11299 100644 --- a/go.sum +++ b/go.sum @@ -701,6 +701,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= +github.com/whilp/git-urls v1.0.0 h1:95f6UMWN5FKW71ECsXRUd3FVYiXdrE7aX4NZKcPmIjU= +github.com/whilp/git-urls v1.0.0/go.mod h1:J16SAmobsqc3Qcy98brfl5f5+e0clUvg1krgwk/qCfE= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0= diff --git a/internal/fileutil/fileutil.go b/internal/fileutil/fileutil.go index 0ca7bdf67..18ff10a4a 100644 --- a/internal/fileutil/fileutil.go +++ b/internal/fileutil/fileutil.go @@ -22,6 +22,7 @@ import ( "os" "path/filepath" "strings" + "time" "archive/tar" "bytes" @@ -55,20 +56,32 @@ func AtomicWriteFile(filename string, reader io.Reader, mode os.FileMode) error return fs.RenameWithFallback(tempName, filename) } -func CompressDirToTgz(src, tmpdir string) (*bytes.Buffer, error) { - // tar => gzip => buf +func CompressDirToTgz(chartTmpDir, tmpdir string) (*bytes.Buffer, error) { + + _, err := os.Stat(chartTmpDir) + if err != nil { + return nil, err + } + + _, err = os.Stat(tmpdir) + if err != nil { + return nil, err + } + // tar => gzip => buf buf := bytes.NewBuffer(nil) zr := gzip.NewWriter(buf) + zr.ModTime = time.Date(1977, time.May, 25, 0, 0, 0, 0, time.UTC) tw := tar.NewWriter(zr) // walk through every file in the folder - walkErr := filepath.Walk(src, func(file string, fi os.FileInfo, err error) error { + walkErr := filepath.Walk(chartTmpDir, func(file string, fi os.FileInfo, err error) error { // generate tar header if err != nil { return err } + header, err := tar.FileInfoHeader(fi, strings.TrimPrefix(file, tmpdir+"/")) if err != nil { return err @@ -77,11 +90,13 @@ func CompressDirToTgz(src, tmpdir string) (*bytes.Buffer, error) { // must provide real name // (see https://golang.org/src/archive/tar/common.go?#L626) header.Name = strings.TrimPrefix(filepath.ToSlash(file), tmpdir+"/") + header.ModTime = time.Date(1977, time.May, 25, 0, 0, 0, 0, time.UTC) // write header if err := tw.WriteHeader(header); err != nil { return err } + // if not a dir, write file content if !fi.IsDir() { data, err := os.Open(file) diff --git a/internal/fileutil/fileutil_test.go b/internal/fileutil/fileutil_test.go index 76cd8f074..6d2b8d03e 100644 --- a/internal/fileutil/fileutil_test.go +++ b/internal/fileutil/fileutil_test.go @@ -18,6 +18,8 @@ package fileutil import ( "bytes" + "crypto/md5" + "encoding/hex" "io/ioutil" "os" "path/filepath" @@ -56,3 +58,25 @@ func TestAtomicWriteFile(t *testing.T) { mode, gotinfo.Mode()) } } + +func TestCompressDirToTgz(t *testing.T) { + + testDataDir := "testdata" + chartTestDir := "testdata/helmchart" + expectMd5Value := "47e407d2251866226cb7df4c44028091" + expectChartBytesLen := 3990 + + chartBytes, err := CompressDirToTgz(chartTestDir, testDataDir) + if err != nil { + t.Fatal(err) + } + chartBytesLen := chartBytes.Len() + hash := md5.Sum(chartBytes.Bytes()) + currentMd5Value := hex.EncodeToString(hash[:]) + if currentMd5Value != expectMd5Value { + t.Fatalf("Expect md5 %s, but get md5 %s, len %d", expectMd5Value, currentMd5Value, chartBytesLen) + } + if chartBytesLen != expectChartBytesLen { + t.Fatalf("Expect chartBytesLen %d, but get %d", expectChartBytesLen, chartBytesLen) + } +} diff --git a/internal/fileutil/testdata/helmchart/.helmignore b/internal/fileutil/testdata/helmchart/.helmignore new file mode 100644 index 000000000..0e8a0eb36 --- /dev/null +++ b/internal/fileutil/testdata/helmchart/.helmignore @@ -0,0 +1,23 @@ +# Patterns to ignore when building packages. +# This supports shell glob matching, relative path matching, and +# negation (prefixed with !). Only one pattern per line. +.DS_Store +# Common VCS dirs +.git/ +.gitignore +.bzr/ +.bzrignore +.hg/ +.hgignore +.svn/ +# Common backup files +*.swp +*.bak +*.tmp +*.orig +*~ +# Various IDEs +.project +.idea/ +*.tmproj +.vscode/ diff --git a/internal/fileutil/testdata/helmchart/Chart.yaml b/internal/fileutil/testdata/helmchart/Chart.yaml new file mode 100644 index 000000000..c4239c942 --- /dev/null +++ b/internal/fileutil/testdata/helmchart/Chart.yaml @@ -0,0 +1,24 @@ +apiVersion: v2 +name: helmchart +description: A Helm chart for Kubernetes + +# A chart can be either an 'application' or a 'library' chart. +# +# Application charts are a collection of templates that can be packaged into versioned archives +# to be deployed. +# +# Library charts provide useful utilities or functions for the chart developer. They're included as +# a dependency of application charts to inject those utilities and functions into the rendering +# pipeline. Library charts do not define any templates and therefore cannot be deployed. +type: application + +# This is the chart version. This version number should be incremented each time you make changes +# to the chart and its templates, including the app version. +# Versions are expected to follow Semantic Versioning (https://semver.org/) +version: 0.1.0 + +# This is the version number of the application being deployed. This version number should be +# incremented each time you make changes to the application. Versions are not expected to +# follow Semantic Versioning. They should reflect the version the application is using. +# It is recommended to use it with quotes. +appVersion: "1.16.0" diff --git a/internal/fileutil/testdata/helmchart/templates/NOTES.txt b/internal/fileutil/testdata/helmchart/templates/NOTES.txt new file mode 100644 index 000000000..41d5cffaf --- /dev/null +++ b/internal/fileutil/testdata/helmchart/templates/NOTES.txt @@ -0,0 +1,22 @@ +1. Get the application URL by running these commands: +{{- if .Values.ingress.enabled }} +{{- range $host := .Values.ingress.hosts }} + {{- range .paths }} + http{{ if $.Values.ingress.tls }}s{{ end }}://{{ $host.host }}{{ .path }} + {{- end }} +{{- end }} +{{- else if contains "NodePort" .Values.service.type }} + export NODE_PORT=$(kubectl get --namespace {{ .Release.Namespace }} -o jsonpath="{.spec.ports[0].nodePort}" services {{ include "helmchart.fullname" . }}) + export NODE_IP=$(kubectl get nodes --namespace {{ .Release.Namespace }} -o jsonpath="{.items[0].status.addresses[0].address}") + echo http://$NODE_IP:$NODE_PORT +{{- else if contains "LoadBalancer" .Values.service.type }} + NOTE: It may take a few minutes for the LoadBalancer IP to be available. + You can watch the status of by running 'kubectl get --namespace {{ .Release.Namespace }} svc -w {{ include "helmchart.fullname" . }}' + export SERVICE_IP=$(kubectl get svc --namespace {{ .Release.Namespace }} {{ include "helmchart.fullname" . }} --template "{{"{{ range (index .status.loadBalancer.ingress 0) }}{{.}}{{ end }}"}}") + echo http://$SERVICE_IP:{{ .Values.service.port }} +{{- else if contains "ClusterIP" .Values.service.type }} + export POD_NAME=$(kubectl get pods --namespace {{ .Release.Namespace }} -l "app.kubernetes.io/name={{ include "helmchart.name" . }},app.kubernetes.io/instance={{ .Release.Name }}" -o jsonpath="{.items[0].metadata.name}") + export CONTAINER_PORT=$(kubectl get pod --namespace {{ .Release.Namespace }} $POD_NAME -o jsonpath="{.spec.containers[0].ports[0].containerPort}") + echo "Visit http://127.0.0.1:8080 to use your application" + kubectl --namespace {{ .Release.Namespace }} port-forward $POD_NAME 8080:$CONTAINER_PORT +{{- end }} diff --git a/internal/fileutil/testdata/helmchart/templates/_helpers.tpl b/internal/fileutil/testdata/helmchart/templates/_helpers.tpl new file mode 100644 index 000000000..28bcb84bd --- /dev/null +++ b/internal/fileutil/testdata/helmchart/templates/_helpers.tpl @@ -0,0 +1,62 @@ +{{/* +Expand the name of the chart. +*/}} +{{- define "helmchart.name" -}} +{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }} +{{- end }} + +{{/* +Create a default fully qualified app name. +We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec). +If release name contains chart name it will be used as a full name. +*/}} +{{- define "helmchart.fullname" -}} +{{- if .Values.fullnameOverride }} +{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }} +{{- else }} +{{- $name := default .Chart.Name .Values.nameOverride }} +{{- if contains $name .Release.Name }} +{{- .Release.Name | trunc 63 | trimSuffix "-" }} +{{- else }} +{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }} +{{- end }} +{{- end }} +{{- end }} + +{{/* +Create chart name and version as used by the chart label. +*/}} +{{- define "helmchart.chart" -}} +{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" }} +{{- end }} + +{{/* +Common labels +*/}} +{{- define "helmchart.labels" -}} +helm.sh/chart: {{ include "helmchart.chart" . }} +{{ include "helmchart.selectorLabels" . }} +{{- if .Chart.AppVersion }} +app.kubernetes.io/version: {{ .Chart.AppVersion | quote }} +{{- end }} +app.kubernetes.io/managed-by: {{ .Release.Service }} +{{- end }} + +{{/* +Selector labels +*/}} +{{- define "helmchart.selectorLabels" -}} +app.kubernetes.io/name: {{ include "helmchart.name" . }} +app.kubernetes.io/instance: {{ .Release.Name }} +{{- end }} + +{{/* +Create the name of the service account to use +*/}} +{{- define "helmchart.serviceAccountName" -}} +{{- if .Values.serviceAccount.create }} +{{- default (include "helmchart.fullname" .) .Values.serviceAccount.name }} +{{- else }} +{{- default "default" .Values.serviceAccount.name }} +{{- end }} +{{- end }} diff --git a/internal/fileutil/testdata/helmchart/templates/deployment.yaml b/internal/fileutil/testdata/helmchart/templates/deployment.yaml new file mode 100644 index 000000000..0c260cc9b --- /dev/null +++ b/internal/fileutil/testdata/helmchart/templates/deployment.yaml @@ -0,0 +1,61 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ include "helmchart.fullname" . }} + labels: + {{- include "helmchart.labels" . | nindent 4 }} +spec: + {{- if not .Values.autoscaling.enabled }} + replicas: {{ .Values.replicaCount }} + {{- end }} + selector: + matchLabels: + {{- include "helmchart.selectorLabels" . | nindent 6 }} + template: + metadata: + {{- with .Values.podAnnotations }} + annotations: + {{- toYaml . | nindent 8 }} + {{- end }} + labels: + {{- include "helmchart.selectorLabels" . | nindent 8 }} + spec: + {{- with .Values.imagePullSecrets }} + imagePullSecrets: + {{- toYaml . | nindent 8 }} + {{- end }} + serviceAccountName: {{ include "helmchart.serviceAccountName" . }} + securityContext: + {{- toYaml .Values.podSecurityContext | nindent 8 }} + containers: + - name: {{ .Chart.Name }} + securityContext: + {{- toYaml .Values.securityContext | nindent 12 }} + image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" + imagePullPolicy: {{ .Values.image.pullPolicy }} + ports: + - name: http + containerPort: 80 + protocol: TCP + livenessProbe: + httpGet: + path: / + port: http + readinessProbe: + httpGet: + path: / + port: http + resources: + {{- toYaml .Values.resources | nindent 12 }} + {{- with .Values.nodeSelector }} + nodeSelector: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.affinity }} + affinity: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.tolerations }} + tolerations: + {{- toYaml . | nindent 8 }} + {{- end }} diff --git a/internal/fileutil/testdata/helmchart/templates/hpa.yaml b/internal/fileutil/testdata/helmchart/templates/hpa.yaml new file mode 100644 index 000000000..acd8e4207 --- /dev/null +++ b/internal/fileutil/testdata/helmchart/templates/hpa.yaml @@ -0,0 +1,28 @@ +{{- if .Values.autoscaling.enabled }} +apiVersion: autoscaling/v2beta1 +kind: HorizontalPodAutoscaler +metadata: + name: {{ include "helmchart.fullname" . }} + labels: + {{- include "helmchart.labels" . | nindent 4 }} +spec: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: {{ include "helmchart.fullname" . }} + minReplicas: {{ .Values.autoscaling.minReplicas }} + maxReplicas: {{ .Values.autoscaling.maxReplicas }} + metrics: + {{- if .Values.autoscaling.targetCPUUtilizationPercentage }} + - type: Resource + resource: + name: cpu + targetAverageUtilization: {{ .Values.autoscaling.targetCPUUtilizationPercentage }} + {{- end }} + {{- if .Values.autoscaling.targetMemoryUtilizationPercentage }} + - type: Resource + resource: + name: memory + targetAverageUtilization: {{ .Values.autoscaling.targetMemoryUtilizationPercentage }} + {{- end }} +{{- end }} diff --git a/internal/fileutil/testdata/helmchart/templates/ingress.yaml b/internal/fileutil/testdata/helmchart/templates/ingress.yaml new file mode 100644 index 000000000..e9440ab38 --- /dev/null +++ b/internal/fileutil/testdata/helmchart/templates/ingress.yaml @@ -0,0 +1,41 @@ +{{- if .Values.ingress.enabled -}} +{{- $fullName := include "helmchart.fullname" . -}} +{{- $svcPort := .Values.service.port -}} +{{- if semverCompare ">=1.14-0" .Capabilities.KubeVersion.GitVersion -}} +apiVersion: networking.k8s.io/v1beta1 +{{- else -}} +apiVersion: extensions/v1beta1 +{{- end }} +kind: Ingress +metadata: + name: {{ $fullName }} + labels: + {{- include "helmchart.labels" . | nindent 4 }} + {{- with .Values.ingress.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + {{- 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 }} + backend: + serviceName: {{ $fullName }} + servicePort: {{ $svcPort }} + {{- end }} + {{- end }} + {{- end }} diff --git a/internal/fileutil/testdata/helmchart/templates/service.yaml b/internal/fileutil/testdata/helmchart/templates/service.yaml new file mode 100644 index 000000000..12e16ef71 --- /dev/null +++ b/internal/fileutil/testdata/helmchart/templates/service.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Service +metadata: + name: {{ include "helmchart.fullname" . }} + labels: + {{- include "helmchart.labels" . | nindent 4 }} +spec: + type: {{ .Values.service.type }} + ports: + - port: {{ .Values.service.port }} + targetPort: http + protocol: TCP + name: http + selector: + {{- include "helmchart.selectorLabels" . | nindent 4 }} diff --git a/internal/fileutil/testdata/helmchart/templates/serviceaccount.yaml b/internal/fileutil/testdata/helmchart/templates/serviceaccount.yaml new file mode 100644 index 000000000..0ff995909 --- /dev/null +++ b/internal/fileutil/testdata/helmchart/templates/serviceaccount.yaml @@ -0,0 +1,12 @@ +{{- if .Values.serviceAccount.create -}} +apiVersion: v1 +kind: ServiceAccount +metadata: + name: {{ include "helmchart.serviceAccountName" . }} + labels: + {{- include "helmchart.labels" . | nindent 4 }} + {{- with .Values.serviceAccount.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +{{- end }} diff --git a/internal/fileutil/testdata/helmchart/templates/tests/test-connection.yaml b/internal/fileutil/testdata/helmchart/templates/tests/test-connection.yaml new file mode 100644 index 000000000..5e64cd3ac --- /dev/null +++ b/internal/fileutil/testdata/helmchart/templates/tests/test-connection.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Pod +metadata: + name: "{{ include "helmchart.fullname" . }}-test-connection" + labels: + {{- include "helmchart.labels" . | nindent 4 }} + annotations: + "helm.sh/hook": test +spec: + containers: + - name: wget + image: busybox + command: ['wget'] + args: ['{{ include "helmchart.fullname" . }}:{{ .Values.service.port }}'] + restartPolicy: Never diff --git a/internal/fileutil/testdata/helmchart/values.yaml b/internal/fileutil/testdata/helmchart/values.yaml new file mode 100644 index 000000000..211528bf7 --- /dev/null +++ b/internal/fileutil/testdata/helmchart/values.yaml @@ -0,0 +1,83 @@ +# Default values for helmchart. +# This is a YAML-formatted file. +# Declare variables to be passed into your templates. + +replicaCount: 1 + +image: + repository: nginx + pullPolicy: IfNotPresent + # Overrides the image tag whose default is the chart appVersion. + tag: "" + +imagePullSecrets: [] +nameOverride: "" +fullnameOverride: "" + +serviceAccount: + # Specifies whether a service account should be created + create: true + # Annotations to add to the service account + annotations: {} + # The name of the service account to use. + # If not set and create is true, a name is generated using the fullname template + name: "" + +podAnnotations: {} + +podSecurityContext: {} + # fsGroup: 2000 + +securityContext: {} + # capabilities: + # drop: + # - ALL + # readOnlyRootFilesystem: true + # runAsNonRoot: true + # runAsUser: 1000 + +service: + type: ClusterIP + port: 80 + +ingress: + enabled: false + annotations: {} + # kubernetes.io/ingress.class: nginx + # kubernetes.io/tls-acme: "true" + hosts: + - host: chart-example.local + paths: + - path: / + backend: + serviceName: chart-example.local + servicePort: 80 + tls: [] + # - secretName: chart-example-tls + # hosts: + # - chart-example.local + +resources: {} + # We usually recommend not to specify default resources and to leave this as a conscious + # choice for the user. This also increases chances charts run on environments with little + # resources, such as Minikube. If you do want to specify resources, uncomment the following + # lines, adjust them as necessary, and remove the curly braces after 'resources:'. + # limits: + # cpu: 100m + # memory: 128Mi + # requests: + # cpu: 100m + # memory: 128Mi + +autoscaling: + enabled: false + minReplicas: 1 + maxReplicas: 100 + targetCPUUtilizationPercentage: 80 + # targetMemoryUtilizationPercentage: 80 + +nodeSelector: {} + +tolerations: [] + +affinity: {} diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 86cf7227d..f4ebe2a91 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -124,7 +124,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string if !found { return nil, fmt.Errorf(`dependency %q is missing git branch or tag: %s. - When using a "git:" type repository, the "version" should be a valid branch or tag name`, d.Name, d.Version) + When using a "git://" type repository, the "version" should be a valid branch or tag name`, d.Name, d.Version) } locked[i] = &chart.Dependency{ @@ -135,10 +135,6 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string continue } - if err != nil { - return nil, errors.Wrapf(err, "dependency %q has an invalid version/constraint format", d.Name) - } - repoName := repoNames[d.Name] // if the repository was not defined, but the dependency defines a repository url, bypass the cache if repoName == "" && d.Repository != "" { diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 37dabb769..e8fe63d73 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -25,7 +25,7 @@ import ( func fakeGetRefs(gitRepoURL string) (map[string]string, error) { refs := map[string]string{ - "master": "9668ad4d90c5e95bd520e58e7387607be6b63bb6", + "1.0.0": "9668ad4d90c5e95bd520e58e7387607be6b63bb6", } return refs, nil } @@ -146,13 +146,24 @@ func TestResolve(t *testing.T) { err: true, }, { - name: "repo from git url", + name: "repo from git ssh url", req: []*chart.Dependency{ - {Name: "gitdependency", Repository: "git:git@github.com:helm/gitdependency.git", Version: "master"}, + {Name: "gitdependency", Repository: "git:git@github.com:helm/gitdependency.git", Version: "1.0.0"}, }, expect: &chart.Lock{ Dependencies: []*chart.Dependency{ - {Name: "gitdependency", Repository: "git:git@github.com:helm/gitdependency.git", Version: "master"}, + {Name: "gitdependency", Repository: "git:git@github.com:helm/gitdependency.git", Version: "1.0.0"}, + }, + }, + }, + { + name: "repo from git https url", + req: []*chart.Dependency{ + {Name: "gitdependency", Repository: "https://github.com/helm/gitdependency.git", Version: "1.0.0"}, + }, + expect: &chart.Lock{ + Dependencies: []*chart.Dependency{ + {Name: "gitdependency", Repository: "https://github.com/helm/gitdependency.git", Version: "1.0.0"}, }, }, }, diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 23bd50ed6..f07cf274b 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -26,6 +26,8 @@ import ( "github.com/Masterminds/semver/v3" "github.com/pkg/errors" + giturls "github.com/whilp/git-urls" + "helm.sh/helm/v3/internal/fileutil" "helm.sh/helm/v3/internal/urlutil" "helm.sh/helm/v3/pkg/getter" @@ -92,19 +94,20 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven return "", nil, err } - g, err := c.Getters.ByScheme(u.Scheme) - if err != nil { - return "", nil, err - } + scheme := "" - downloadURL := "" - if u.Scheme == "git" { - downloadURL = u.Host + u.Path + if strings.HasPrefix(ref, "git://") { + scheme = "git" } else { - downloadURL = u.String() + scheme = u.Scheme } - data, err := g.Get(downloadURL, c.Options...) + g, err := c.Getters.ByScheme(scheme) + if err != nil { + return "", nil, err + } + + data, err := g.Get(u.String(), c.Options...) if err != nil { return "", nil, err } @@ -114,6 +117,13 @@ func (c *ChartDownloader) DownloadTo(ref, version, dest string) (string, *proven idx := strings.LastIndexByte(name, ':') name = fmt.Sprintf("%s-%s.tgz", name[:idx], name[idx+1:]) } + if scheme == "git" { + gitGetter, ok := g.(*getter.GITGetter) + if !ok { + return "", nil, fmt.Errorf("can't convert to GITGetter") + } + name = fmt.Sprintf("%s-%s.tgz", gitGetter.ChartName(), version) + } destfile := filepath.Join(dest, name) if err := fileutil.AtomicWriteFile(destfile, data, 0644); err != nil { @@ -187,7 +197,7 @@ func (c *ChartDownloader) getOciURI(ref, version string, u *url.URL) (*url.URL, // the URL using the appropriate Getter. // // A reference may be an HTTP URL, an oci reference URL, a 'reponame/chartname' -// reference, or a local path. +// reference, git URL, or a local path. // // A version is a SemVer string (1.2.3-beta.1+f334a6789). // @@ -197,18 +207,13 @@ func (c *ChartDownloader) getOciURI(ref, version string, u *url.URL) (*url.URL, // * If version is empty, this will return the URL for the latest version // * If no version can be found, an error is returned func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, error) { - if strings.HasPrefix(ref, "git:") { - gitRefSplitResult := strings.Split(ref, "git:") - gitURL := gitRefSplitResult[1] - u, err := url.Parse(gitURL) + if strings.HasPrefix(ref, "git://") { + gitURL := strings.TrimPrefix(ref, "git://") + u, err := giturls.Parse(gitURL) if err != nil { return nil, errors.Errorf("invalid git URL format: %s", gitURL) } - return &url.URL{ - Scheme: "git", - Host: u.Scheme + "://" + u.Host, - Path: u.Path, - }, nil + return u, nil } u, err := url.Parse(ref) if err != nil { diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index f70a56422..3c27f1cfa 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -40,6 +40,7 @@ func TestResolveChartRef(t *testing.T) { {name: "full URL", ref: "http://example.com/foo-1.2.3.tgz", expect: "http://example.com/foo-1.2.3.tgz"}, {name: "full URL, HTTPS", ref: "https://example.com/foo-1.2.3.tgz", expect: "https://example.com/foo-1.2.3.tgz"}, {name: "full URL, with authentication", ref: "http://username:password@example.com/foo-1.2.3.tgz", expect: "http://username:password@example.com/foo-1.2.3.tgz"}, + {name: "helmchart", ref: "git://https://github.com/helmchart/helmchart.git", expect: "https://github.com/helmchart/helmchart.git"}, {name: "reference, testing repo", ref: "testing/alpine", expect: "http://example.com/alpine-1.2.3.tgz"}, {name: "reference, version, testing repo", ref: "testing/alpine", version: "0.2.0", expect: "http://example.com/alpine-0.2.0.tgz"}, {name: "reference, version, malformed repo", ref: "malformed/alpine", version: "1.2.3", expect: "http://dl.example.com/alpine-1.2.3.tgz"}, diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 4222d3d96..c9a95ace7 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -619,7 +619,7 @@ func (m *Manager) resolveRepoNames(deps []*chart.Dependency) (map[string]string, // if dep chart is from a git url, assume it is valid for now. // if the repo does not exist then it will later error when we try to fetch branches and tags. // we could check for the repo existence here, but trying to avoid anotehr git request. - if strings.HasPrefix(dd.Repository, "git:") { + if strings.HasPrefix(dd.Repository, "git://") { if m.Debug { fmt.Fprintf(m.Out, "Repository from git url: %s\n", strings.TrimPrefix(dd.Repository, "git:")) } diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index b31533517..0652f6440 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -80,55 +80,36 @@ func TestFindChartURL(t *testing.T) { t.Fatal(err) } - name := "alpine" - version := "0.1.0" - repoURL := "http://example.com/charts" - - churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(name, version, repoURL, repos) - if err != nil { - t.Fatal(err) - } - - if churl != "https://charts.helm.sh/stable/alpine-0.1.0.tgz" { - t.Errorf("Unexpected URL %q", churl) - } - if username != "" { - t.Errorf("Unexpected username %q", username) - } - if password != "" { - t.Errorf("Unexpected password %q", password) - } - if passcredentialsall != false { - t.Errorf("Unexpected passcredentialsall %t", passcredentialsall) - } - if insecureSkipTLSVerify { - t.Errorf("Unexpected insecureSkipTLSVerify %t", insecureSkipTLSVerify) + tests := []struct { + name, version, repoURL, expectChurl, expectUserName, expectPassword string + expectInsecureSkipTLSVerify, expectPasscredentialsall bool + }{ + {name: "alpine", version: "0.1.0", repoURL: "http://example.com/charts", expectChurl: "https://charts.helm.sh/stable/alpine-0.1.0.tgz", expectUserName: "", expectPassword: "", expectInsecureSkipTLSVerify: false, expectPasscredentialsall: false}, + {name: "tlsfoo", version: "1.2.3", repoURL: "https://example-https-insecureskiptlsverify.com", expectChurl: "https://example.com/tlsfoo-1.2.3.tgz", expectUserName: "", expectPassword: "", expectInsecureSkipTLSVerify: true, expectPasscredentialsall: false}, + {name: "helm-test", version: "master", repoURL: "git://https://github.com/rally25rs/helm-test-chart.git", expectChurl: "git://https://github.com/rally25rs/helm-test-chart.git", expectUserName: "", expectPassword: "", expectInsecureSkipTLSVerify: false, expectPasscredentialsall: false}, } - - name = "tlsfoo" - version = "1.2.3" - repoURL = "https://example-https-insecureskiptlsverify.com" - - churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err = m.findChartURL(name, version, repoURL, repos) - if err != nil { - t.Fatal(err) + for _, tt := range tests { + churl, username, password, insecureSkipTLSVerify, passcredentialsall, _, _, _, err := m.findChartURL(tt.name, tt.version, tt.repoURL, repos) + if err != nil { + t.Fatal(err) + } + if churl != tt.expectChurl { + t.Errorf("Unexpected URL %q", churl) + } + if username != tt.expectUserName { + t.Errorf("Unexpected username %q", username) + } + if password != tt.expectPassword { + t.Errorf("Unexpected password %q", password) + } + if insecureSkipTLSVerify != tt.expectInsecureSkipTLSVerify { + t.Errorf("Unexpected insecureSkipTLSVerify %t", insecureSkipTLSVerify) + } + if passcredentialsall != tt.expectPasscredentialsall { + t.Errorf("Unexpected passcredentialsall %t", passcredentialsall) + } } - if !insecureSkipTLSVerify { - t.Errorf("Unexpected insecureSkipTLSVerify %t", insecureSkipTLSVerify) - } - if churl != "https://example.com/tlsfoo-1.2.3.tgz" { - t.Errorf("Unexpected URL %q", churl) - } - if username != "" { - t.Errorf("Unexpected username %q", username) - } - if password != "" { - t.Errorf("Unexpected password %q", password) - } - if passcredentialsall != false { - t.Errorf("Unexpected passcredentialsall %t", passcredentialsall) - } } func TestGetRepoNames(t *testing.T) { @@ -196,9 +177,9 @@ func TestGetRepoNames(t *testing.T) { { name: "repo from git url", req: []*chart.Dependency{ - {Name: "local-dep", Repository: "git:https://github.com/git/git"}, + {Name: "local-dep", Repository: "git://https://github.com/git/git"}, }, - expect: map[string]string{"local-dep": "git:https://github.com/git/git"}, + expect: map[string]string{"local-dep": "git://https://github.com/git/git"}, }, } diff --git a/pkg/getter/gitgetter.go b/pkg/getter/gitgetter.go index c4b46a530..470e51681 100644 --- a/pkg/getter/gitgetter.go +++ b/pkg/getter/gitgetter.go @@ -20,7 +20,6 @@ import ( "strings" "fmt" - "io/ioutil" "os" "path/filepath" @@ -36,6 +35,10 @@ type GITGetter struct { opts options } +func (g *GITGetter) ChartName() string { + return g.opts.chartName +} + // ensureGitDirIgnored will append ".git/" to the .helmignore file in a directory. // Create the .helmignore file if it does not exist. func (g *GITGetter) ensureGitDirIgnored(repoPath string) error { @@ -60,13 +63,13 @@ func (g *GITGetter) Get(href string, options ...Option) (*bytes.Buffer, error) { } func (g *GITGetter) get(href string) (*bytes.Buffer, error) { - gitURL := strings.TrimPrefix(href, "git:") + gitURL := strings.TrimPrefix(href, "git://") version := g.opts.version chartName := g.opts.chartName if version == "" { return nil, fmt.Errorf("The version must be a valid tag or branch name for the git repo, not nil") } - tmpDir, err := ioutil.TempDir("", "helm") + tmpDir, err := os.MkdirTemp("", "helm") if err != nil { return nil, err } @@ -84,7 +87,7 @@ func (g *GITGetter) get(href string) (*bytes.Buffer, error) { // A .helmignore that includes an ignore for .git/ should be included in the git repo itself, // but a lot of people will probably not think about that. // To prevent the git history from bleeding into the charts archive, append/create .helmignore. - g.ensureGitDirIgnored(tmpDir) + g.ensureGitDirIgnored(chartTmpDir) buf, err := fileutil.CompressDirToTgz(chartTmpDir, tmpDir) if err != nil { @@ -93,7 +96,7 @@ func (g *GITGetter) get(href string) (*bytes.Buffer, error) { return buf, nil } -// NewGITGetter constructs a valid http/https client as a Getter +// NewGITGetter constructs a valid git client as a Getter func NewGITGetter(ops ...Option) (Getter, error) { client := GITGetter{}