diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 286eb7eb0f215..c579eeb5b6591 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -345,7 +345,7 @@ func (s *Service) runRepoOperation( if source.IsHelm() { if settings.noCache { - err = helmClient.CleanChartCache(source.Chart, revision, repo.Project) + err = helmClient.CleanChartCache(source.Chart, revision) if err != nil { return err } @@ -354,7 +354,7 @@ func (s *Service) runRepoOperation( if source.Helm != nil { helmPassCredentials = source.Helm.PassCredentials } - chartPath, closer, err := helmClient.ExtractChart(source.Chart, revision, repo.Project, helmPassCredentials, s.initConstants.HelmManifestMaxExtractedSize, s.initConstants.DisableHelmManifestMaxExtractedSize) + chartPath, closer, err := helmClient.ExtractChart(source.Chart, revision, helmPassCredentials, s.initConstants.HelmManifestMaxExtractedSize, s.initConstants.DisableHelmManifestMaxExtractedSize) if err != nil { return err } @@ -1180,7 +1180,7 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie referencedSource := getReferencedSource(p.Path, q.RefSources) if referencedSource != nil { // If the $-prefixed path appears to reference another source, do env substitution _after_ resolving the source - resolvedPath, err = getResolvedRefValueFile(p.Path, env, q.GetValuesFileSchemes(), referencedSource.Repo.Repo, gitRepoPaths, referencedSource.Repo.Project) + resolvedPath, err = getResolvedRefValueFile(p.Path, env, q.GetValuesFileSchemes(), referencedSource.Repo.Repo, gitRepoPaths) if err != nil { return nil, "", fmt.Errorf("error resolving set-file path: %w", err) } @@ -1306,7 +1306,7 @@ func getResolvedValueFiles( referencedSource := getReferencedSource(rawValueFile, refSources) if referencedSource != nil { // If the $-prefixed path appears to reference another source, do env substitution _after_ resolving that source. - resolvedPath, err = getResolvedRefValueFile(rawValueFile, env, allowedValueFilesSchemas, referencedSource.Repo.Repo, gitRepoPaths, referencedSource.Repo.Project) + resolvedPath, err = getResolvedRefValueFile(rawValueFile, env, allowedValueFilesSchemas, referencedSource.Repo.Repo, gitRepoPaths) if err != nil { return nil, fmt.Errorf("error resolving value file path: %w", err) } @@ -1339,15 +1339,9 @@ func getResolvedRefValueFile( allowedValueFilesSchemas []string, refSourceRepo string, gitRepoPaths io.TempPaths, - project string, ) (pathutil.ResolvedFilePath, error) { pathStrings := strings.Split(rawValueFile, "/") - - keyData, err := json.Marshal(map[string]string{"url": git.NormalizeGitURL(refSourceRepo), "project": project}) - if err != nil { - return "", err - } - repoPath := gitRepoPaths.GetPathIfExists(string(keyData)) + repoPath := gitRepoPaths.GetPathIfExists(git.NormalizeGitURL(refSourceRepo)) if repoPath == "" { return "", fmt.Errorf("failed to find repo %q", refSourceRepo) } @@ -2382,7 +2376,7 @@ func (s *Service) GetRevisionChartDetails(ctx context.Context, q *apiclient.Repo if err != nil { return nil, fmt.Errorf("helm client error: %w", err) } - chartPath, closer, err := helmClient.ExtractChart(q.Name, revision, q.Repo.Project, false, s.initConstants.HelmManifestMaxExtractedSize, s.initConstants.DisableHelmManifestMaxExtractedSize) + chartPath, closer, err := helmClient.ExtractChart(q.Name, revision, false, s.initConstants.HelmManifestMaxExtractedSize, s.initConstants.DisableHelmManifestMaxExtractedSize) if err != nil { return nil, fmt.Errorf("error extracting chart: %w", err) } @@ -2412,11 +2406,7 @@ func fileParameters(q *apiclient.RepoServerAppDetailsQuery) []v1alpha1.HelmFileP } func (s *Service) newClient(repo *v1alpha1.Repository, opts ...git.ClientOpts) (git.Client, error) { - keyData, err := json.Marshal(map[string]string{"url": git.NormalizeGitURL(repo.Repo), "project": repo.Project}) - if err != nil { - return nil, err - } - repoPath, err := s.gitRepoPaths.GetPath(string(keyData)) + repoPath, err := s.gitRepoPaths.GetPath(git.NormalizeGitURL(repo.Repo)) if err != nil { return nil, err } diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index ea10c92ad9183..9d4909e90743b 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -127,10 +127,10 @@ func newServiceWithMocks(t *testing.T, root string, signed bool) (*Service, *git chart: {{Version: "1.0.0"}, {Version: version}}, oobChart: {{Version: "1.0.0"}, {Version: version}}, }}, nil) - helmClient.On("ExtractChart", chart, version, "", false, int64(0), false).Return("./testdata/my-chart", io.NopCloser, nil) - helmClient.On("ExtractChart", oobChart, version, "", false, int64(0), false).Return("./testdata2/out-of-bounds-chart", io.NopCloser, nil) - helmClient.On("CleanChartCache", chart, version, "").Return(nil) - helmClient.On("CleanChartCache", oobChart, version, "").Return(nil) + helmClient.On("ExtractChart", chart, version, false, int64(0), false).Return("./testdata/my-chart", io.NopCloser, nil) + helmClient.On("ExtractChart", oobChart, version, false, int64(0), false).Return("./testdata2/out-of-bounds-chart", io.NopCloser, nil) + helmClient.On("CleanChartCache", chart, version).Return(nil) + helmClient.On("CleanChartCache", oobChart, version).Return(nil) helmClient.On("DependencyBuild").Return(nil) paths.On("Add", mock.Anything, mock.Anything).Return(root, nil) @@ -3283,8 +3283,7 @@ func Test_getResolvedValueFiles(t *testing.T) { tempDir := t.TempDir() paths := io.NewRandomizedTempPaths(tempDir) - key, _ := json.Marshal(map[string]string{"url": git.NormalizeGitURL("https://github.com/org/repo1"), "project": ""}) - paths.Add(string(key), path.Join(tempDir, "repo1")) + paths.Add(git.NormalizeGitURL("https://github.com/org/repo1"), path.Join(tempDir, "repo1")) testCases := []struct { name string diff --git a/test/e2e/scoped_repository_test.go b/test/e2e/scoped_repository_test.go index 0075b6bd1b40a..3d4fd71dc9866 100644 --- a/test/e2e/scoped_repository_test.go +++ b/test/e2e/scoped_repository_test.go @@ -232,3 +232,41 @@ func TestGetRepoCLIOutput(t *testing.T) { git https://github.com/argoproj/argo-cd.git false false false false Successful argo-project`, output) }) } + +func TestCreateRepoWithSameURLInTwoProjects(t *testing.T) { + projectFixture.Given(t). + When(). + Name("project-one"). + Create(). + Then() + + projectFixture.Given(t). + When(). + Name("project-two"). + Create(). + Then() + + path := "https://github.com/argoproj/argo-cd.git" + + // Create repository in first project + repoFixture.GivenWithSameState(t). + When(). + Path(path). + Project("project-one"). + Create(). + Then(). + And(func(r *Repository, _ error) { + assert.Equal(t, r.Repo, path) + }) + + // Create repository with same URL in second project + repoFixture.GivenWithSameState(t). + When(). + Path(path). + Project("project-two"). + Create(). + Then(). + And(func(r *Repository, _ error) { + assert.Equal(t, r.Repo, path) + }) +} diff --git a/util/helm/client.go b/util/helm/client.go index d9972adb04968..e2e03fa41271e 100644 --- a/util/helm/client.go +++ b/util/helm/client.go @@ -55,8 +55,8 @@ type indexCache interface { } type Client interface { - CleanChartCache(chart string, version string, project string) error - ExtractChart(chart string, version string, project string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, argoio.Closer, error) + CleanChartCache(chart string, version string) error + ExtractChart(chart string, version string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, argoio.Closer, error) GetIndex(noCache bool, maxIndexSize int64) (*Index, error) GetTags(chart string, noCache bool) (*TagsList, error) TestHelmOCI() (bool, error) @@ -120,8 +120,8 @@ func fileExist(filePath string) (bool, error) { return true, nil } -func (c *nativeHelmChart) CleanChartCache(chart string, version string, project string) error { - cachePath, err := c.getCachedChartPath(chart, version, project) +func (c *nativeHelmChart) CleanChartCache(chart string, version string) error { + cachePath, err := c.getCachedChartPath(chart, version) if err != nil { return fmt.Errorf("error getting cached chart path: %w", err) } @@ -148,7 +148,7 @@ func untarChart(tempDir string, cachedChartPath string, manifestMaxExtractedSize return files.Untgz(tempDir, reader, manifestMaxExtractedSize, false) } -func (c *nativeHelmChart) ExtractChart(chart string, version string, project string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, argoio.Closer, error) { +func (c *nativeHelmChart) ExtractChart(chart string, version string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, argoio.Closer, error) { // always use Helm V3 since we don't have chart content to determine correct Helm version helmCmd, err := NewCmdWithVersion("", c.enableOci, c.proxy, c.noProxy) if err != nil { @@ -162,7 +162,7 @@ func (c *nativeHelmChart) ExtractChart(chart string, version string, project str return "", nil, fmt.Errorf("error creating temporary directory: %w", err) } - cachedChartPath, err := c.getCachedChartPath(chart, version, project) + cachedChartPath, err := c.getCachedChartPath(chart, version) if err != nil { _ = os.RemoveAll(tempDir) return "", nil, fmt.Errorf("error getting cached chart path: %w", err) @@ -385,8 +385,8 @@ func normalizeChartName(chart string) string { return nc } -func (c *nativeHelmChart) getCachedChartPath(chart string, version string, project string) (string, error) { - keyData, err := json.Marshal(map[string]string{"url": c.repoURL, "chart": chart, "version": version, "project": project}) +func (c *nativeHelmChart) getCachedChartPath(chart string, version string) (string, error) { + keyData, err := json.Marshal(map[string]string{"url": c.repoURL, "chart": chart, "version": version}) if err != nil { return "", fmt.Errorf("error marshaling cache key data: %w", err) } diff --git a/util/helm/client_test.go b/util/helm/client_test.go index 97c27d6c1f62c..e32f3d3c7098f 100644 --- a/util/helm/client_test.go +++ b/util/helm/client_test.go @@ -79,7 +79,7 @@ func TestIndex(t *testing.T) { func Test_nativeHelmChart_ExtractChart(t *testing.T) { client := NewClient("https://argoproj.github.io/argo-helm", Creds{}, false, "", "") - path, closer, err := client.ExtractChart("argo-cd", "0.7.1", "", false, math.MaxInt64, true) + path, closer, err := client.ExtractChart("argo-cd", "0.7.1", false, math.MaxInt64, true) require.NoError(t, err) defer io.Close(closer) info, err := os.Stat(path) @@ -89,13 +89,13 @@ func Test_nativeHelmChart_ExtractChart(t *testing.T) { func Test_nativeHelmChart_ExtractChartWithLimiter(t *testing.T) { client := NewClient("https://argoproj.github.io/argo-helm", Creds{}, false, "", "") - _, _, err := client.ExtractChart("argo-cd", "0.7.1", "", false, 100, false) + _, _, err := client.ExtractChart("argo-cd", "0.7.1", false, 100, false) require.Error(t, err, "error while iterating on tar reader: unexpected EOF") } func Test_nativeHelmChart_ExtractChart_insecure(t *testing.T) { client := NewClient("https://argoproj.github.io/argo-helm", Creds{InsecureSkipVerify: true}, false, "", "") - path, closer, err := client.ExtractChart("argo-cd", "0.7.1", "", false, math.MaxInt64, true) + path, closer, err := client.ExtractChart("argo-cd", "0.7.1", false, math.MaxInt64, true) require.NoError(t, err) defer io.Close(closer) info, err := os.Stat(path) diff --git a/util/helm/mocks/Client.go b/util/helm/mocks/Client.go index a9ba09cb187a1..40ac2e48b16b7 100644 --- a/util/helm/mocks/Client.go +++ b/util/helm/mocks/Client.go @@ -14,17 +14,17 @@ type Client struct { mock.Mock } -// CleanChartCache provides a mock function with given fields: chart, version, project -func (_m *Client) CleanChartCache(chart string, version string, project string) error { - ret := _m.Called(chart, version, project) +// CleanChartCache provides a mock function with given fields: chart, version +func (_m *Client) CleanChartCache(chart string, version string) error { + ret := _m.Called(chart, version) if len(ret) == 0 { panic("no return value specified for CleanChartCache") } var r0 error - if rf, ok := ret.Get(0).(func(string, string, string) error); ok { - r0 = rf(chart, version, project) + if rf, ok := ret.Get(0).(func(string, string) error); ok { + r0 = rf(chart, version) } else { r0 = ret.Error(0) } @@ -32,9 +32,9 @@ func (_m *Client) CleanChartCache(chart string, version string, project string) return r0 } -// ExtractChart provides a mock function with given fields: chart, version, project, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize -func (_m *Client) ExtractChart(chart string, version string, project string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, io.Closer, error) { - ret := _m.Called(chart, version, project, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize) +// ExtractChart provides a mock function with given fields: chart, version, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize +func (_m *Client) ExtractChart(chart string, version string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, io.Closer, error) { + ret := _m.Called(chart, version, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize) if len(ret) == 0 { panic("no return value specified for ExtractChart") @@ -43,25 +43,25 @@ func (_m *Client) ExtractChart(chart string, version string, project string, pas var r0 string var r1 io.Closer var r2 error - if rf, ok := ret.Get(0).(func(string, string, string, bool, int64, bool) (string, io.Closer, error)); ok { - return rf(chart, version, project, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize) + if rf, ok := ret.Get(0).(func(string, string, bool, int64, bool) (string, io.Closer, error)); ok { + return rf(chart, version, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize) } - if rf, ok := ret.Get(0).(func(string, string, string, bool, int64, bool) string); ok { - r0 = rf(chart, version, project, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize) + if rf, ok := ret.Get(0).(func(string, string, bool, int64, bool) string); ok { + r0 = rf(chart, version, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize) } else { r0 = ret.Get(0).(string) } - if rf, ok := ret.Get(1).(func(string, string, string, bool, int64, bool) io.Closer); ok { - r1 = rf(chart, version, project, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize) + if rf, ok := ret.Get(1).(func(string, string, bool, int64, bool) io.Closer); ok { + r1 = rf(chart, version, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(io.Closer) } } - if rf, ok := ret.Get(2).(func(string, string, string, bool, int64, bool) error); ok { - r2 = rf(chart, version, project, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize) + if rf, ok := ret.Get(2).(func(string, string, bool, int64, bool) error); ok { + r2 = rf(chart, version, passCredentials, manifestMaxExtractedSize, disableManifestMaxExtractedSize) } else { r2 = ret.Error(2) }