Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
Make chart ops insensitive to (missing) slashes
Browse files Browse the repository at this point in the history
Before this change it was possible that a user provided correct
credentials for a repository, but the operator was unable to find them
due to too strict comparisons, also, the URL the operator constructed
to download a tarball of a chart was missing a path element when a
given repo URL did not end with a trailing slash.

To get around the first issue the repo URL from the `repositories.yaml`
and the repo URL from the given HelmRelease are both stripped from their
trailing slashes during comparison.

To get around the second issue a CleanRepoURL function was added and
implemented, which returns the RepoURL but ensures it always ends with
a trailing slash.
  • Loading branch information
hiddeco committed Feb 14, 2019
1 parent b9ec3e7 commit 65f8545
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 6 deletions.
12 changes: 11 additions & 1 deletion integrations/apis/flux.weave.works/v1beta1/types.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package v1beta1

import (
"strings"

"github.com/ghodss/yaml"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/helm/pkg/chartutil"

Expand Down Expand Up @@ -62,6 +64,14 @@ type RepoChartSource struct {
ChartPullSecret *v1.LocalObjectReference `json:"chartPullSecret,omitempty"`
}

// CleanRepoURL returns the RepoURL but ensures it ends with a trailing slash
func (s RepoChartSource) CleanRepoURL() string {
if strings.HasSuffix(s.RepoURL, "/") {
return s.RepoURL
}
return s.RepoURL + "/"
}

// FluxHelmReleaseSpec is the spec for a FluxHelmRelease resource
// FluxHelmReleaseSpec
type HelmReleaseSpec struct {
Expand Down
15 changes: 10 additions & 5 deletions integrations/helm/chartsync/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/url"
"os"
"path/filepath"
"strings"

"github.com/spf13/pflag"
"k8s.io/helm/pkg/getter"
Expand All @@ -23,7 +24,7 @@ func makeChartPath(base string, source *flux_v1beta1.RepoChartSource) string {
// We don't need to obscure the location of the charts in the
// filesystem; but we do need a stable, filesystem-friendly path
// to them that is based on the URL.
repoPath := filepath.Join(base, base64.URLEncoding.EncodeToString([]byte(source.RepoURL)))
repoPath := filepath.Join(base, base64.URLEncoding.EncodeToString([]byte(source.CleanRepoURL())))
if err := os.MkdirAll(repoPath, os.FileMode(os.ModeDir+0660)); err != nil {
panic(err)
}
Expand Down Expand Up @@ -82,7 +83,7 @@ func downloadChart(destFile string, source *flux_v1beta1.RepoChartSource) error
// we'll assume there's no auth needed.
repoEntry := &repo.Entry{}
for _, entry := range repoFile.Repositories {
if entry.URL == source.RepoURL {
if urlsMatch(entry.URL, source.CleanRepoURL()) {
repoEntry = entry
break
}
Expand All @@ -91,7 +92,7 @@ func downloadChart(destFile string, source *flux_v1beta1.RepoChartSource) error
// TODO(michael): could look for an existing index file here,
// and/or update it. Then we're _pretty_ close to just using
// `repo.DownloadTo(...)`.
chartUrl, err := repo.FindChartInAuthRepoURL(source.RepoURL, repoEntry.Username, repoEntry.Password, source.Name, source.Version, repoEntry.CertFile, repoEntry.KeyFile, repoEntry.CAFile, getters)
chartURL, err := repo.FindChartInAuthRepoURL(source.CleanRepoURL(), repoEntry.Username, repoEntry.Password, source.Name, source.Version, repoEntry.CertFile, repoEntry.KeyFile, repoEntry.CAFile, getters)
if err != nil {
return err
}
Expand All @@ -102,7 +103,7 @@ func downloadChart(destFile string, source *flux_v1beta1.RepoChartSource) error
// former interacts with Helm's local caching, which would mean
// having to maintain the local cache. Since we already have the
// information we need, we can just go ahead and get the file.
u, err := url.Parse(chartUrl)
u, err := url.Parse(chartURL)
if err != nil {
return err
}
Expand All @@ -111,7 +112,7 @@ func downloadChart(destFile string, source *flux_v1beta1.RepoChartSource) error
return err
}

g, err := getterConstructor(chartUrl, repoEntry.CertFile, repoEntry.KeyFile, repoEntry.CAFile)
g, err := getterConstructor(chartURL, repoEntry.CertFile, repoEntry.KeyFile, repoEntry.CAFile)
if t, ok := g.(*getter.HttpGetter); ok {
t.SetCredentials(repoEntry.Username, repoEntry.Password)
}
Expand All @@ -126,3 +127,7 @@ func downloadChart(destFile string, source *flux_v1beta1.RepoChartSource) error

return nil
}

func urlsMatch(entryURL, sourceURL string) bool {
return strings.TrimRight(entryURL, "/") == strings.TrimRight(sourceURL, "/")
}

0 comments on commit 65f8545

Please sign in to comment.