feat: Add wildcard support in OCI Helm Repositories targetRevision (a…

…rgoproj#6686) (argoproj#10641) * Add wildcard support in OCI Helm Repositories A naive approach, adapting existing code for fetching the index. Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * Fix unittest missing mock Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * Fix release resolution also in Manual Sync dialog Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * Show target revision in application list. Tiles and Table updated Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * Follow Link rel=next in tags response for tag list completion (signed) Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * Wrap errors into fmt.Errorf according to PR review Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * Address PR comments, add test for tags MaxVersion and pagination Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * Apply suggestions from code review Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * more feedback from pr Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * Revert url.JoinPath change - only available in 1.19 Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * use strings.Join. add unittest Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * Safe access to app.status.sync object Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * Remove status.revision from UI. It doesn't bring much value and it does clutter the ui a bit Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> * Update util/helm/client.go Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> * Update util/helm/client.go Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com> Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Signed-off-by: emirot <emirot.nolan@gmail.com>
emirot · Jan 27, 2023 · 727be5e · 727be5e
1 parent 23e7089
commit 727be5e
Show file tree

Hide file tree

Showing 8 changed files with 330 additions and 22 deletions.
diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go
@@ -1956,14 +1956,27 @@ func (s *Service) newClientResolveRevision(repo *v1alpha1.Repository, revision s
 func (s *Service) newHelmClientResolveRevision(repo *v1alpha1.Repository, revision string, chart string, noRevisionCache bool) (helm.Client, string, error) {
 	enableOCI := repo.EnableOCI || helm.IsHelmOciRepo(repo.Repo)
 	helmClient := s.newHelmClient(repo.Repo, repo.GetHelmCreds(), enableOCI, repo.Proxy, helm.WithIndexCache(s.cache), helm.WithChartPaths(s.chartPaths))
-	// OCI helm registers don't support semver ranges. Assuming that given revision is exact version
-	if helm.IsVersion(revision) || enableOCI {
+	if helm.IsVersion(revision) {
 		return helmClient, revision, nil
 	}
 	constraints, err := semver.NewConstraint(revision)
 	if err != nil {
 		return nil, "", fmt.Errorf("invalid revision '%s': %v", revision, err)
 	}
+
+	if enableOCI {
+		tags, err := helmClient.GetTags(chart, noRevisionCache)
+		if err != nil {
+			return nil, "", fmt.Errorf("unable to get tags: %v", err)
+		}
+
+		version, err := tags.MaxVersion(constraints)
+		if err != nil {
+			return nil, "", fmt.Errorf("no version for constraints: %v", err)
+		}
+		return helmClient, version.String(), nil
+	}
+
 	index, err := helmClient.GetIndex(noRevisionCache)
 	if err != nil {
 		return nil, "", err
@@ -2099,30 +2112,14 @@ func (s *Service) ResolveRevision(ctx context.Context, q *apiclient.ResolveRevis
 	ambiguousRevision := q.AmbiguousRevision
 	var revision string
 	if app.Spec.Source.IsHelm() {
+		_, revision, err := s.newHelmClientResolveRevision(repo, ambiguousRevision, app.Spec.Source.Chart, true)
 
-		if helm.IsVersion(ambiguousRevision) {
-			return &apiclient.ResolveRevisionResponse{Revision: ambiguousRevision, AmbiguousRevision: ambiguousRevision}, nil
-		}
-		client := helm.NewClient(repo.Repo, repo.GetHelmCreds(), repo.EnableOCI || app.Spec.Source.IsHelmOci(), repo.Proxy, helm.WithChartPaths(s.chartPaths))
-		index, err := client.GetIndex(false)
-		if err != nil {
-			return &apiclient.ResolveRevisionResponse{Revision: "", AmbiguousRevision: ""}, err
-		}
-		entries, err := index.GetEntries(app.Spec.Source.Chart)
-		if err != nil {
-			return &apiclient.ResolveRevisionResponse{Revision: "", AmbiguousRevision: ""}, err
-		}
-		constraints, err := semver.NewConstraint(ambiguousRevision)
-		if err != nil {
-			return &apiclient.ResolveRevisionResponse{Revision: "", AmbiguousRevision: ""}, err
-		}
-		version, err := entries.MaxVersion(constraints)
 		if err != nil {
 			return &apiclient.ResolveRevisionResponse{Revision: "", AmbiguousRevision: ""}, err
 		}
 		return &apiclient.ResolveRevisionResponse{
-			Revision:          version.String(),
-			AmbiguousRevision: fmt.Sprintf("%v (%v)", ambiguousRevision, version.String()),
+			Revision:          revision,
+			AmbiguousRevision: fmt.Sprintf("%v (%v)", ambiguousRevision, revision),
 		}, nil
 	} else {
 		gitClient, err := git.NewClient(repo.Repo, repo.GetGitCreds(s.gitCredsStore), repo.IsInsecure(), repo.IsLFSEnabled(), repo.Proxy)

diff --git a/ui/src/app/applications/components/applications-list/applications-list.tsx b/ui/src/app/applications/components/applications-list/applications-list.tsx
@@ -40,6 +40,7 @@ const APP_FIELDS = [
     'spec',
     'operation.sync',
     'status.sync.status',
+    'status.sync.revision',
     'status.health',
     'status.operationState.phase',
     'status.operationState.operation.sync',

diff --git a/ui/src/app/applications/components/applications-list/applications-tiles.tsx b/ui/src/app/applications/components/applications-list/applications-tiles.tsx
@@ -224,7 +224,7 @@ export const ApplicationTiles = ({applications, syncApplication, refreshApplicat
                                                         <div className='columns small-3' title='Target Revision:'>
                                                             Target Revision:
                                                         </div>
-                                                        <div className='columns small-9'>{app.spec.source.targetRevision}</div>
+                                                        <div className='columns small-9'>{app.spec.source.targetRevision || 'HEAD'}</div>
                                                     </div>
                                                     {app.spec.source.path && (
                                                         <div className='row'>

diff --git a/util/helm/client.go b/util/helm/client.go
@@ -21,11 +21,13 @@ import (
 	log "github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v2"
 
+	"github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/util/cache"
 	executil "github.com/argoproj/argo-cd/v2/util/exec"
 	argoio "github.com/argoproj/argo-cd/v2/util/io"
 	"github.com/argoproj/argo-cd/v2/util/io/files"
 	"github.com/argoproj/argo-cd/v2/util/proxy"
+	"github.com/argoproj/argo-cd/v2/util/text"
 )
 
 var (
@@ -51,6 +53,7 @@ type Client interface {
 	CleanChartCache(chart string, version string) error
 	ExtractChart(chart string, version string, passCredentials bool) (string, argoio.Closer, error)
 	GetIndex(noCache bool) (*Index, error)
+	GetTags(chart string, noCache bool) (*TagsList, error)
 	TestHelmOCI() (bool, error)
 }
 
@@ -381,3 +384,148 @@ func getIndexURL(rawURL string) (string, error) {
 	repoURL.RawPath = path.Join(repoURL.RawPath, indexFile)
 	return repoURL.String(), nil
 }
+
+func getTagsListURL(rawURL string, chart string) (string, error) {
+	repoURL, err := url.Parse(strings.Trim(rawURL, "/"))
+	if err != nil {
+		return "", fmt.Errorf("unable to parse repo url: %v", err)
+	}
+	repoURL.Scheme = "https"
+	tagsList := strings.Join([]string{"v2", url.PathEscape(chart), "tags/list"}, "/")
+	repoURL.Path = strings.Join([]string{repoURL.Path, tagsList}, "/")
+	repoURL.RawPath = strings.Join([]string{repoURL.RawPath, tagsList}, "/")
+	return repoURL.String(), nil
+}
+
+func (c *nativeHelmChart) getTags(chart string) ([]byte, error) {
+	nextURL, err := getTagsListURL(c.repoURL, chart)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get tag list url: %v", err)
+	}
+
+	allTags := &TagsList{}
+	var data []byte
+	for nextURL != "" {
+		log.Debugf("fetching %s tags from %s", chart, text.Trunc(nextURL, 100))
+		data, nextURL, err = c.getTagsFromUrl(nextURL)
+		if err != nil {
+			return nil, fmt.Errorf("failed tags part: %v", err)
+		}
+
+		tags := &TagsList{}
+		err := json.Unmarshal(data, tags)
+		if err != nil {
+			return nil, fmt.Errorf("unable to decode json: %v", err)
+		}
+		allTags.Tags = append(allTags.Tags, tags.Tags...)
+	}
+	data, err = json.Marshal(allTags)
+	if err != nil {
+		return nil, fmt.Errorf("failed to marshal tag json: %w", err)
+	}
+	return data, nil
+}
+
+func getNextUrl(linkHeader string) string {
+	nextUrl := ""
+	if linkHeader != "" {
+		// drop < >; ref= from the Link header, see: https://docs.docker.com/registry/spec/api/#pagination
+		nextUrl = strings.Split(linkHeader, ";")[0][1:]
+		nextUrl = nextUrl[:len(nextUrl)-1]
+	}
+	return nextUrl
+}
+
+func (c *nativeHelmChart) getTagsFromUrl(tagsURL string) ([]byte, string, error) {
+	req, err := http.NewRequest("GET", tagsURL, nil)
+	if err != nil {
+		return nil, "", fmt.Errorf("failed create request: %v", err)
+	}
+	req.Header.Add("Accept", `application/json`)
+	if c.creds.Username != "" || c.creds.Password != "" {
+		// only basic supported
+		req.SetBasicAuth(c.creds.Username, c.creds.Password)
+	}
+
+	tlsConf, err := newTLSConfig(c.creds)
+	if err != nil {
+		return nil, "", fmt.Errorf("failed setup tlsConfig: %v", err)
+	}
+
+	tr := &http.Transport{
+		Proxy:           proxy.GetCallback(c.proxy),
+		TLSClientConfig: tlsConf,
+	}
+	client := http.Client{Transport: tr}
+	resp, err := client.Do(req)
+	if err != nil {
+		return nil, "", fmt.Errorf("request failed: %v", err)
+	}
+	defer func() {
+		if err = resp.Body.Close(); err != nil {
+			log.WithFields(log.Fields{
+				common.SecurityField:    common.SecurityMedium,
+				common.SecurityCWEField: 775,
+			}).Errorf("error closing response %q: %v", text.Trunc(tagsURL, 100), err)
+		}
+	}()
+
+	if resp.StatusCode != 200 {
+		data, err := io.ReadAll(resp.Body)
+		var responseExcerpt string
+		if err != nil {
+			responseExcerpt = fmt.Sprintf("err: %v", err)
+		} else {
+			responseExcerpt = text.Trunc(string(data), 100)
+		}
+		return nil, "", fmt.Errorf("invalid response: %s %s", resp.Status, responseExcerpt)
+	}
+	data, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, "", fmt.Errorf("failed to read body: %v", err)
+	}
+	nextUrl := getNextUrl(resp.Header.Get("Link"))
+	return data, nextUrl, nil
+}
+
+func (c *nativeHelmChart) GetTags(chart string, noCache bool) (*TagsList, error) {
+	tagsURL, err := getTagsListURL(c.repoURL, chart)
+	if err != nil {
+		return nil, fmt.Errorf("invalid tags url: %v", err)
+	}
+	indexLock.Lock(tagsURL)
+	defer indexLock.Unlock(tagsURL)
+
+	var data []byte
+	if !noCache && c.indexCache != nil {
+		if err := c.indexCache.GetHelmIndex(tagsURL, &data); err != nil && err != cache.ErrCacheMiss {
+			log.Warnf("Failed to load index cache for repo: %s: %v", tagsURL, err)
+		}
+	}
+
+	if len(data) == 0 {
+		start := time.Now()
+		var err error
+		data, err = c.getTags(chart)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get tags: %v", err)
+		}
+		log.WithFields(
+			log.Fields{"seconds": time.Since(start).Seconds(), "chart": chart, "repo": c.repoURL},
+		).Info("took to get tags")
+
+		if c.indexCache != nil {
+			if err := c.indexCache.SetHelmIndex(tagsURL, data); err != nil {
+				log.Warnf("Failed to store tags list cache for repo: %s: %v", tagsURL, err)
+			}
+		}
+	}
+
+	tags := &TagsList{}
+	err = json.Unmarshal(data, tags)
+	if err != nil {
+		return nil, fmt.Errorf("failed to decode tags: %v", err)
+	}
+
+	return tags, nil
+}
diff --git a/util/helm/client_test.go b/util/helm/client_test.go
@@ -2,10 +2,15 @@ package helm
 
 import (
 	"bytes"
+	"encoding/json"
 	"fmt"
 	"os"
+	"strings"
 	"testing"
 
+	"net/http"
+	"net/http/httptest"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/yaml.v2"
@@ -145,3 +150,51 @@ func TestGetIndexURL(t *testing.T) {
 		assert.Error(t, err)
 	})
 }
+
+func TestGetTagsFromUrl(t *testing.T) {
+	server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		responseTags := TagsList{}
+		w.Header().Set("Content-Type", "application/json")
+		if !strings.Contains(r.URL.String(), "token") {
+			w.Header().Set("Link", fmt.Sprintf("<https://%s%s?token=next-token>; rel=next", r.Host, r.URL.Path))
+			responseTags.Tags = []string{"first"}
+		} else {
+			responseTags.Tags = []string{"second"}
+		}
+		w.WriteHeader(http.StatusOK)
+		err := json.NewEncoder(w).Encode(responseTags)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}))
+
+	client := NewClient(server.URL, Creds{InsecureSkipVerify: true}, true, "")
+
+	tags, err := client.GetTags("mychart", true)
+	assert.NoError(t, err)
+	assert.Equal(t, tags.Tags[0], "first")
+	assert.Equal(t, tags.Tags[1], "second")
+}
+
+func Test_getNextUrl(t *testing.T) {
+	nextUrl := getNextUrl("")
+	assert.Equal(t, nextUrl, "")
+
+	nextUrl = getNextUrl("<https://my.repo.com/v2/chart/tags/list?token=123>; rel=next")
+	assert.Equal(t, nextUrl, "https://my.repo.com/v2/chart/tags/list?token=123")
+}
+
+func Test_getTagsListURL(t *testing.T) {
+	tagsListURL, err := getTagsListURL("account.dkr.ecr.eu-central-1.amazonaws.com", "dss")
+	assert.Nil(t, err)
+	assert.Equal(t, tagsListURL, "https://account.dkr.ecr.eu-central-1.amazonaws.com/v2/dss/tags/list")
+
+	tagsListURL, err = getTagsListURL("http://account.dkr.ecr.eu-central-1.amazonaws.com", "dss")
+	assert.Nil(t, err)
+	assert.Equal(t, tagsListURL, "https://account.dkr.ecr.eu-central-1.amazonaws.com/v2/dss/tags/list")
+
+	// with trailing /
+	tagsListURL, err = getTagsListURL("https://account.dkr.ecr.eu-central-1.amazonaws.com/", "dss")
+	assert.Nil(t, err)
+	assert.Equal(t, tagsListURL, "https://account.dkr.ecr.eu-central-1.amazonaws.com/v2/dss/tags/list")
+}
diff --git a/util/helm/mocks/Client.go b/util/helm/mocks/Client.go
diff --git a/util/helm/tags.go b/util/helm/tags.go
@@ -0,0 +1,42 @@
+package helm
+
+import (
+    "fmt"
+
+    log "github.com/sirupsen/logrus"
+
+    "github.com/Masterminds/semver/v3"
+)
+
+type TagsList struct {
+	Tags []string
+}
+
+func (t TagsList) MaxVersion(constraints *semver.Constraints) (*semver.Version, error) {
+	versions := semver.Collection{}
+	for _, tag := range t.Tags {
+		v, err := semver.NewVersion(tag)
+
+		//Invalid semantic version ignored
+		if err == semver.ErrInvalidSemVer {
+			log.Debugf("Invalid semantic version: %s", tag)
+			continue
+		}
+		if err != nil {
+			return nil, fmt.Errorf("invalid constraint in tags: %v", err)
+		}
+		if constraints.Check(v) {
+			versions = append(versions, v)
+		}
+	}
+	if len(versions) == 0 {
+		return nil, fmt.Errorf("constraint not found in %v tags", len(t.Tags))
+	}
+	maxVersion := versions[0]
+	for _, v := range versions {
+		if v.GreaterThan(maxVersion) {
+			maxVersion = v
+		}
+	}
+	return maxVersion, nil
+}