Adds support for Helm 1st-class. Closes #1145 #1865
Conversation
|
Failed |
|
|
| @@ -122,7 +122,7 @@ func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, source v1alpha1 | |||
| } | |||
| manifestInfo, err := repoClient.GenerateManifest(context.Background(), &apiclient.ManifestRequest{ | |||
| Repo: repo, | |||
| HelmRepos: helmRepos, | |||
There was a problem hiding this comment.
I would remove Repo field, since now we are passing all repositories and repo server just need to find matching one by URL
There was a problem hiding this comment.
sorry - don't think it works like that
There was a problem hiding this comment.
repos field now contains all repositories ( git and helm ). Repository server already filters give repos by type for helm applications. In the same way repo server might filter repo by URL. Is not it?
There was a problem hiding this comment.
Added few comments. As we discuss offline we should make following improvements:
- allow user to specify repo type during app creation without registering it. Now we are forcing user to register repo url using type helm even if it is publicly available.
- app creation URL for helm case should be different from git (path field should be renamed to chart, we should try to auto-suggest available revisions)
- same for repo creation UI. If user selects
helmtype when helm specific fields should be shown. - as @jannfis mentioned we should reuse existing CA management feature
We definitely have to implement these enhancements, so I'm ok to merge this PR and implement improvements later.
Please fix regression before merging this PR.
| return HTTPSCreds{ | ||
| username, | ||
| password, | ||
| insecure, | ||
| clientCertData, | ||
| clientCertKey, | ||
| clientCAData, |
There was a problem hiding this comment.
100% agree. We should remove CA data and reuse existing functionality.
| @@ -98,32 +98,28 @@ type OIDCConfig struct { | |||
| type RepoCredentials struct { | |||
| // The URL to the repository | |||
| URL string `json:"url,omitempty"` | |||
| // the type of the repo, "git" or "helm", assumed to be "git" if empty or absent | |||
There was a problem hiding this comment.
Instead of type lets introduce two mutually exclusive fields git and helm and move type-specific fields there.
This commit fixes Helm parsing of parameters values containing a comma. The issue was first found as argoproj#1660, and fixed in argoproj#1720. However, commit 4e9772e, in argoproj#1865 removed the call to `cleanHelmParameters`, hence the regression.
This contains a couple of changes that rollup to the new feature:
helm.repositoriesis merged intorepositories. This is a breaking change that actually simplifies the code.util/repowhich contains theRepointerface and value-objects that specify a more generalized concept of a repository.util/repo/factorywhich contains a factory that takes aRepository(as defined in the core filetypes.go) and returns aRepoto create one.helmcommand intoutil/helm/cmd.go, to allow for reuse of that code.util/git/repo.gothat implements theRepointerface.util/helm/repo.gothat implements theRepointerface.Another key consideration is that the concept of
HEADis generalized intolatest.Closes #1145