From f06ce4b0250ea984c2bec5baf47ef984bcf0aad5 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Wed, 18 Dec 2024 17:41:40 -0500 Subject: [PATCH 1/6] chore!: remove legacy repo support (#19678) Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- cmd/argocd/commands/admin/admin.go | 97 +--- cmd/argocd/commands/admin/backup.go | 29 -- cmd/argocd/commands/admin/settings.go | 13 - cmd/argocd/commands/admin/settings_test.go | 9 - docs/operator-manual/declarative-setup.md | 37 -- .../argocd_admin_settings_validate.md | 2 +- test/e2e/fixture/fixture.go | 22 - test/e2e/helm_test.go | 39 +- test/e2e/repo_creds_test.go | 53 +- test/e2e/repo_management_test.go | 2 - util/db/db.go | 17 - util/db/db_test.go | 456 +++-------------- util/db/helmrepository.go | 49 +- util/db/repository.go | 121 +---- util/db/repository_legacy.go | 475 ------------------ util/db/repository_legacy_test.go | 35 -- util/db/repository_secrets_test.go | 22 +- util/db/repository_test.go | 120 ++--- util/settings/settings.go | 111 ---- util/settings/settings_test.go | 55 -- 20 files changed, 147 insertions(+), 1617 deletions(-) delete mode 100644 util/db/repository_legacy.go delete mode 100644 util/db/repository_legacy_test.go diff --git a/cmd/argocd/commands/admin/admin.go b/cmd/argocd/commands/admin/admin.go index 6c120bd425cdb..2c7f54625b48a 100644 --- a/cmd/argocd/commands/admin/admin.go +++ b/cmd/argocd/commands/admin/admin.go @@ -14,15 +14,12 @@ import ( "k8s.io/client-go/dynamic" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" - "sigs.k8s.io/yaml" cmdutil "github.com/argoproj/argo-cd/v2/cmd/util" "github.com/argoproj/argo-cd/v2/common" argocdclient "github.com/argoproj/argo-cd/v2/pkg/apiclient" - "github.com/argoproj/argo-cd/v2/util/errors" - "github.com/argoproj/argo-cd/v2/util/settings" - "github.com/argoproj/argo-cd/v2/pkg/apis/application" + "github.com/argoproj/argo-cd/v2/util/errors" ) const ( @@ -95,98 +92,6 @@ func newArgoCDClientsets(config *rest.Config, namespace string) *argoCDClientset } } -// getReferencedSecrets examines the argocd-cm config for any referenced repo secrets and returns a -// map of all referenced secrets. -func getReferencedSecrets(un unstructured.Unstructured) map[string]bool { - var cm apiv1.ConfigMap - err := runtime.DefaultUnstructuredConverter.FromUnstructured(un.Object, &cm) - errors.CheckError(err) - referencedSecrets := make(map[string]bool) - - // Referenced repository secrets - if reposRAW, ok := cm.Data["repositories"]; ok { - repos := make([]settings.Repository, 0) - err := yaml.Unmarshal([]byte(reposRAW), &repos) - errors.CheckError(err) - for _, cred := range repos { - if cred.PasswordSecret != nil { - referencedSecrets[cred.PasswordSecret.Name] = true - } - if cred.SSHPrivateKeySecret != nil { - referencedSecrets[cred.SSHPrivateKeySecret.Name] = true - } - if cred.UsernameSecret != nil { - referencedSecrets[cred.UsernameSecret.Name] = true - } - if cred.TLSClientCertDataSecret != nil { - referencedSecrets[cred.TLSClientCertDataSecret.Name] = true - } - if cred.TLSClientCertKeySecret != nil { - referencedSecrets[cred.TLSClientCertKeySecret.Name] = true - } - } - } - - // Referenced repository credentials secrets - if reposRAW, ok := cm.Data["repository.credentials"]; ok { - creds := make([]settings.RepositoryCredentials, 0) - err := yaml.Unmarshal([]byte(reposRAW), &creds) - errors.CheckError(err) - for _, cred := range creds { - if cred.PasswordSecret != nil { - referencedSecrets[cred.PasswordSecret.Name] = true - } - if cred.SSHPrivateKeySecret != nil { - referencedSecrets[cred.SSHPrivateKeySecret.Name] = true - } - if cred.UsernameSecret != nil { - referencedSecrets[cred.UsernameSecret.Name] = true - } - if cred.TLSClientCertDataSecret != nil { - referencedSecrets[cred.TLSClientCertDataSecret.Name] = true - } - if cred.TLSClientCertKeySecret != nil { - referencedSecrets[cred.TLSClientCertKeySecret.Name] = true - } - } - } - return referencedSecrets -} - -// isArgoCDSecret returns whether or not the given secret is a part of Argo CD configuration -// (e.g. argocd-secret, repo credentials, or cluster credentials) -func isArgoCDSecret(repoSecretRefs map[string]bool, un unstructured.Unstructured) bool { - secretName := un.GetName() - if secretName == common.ArgoCDSecretName { - return true - } - if repoSecretRefs != nil { - if _, ok := repoSecretRefs[secretName]; ok { - return true - } - } - if labels := un.GetLabels(); labels != nil { - if _, ok := labels[common.LabelKeySecretType]; ok { - return true - } - } - if annotations := un.GetAnnotations(); annotations != nil { - if annotations[common.AnnotationKeyManagedBy] == common.AnnotationValueManagedByArgoCD { - return true - } - } - return false -} - -// isArgoCDConfigMap returns true if the configmap name is one of argo cd's well known configmaps -func isArgoCDConfigMap(name string) bool { - switch name { - case common.ArgoCDConfigMapName, common.ArgoCDRBACConfigMapName, common.ArgoCDKnownHostsConfigMapName, common.ArgoCDTLSCertsConfigMapName: - return true - } - return false -} - // specsEqual returns if the spec, data, labels, annotations, and finalizers of the two // supplied objects are equal, indicating that no update is necessary during importing func specsEqual(left, right unstructured.Unstructured) bool { diff --git a/cmd/argocd/commands/admin/backup.go b/cmd/argocd/commands/admin/backup.go index b644b34e860b1..41b94375c0fd6 100644 --- a/cmd/argocd/commands/admin/backup.go +++ b/cmd/argocd/commands/admin/backup.go @@ -74,14 +74,6 @@ func NewExportCommand() *cobra.Command { errors.CheckError(err) export(writer, *acdTLSCertsConfigMap, namespace) - referencedSecrets := getReferencedSecrets(*acdConfigMap) - secrets, err := acdClients.secrets.List(ctx, v1.ListOptions{}) - errors.CheckError(err) - for _, secret := range secrets.Items { - if isArgoCDSecret(referencedSecrets, secret) { - export(writer, secret, namespace) - } - } projects, err := acdClients.projects.List(ctx, v1.ListOptions{}) errors.CheckError(err) for _, proj := range projects.Items { @@ -188,27 +180,6 @@ func NewImportCommand() *cobra.Command { // items in this map indicates the resource should be pruned since it no longer appears // in the backup pruneObjects := make(map[kube.ResourceKey]unstructured.Unstructured) - configMaps, err := acdClients.configMaps.List(ctx, v1.ListOptions{}) - errors.CheckError(err) - // referencedSecrets holds any secrets referenced in the argocd-cm configmap. These - // secrets need to be imported too - var referencedSecrets map[string]bool - for _, cm := range configMaps.Items { - if isArgoCDConfigMap(cm.GetName()) { - pruneObjects[kube.ResourceKey{Group: "", Kind: "ConfigMap", Name: cm.GetName(), Namespace: cm.GetNamespace()}] = cm - } - if cm.GetName() == common.ArgoCDConfigMapName { - referencedSecrets = getReferencedSecrets(cm) - } - } - - secrets, err := acdClients.secrets.List(ctx, v1.ListOptions{}) - errors.CheckError(err) - for _, secret := range secrets.Items { - if isArgoCDSecret(referencedSecrets, secret) { - pruneObjects[kube.ResourceKey{Group: "", Kind: "Secret", Name: secret.GetName(), Namespace: secret.GetNamespace()}] = secret - } - } applications, err := acdClients.applications.List(ctx, v1.ListOptions{}) errors.CheckError(err) for _, app := range applications.Items { diff --git a/cmd/argocd/commands/admin/settings.go b/cmd/argocd/commands/admin/settings.go index 85baf9bfd1389..1c6d6e894e0ee 100644 --- a/cmd/argocd/commands/admin/settings.go +++ b/cmd/argocd/commands/admin/settings.go @@ -244,19 +244,6 @@ var validatorsByGroup = map[string]settingValidator{ } return summary, err }, - "repositories": joinValidators(func(manager *settings.SettingsManager) (string, error) { - repos, err := manager.GetRepositories() - if err != nil { - return "", err - } - return fmt.Sprintf("%d repositories", len(repos)), nil - }, func(manager *settings.SettingsManager) (string, error) { - creds, err := manager.GetRepositoryCredentials() - if err != nil { - return "", err - } - return fmt.Sprintf("%d repository credentials", len(creds)), nil - }), "accounts": func(manager *settings.SettingsManager) (string, error) { accounts, err := manager.GetAccounts() if err != nil { diff --git a/cmd/argocd/commands/admin/settings_test.go b/cmd/argocd/commands/admin/settings_test.go index 9ff9af5411f97..013a8114c8d0e 100644 --- a/cmd/argocd/commands/admin/settings_test.go +++ b/cmd/argocd/commands/admin/settings_test.go @@ -158,15 +158,6 @@ clientSecret: aaaabbbbccccddddeee`, }, containsSummary: "updated-options", }, - "Repositories": { - validator: "repositories", - data: map[string]string{ - "repositories": ` -- url: https://github.com/argoproj/my-private-repository1 -- url: https://github.com/argoproj/my-private-repository2`, - }, - containsSummary: "2 repositories", - }, "Accounts": { validator: "accounts", data: map[string]string{ diff --git a/docs/operator-manual/declarative-setup.md b/docs/operator-manual/declarative-setup.md index d3b93d27c1601..3ffa009de11a2 100644 --- a/docs/operator-manual/declarative-setup.md +++ b/docs/operator-manual/declarative-setup.md @@ -493,43 +493,6 @@ stringData: A note on noProxy: Argo CD uses exec to interact with different tools such as helm and kustomize. Not all of these tools support the same noProxy syntax as the [httpproxy go package](https://cs.opensource.google/go/x/net/+/internal-branch.go1.21-vendor:http/httpproxy/proxy.go;l=38-50) does. In case you run in trouble with noProxy not beeing respected you might want to try using the full domain instead of a wildcard pattern or IP range to find a common syntax that all tools support. -### Legacy behaviour - -In Argo CD version 2.0 and earlier, repositories were stored as part of the `argocd-cm` config map. For -backward-compatibility, Argo CD will still honor repositories in the config map, but this style of repository -configuration is deprecated and support for it will be removed in a future version. - -```yaml -apiVersion: v1 -kind: ConfigMap -data: - repositories: | - - url: https://github.com/argoproj/my-private-repository - passwordSecret: - name: my-secret - key: password - usernameSecret: - name: my-secret - key: username - repository.credentials: | - - url: https://github.com/argoproj - passwordSecret: - name: my-secret - key: password - usernameSecret: - name: my-secret - key: username ---- -apiVersion: v1 -kind: Secret -metadata: - name: my-secret - namespace: argocd -stringData: - password: my-password - username: my-username -``` - ## Clusters Cluster credentials are stored in secrets same as repositories or repository credentials. Each secret must have label diff --git a/docs/user-guide/commands/argocd_admin_settings_validate.md b/docs/user-guide/commands/argocd_admin_settings_validate.md index 7a7612575c07c..f78aa4040f447 100644 --- a/docs/user-guide/commands/argocd_admin_settings_validate.md +++ b/docs/user-guide/commands/argocd_admin_settings_validate.md @@ -26,7 +26,7 @@ argocd admin settings validate --group accounts --group plugins --load-cluster-s ### Options ``` - --group stringArray Optional list of setting groups that have to be validated ( one of: accounts, general, kustomize, repositories, resource-overrides) + --group stringArray Optional list of setting groups that have to be validated ( one of: accounts, general, kustomize, resource-overrides) -h, --help help for validate ``` diff --git a/test/e2e/fixture/fixture.go b/test/e2e/fixture/fixture.go index 03863a5ef53a0..d2d1fcc6c5817 100644 --- a/test/e2e/fixture/fixture.go +++ b/test/e2e/fixture/fixture.go @@ -542,28 +542,6 @@ func SetResourceFilter(filters settings.ResourcesFilter) error { }) } -func SetHelmRepos(repos ...settings.HelmRepoCredentials) error { - return updateSettingConfigMap(func(cm *corev1.ConfigMap) error { - yamlBytes, err := yaml.Marshal(repos) - if err != nil { - return err - } - cm.Data["helm.repositories"] = string(yamlBytes) - return nil - }) -} - -func SetRepos(repos ...settings.RepositoryCredentials) error { - return updateSettingConfigMap(func(cm *corev1.ConfigMap) error { - yamlBytes, err := yaml.Marshal(repos) - if err != nil { - return err - } - cm.Data["repositories"] = string(yamlBytes) - return nil - }) -} - func SetProjectSpec(project string, spec v1alpha1.AppProjectSpec) error { proj, err := AppClientset.ArgoprojV1alpha1().AppProjects(TestNamespace()).Get(context.Background(), project, v1.GetOptions{}) if err != nil { diff --git a/test/e2e/helm_test.go b/test/e2e/helm_test.go index a893486811df5..9d06eecf269c2 100644 --- a/test/e2e/helm_test.go +++ b/test/e2e/helm_test.go @@ -1,7 +1,6 @@ package e2e import ( - "context" "fmt" "net" "net/http" @@ -13,17 +12,12 @@ import ( . "github.com/argoproj/gitops-engine/pkg/sync/common" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" . "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "github.com/argoproj/argo-cd/v2/test/e2e/fixture" . "github.com/argoproj/argo-cd/v2/test/e2e/fixture" . "github.com/argoproj/argo-cd/v2/test/e2e/fixture/app" projectFixture "github.com/argoproj/argo-cd/v2/test/e2e/fixture/project" - "github.com/argoproj/argo-cd/v2/test/e2e/fixture/repos" . "github.com/argoproj/argo-cd/v2/util/errors" - "github.com/argoproj/argo-cd/v2/util/settings" ) func TestHelmHooksAreCreated(t *testing.T) { @@ -437,7 +431,7 @@ func TestHelmValuesHiddenDirectory(t *testing.T) { func TestHelmWithDependencies(t *testing.T) { SkipOnEnv(t, "HELM") - testHelmWithDependencies(t, "helm-with-dependencies", false) + testHelmWithDependencies(t, "helm-with-dependencies") } func TestHelmWithMultipleDependencies(t *testing.T) { @@ -495,42 +489,13 @@ func TestHelmDependenciesPermissionDenied(t *testing.T) { Expect(Error("", expectedErr)) } -func TestHelmWithDependenciesLegacyRepo(t *testing.T) { - SkipOnEnv(t, "HELM") - testHelmWithDependencies(t, "helm-with-dependencies", true) -} - -func testHelmWithDependencies(t *testing.T, chartPath string, legacyRepo bool) { +func testHelmWithDependencies(t *testing.T, chartPath string) { t.Helper() ctx := Given(t). CustomCACertAdded(). // these are slow tests Timeout(30). HelmPassCredentials() - if legacyRepo { - ctx.And(func() { - FailOnErr(fixture.Run("", "kubectl", "create", "secret", "generic", "helm-repo", - "-n", fixture.TestNamespace(), - fmt.Sprintf("--from-file=certSecret=%s", repos.CertPath), - fmt.Sprintf("--from-file=keySecret=%s", repos.CertKeyPath), - fmt.Sprintf("--from-literal=username=%s", GitUsername), - fmt.Sprintf("--from-literal=password=%s", GitPassword), - )) - FailOnErr(fixture.KubeClientset.CoreV1().Secrets(fixture.TestNamespace()).Patch(context.Background(), - "helm-repo", types.MergePatchType, []byte(`{"metadata": { "labels": {"e2e.argoproj.io": "true"} }}`), metav1.PatchOptions{})) - - CheckError(fixture.SetHelmRepos(settings.HelmRepoCredentials{ - URL: RepoURL(RepoURLTypeHelm), - Name: "custom-repo", - KeySecret: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "helm-repo"}, Key: "keySecret"}, - CertSecret: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "helm-repo"}, Key: "certSecret"}, - UsernameSecret: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "helm-repo"}, Key: "username"}, - PasswordSecret: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "helm-repo"}, Key: "password"}, - })) - }) - } else { - ctx = ctx.HelmRepoAdded("custom-repo") - } helmVer := "" diff --git a/test/e2e/repo_creds_test.go b/test/e2e/repo_creds_test.go index 59df113605a7b..ba8f1669a89de 100644 --- a/test/e2e/repo_creds_test.go +++ b/test/e2e/repo_creds_test.go @@ -1,7 +1,6 @@ package e2e import ( - "fmt" "testing" "github.com/argoproj/argo-cd/v2/test/e2e/fixture" @@ -33,7 +32,7 @@ func TestCannotAddAppFromClientCertRepoWithoutCfg(t *testing.T) { Expect(Error("", "repository not accessible")) } -// make sure you can create an app from a private repo, if the repo is set-up in the CM +// make sure you can create an app from a private repo, if the repo is set-up func TestCanAddAppFromPrivateRepoWithRepoCfg(t *testing.T) { Given(t). RepoURLType(fixture.RepoURLTypeHTTPS). @@ -48,22 +47,16 @@ func TestCanAddAppFromPrivateRepoWithRepoCfg(t *testing.T) { Expect(Success("")) } -// make sure you can create an app from a private repo, if the creds are set-up in the CM +// make sure you can create an app from a private repo, if the creds are set-up func TestCanAddAppFromInsecurePrivateRepoWithCredCfg(t *testing.T) { Given(t). CustomCACertAdded(). + HTTPSCredentialsUserPassAdded(). RepoURLType(fixture.RepoURLTypeHTTPS). Path(fixture.LocalOrRemotePath("https-kustomize-base")). And(func() { - secretName := fixture.CreateSecret(fixture.GitUsername, fixture.GitPassword) - FailOnErr(fixture.Run("", "kubectl", "patch", "cm", "argocd-cm", - "-n", fixture.TestNamespace(), - "-p", fmt.Sprintf( - `{"data": {"repository.credentials": "- passwordSecret:\n key: password\n name: %s\n url: %s\n insecure: true\n usernameSecret:\n key: username\n name: %s\n"}}`, - secretName, - fixture.RepoURL(fixture.RepoURLTypeHTTPS), - secretName, - ))) + // I use CLI, but you could also modify the settings, we get a free test of the CLI here + FailOnErr(fixture.RunCli("repocreds", "update", "--insecure-skip-server-verification")) }). When(). CreateApp(). @@ -80,42 +73,6 @@ func TestCanAddAppFromPrivateRepoWithCredCfg(t *testing.T) { HTTPSRepoURLAdded(false). RepoURLType(fixture.RepoURLTypeHTTPS). Path(fixture.LocalOrRemotePath("https-kustomize-base")). - And(func() { - secretName := fixture.CreateSecret(fixture.GitUsername, fixture.GitPassword) - FailOnErr(fixture.Run("", "kubectl", "patch", "cm", "argocd-cm", - "-n", fixture.TestNamespace(), - "-p", fmt.Sprintf( - `{"data": {"repository.credentials": "- passwordSecret:\n key: password\n name: %s\n url: %s\n usernameSecret:\n key: username\n name: %s\n"}}`, - secretName, - fixture.RepoURL(fixture.RepoURLTypeHTTPS), - secretName, - ))) - }). - When(). - CreateApp(). - Then(). - Expect(Success("")) -} - -// make sure we can create an app from a private repo, in a secure manner using -// a custom CA certificate bundle -func TestCanAddAppFromClientCertRepoWithCredCfg(t *testing.T) { - Given(t). - CustomCACertAdded(). - HTTPSRepoURLWithClientCertAdded(). - RepoURLType(fixture.RepoURLTypeHTTPSClientCert). - Path(fixture.LocalOrRemotePath("https-kustomize-base")). - And(func() { - secretName := fixture.CreateSecret(fixture.GitUsername, fixture.GitPassword) - FailOnErr(fixture.Run("", "kubectl", "patch", "cm", "argocd-cm", - "-n", fixture.TestNamespace(), - "-p", fmt.Sprintf( - `{"data": {"repository.credentials": "- passwordSecret:\n key: password\n name: %s\n url: %s\n usernameSecret:\n key: username\n name: %s\n"}}`, - secretName, - fixture.RepoURL(fixture.RepoURLTypeHTTPS), - secretName, - ))) - }). When(). CreateApp(). Then(). diff --git a/test/e2e/repo_management_test.go b/test/e2e/repo_management_test.go index 97627d84f31d8..a9483bdf3a224 100644 --- a/test/e2e/repo_management_test.go +++ b/test/e2e/repo_management_test.go @@ -14,7 +14,6 @@ import ( "github.com/argoproj/argo-cd/v2/test/e2e/fixture/repos" . "github.com/argoproj/argo-cd/v2/util/errors" argoio "github.com/argoproj/argo-cd/v2/util/io" - "github.com/argoproj/argo-cd/v2/util/settings" ) func TestAddRemovePublicRepo(t *testing.T) { @@ -89,7 +88,6 @@ func TestGetRepoWithInheritedCreds(t *testing.T) { func TestUpsertExistingRepo(t *testing.T) { app.Given(t).And(func() { - CheckError(fixture.SetRepos(settings.RepositoryCredentials{URL: fixture.RepoURL(fixture.RepoURLTypeFile)})) repoUrl := fixture.RepoURL(fixture.RepoURLTypeFile) _, err := fixture.RunCli("repo", "add", repoUrl) require.NoError(t, err) diff --git a/util/db/db.go b/util/db/db.go index f516a410bc190..9975409fc7cb3 100644 --- a/util/db/db.go +++ b/util/db/db.go @@ -150,23 +150,6 @@ func (db *db) getSecret(name string, cache map[string]*v1.Secret) (*v1.Secret, e return cache[name], nil } -func (db *db) unmarshalFromSecretsStr(secrets map[*SecretMaperValidation]*v1.SecretKeySelector, cache map[string]*v1.Secret) error { - for dst, src := range secrets { - if src != nil { - secret, err := db.getSecret(src.Name, cache) - if err != nil { - return err - } - if dst.Transform != nil { - *dst.Dest = dst.Transform(string(secret.Data[src.Key])) - } else { - *dst.Dest = string(secret.Data[src.Key]) - } - } - } - return nil -} - // StripCRLFCharacter strips the trailing CRLF characters func StripCRLFCharacter(input string) string { return strings.TrimSpace(input) diff --git a/util/db/db_test.go b/util/db/db_test.go index 8227f86964bcc..dde04c73eacae 100644 --- a/util/db/db_test.go +++ b/util/db/db_test.go @@ -2,7 +2,6 @@ package db import ( "context" - "strings" "testing" "time" @@ -26,7 +25,7 @@ const ( testNamespace = "default" ) -func getClientset(config map[string]string, objects ...runtime.Object) *fake.Clientset { +func getClientset(objects ...runtime.Object) *fake.Clientset { secret := v1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "argocd-secret", @@ -45,13 +44,12 @@ func getClientset(config map[string]string, objects ...runtime.Object) *fake.Cli "app.kubernetes.io/part-of": "argocd", }, }, - Data: config, } return fake.NewClientset(append(objects, &cm, &secret)...) } func TestCreateRepository(t *testing.T) { - clientset := getClientset(nil) + clientset := getClientset() db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) repo, err := db.CreateRepository(context.Background(), &v1alpha1.Repository{ @@ -72,7 +70,7 @@ func TestCreateRepository(t *testing.T) { } func TestCreateProjectScopedRepository(t *testing.T) { - clientset := getClientset(nil) + clientset := getClientset() db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) repo, err := db.CreateRepository(context.Background(), &v1alpha1.Repository{ @@ -119,7 +117,7 @@ func TestCreateProjectScopedRepository(t *testing.T) { } func TestCreateRepoCredentials(t *testing.T) { - clientset := getClientset(nil) + clientset := getClientset() db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) creds, err := db.CreateRepositoryCredentials(context.Background(), &v1alpha1.RepoCreds{ @@ -155,83 +153,53 @@ func TestCreateRepoCredentials(t *testing.T) { } func TestGetRepositoryCredentials(t *testing.T) { - config := map[string]string{ - "repositories": ` -- url: https://known/repo -- url: https://secured/repo -- url: https://missing/repo -`, - "repository.credentials": ` -- url: https://secured - usernameSecret: - name: managed-secret - key: username - passwordSecret: - name: managed-secret - key: password -- url: https://missing - usernameSecret: - name: managed-secret - key: username - passwordSecret: - name: missing-managed-secret - key: password -`, - } - clientset := getClientset(config, newManagedSecret()) + clientset := getClientset() db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) + _, err := db.CreateRepositoryCredentials(context.Background(), &v1alpha1.RepoCreds{ + URL: "https://secured", + Username: "test-username", + Password: "test-password", + }) + require.NoError(t, err) tests := []struct { name string repoURL string want *v1alpha1.RepoCreds - wantErr bool }{ { name: "TestUnknownRepo", repoURL: "https://unknown/repo", want: nil, - wantErr: false, }, { name: "TestKnownRepo", repoURL: "https://known/repo", want: nil, - wantErr: false, }, { name: "TestSecuredRepo", repoURL: "https://secured/repo", want: &v1alpha1.RepoCreds{URL: "https://secured", Username: "test-username", Password: "test-password"}, - wantErr: false, }, { name: "TestMissingRepo", repoURL: "https://missing/repo", want: nil, - wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := db.GetRepositoryCredentials(context.TODO(), tt.repoURL) - if tt.wantErr { - require.Error(t, err) - assert.True(t, errors.IsNotFound(err)) - } else { - require.NoError(t, err) - } - + require.NoError(t, err) assert.Equal(t, tt.want, got) }) } } func TestCreateExistingRepository(t *testing.T) { - clientset := getClientset(map[string]string{ - "repositories": `- url: https://github.com/argoproj/argocd-example-apps`, - }) + clientset := getClientset() db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) _, err := db.CreateRepository(context.Background(), &v1alpha1.Repository{ @@ -239,273 +207,99 @@ func TestCreateExistingRepository(t *testing.T) { Username: "test-username", Password: "test-password", }) + require.NoError(t, err) + + _, err = db.CreateRepository(context.Background(), &v1alpha1.Repository{ + Repo: "https://github.com/argoproj/argocd-example-apps", + Username: "test-username", + Password: "test-password", + }) require.Error(t, err) assert.Equal(t, codes.AlreadyExists, status.Convert(err).Code()) } func TestGetRepository(t *testing.T) { - config := map[string]string{ - "repositories": ` -- url: https://known/repo -- url: https://secured/repo -`, - "repository.credentials": ` -- url: https://secured - usernameSecret: - name: managed-secret - key: username - passwordSecret: - name: managed-secret - key: password -`, - } - clientset := getClientset(config, newManagedSecret()) - db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) - - tests := []struct { - name string - repoURL string - want *v1alpha1.Repository - }{ - { - name: "TestUnknownRepo", - repoURL: "https://unknown/repo", - want: &v1alpha1.Repository{Repo: "https://unknown/repo"}, - }, - { - name: "TestKnownRepo", - repoURL: "https://known/repo", - want: &v1alpha1.Repository{Repo: "https://known/repo"}, - }, - { - name: "TestSecuredRepo", - repoURL: "https://secured/repo", - want: &v1alpha1.Repository{Repo: "https://secured/repo", Username: "test-username", Password: "test-password", InheritedCreds: true}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := db.GetRepository(context.TODO(), tt.repoURL, "") - require.NoError(t, err) - assert.Equal(t, tt.want, got) - }) - } -} - -func newManagedSecret() *v1.Secret { - return &v1.Secret{ + clientset := getClientset(&v1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "managed-secret", Namespace: testNamespace, + Name: "known-repo-secret", Annotations: map[string]string{ common.AnnotationKeyManagedBy: common.AnnotationValueManagedByArgoCD, }, + Labels: map[string]string{ + common.LabelKeySecretType: common.LabelValueSecretTypeRepository, + }, }, Data: map[string][]byte{ - username: []byte("test-username"), - password: []byte("test-password"), - }, - } -} - -func TestDeleteRepositoryManagedSecrets(t *testing.T) { - config := map[string]string{ - "repositories": ` -- url: https://github.com/argoproj/argocd-example-apps - usernameSecret: - name: managed-secret - key: username - passwordSecret: - name: managed-secret - key: password -`, - } - clientset := getClientset(config, newManagedSecret()) - db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) - - err := db.DeleteRepository(context.Background(), "https://github.com/argoproj/argocd-example-apps", "") - require.NoError(t, err) - - _, err = clientset.CoreV1().Secrets(testNamespace).Get(context.Background(), "managed-secret", metav1.GetOptions{}) - require.Error(t, err) - assert.True(t, errors.IsNotFound(err)) - - cm, err := clientset.CoreV1().ConfigMaps(testNamespace).Get(context.Background(), "argocd-cm", metav1.GetOptions{}) - require.NoError(t, err) - assert.Equal(t, "", cm.Data["repositories"]) -} - -func TestDeleteRepositoryUnmanagedSecrets(t *testing.T) { - config := map[string]string{ - "repositories": ` -- url: https://github.com/argoproj/argocd-example-apps - usernameSecret: - name: unmanaged-secret - key: username - passwordSecret: - name: unmanaged-secret - key: password -`, - } - clientset := getClientset(config, &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "unmanaged-secret", - Namespace: testNamespace, - }, - Data: map[string][]byte{ - username: []byte("test-username"), - password: []byte("test-password"), + "url": []byte("https://known/repo"), }, - }) - db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) - - err := db.DeleteRepository(context.Background(), "https://github.com/argoproj/argocd-example-apps", "") - require.NoError(t, err) - - s, err := clientset.CoreV1().Secrets(testNamespace).Get(context.Background(), "unmanaged-secret", metav1.GetOptions{}) - require.NoError(t, err) - assert.Equal(t, "test-username", string(s.Data[username])) - assert.Equal(t, "test-password", string(s.Data[password])) - - cm, err := clientset.CoreV1().ConfigMaps(testNamespace).Get(context.Background(), "argocd-cm", metav1.GetOptions{}) - require.NoError(t, err) - assert.Equal(t, "", cm.Data["repositories"]) -} - -func TestUpdateRepositoryWithManagedSecrets(t *testing.T) { - config := map[string]string{ - "repositories": ` -- url: https://github.com/argoproj/argocd-example-apps - usernameSecret: - name: managed-secret - key: username - passwordSecret: - name: managed-secret - key: password - sshPrivateKeySecret: - name: managed-secret - key: sshPrivateKey -`, - } - clientset := getClientset(config, &v1.Secret{ + }, &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "managed-secret", Namespace: testNamespace, + Name: "secured-repo-secret", Annotations: map[string]string{ common.AnnotationKeyManagedBy: common.AnnotationValueManagedByArgoCD, }, + Labels: map[string]string{ + common.LabelKeySecretType: common.LabelValueSecretTypeRepository, + }, }, Data: map[string][]byte{ - username: []byte("test-username"), - password: []byte("test-password"), - sshPrivateKey: []byte("test-ssh-private-key"), + "url": []byte("https://secured/repo"), }, - }) - db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) - - repo, err := db.GetRepository(context.Background(), "https://github.com/argoproj/argocd-example-apps", "") - require.NoError(t, err) - assert.Equal(t, "test-username", repo.Username) - assert.Equal(t, "test-password", repo.Password) - assert.Equal(t, "test-ssh-private-key", repo.SSHPrivateKey) - - _, err = db.UpdateRepository(context.Background(), &v1alpha1.Repository{ - Repo: "https://github.com/argoproj/argocd-example-apps", Password: "", Username: "", SSHPrivateKey: "", - }) - require.NoError(t, err) - - _, err = clientset.CoreV1().Secrets(testNamespace).Get(context.Background(), "managed-secret", metav1.GetOptions{}) - require.Error(t, err) - assert.True(t, errors.IsNotFound(err)) - - cm, err := clientset.CoreV1().ConfigMaps(testNamespace).Get(context.Background(), "argocd-cm", metav1.GetOptions{}) - require.NoError(t, err) - assert.Equal(t, "- url: https://github.com/argoproj/argocd-example-apps", strings.Trim(cm.Data["repositories"], "\n")) -} - -func TestRepositorySecretsTrim(t *testing.T) { - config := map[string]string{ - "repositories": ` -- url: https://github.com/argoproj/argocd-example-apps - usernameSecret: - name: managed-secret - key: username - passwordSecret: - name: managed-secret - key: password - sshPrivateKeySecret: - name: managed-secret - key: sshPrivateKey - tlsClientCertDataSecret: - name: managed-secret - key: tlsClientCertData - tlsClientCertKeySecret: - name: managed-secret - key: tlsClientCertKey - githubAppPrivateKeySecret: - name: managed-secret - key: githubAppPrivateKey -`, - } - clientset := getClientset(config, &v1.Secret{ + }, &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: "managed-secret", Namespace: testNamespace, + Name: "secured-repo-creds-secret", Annotations: map[string]string{ common.AnnotationKeyManagedBy: common.AnnotationValueManagedByArgoCD, }, + Labels: map[string]string{ + common.LabelKeySecretType: common.LabelValueSecretTypeRepoCreds, + }, }, Data: map[string][]byte{ - username: []byte("test-username\n\n"), - password: []byte("test-password\r\r"), - sshPrivateKey: []byte("test-ssh-private-key\n\r"), - tlsClientCertData: []byte("test-tls-client-cert-data\n\r"), - tlsClientCertKey: []byte("test-tls-client-cert-key\n\r"), - githubAppPrivateKey: []byte("test-github-app-private-key\n\r"), + "url": []byte("https://secured"), + "username": []byte("test-username"), + "password": []byte("test-password"), }, }) db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) - repo, err := db.GetRepository(context.Background(), "https://github.com/argoproj/argocd-example-apps", "") - require.NoError(t, err) - teststruct := []struct { - expectedSecret string - retrievedSecret string + tests := []struct { + name string + repoURL string + want *v1alpha1.Repository }{ { - "test-username", - repo.Username, - }, - { - "test-password", - repo.Password, - }, - { - "test-ssh-private-key", - repo.SSHPrivateKey, - }, - { - "test-tls-client-cert-data", - repo.TLSClientCertData, + name: "TestUnknownRepo", + repoURL: "https://unknown/repo", + want: &v1alpha1.Repository{Repo: "https://unknown/repo"}, }, { - "test-tls-client-cert-key", - repo.TLSClientCertKey, + name: "TestKnownRepo", + repoURL: "https://known/repo", + want: &v1alpha1.Repository{Repo: "https://known/repo"}, }, { - "test-github-app-private-key", - repo.GithubAppPrivateKey, + name: "TestSecuredRepo", + repoURL: "https://secured/repo", + want: &v1alpha1.Repository{Repo: "https://secured/repo", Username: "test-username", Password: "test-password", InheritedCreds: true}, }, } - for _, tt := range teststruct { - assert.Equal(t, tt.expectedSecret, tt.retrievedSecret) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := db.GetRepository(context.TODO(), tt.repoURL, "") + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) } } func TestGetClusterSuccessful(t *testing.T) { server := "my-cluster" name := "my-name" - clientset := getClientset(nil, &v1.Secret{ + clientset := getClientset(&v1.Secret{ ObjectMeta: metav1.ObjectMeta{ Namespace: testNamespace, Labels: map[string]string{ @@ -528,7 +322,7 @@ func TestGetClusterSuccessful(t *testing.T) { func TestGetNonExistingCluster(t *testing.T) { server := "https://mycluster" - clientset := getClientset(nil) + clientset := getClientset() db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) _, err := db.GetCluster(context.Background(), server) @@ -540,7 +334,7 @@ func TestGetNonExistingCluster(t *testing.T) { func TestCreateClusterSuccessful(t *testing.T) { server := "https://mycluster" - clientset := getClientset(nil) + clientset := getClientset() db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) _, err := db.CreateCluster(context.Background(), &v1alpha1.Cluster{ @@ -559,7 +353,7 @@ func TestDeleteClusterWithManagedSecret(t *testing.T) { clusterURL := "https://mycluster" clusterName := "cluster-mycluster-3274446258" - clientset := getClientset(nil, &v1.Secret{ + clientset := getClientset(&v1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: clusterName, Namespace: testNamespace, @@ -590,7 +384,7 @@ func TestDeleteClusterWithUnmanagedSecret(t *testing.T) { clusterURL := "https://mycluster" clusterName := "mycluster-443" - clientset := getClientset(nil, &v1.Secret{ + clientset := getClientset(&v1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: clusterName, Namespace: testNamespace, @@ -615,7 +409,7 @@ func TestDeleteClusterWithUnmanagedSecret(t *testing.T) { } func TestFuzzyEquivalence(t *testing.T) { - clientset := getClientset(nil) + clientset := getClientset() ctx := context.Background() db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) @@ -642,118 +436,8 @@ func TestFuzzyEquivalence(t *testing.T) { assert.Equal(t, "https://github.com/argoproj/argocd-example-apps", repo.Repo) } -func TestListHelmRepositories(t *testing.T) { - config := map[string]string{ - "repositories": ` -- url: https://argoproj.github.io/argo-helm - name: argo - type: helm - usernameSecret: - name: test-secret - key: username - passwordSecret: - name: test-secret - key: password - tlsClientCertDataSecret: - name: test-secret - key: cert - tlsClientCertKeySecret: - name: test-secret - key: key -`, - } - clientset := getClientset(config, &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-secret", - Namespace: testNamespace, - }, - Data: map[string][]byte{ - "username": []byte("test-username"), - "password": []byte("test-password"), - "ca": []byte("test-ca"), - "cert": []byte("test-cert"), - "key": []byte("test-key"), - }, - }) - db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) - - repos, err := db.ListRepositories(context.Background()) - require.NoError(t, err) - assert.Len(t, repos, 1) - repo := repos[0] - assert.Equal(t, "https://argoproj.github.io/argo-helm", repo.Repo) - assert.Equal(t, "helm", repo.Type) - assert.Equal(t, "argo", repo.Name) - assert.Equal(t, "test-username", repo.Username) - assert.Equal(t, "test-password", repo.Password) - assert.Equal(t, "test-cert", repo.TLSClientCertData) - assert.Equal(t, "test-key", repo.TLSClientCertKey) -} - -func TestHelmRepositorySecretsTrim(t *testing.T) { - config := map[string]string{ - "repositories": ` -- url: https://argoproj.github.io/argo-helm - name: argo - type: helm - usernameSecret: - name: test-secret - key: username - passwordSecret: - name: test-secret - key: password - tlsClientCertDataSecret: - name: test-secret - key: cert - tlsClientCertKeySecret: - name: test-secret - key: key -`, - } - clientset := getClientset(config, &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-secret", - Namespace: testNamespace, - }, - Data: map[string][]byte{ - "username": []byte("test-username\r\n"), - "password": []byte("test-password\r\n"), - "cert": []byte("test-cert\n\r"), - "key": []byte("test-key\n\r"), - }, - }) - db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) - repo, err := db.GetRepository(context.Background(), "https://argoproj.github.io/argo-helm", "") - - require.NoError(t, err) - teststruct := []struct { - expectedSecret string - retrievedSecret string - }{ - { - "test-username", - repo.Username, - }, - { - "test-password", - repo.Password, - }, - { - "test-cert", - repo.TLSClientCertData, - }, - { - "test-key", - repo.TLSClientCertKey, - }, - } - for _, tt := range teststruct { - assert.Equal(t, tt.expectedSecret, tt.retrievedSecret) - } -} - func TestGetClusterServersByName(t *testing.T) { - clientset := getClientset(nil, &v1.Secret{ + clientset := getClientset(&v1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "my-cluster-secret", Namespace: testNamespace, @@ -777,7 +461,7 @@ func TestGetClusterServersByName(t *testing.T) { } func TestGetClusterServersByName_InClusterNotConfigured(t *testing.T) { - clientset := getClientset(nil) + clientset := getClientset() db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) servers, err := db.GetClusterServersByName(context.Background(), "in-cluster") require.NoError(t, err) @@ -785,7 +469,7 @@ func TestGetClusterServersByName_InClusterNotConfigured(t *testing.T) { } func TestGetClusterServersByName_InClusterConfigured(t *testing.T) { - clientset := getClientset(nil, &v1.Secret{ + clientset := getClientset(&v1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "my-cluster-secret", Namespace: testNamespace, @@ -809,7 +493,7 @@ func TestGetClusterServersByName_InClusterConfigured(t *testing.T) { } func TestGetApplicationControllerReplicas(t *testing.T) { - clientset := getClientset(nil) + clientset := getClientset() expectedReplicas := int32(2) t.Setenv(common.EnvControllerReplicas, "2") db := NewDB(testNamespace, settings.NewSettingsManager(context.Background(), clientset, testNamespace), clientset) @@ -817,7 +501,7 @@ func TestGetApplicationControllerReplicas(t *testing.T) { assert.Equal(t, int(expectedReplicas), replicas) expectedReplicas = int32(3) - clientset = getClientset(nil, &appv1.Deployment{ + clientset = getClientset(&appv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: common.ApplicationController, Namespace: testNamespace, diff --git a/util/db/helmrepository.go b/util/db/helmrepository.go index 0cc6bb2742572..a089ca851d281 100644 --- a/util/db/helmrepository.go +++ b/util/db/helmrepository.go @@ -3,62 +3,15 @@ package db import ( "context" "fmt" - "strings" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - v1 "k8s.io/api/core/v1" "k8s.io/utils/ptr" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "github.com/argoproj/argo-cd/v2/util/settings" ) -func getHelmRepoCredIndex(helmRepositories []settings.HelmRepoCredentials, repoURL string) int { - for i, cred := range helmRepositories { - if strings.EqualFold(cred.URL, repoURL) { - return i - } - } - return -1 -} - -func (db *db) getHelmRepo(repoURL string, helmRepositories []settings.HelmRepoCredentials) (*v1alpha1.Repository, error) { - index := getHelmRepoCredIndex(helmRepositories, repoURL) - if index < 0 { - return nil, status.Errorf(codes.NotFound, "repo '%s' not found", repoURL) - } - repoInfo := helmRepositories[index] - - repo := &v1alpha1.Repository{ - Repo: repoInfo.URL, - Type: "helm", - Name: repoInfo.Name, - } - err := db.unmarshalFromSecretsStr(map[*SecretMaperValidation]*v1.SecretKeySelector{ - {Dest: &repo.Username, Transform: StripCRLFCharacter}: repoInfo.UsernameSecret, - {Dest: &repo.Password, Transform: StripCRLFCharacter}: repoInfo.PasswordSecret, - {Dest: &repo.TLSClientCertData, Transform: StripCRLFCharacter}: repoInfo.CertSecret, - {Dest: &repo.TLSClientCertKey, Transform: StripCRLFCharacter}: repoInfo.KeySecret, - }, make(map[string]*v1.Secret)) - return repo, err -} - // ListHelmRepositories lists configured helm repositories func (db *db) ListHelmRepositories(ctx context.Context) ([]*v1alpha1.Repository, error) { - helmRepositories, err := db.settingsMgr.GetHelmRepositories() - if err != nil { - return nil, fmt.Errorf("failed to get list of Helm repositories from settings manager: %w", err) - } - - result := make([]*v1alpha1.Repository, len(helmRepositories)) - for i, helmRepoInfo := range helmRepositories { - repo, err := db.getHelmRepo(helmRepoInfo.URL, helmRepositories) - if err != nil { - return nil, fmt.Errorf("failed to get Helm repository %q: %w", helmRepoInfo.URL, err) - } - result[i] = repo - } + var result []*v1alpha1.Repository repos, err := db.listRepositories(ctx, ptr.To("helm"), false) if err != nil { return nil, fmt.Errorf("failed to list Helm repositories: %w", err) diff --git a/util/db/repository.go b/util/db/repository.go index f6e7a708d04df..43c1b474e2d46 100644 --- a/util/db/repository.go +++ b/util/db/repository.go @@ -29,14 +29,6 @@ const ( project = "project" // The name of the key storing the SSH private in the secret sshPrivateKey = "sshPrivateKey" - // The name of the key storing the TLS client cert data in the secret - tlsClientCertData = "tlsClientCertData" - // The name of the key storing the TLS client cert key in the secret - tlsClientCertKey = "tlsClientCertKey" - // The name of the key storing the GitHub App private key in the secret - githubAppPrivateKey = "githubAppPrivateKey" - // The name of the key storing the service account to access Google Cloud Source repositories - gcpServiceAccountKey = "gcpServiceAccountKey" ) // repositoryBackend defines the API for types that wish to provide interaction with repository storage @@ -60,18 +52,13 @@ type repositoryBackend interface { func (db *db) CreateRepository(ctx context.Context, r *appsv1.Repository) (*appsv1.Repository, error) { secretBackend := db.repoBackend() - legacyBackend := db.legacyRepoBackend() secretExists, err := secretBackend.RepositoryExists(ctx, r.Repo, r.Project, false) if err != nil { return nil, err } - legacyExists, err := legacyBackend.RepositoryExists(ctx, r.Repo, r.Project, false) - if err != nil { - return nil, err - } - if secretExists || legacyExists { + if secretExists { return nil, status.Errorf(codes.AlreadyExists, "repository %q already exists", r.Repo) } @@ -149,13 +136,7 @@ func (db *db) getRepositories(indexer, project string) ([]*appv1.Repository, err func (db *db) RepositoryExists(ctx context.Context, repoURL, project string) (bool, error) { secretsBackend := db.repoBackend() - exists, err := secretsBackend.RepositoryExists(ctx, repoURL, project, true) - if exists || err != nil { - return exists, err - } - - legacyBackend := db.legacyRepoBackend() - return legacyBackend.RepositoryExists(ctx, repoURL, project, true) + return secretsBackend.RepositoryExists(ctx, repoURL, project, true) } func (db *db) WriteRepositoryExists(ctx context.Context, repoURL, project string) (bool, error) { @@ -176,18 +157,6 @@ func (db *db) getRepository(ctx context.Context, repoURL, project string) (*apps return repository, nil } - legacyBackend := db.legacyRepoBackend() - exists, err = legacyBackend.RepositoryExists(ctx, repoURL, project, true) - if err != nil { - return nil, fmt.Errorf("unable to check if repository %q exists from legacy backend: %w", repoURL, err) - } else if exists { - repository, err := legacyBackend.GetRepository(ctx, repoURL, project) - if err != nil { - return nil, fmt.Errorf("unable to get repository %q from legacy backend: %w", repoURL, err) - } - return repository, nil - } - return &appsv1.Repository{Repo: repoURL}, nil } @@ -204,24 +173,17 @@ func (db *db) listRepositories(ctx context.Context, repoType *string, writeCreds // repositories from secrets overlay repositories from legacys. var repositories []*appv1.Repository + var err error if writeCreds { - var err error repositories, err = db.repoWriteBackend().ListRepositories(ctx, repoType) if err != nil { return nil, err } } else { - secretRepositories, err := db.repoBackend().ListRepositories(ctx, repoType) - if err != nil { - return nil, err - } - - legacyRepositories, err := db.legacyRepoBackend().ListRepositories(ctx, repoType) + repositories, err = db.repoBackend().ListRepositories(ctx, repoType) if err != nil { return nil, err } - - repositories = append(secretRepositories, legacyRepositories...) } if err := db.enrichCredsToRepos(ctx, repositories); err != nil { return nil, err @@ -240,14 +202,6 @@ func (db *db) UpdateRepository(ctx context.Context, r *appsv1.Repository) (*apps return secretsBackend.UpdateRepository(ctx, r) } - legacyBackend := db.legacyRepoBackend() - exists, err = legacyBackend.RepositoryExists(ctx, r.Repo, r.Project, false) - if err != nil { - return nil, err - } else if exists { - return legacyBackend.UpdateRepository(ctx, r) - } - return nil, status.Errorf(codes.NotFound, "repo '%s' not found", r.Repo) } @@ -274,14 +228,6 @@ func (db *db) DeleteRepository(ctx context.Context, repoURL, project string) err return secretsBackend.DeleteRepository(ctx, repoURL, project) } - legacyBackend := db.legacyRepoBackend() - exists, err = legacyBackend.RepositoryExists(ctx, repoURL, project, false) - if err != nil { - return err - } else if exists { - return legacyBackend.DeleteRepository(ctx, repoURL, project) - } - return status.Errorf(codes.NotFound, "repo '%s' not found", repoURL) } @@ -301,20 +247,12 @@ func (db *db) DeleteWriteRepository(ctx context.Context, repoURL, project string // ListRepositoryCredentials returns a list of URLs that contain repo credential sets func (db *db) ListRepositoryCredentials(ctx context.Context) ([]string, error) { - // TODO It would be nice to check for duplicates between secret and legacy repositories and make it so that - // repositories from secrets overlay repositories from legacys. - secretRepoCreds, err := db.repoBackend().ListRepoCreds(ctx) if err != nil { return nil, err } - legacyRepoCreds, err := db.legacyRepoBackend().ListRepoCreds(ctx) - if err != nil { - return nil, err - } - - return append(secretRepoCreds, legacyRepoCreds...), nil + return secretRepoCreds, nil } // ListWriteRepositoryCredentials returns a list of URLs that contain repo write credential sets @@ -340,18 +278,6 @@ func (db *db) GetRepositoryCredentials(ctx context.Context, repoURL string) (*ap return creds, nil } - legacyBackend := db.legacyRepoBackend() - exists, err = legacyBackend.RepoCredsExists(ctx, repoURL) - if err != nil { - return nil, fmt.Errorf("unable to check if repository credentials for %q exists from legacy backend: %w", repoURL, err) - } else if exists { - creds, err := legacyBackend.GetRepoCreds(ctx, repoURL) - if err != nil { - return nil, fmt.Errorf("unable to get repository credentials for %q from legacy backend: %w", repoURL, err) - } - return creds, nil - } - return nil, nil } @@ -382,37 +308,24 @@ func (db *db) GetWriteRepositoryCredentials(ctx context.Context, repoURL string) // GetAllHelmRepositoryCredentials retrieves all repository credentials func (db *db) GetAllHelmRepositoryCredentials(ctx context.Context) ([]*appsv1.RepoCreds, error) { - // TODO It would be nice to check for duplicates between secret and legacy repositories and make it so that - // repositories from secrets overlay repositories from legacys. - secretRepoCreds, err := db.repoBackend().GetAllHelmRepoCreds(ctx) if err != nil { return nil, fmt.Errorf("failed to get all Helm repo creds: %w", err) } - legacyRepoCreds, err := db.legacyRepoBackend().GetAllHelmRepoCreds(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get all legacy Helm repo creds: %w", err) - } - - return append(secretRepoCreds, legacyRepoCreds...), nil + return secretRepoCreds, nil } // CreateRepositoryCredentials creates a repository credential set func (db *db) CreateRepositoryCredentials(ctx context.Context, r *appsv1.RepoCreds) (*appsv1.RepoCreds, error) { - legacyBackend := db.legacyRepoBackend() secretBackend := db.repoBackend() secretExists, err := secretBackend.RepositoryExists(ctx, r.URL, "", false) if err != nil { return nil, err } - legacyExists, err := legacyBackend.RepositoryExists(ctx, r.URL, "", false) - if err != nil { - return nil, err - } - if secretExists || legacyExists { + if secretExists { return nil, status.Errorf(codes.AlreadyExists, "repository credentials %q already exists", r.URL) } @@ -444,14 +357,6 @@ func (db *db) UpdateRepositoryCredentials(ctx context.Context, r *appsv1.RepoCre return secretsBackend.UpdateRepoCreds(ctx, r) } - legacyBackend := db.legacyRepoBackend() - exists, err = legacyBackend.RepoCredsExists(ctx, r.URL) - if err != nil { - return nil, err - } else if exists { - return legacyBackend.UpdateRepoCreds(ctx, r) - } - return nil, status.Errorf(codes.NotFound, "repository credentials '%s' not found", r.URL) } @@ -481,14 +386,6 @@ func (db *db) DeleteRepositoryCredentials(ctx context.Context, name string) erro return secretsBackend.DeleteRepoCreds(ctx, name) } - legacyBackend := db.legacyRepoBackend() - exists, err = legacyBackend.RepoCredsExists(ctx, name) - if err != nil { - return err - } else if exists { - return legacyBackend.DeleteRepoCreds(ctx, name) - } - return status.Errorf(codes.NotFound, "repository credentials '%s' not found", name) } @@ -522,10 +419,6 @@ func (db *db) repoWriteBackend() repositoryBackend { return &secretsRepositoryBackend{db: db, writeCreds: true} } -func (db *db) legacyRepoBackend() repositoryBackend { - return &legacyRepositoryBackend{db: db} -} - func (db *db) enrichCredsToRepo(ctx context.Context, repository *appsv1.Repository) error { if !repository.HasCredentials() { creds, err := db.GetRepositoryCredentials(ctx, repository.Repo) diff --git a/util/db/repository_legacy.go b/util/db/repository_legacy.go deleted file mode 100644 index 4f570c653e15e..0000000000000 --- a/util/db/repository_legacy.go +++ /dev/null @@ -1,475 +0,0 @@ -package db - -import ( - "context" - "fmt" - "strings" - - log "github.com/sirupsen/logrus" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - apiv1 "k8s.io/api/core/v1" - apierr "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/argoproj/argo-cd/v2/common" - appsv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "github.com/argoproj/argo-cd/v2/util/errors" - "github.com/argoproj/argo-cd/v2/util/git" - "github.com/argoproj/argo-cd/v2/util/settings" -) - -var _ repositoryBackend = &legacyRepositoryBackend{} - -// legacyRepositoryBackend is a repository backend strategy that maintains backward compatibility with previous versions. -// This can be removed in a future version, once the old "argocd-cm" storage for repositories is removed. -type legacyRepositoryBackend struct { - db *db -} - -func (l *legacyRepositoryBackend) CreateRepository(ctx context.Context, r *appsv1.Repository) (*appsv1.Repository, error) { - // This strategy only kept to preserve backward compatibility, but is deprecated. - // Therefore, no new repositories can be added with this backend. - panic("creating new repositories is not supported for the legacy repository backend") -} - -func (l *legacyRepositoryBackend) GetRepository(ctx context.Context, repoURL, project string) (*appsv1.Repository, error) { - repository, err := l.tryGetRepository(repoURL) - if err != nil { - return nil, fmt.Errorf("unable to get repository: %w", err) - } - return repository, nil -} - -func (l *legacyRepositoryBackend) ListRepositories(ctx context.Context, repoType *string) ([]*appsv1.Repository, error) { - inRepos, err := l.db.settingsMgr.GetRepositories() - if err != nil { - return nil, err - } - - var repos []*appsv1.Repository - for _, inRepo := range inRepos { - if repoType == nil || *repoType == inRepo.Type { - r, err := l.tryGetRepository(inRepo.URL) - if err != nil { - if r != nil && errors.IsCredentialsConfigurationError(err) { - modifiedTime := metav1.Now() - r.ConnectionState = appsv1.ConnectionState{ - Status: appsv1.ConnectionStatusFailed, - Message: "Configuration error - please check the server logs", - ModifiedAt: &modifiedTime, - } - - log.Warnf("could not retrieve repo: %s", err.Error()) - } else { - return nil, err - } - } - repos = append(repos, r) - } - } - return repos, nil -} - -func (l *legacyRepositoryBackend) UpdateRepository(ctx context.Context, r *appsv1.Repository) (*appsv1.Repository, error) { - repos, err := l.db.settingsMgr.GetRepositories() - if err != nil { - return nil, err - } - - index := l.getRepositoryIndex(repos, r.Repo) - if index < 0 { - return nil, status.Errorf(codes.NotFound, "repo '%s' not found", r.Repo) - } - - repoInfo := repos[index] - err = l.updateRepositorySecrets(&repoInfo, r) - if err != nil { - return nil, err - } - - // Update boolean settings - repoInfo.InsecureIgnoreHostKey = r.IsInsecure() - repoInfo.Insecure = r.IsInsecure() - repoInfo.EnableLFS = r.EnableLFS - repoInfo.Proxy = r.Proxy - - repos[index] = repoInfo - err = l.db.settingsMgr.SaveRepositories(repos) - if err != nil { - return nil, err - } - return r, nil -} - -func (l *legacyRepositoryBackend) DeleteRepository(ctx context.Context, repoURL, project string) error { - repos, err := l.db.settingsMgr.GetRepositories() - if err != nil { - return err - } - - index := l.getRepositoryIndex(repos, repoURL) - if index < 0 { - return status.Errorf(codes.NotFound, "repo '%s' not found", repoURL) - } - err = l.updateRepositorySecrets(&repos[index], &appsv1.Repository{ - SSHPrivateKey: "", - Password: "", - Username: "", - TLSClientCertData: "", - TLSClientCertKey: "", - GithubAppPrivateKey: "", - }) - if err != nil { - return err - } - repos = append(repos[:index], repos[index+1:]...) - return l.db.settingsMgr.SaveRepositories(repos) -} - -func (l *legacyRepositoryBackend) RepositoryExists(ctx context.Context, repoURL, project string, allowFallback bool) (bool, error) { - repos, err := l.db.settingsMgr.GetRepositories() - if err != nil { - return false, fmt.Errorf("unable to get repositories: %w", err) - } - - index := l.getRepositoryIndex(repos, repoURL) - return index >= 0, nil -} - -func (l *legacyRepositoryBackend) CreateRepoCreds(ctx context.Context, r *appsv1.RepoCreds) (*appsv1.RepoCreds, error) { - // This strategy only kept to preserve backward compatibility, but is deprecated. - // Therefore, no new repositories can be added with this backend. - panic("creating new repository credentials is not supported for the legacy repository backend") -} - -func (l *legacyRepositoryBackend) GetRepoCreds(ctx context.Context, repoURL string) (*appsv1.RepoCreds, error) { - var credential *appsv1.RepoCreds - - repoCredentials, err := l.db.settingsMgr.GetRepositoryCredentials() - if err != nil { - return nil, err - } - index := getRepositoryCredentialIndex(repoCredentials, repoURL) - if index >= 0 { - credential, err = l.credentialsToRepositoryCredentials(repoCredentials[index]) - if err != nil { - return nil, err - } - } - - return credential, err -} - -func (l *legacyRepositoryBackend) ListRepoCreds(ctx context.Context) ([]string, error) { - repos, err := l.db.settingsMgr.GetRepositoryCredentials() - if err != nil { - return nil, err - } - - urls := make([]string, len(repos)) - for i := range repos { - urls[i] = repos[i].URL - } - - return urls, nil -} - -func (l *legacyRepositoryBackend) UpdateRepoCreds(ctx context.Context, r *appsv1.RepoCreds) (*appsv1.RepoCreds, error) { - repos, err := l.db.settingsMgr.GetRepositoryCredentials() - if err != nil { - return nil, err - } - - index := getRepositoryCredentialIndex(repos, r.URL) - if index < 0 { - return nil, status.Errorf(codes.NotFound, "repository credentials '%s' not found", r.URL) - } - - repoInfo := repos[index] - err = l.updateCredentialsSecret(&repoInfo, r) - if err != nil { - return nil, err - } - - repos[index] = repoInfo - err = l.db.settingsMgr.SaveRepositoryCredentials(repos) - if err != nil { - return nil, err - } - return r, nil -} - -func (l *legacyRepositoryBackend) DeleteRepoCreds(ctx context.Context, name string) error { - repos, err := l.db.settingsMgr.GetRepositoryCredentials() - if err != nil { - return err - } - - index := getRepositoryCredentialIndex(repos, name) - if index < 0 { - return status.Errorf(codes.NotFound, "repository credentials '%s' not found", name) - } - err = l.updateCredentialsSecret(&repos[index], &appsv1.RepoCreds{ - SSHPrivateKey: "", - Password: "", - Username: "", - TLSClientCertData: "", - TLSClientCertKey: "", - GithubAppPrivateKey: "", - }) - if err != nil { - return err - } - repos = append(repos[:index], repos[index+1:]...) - return l.db.settingsMgr.SaveRepositoryCredentials(repos) -} - -func (l *legacyRepositoryBackend) RepoCredsExists(ctx context.Context, repoURL string) (bool, error) { - creds, err := l.db.settingsMgr.GetRepositoryCredentials() - if err != nil { - return false, err - } - - index := getRepositoryCredentialIndex(creds, repoURL) - return index >= 0, nil -} - -func (l *legacyRepositoryBackend) GetAllHelmRepoCreds(ctx context.Context) ([]*appsv1.RepoCreds, error) { - var allCredentials []*appsv1.RepoCreds - repoCredentials, err := l.db.settingsMgr.GetRepositoryCredentials() - if err != nil { - return nil, err - } - for _, v := range repoCredentials { - if strings.EqualFold(v.Type, "helm") { - credential, err := l.credentialsToRepositoryCredentials(v) - if err != nil { - return nil, err - } - allCredentials = append(allCredentials, credential) - } - } - return allCredentials, err -} - -func (l *legacyRepositoryBackend) updateRepositorySecrets(repoInfo *settings.Repository, r *appsv1.Repository) error { - secretsData := make(map[string]map[string][]byte) - - repoInfo.UsernameSecret = l.setSecretData(repoSecretPrefix, r.Repo, secretsData, repoInfo.UsernameSecret, r.Username, username) - repoInfo.PasswordSecret = l.setSecretData(repoSecretPrefix, r.Repo, secretsData, repoInfo.PasswordSecret, r.Password, password) - repoInfo.SSHPrivateKeySecret = l.setSecretData(repoSecretPrefix, r.Repo, secretsData, repoInfo.SSHPrivateKeySecret, r.SSHPrivateKey, sshPrivateKey) - repoInfo.TLSClientCertDataSecret = l.setSecretData(repoSecretPrefix, r.Repo, secretsData, repoInfo.TLSClientCertDataSecret, r.TLSClientCertData, tlsClientCertData) - repoInfo.TLSClientCertKeySecret = l.setSecretData(repoSecretPrefix, r.Repo, secretsData, repoInfo.TLSClientCertKeySecret, r.TLSClientCertKey, tlsClientCertKey) - repoInfo.GithubAppPrivateKeySecret = l.setSecretData(repoSecretPrefix, r.Repo, secretsData, repoInfo.GithubAppPrivateKeySecret, r.GithubAppPrivateKey, githubAppPrivateKey) - repoInfo.GCPServiceAccountKey = l.setSecretData(repoSecretPrefix, r.Repo, secretsData, repoInfo.GCPServiceAccountKey, r.GCPServiceAccountKey, gcpServiceAccountKey) - for k, v := range secretsData { - err := l.upsertSecret(k, v) - if err != nil { - return err - } - } - return nil -} - -func (l *legacyRepositoryBackend) updateCredentialsSecret(credsInfo *settings.RepositoryCredentials, c *appsv1.RepoCreds) error { - r := &appsv1.Repository{ - Repo: c.URL, - Username: c.Username, - Password: c.Password, - SSHPrivateKey: c.SSHPrivateKey, - TLSClientCertData: c.TLSClientCertData, - TLSClientCertKey: c.TLSClientCertKey, - GithubAppPrivateKey: c.GithubAppPrivateKey, - GithubAppId: c.GithubAppId, - GithubAppInstallationId: c.GithubAppInstallationId, - GitHubAppEnterpriseBaseURL: c.GitHubAppEnterpriseBaseURL, - GCPServiceAccountKey: c.GCPServiceAccountKey, - } - secretsData := make(map[string]map[string][]byte) - - credsInfo.UsernameSecret = l.setSecretData(credSecretPrefix, r.Repo, secretsData, credsInfo.UsernameSecret, r.Username, username) - credsInfo.PasswordSecret = l.setSecretData(credSecretPrefix, r.Repo, secretsData, credsInfo.PasswordSecret, r.Password, password) - credsInfo.SSHPrivateKeySecret = l.setSecretData(credSecretPrefix, r.Repo, secretsData, credsInfo.SSHPrivateKeySecret, r.SSHPrivateKey, sshPrivateKey) - credsInfo.TLSClientCertDataSecret = l.setSecretData(credSecretPrefix, r.Repo, secretsData, credsInfo.TLSClientCertDataSecret, r.TLSClientCertData, tlsClientCertData) - credsInfo.TLSClientCertKeySecret = l.setSecretData(credSecretPrefix, r.Repo, secretsData, credsInfo.TLSClientCertKeySecret, r.TLSClientCertKey, tlsClientCertKey) - credsInfo.GithubAppPrivateKeySecret = l.setSecretData(repoSecretPrefix, r.Repo, secretsData, credsInfo.GithubAppPrivateKeySecret, r.GithubAppPrivateKey, githubAppPrivateKey) - credsInfo.GCPServiceAccountKey = l.setSecretData(repoSecretPrefix, r.Repo, secretsData, credsInfo.GCPServiceAccountKey, r.GCPServiceAccountKey, gcpServiceAccountKey) - for k, v := range secretsData { - err := l.upsertSecret(k, v) - if err != nil { - return err - } - } - return nil -} - -func (l *legacyRepositoryBackend) upsertSecret(name string, data map[string][]byte) error { - secret, err := l.db.kubeclientset.CoreV1().Secrets(l.db.ns).Get(context.Background(), name, metav1.GetOptions{}) - if err != nil { - if apierr.IsNotFound(err) { - if len(data) == 0 { - return nil - } - _, err = l.db.kubeclientset.CoreV1().Secrets(l.db.ns).Create(context.Background(), &apiv1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Annotations: map[string]string{ - common.AnnotationKeyManagedBy: common.AnnotationValueManagedByArgoCD, - }, - }, - Data: data, - }, metav1.CreateOptions{}) - if err != nil { - return err - } - } - } else { - for _, key := range []string{username, password, sshPrivateKey, tlsClientCertData, tlsClientCertKey, githubAppPrivateKey} { - if secret.Data == nil { - secret.Data = make(map[string][]byte) - } - if val, ok := data[key]; ok && len(val) > 0 { - secret.Data[key] = val - } else { - delete(secret.Data, key) - } - } - if len(secret.Data) == 0 { - isManagedByArgo := secret.Annotations != nil && secret.Annotations[common.AnnotationKeyManagedBy] == common.AnnotationValueManagedByArgoCD - if isManagedByArgo { - return l.db.kubeclientset.CoreV1().Secrets(l.db.ns).Delete(context.Background(), name, metav1.DeleteOptions{}) - } - return nil - } else { - _, err = l.db.kubeclientset.CoreV1().Secrets(l.db.ns).Update(context.Background(), secret, metav1.UpdateOptions{}) - if err != nil { - return err - } - } - } - return nil -} - -// tryGetRepository returns a repository by URL. -// It provides the same functionality as GetRepository, with the additional behaviour of still returning a repository, -// even if an error occurred during the resolving of credentials for the repository. Otherwise this function behaves -// just as one would expect. -func (l *legacyRepositoryBackend) tryGetRepository(repoURL string) (*appsv1.Repository, error) { - repos, err := l.db.settingsMgr.GetRepositories() - if err != nil { - return nil, err - } - - repo := &appsv1.Repository{Repo: repoURL} - index := l.getRepositoryIndex(repos, repoURL) - if index >= 0 { - repo, err = l.credentialsToRepository(repos[index]) - if err != nil { - return repo, errors.NewCredentialsConfigurationError(err) - } - } - - return repo, err -} - -func (l *legacyRepositoryBackend) credentialsToRepository(repoInfo settings.Repository) (*appsv1.Repository, error) { - repo := &appsv1.Repository{ - Repo: repoInfo.URL, - Type: repoInfo.Type, - Name: repoInfo.Name, - InsecureIgnoreHostKey: repoInfo.InsecureIgnoreHostKey, - Insecure: repoInfo.Insecure, - EnableLFS: repoInfo.EnableLFS, - EnableOCI: repoInfo.EnableOci, - GithubAppId: repoInfo.GithubAppId, - GithubAppInstallationId: repoInfo.GithubAppInstallationId, - GitHubAppEnterpriseBaseURL: repoInfo.GithubAppEnterpriseBaseURL, - Proxy: repoInfo.Proxy, - } - err := l.db.unmarshalFromSecretsStr(map[*SecretMaperValidation]*apiv1.SecretKeySelector{ - {Dest: &repo.Username, Transform: StripCRLFCharacter}: repoInfo.UsernameSecret, - {Dest: &repo.Password, Transform: StripCRLFCharacter}: repoInfo.PasswordSecret, - {Dest: &repo.SSHPrivateKey, Transform: StripCRLFCharacter}: repoInfo.SSHPrivateKeySecret, - {Dest: &repo.TLSClientCertData, Transform: StripCRLFCharacter}: repoInfo.TLSClientCertDataSecret, - {Dest: &repo.TLSClientCertKey, Transform: StripCRLFCharacter}: repoInfo.TLSClientCertKeySecret, - {Dest: &repo.GithubAppPrivateKey, Transform: StripCRLFCharacter}: repoInfo.GithubAppPrivateKeySecret, - {Dest: &repo.GCPServiceAccountKey, Transform: StripCRLFCharacter}: repoInfo.GCPServiceAccountKey, - }, make(map[string]*apiv1.Secret)) - return repo, err -} - -func (l *legacyRepositoryBackend) credentialsToRepositoryCredentials(repoInfo settings.RepositoryCredentials) (*appsv1.RepoCreds, error) { - creds := &appsv1.RepoCreds{ - URL: repoInfo.URL, - GithubAppId: repoInfo.GithubAppId, - GithubAppInstallationId: repoInfo.GithubAppInstallationId, - GitHubAppEnterpriseBaseURL: repoInfo.GithubAppEnterpriseBaseURL, - EnableOCI: repoInfo.EnableOCI, - } - err := l.db.unmarshalFromSecretsStr(map[*SecretMaperValidation]*apiv1.SecretKeySelector{ - {Dest: &creds.Username}: repoInfo.UsernameSecret, - {Dest: &creds.Password}: repoInfo.PasswordSecret, - {Dest: &creds.SSHPrivateKey}: repoInfo.SSHPrivateKeySecret, - {Dest: &creds.TLSClientCertData}: repoInfo.TLSClientCertDataSecret, - {Dest: &creds.TLSClientCertKey}: repoInfo.TLSClientCertKeySecret, - {Dest: &creds.GithubAppPrivateKey}: repoInfo.GithubAppPrivateKeySecret, - {Dest: &creds.GCPServiceAccountKey}: repoInfo.GCPServiceAccountKey, - }, make(map[string]*apiv1.Secret)) - return creds, err -} - -// Set data to be stored in a given secret used for repository credentials and templates. -// The name of the secret is a combination of the prefix given, and a calculated value -// from the repository or template URL. -func (l *legacyRepositoryBackend) setSecretData(prefix string, url string, secretsData map[string]map[string][]byte, secretKey *apiv1.SecretKeySelector, value string, defaultKeyName string) *apiv1.SecretKeySelector { - if secretKey == nil && value != "" { - secretKey = &apiv1.SecretKeySelector{ - LocalObjectReference: apiv1.LocalObjectReference{Name: RepoURLToSecretName(prefix, url, "")}, - Key: defaultKeyName, - } - } - - if secretKey != nil { - data, ok := secretsData[secretKey.Name] - if !ok { - data = map[string][]byte{} - } - if value != "" { - data[secretKey.Key] = []byte(value) - } - secretsData[secretKey.Name] = data - } - - if value == "" { - secretKey = nil - } - - return secretKey -} - -func (l *legacyRepositoryBackend) getRepositoryIndex(repos []settings.Repository, repoURL string) int { - for i, repo := range repos { - if git.SameURL(repo.URL, repoURL) { - return i - } - } - return -1 -} - -// getRepositoryCredentialIndex returns the index of the best matching repository credential -// configuration, i.e. the one with the longest match -func getRepositoryCredentialIndex(repoCredentials []settings.RepositoryCredentials, repoURL string) int { - var max, idx int = 0, -1 - repoURL = git.NormalizeGitURL(repoURL) - for i, cred := range repoCredentials { - credUrl := git.NormalizeGitURL(cred.URL) - if strings.HasPrefix(repoURL, credUrl) { - if len(credUrl) > max { - max = len(credUrl) - idx = i - } - } - } - return idx -} diff --git a/util/db/repository_legacy_test.go b/util/db/repository_legacy_test.go deleted file mode 100644 index b414ffa9cc577..0000000000000 --- a/util/db/repository_legacy_test.go +++ /dev/null @@ -1,35 +0,0 @@ -package db - -import ( - "testing" - - "github.com/argoproj/argo-cd/v2/util/settings" -) - -func Test_getRepositoryCredentialIndex(t *testing.T) { - repositoryCredentials := []settings.RepositoryCredentials{ - {URL: "http://known"}, - {URL: "http://known/repos"}, - {URL: "http://known/other"}, - {URL: "http://known/other/other"}, - } - tests := []struct { - name string - repoURL string - want int - }{ - {"TestNotFound", "", -1}, - {"TestNotFound", "http://unknown/repos", -1}, - {"TestNotFound", "http://unknown/repo/repo", -1}, - {"TestFoundFound", "http://known/repos/repo", 1}, - {"TestFoundFound", "http://known/other/repo/foo", 2}, - {"TestFoundFound", "http://known/other/other/repo", 3}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := getRepositoryCredentialIndex(repositoryCredentials, tt.repoURL); got != tt.want { - t.Errorf("getRepositoryCredentialIndex() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/util/db/repository_secrets_test.go b/util/db/repository_secrets_test.go index e484999f884e7..d3fcc8ffbca3a 100644 --- a/util/db/repository_secrets_test.go +++ b/util/db/repository_secrets_test.go @@ -37,7 +37,7 @@ func TestSecretsRepositoryBackend_CreateRepository(t *testing.T) { EnableLFS: true, } setupWithK8sObjects := func(objects ...runtime.Object) *fixture { - clientset := getClientset(map[string]string{}, objects...) + clientset := getClientset(objects...) settingsMgr := settings.NewSettingsManager(context.Background(), clientset, testNamespace) repoBackend := &secretsRepositoryBackend{db: &db{ ns: testNamespace, @@ -210,7 +210,7 @@ func TestSecretsRepositoryBackend_GetRepository(t *testing.T) { }, } - clientset := getClientset(map[string]string{}, repoSecrets...) + clientset := getClientset(repoSecrets...) testee := &secretsRepositoryBackend{db: &db{ ns: testNamespace, kubeclientset: clientset, @@ -283,7 +283,7 @@ func TestSecretsRepositoryBackend_ListRepositories(t *testing.T) { }, } - clientset := getClientset(map[string]string{}, repoSecrets...) + clientset := getClientset(repoSecrets...) testee := &secretsRepositoryBackend{db: &db{ ns: testNamespace, kubeclientset: clientset, @@ -405,7 +405,7 @@ func TestSecretsRepositoryBackend_UpdateRepository(t *testing.T) { }, } - clientset := getClientset(map[string]string{}, repoSecrets...) + clientset := getClientset(repoSecrets...) testee := &secretsRepositoryBackend{db: &db{ ns: testNamespace, kubeclientset: clientset, @@ -514,7 +514,7 @@ func TestSecretsRepositoryBackend_DeleteRepository(t *testing.T) { }, } - clientset := getClientset(map[string]string{}, repoSecrets...) + clientset := getClientset(repoSecrets...) testee := &secretsRepositoryBackend{db: &db{ ns: testNamespace, kubeclientset: clientset, @@ -546,7 +546,7 @@ func TestSecretsRepositoryBackend_DeleteRepository(t *testing.T) { } func TestSecretsRepositoryBackend_CreateRepoCreds(t *testing.T) { - clientset := getClientset(map[string]string{}) + clientset := getClientset() testee := &secretsRepositoryBackend{db: &db{ ns: testNamespace, kubeclientset: clientset, @@ -675,7 +675,7 @@ func TestSecretsRepositoryBackend_GetRepoCreds(t *testing.T) { }, } - clientset := getClientset(map[string]string{}, repoCredSecrets...) + clientset := getClientset(repoCredSecrets...) testee := &secretsRepositoryBackend{db: &db{ ns: testNamespace, kubeclientset: clientset, @@ -732,7 +732,7 @@ func TestSecretsRepositoryBackend_ListRepoCreds(t *testing.T) { }, } - clientset := getClientset(map[string]string{}, repoCredSecrets...) + clientset := getClientset(repoCredSecrets...) testee := &secretsRepositoryBackend{db: &db{ ns: testNamespace, kubeclientset: clientset, @@ -793,7 +793,7 @@ func TestSecretsRepositoryBackend_UpdateRepoCreds(t *testing.T) { }, } - clientset := getClientset(map[string]string{}, repoCredSecrets...) + clientset := getClientset(repoCredSecrets...) testee := &secretsRepositoryBackend{db: &db{ ns: testNamespace, kubeclientset: clientset, @@ -862,7 +862,7 @@ func TestSecretsRepositoryBackend_DeleteRepoCreds(t *testing.T) { }, } - clientset := getClientset(map[string]string{}, repoSecrets...) + clientset := getClientset(repoSecrets...) testee := &secretsRepositoryBackend{db: &db{ ns: testNamespace, kubeclientset: clientset, @@ -916,7 +916,7 @@ func TestSecretsRepositoryBackend_GetAllHelmRepoCreds(t *testing.T) { }, } - clientset := getClientset(map[string]string{}, repoCredSecrets...) + clientset := getClientset(repoCredSecrets...) testee := &secretsRepositoryBackend{db: &db{ ns: testNamespace, kubeclientset: clientset, diff --git a/util/db/repository_test.go b/util/db/repository_test.go index a0601815e901f..f66cd0e39220b 100644 --- a/util/db/repository_test.go +++ b/util/db/repository_test.go @@ -14,19 +14,6 @@ import ( "github.com/argoproj/argo-cd/v2/util/settings" ) -const ( - repoArgoProj = ` -- name: OtherRepo - url: git@github.com:argoproj/argoproj.git - usernameSecret: - name: managed-secret - key: username - passwordSecret: - name: managed-secret - key: password - type: git` -) - var repoArgoCD = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Namespace: testNamespace, @@ -47,8 +34,28 @@ var repoArgoCD = &corev1.Secret{ }, } +var repoArgoProj = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: "some-other-repo-secret", + Annotations: map[string]string{ + common.AnnotationKeyManagedBy: common.AnnotationValueManagedByArgoCD, + }, + Labels: map[string]string{ + common.LabelKeySecretType: common.LabelValueSecretTypeRepository, + }, + }, + Data: map[string][]byte{ + "name": []byte("OtherRepo"), + "url": []byte("git@github.com:argoproj/argoproj.git"), + "username": []byte("someUsername"), + "password": []byte("somePassword"), + "type": []byte("git"), + }, +} + func TestDb_CreateRepository(t *testing.T) { - clientset := getClientset(map[string]string{}) + clientset := getClientset() settingsManager := settings.NewSettingsManager(context.TODO(), clientset, testNamespace) testee := &db{ ns: testNamespace, @@ -68,12 +75,6 @@ func TestDb_CreateRepository(t *testing.T) { require.NoError(t, err) assert.Same(t, input, output) - // New repositories should not be stored in the settings anymore - settingRepositories, err := settingsManager.GetRepositories() - require.NoError(t, err) - assert.Empty(t, settingRepositories) - - // New repositories should be now stored as secrets secret, err := clientset.CoreV1().Secrets(testNamespace).Get( context.TODO(), RepoURLToSecretName(repoSecretPrefix, input.Repo, ""), @@ -84,7 +85,7 @@ func TestDb_CreateRepository(t *testing.T) { } func TestDb_GetRepository(t *testing.T) { - clientset := getClientset(map[string]string{"repositories": repoArgoProj}, newManagedSecret(), repoArgoCD) + clientset := getClientset(repoArgoCD, repoArgoProj) settingsManager := settings.NewSettingsManager(context.TODO(), clientset, testNamespace) testee := &db{ ns: testNamespace, @@ -94,12 +95,12 @@ func TestDb_GetRepository(t *testing.T) { repository, err := testee.GetRepository(context.TODO(), "git@github.com:argoproj/argoproj.git", "") require.NoError(t, err) - assert.NotNil(t, repository) + require.NotNil(t, repository) assert.Equal(t, "OtherRepo", repository.Name) repository, err = testee.GetRepository(context.TODO(), "git@github.com:argoproj/argo-cd.git", "") require.NoError(t, err) - assert.NotNil(t, repository) + require.NotNil(t, repository) assert.Equal(t, "SomeRepo", repository.Name) repository, err = testee.GetRepository(context.TODO(), "git@github.com:argoproj/not-existing.git", "") @@ -109,7 +110,7 @@ func TestDb_GetRepository(t *testing.T) { } func TestDb_ListRepositories(t *testing.T) { - clientset := getClientset(map[string]string{"repositories": repoArgoProj}, newManagedSecret(), repoArgoCD) + clientset := getClientset(repoArgoCD, repoArgoProj) settingsManager := settings.NewSettingsManager(context.TODO(), clientset, testNamespace) testee := &db{ ns: testNamespace, @@ -130,15 +131,8 @@ func TestDb_UpdateRepository(t *testing.T) { Password: "somePassword", Type: "git", } - settingRepository := &appsv1.Repository{ - Name: "OtherRepo", - Repo: "git@github.com:argoproj/argoproj.git", - Username: "otherUsername", - Password: "otherPassword", - Type: "git", - } - clientset := getClientset(map[string]string{"repositories": repoArgoProj}, newManagedSecret(), repoArgoCD) + clientset := getClientset(repoArgoCD) settingsManager := settings.NewSettingsManager(context.TODO(), clientset, testNamespace) testee := &db{ ns: testNamespace, @@ -146,30 +140,13 @@ func TestDb_UpdateRepository(t *testing.T) { settingsMgr: settingsManager, } - // Verify that legacy repository can still be updated - settingRepository.Username = "OtherUpdatedUsername" - repository, err := testee.UpdateRepository(context.TODO(), settingRepository) - require.NoError(t, err) - assert.NotNil(t, repository) - assert.Same(t, settingRepository, repository) - - secret, err := clientset.CoreV1().Secrets(testNamespace).Get( - context.TODO(), - "managed-secret", - metav1.GetOptions{}, - ) - require.NoError(t, err) - assert.NotNil(t, secret) - assert.Equal(t, "OtherUpdatedUsername", string(secret.Data["username"])) - - // Verify that secret-based repository can be updated secretRepository.Username = "UpdatedUsername" - repository, err = testee.UpdateRepository(context.TODO(), secretRepository) + repository, err := testee.UpdateRepository(context.TODO(), secretRepository) require.NoError(t, err) assert.NotNil(t, repository) assert.Same(t, secretRepository, repository) - secret, err = clientset.CoreV1().Secrets(testNamespace).Get( + secret, err := clientset.CoreV1().Secrets(testNamespace).Get( context.TODO(), "some-repo-secret", metav1.GetOptions{}, @@ -180,7 +157,7 @@ func TestDb_UpdateRepository(t *testing.T) { } func TestDb_DeleteRepository(t *testing.T) { - clientset := getClientset(map[string]string{"repositories": repoArgoProj}, newManagedSecret(), repoArgoCD) + clientset := getClientset(repoArgoCD, repoArgoProj) settingsManager := settings.NewSettingsManager(context.TODO(), clientset, testNamespace) testee := &db{ ns: testNamespace, @@ -191,10 +168,6 @@ func TestDb_DeleteRepository(t *testing.T) { err := testee.DeleteRepository(context.TODO(), "git@github.com:argoproj/argoproj.git", "") require.NoError(t, err) - repositories, err := settingsManager.GetRepositories() - require.NoError(t, err) - assert.Empty(t, repositories) - err = testee.DeleteRepository(context.TODO(), "git@github.com:argoproj/argo-cd.git", "") require.NoError(t, err) @@ -203,17 +176,7 @@ func TestDb_DeleteRepository(t *testing.T) { } func TestDb_GetRepositoryCredentials(t *testing.T) { - repositoryCredentialsSettings := ` -- type: git - url: git@github.com:argoproj - usernameSecret: - name: managed-secret - key: username - passwordSecret: - name: managed-secret - key: password -` - repoCredsSecret := &corev1.Secret{ + gitHubRepoCredsSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Namespace: testNamespace, Name: "some-repocreds-secret", @@ -221,6 +184,21 @@ func TestDb_GetRepositoryCredentials(t *testing.T) { common.LabelKeySecretType: common.LabelValueSecretTypeRepoCreds, }, }, + Data: map[string][]byte{ + "type": []byte("git"), + "url": []byte("git@github.com:argoproj"), + "username": []byte("someUsername"), + "password": []byte("somePassword"), + }, + } + gitLabRepoCredsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: "some-other-repocreds-secret", + Labels: map[string]string{ + common.LabelKeySecretType: common.LabelValueSecretTypeRepoCreds, + }, + }, Data: map[string][]byte{ "type": []byte("git"), "url": []byte("git@gitlab.com"), @@ -229,17 +207,17 @@ func TestDb_GetRepositoryCredentials(t *testing.T) { }, } - clientset := getClientset(map[string]string{"repository.credentials": repositoryCredentialsSettings}, newManagedSecret(), repoCredsSecret) + clientset := getClientset(gitHubRepoCredsSecret, gitLabRepoCredsSecret) testee := NewDB(testNamespace, settings.NewSettingsManager(context.TODO(), clientset, testNamespace), clientset) repoCreds, err := testee.GetRepositoryCredentials(context.TODO(), "git@github.com:argoproj/argoproj.git") require.NoError(t, err) - assert.NotNil(t, repoCreds) + require.NotNil(t, repoCreds) assert.Equal(t, "git@github.com:argoproj", repoCreds.URL) repoCreds, err = testee.GetRepositoryCredentials(context.TODO(), "git@gitlab.com:someorg/foobar.git") require.NoError(t, err) - assert.NotNil(t, repoCreds) + require.NotNil(t, repoCreds) assert.Equal(t, "git@gitlab.com", repoCreds.URL) repoCreds, err = testee.GetRepositoryCredentials(context.TODO(), "git@github.com:example/not-existing.git") @@ -354,7 +332,7 @@ func Test_GetProjectRepositories(t *testing.T) { }, } - clientset := getClientset(map[string]string{}, repoSecretWithProject, repoSecretWithoutProject) + clientset := getClientset(repoSecretWithProject, repoSecretWithoutProject) argoDB := NewDB(testNamespace, settings.NewSettingsManager(context.TODO(), clientset, testNamespace), clientset) repos, err := argoDB.GetProjectRepositories(context.TODO(), "some-project") diff --git a/util/settings/settings.go b/util/settings/settings.go index a4e75440cc208..ac06d4c96abdc 100644 --- a/util/settings/settings.go +++ b/util/settings/settings.go @@ -418,12 +418,6 @@ const ( settingURLKey = "url" // settingAdditionalUrlsKey designates the key where Argo CD's additional external URLs are set settingAdditionalUrlsKey = "additionalUrls" - // repositoriesKey designates the key where ArgoCDs repositories list is set - repositoriesKey = "repositories" - // repositoryCredentialsKey designates the key where ArgoCDs repositories credentials list is set - repositoryCredentialsKey = "repository.credentials" - // helmRepositoriesKey designates the key where list of helm repositories is set - helmRepositoriesKey = "helm.repositories" // settingDexConfigKey designates the key for the dex config settingDexConfigKey = "dex.config" // settingsOIDCConfigKey designates the key for OIDC config @@ -1209,111 +1203,6 @@ func addKustomizeVersion(prefix, name, path string, kvMap map[string]KustomizeVe return nil } -// DEPRECATED. Helm repository credentials are now managed using RepoCredentials -func (mgr *SettingsManager) GetHelmRepositories() ([]HelmRepoCredentials, error) { - argoCDCM, err := mgr.getConfigMap() - if err != nil { - return nil, fmt.Errorf("error retrieving config map: %w", err) - } - helmRepositories := make([]HelmRepoCredentials, 0) - helmRepositoriesStr := argoCDCM.Data[helmRepositoriesKey] - if helmRepositoriesStr != "" { - err := yaml.Unmarshal([]byte(helmRepositoriesStr), &helmRepositories) - if err != nil { - return nil, fmt.Errorf("error unmarshalling helm repositories: %w", err) - } - } - return helmRepositories, nil -} - -func (mgr *SettingsManager) GetRepositories() ([]Repository, error) { - mgr.mutex.Lock() - reposCache := mgr.reposCache - mgr.mutex.Unlock() - if reposCache != nil { - return reposCache, nil - } - - // Get the config map outside of the lock - argoCDCM, err := mgr.getConfigMap() - if err != nil { - return nil, fmt.Errorf("failed to get argo-cd config map: %w", err) - } - - mgr.mutex.Lock() - defer mgr.mutex.Unlock() - repositories := make([]Repository, 0) - repositoriesStr := argoCDCM.Data[repositoriesKey] - if repositoriesStr != "" { - err := yaml.Unmarshal([]byte(repositoriesStr), &repositories) - if err != nil { - return nil, fmt.Errorf("failed to unmarshal repositories from config map key %q: %w", repositoriesKey, err) - } - } - mgr.reposCache = repositories - - return mgr.reposCache, nil -} - -func (mgr *SettingsManager) SaveRepositories(repos []Repository) error { - return mgr.updateConfigMap(func(argoCDCM *apiv1.ConfigMap) error { - if len(repos) > 0 { - yamlStr, err := yaml.Marshal(repos) - if err != nil { - return err - } - argoCDCM.Data[repositoriesKey] = string(yamlStr) - } else { - delete(argoCDCM.Data, repositoriesKey) - } - return nil - }) -} - -func (mgr *SettingsManager) SaveRepositoryCredentials(creds []RepositoryCredentials) error { - return mgr.updateConfigMap(func(argoCDCM *apiv1.ConfigMap) error { - if len(creds) > 0 { - yamlStr, err := yaml.Marshal(creds) - if err != nil { - return err - } - argoCDCM.Data[repositoryCredentialsKey] = string(yamlStr) - } else { - delete(argoCDCM.Data, repositoryCredentialsKey) - } - return nil - }) -} - -func (mgr *SettingsManager) GetRepositoryCredentials() ([]RepositoryCredentials, error) { - mgr.mutex.Lock() - repoCredsCache := mgr.repoCredsCache - mgr.mutex.Unlock() - if repoCredsCache != nil { - return repoCredsCache, nil - } - - // Get the config map outside of the lock - argoCDCM, err := mgr.getConfigMap() - if err != nil { - return nil, fmt.Errorf("error retrieving config map: %w", err) - } - - mgr.mutex.Lock() - defer mgr.mutex.Unlock() - creds := make([]RepositoryCredentials, 0) - credsStr := argoCDCM.Data[repositoryCredentialsKey] - if credsStr != "" { - err := yaml.Unmarshal([]byte(credsStr), &creds) - if err != nil { - return nil, err - } - } - mgr.repoCredsCache = creds - - return mgr.repoCredsCache, nil -} - func (mgr *SettingsManager) GetGoogleAnalytics() (*GoogleAnalytics, error) { argoCDCM, err := mgr.getConfigMap() if err != nil { diff --git a/util/settings/settings_test.go b/util/settings/settings_test.go index 59ff122704fb3..f470d202fc4c1 100644 --- a/util/settings/settings_test.go +++ b/util/settings/settings_test.go @@ -102,15 +102,6 @@ func TestGetSecretByName(t *testing.T) { }) } -func TestGetRepositories(t *testing.T) { - _, settingsManager := fixtures(map[string]string{ - "repositories": "\n - url: http://foo\n", - }) - filter, err := settingsManager.GetRepositories() - require.NoError(t, err) - assert.Equal(t, []Repository{{URL: "http://foo"}}, filter) -} - func TestGetExtensionConfigs(t *testing.T) { type cases struct { name string @@ -157,52 +148,6 @@ func TestGetExtensionConfigs(t *testing.T) { } } -func TestSaveRepositories(t *testing.T) { - kubeClient, settingsManager := fixtures(nil) - err := settingsManager.SaveRepositories([]Repository{{URL: "http://foo"}}) - require.NoError(t, err) - cm, err := kubeClient.CoreV1().ConfigMaps("default").Get(context.Background(), common.ArgoCDConfigMapName, metav1.GetOptions{}) - require.NoError(t, err) - assert.Equal(t, "- url: http://foo\n", cm.Data["repositories"]) - - repos, err := settingsManager.GetRepositories() - require.NoError(t, err) - assert.ElementsMatch(t, repos, []Repository{{URL: "http://foo"}}) -} - -func TestSaveRepositoriesNoConfigMap(t *testing.T) { - kubeClient := fake.NewClientset() - settingsManager := NewSettingsManager(context.Background(), kubeClient, "default") - - err := settingsManager.SaveRepositories([]Repository{{URL: "http://foo"}}) - require.NoError(t, err) - cm, err := kubeClient.CoreV1().ConfigMaps("default").Get(context.Background(), common.ArgoCDConfigMapName, metav1.GetOptions{}) - require.NoError(t, err) - assert.Equal(t, "- url: http://foo\n", cm.Data["repositories"]) -} - -func TestSaveRepositoryCredentials(t *testing.T) { - kubeClient, settingsManager := fixtures(nil) - err := settingsManager.SaveRepositoryCredentials([]RepositoryCredentials{{URL: "http://foo"}}) - require.NoError(t, err) - cm, err := kubeClient.CoreV1().ConfigMaps("default").Get(context.Background(), common.ArgoCDConfigMapName, metav1.GetOptions{}) - require.NoError(t, err) - assert.Equal(t, "- url: http://foo\n", cm.Data["repository.credentials"]) - - creds, err := settingsManager.GetRepositoryCredentials() - require.NoError(t, err) - assert.ElementsMatch(t, creds, []RepositoryCredentials{{URL: "http://foo"}}) -} - -func TestGetRepositoryCredentials(t *testing.T) { - _, settingsManager := fixtures(map[string]string{ - "repository.credentials": "\n - url: http://foo\n", - }) - filter, err := settingsManager.GetRepositoryCredentials() - require.NoError(t, err) - assert.Equal(t, []RepositoryCredentials{{URL: "http://foo"}}, filter) -} - func TestGetResourceFilter(t *testing.T) { data := map[string]string{ "resource.exclusions": "\n - apiGroups: [\"group1\"]\n kinds: [\"kind1\"]\n clusters: [\"cluster1\"]\n", From 84b1aa7dc63c33d84ecbb9b03cf0967b99332b2d Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Wed, 18 Dec 2024 22:05:26 -0500 Subject: [PATCH 2/6] fix some tests, delete some others Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- cmd/argocd/commands/admin/admin.go | 34 ++++++++++++++++++++++++++ cmd/argocd/commands/admin/backup.go | 7 ++++++ test/e2e/fixture/admin/utils/backup.go | 2 +- test/e2e/helm_test.go | 34 ++++++++++++-------------- test/e2e/repo_creds_test.go | 17 ------------- 5 files changed, 57 insertions(+), 37 deletions(-) diff --git a/cmd/argocd/commands/admin/admin.go b/cmd/argocd/commands/admin/admin.go index 2c7f54625b48a..51908b0d63900 100644 --- a/cmd/argocd/commands/admin/admin.go +++ b/cmd/argocd/commands/admin/admin.go @@ -92,6 +92,40 @@ func newArgoCDClientsets(config *rest.Config, namespace string) *argoCDClientset } } +// isArgoCDSecret returns whether or not the given secret is a part of Argo CD configuration +// (e.g. argocd-secret, repo credentials, or cluster credentials) +func isArgoCDSecret(repoSecretRefs map[string]bool, un unstructured.Unstructured) bool { + secretName := un.GetName() + if secretName == common.ArgoCDSecretName { + return true + } + if repoSecretRefs != nil { + if _, ok := repoSecretRefs[secretName]; ok { + return true + } + } + if labels := un.GetLabels(); labels != nil { + if _, ok := labels[common.LabelKeySecretType]; ok { + return true + } + } + if annotations := un.GetAnnotations(); annotations != nil { + if annotations[common.AnnotationKeyManagedBy] == common.AnnotationValueManagedByArgoCD { + return true + } + } + return false +} + +// isArgoCDConfigMap returns true if the configmap name is one of argo cd's well known configmaps +func isArgoCDConfigMap(name string) bool { + switch name { + case common.ArgoCDConfigMapName, common.ArgoCDRBACConfigMapName, common.ArgoCDKnownHostsConfigMapName, common.ArgoCDTLSCertsConfigMapName: + return true + } + return false +} + // specsEqual returns if the spec, data, labels, annotations, and finalizers of the two // supplied objects are equal, indicating that no update is necessary during importing func specsEqual(left, right unstructured.Unstructured) bool { diff --git a/cmd/argocd/commands/admin/backup.go b/cmd/argocd/commands/admin/backup.go index 41b94375c0fd6..9c8ad449da488 100644 --- a/cmd/argocd/commands/admin/backup.go +++ b/cmd/argocd/commands/admin/backup.go @@ -180,6 +180,13 @@ func NewImportCommand() *cobra.Command { // items in this map indicates the resource should be pruned since it no longer appears // in the backup pruneObjects := make(map[kube.ResourceKey]unstructured.Unstructured) + configMaps, err := acdClients.configMaps.List(ctx, v1.ListOptions{}) + errors.CheckError(err) + for _, cm := range configMaps.Items { + if isArgoCDConfigMap(cm.GetName()) { + pruneObjects[kube.ResourceKey{Group: "", Kind: "ConfigMap", Name: cm.GetName(), Namespace: cm.GetNamespace()}] = cm + } + } applications, err := acdClients.applications.List(ctx, v1.ListOptions{}) errors.CheckError(err) for _, app := range applications.Items { diff --git a/test/e2e/fixture/admin/utils/backup.go b/test/e2e/fixture/admin/utils/backup.go index 79bd890518603..d06932e7133a9 100644 --- a/test/e2e/fixture/admin/utils/backup.go +++ b/test/e2e/fixture/admin/utils/backup.go @@ -13,7 +13,7 @@ type ExportedResources []unstructured.Unstructured func GetExportedResourcesFromOutput(output string) (ExportedResources, error) { var resources []unstructured.Unstructured - docs := strings.Split(output, "---") + docs := strings.Split(output, "\n---\n") for _, doc := range docs { doc = strings.TrimSpace(doc) diff --git a/test/e2e/helm_test.go b/test/e2e/helm_test.go index 9d06eecf269c2..22b08d4bff2fc 100644 --- a/test/e2e/helm_test.go +++ b/test/e2e/helm_test.go @@ -431,7 +431,21 @@ func TestHelmValuesHiddenDirectory(t *testing.T) { func TestHelmWithDependencies(t *testing.T) { SkipOnEnv(t, "HELM") - testHelmWithDependencies(t, "helm-with-dependencies") + + ctx := Given(t). + CustomCACertAdded(). + // these are slow tests + Timeout(30). + HelmPassCredentials() + + ctx = ctx.HelmRepoAdded("custom-repo") + + ctx.Path("helm-with-dependencies"). + When(). + CreateApp("--helm-version", ""). + Sync(). + Then(). + Expect(SyncStatusIs(SyncStatusCodeSynced)) } func TestHelmWithMultipleDependencies(t *testing.T) { @@ -489,24 +503,6 @@ func TestHelmDependenciesPermissionDenied(t *testing.T) { Expect(Error("", expectedErr)) } -func testHelmWithDependencies(t *testing.T, chartPath string) { - t.Helper() - ctx := Given(t). - CustomCACertAdded(). - // these are slow tests - Timeout(30). - HelmPassCredentials() - - helmVer := "" - - ctx.Path(chartPath). - When(). - CreateApp("--helm-version", helmVer). - Sync(). - Then(). - Expect(SyncStatusIs(SyncStatusCodeSynced)) -} - func TestHelm3CRD(t *testing.T) { SkipOnEnv(t, "HELM") Given(t). diff --git a/test/e2e/repo_creds_test.go b/test/e2e/repo_creds_test.go index ba8f1669a89de..e97c36dfddf7b 100644 --- a/test/e2e/repo_creds_test.go +++ b/test/e2e/repo_creds_test.go @@ -47,23 +47,6 @@ func TestCanAddAppFromPrivateRepoWithRepoCfg(t *testing.T) { Expect(Success("")) } -// make sure you can create an app from a private repo, if the creds are set-up -func TestCanAddAppFromInsecurePrivateRepoWithCredCfg(t *testing.T) { - Given(t). - CustomCACertAdded(). - HTTPSCredentialsUserPassAdded(). - RepoURLType(fixture.RepoURLTypeHTTPS). - Path(fixture.LocalOrRemotePath("https-kustomize-base")). - And(func() { - // I use CLI, but you could also modify the settings, we get a free test of the CLI here - FailOnErr(fixture.RunCli("repocreds", "update", "--insecure-skip-server-verification")) - }). - When(). - CreateApp(). - Then(). - Expect(Success("")) -} - // make sure we can create an app from a private repo, in a secure manner using // a custom CA certificate bundle func TestCanAddAppFromPrivateRepoWithCredCfg(t *testing.T) { From 4ab69bd0b8f84c3f0aebdc133ebacf25d6adab7e Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Thu, 19 Dec 2024 10:41:09 -0500 Subject: [PATCH 3/6] remove unused function Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- cmd/argocd/commands/admin/admin.go | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/cmd/argocd/commands/admin/admin.go b/cmd/argocd/commands/admin/admin.go index 51908b0d63900..3a09259ed59c3 100644 --- a/cmd/argocd/commands/admin/admin.go +++ b/cmd/argocd/commands/admin/admin.go @@ -92,31 +92,6 @@ func newArgoCDClientsets(config *rest.Config, namespace string) *argoCDClientset } } -// isArgoCDSecret returns whether or not the given secret is a part of Argo CD configuration -// (e.g. argocd-secret, repo credentials, or cluster credentials) -func isArgoCDSecret(repoSecretRefs map[string]bool, un unstructured.Unstructured) bool { - secretName := un.GetName() - if secretName == common.ArgoCDSecretName { - return true - } - if repoSecretRefs != nil { - if _, ok := repoSecretRefs[secretName]; ok { - return true - } - } - if labels := un.GetLabels(); labels != nil { - if _, ok := labels[common.LabelKeySecretType]; ok { - return true - } - } - if annotations := un.GetAnnotations(); annotations != nil { - if annotations[common.AnnotationKeyManagedBy] == common.AnnotationValueManagedByArgoCD { - return true - } - } - return false -} - // isArgoCDConfigMap returns true if the configmap name is one of argo cd's well known configmaps func isArgoCDConfigMap(name string) bool { switch name { From c326e47d9e00ccf639fa2a750dfada3a3bc885d8 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Mon, 13 Jan 2025 13:04:41 -0500 Subject: [PATCH 4/6] add docs Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- docs/operator-manual/upgrading/2.14-3.0.md | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/docs/operator-manual/upgrading/2.14-3.0.md b/docs/operator-manual/upgrading/2.14-3.0.md index 8605ef6b2ab22..28e6dc5b13372 100644 --- a/docs/operator-manual/upgrading/2.14-3.0.md +++ b/docs/operator-manual/upgrading/2.14-3.0.md @@ -9,4 +9,23 @@ most recent minor versions (so 2.14 until 3.2 is released, and 2.13 until 3.1 is ## Breaking Changes -So far no breaking changes have been merged. We will update this section as changes are merged. +### Removed support for legacy repo config in argocd-cm + +Before repositories were managed as Secrets, they were configured in the argocd-cm ConfigMap. The argocd-cm option has +been deprecated for some time and is no longer available in Argo CD 3.0. + +#### Detection + +To check whether you have any repositories configured in argocd-cm, run the following command: + +```shell +kubectl get cm argocd-cm -o=jsonpath="[{.data.repositories}, {.data['repository.credentials']}, {.data['helm.repositories']}]" +``` + +If you have no repositories configured in argocd-cm, the output will be `[, , ]`, and you are not impacted by this +change. + +#### Resolution + +To convert your repositories to Secrets, follow the documentation for +[declarative management of repositories](../declarative-setup.md#repositories). From 37b2f087ece279dd4e34312d6cd3036e626e94e1 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Mon, 3 Feb 2025 14:31:53 -0500 Subject: [PATCH 5/6] handle non-referenced secrets Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- cmd/argocd/commands/admin/admin.go | 20 ++++++++++++++++++++ cmd/argocd/commands/admin/backup.go | 16 ++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/cmd/argocd/commands/admin/admin.go b/cmd/argocd/commands/admin/admin.go index 7c860043f82dc..e4e67a89ca8f1 100644 --- a/cmd/argocd/commands/admin/admin.go +++ b/cmd/argocd/commands/admin/admin.go @@ -92,6 +92,26 @@ func newArgoCDClientsets(config *rest.Config, namespace string) *argoCDClientset } } +// isArgoCDSecret returns whether or not the given secret is a part of Argo CD configuration +// (e.g. argocd-secret, repo credentials, or cluster credentials) +func isArgoCDSecret(un unstructured.Unstructured) bool { + secretName := un.GetName() + if secretName == common.ArgoCDSecretName { + return true + } + if labels := un.GetLabels(); labels != nil { + if _, ok := labels[common.LabelKeySecretType]; ok { + return true + } + } + if annotations := un.GetAnnotations(); annotations != nil { + if annotations[common.AnnotationKeyManagedBy] == common.AnnotationValueManagedByArgoCD { + return true + } + } + return false +} + // isArgoCDConfigMap returns true if the configmap name is one of argo cd's well known configmaps func isArgoCDConfigMap(name string) bool { switch name { diff --git a/cmd/argocd/commands/admin/backup.go b/cmd/argocd/commands/admin/backup.go index 9483f759a4e23..8232055c9f1e4 100644 --- a/cmd/argocd/commands/admin/backup.go +++ b/cmd/argocd/commands/admin/backup.go @@ -74,6 +74,14 @@ func NewExportCommand() *cobra.Command { errors.CheckError(err) export(writer, *acdTLSCertsConfigMap, namespace) + secrets, err := acdClients.secrets.List(ctx, metav1.ListOptions{}) + errors.CheckError(err) + for _, secret := range secrets.Items { + if isArgoCDSecret(secret) { + export(writer, secret, namespace) + } + } + projects, err := acdClients.projects.List(ctx, metav1.ListOptions{}) errors.CheckError(err) for _, proj := range projects.Items { @@ -187,6 +195,14 @@ func NewImportCommand() *cobra.Command { pruneObjects[kube.ResourceKey{Group: "", Kind: "ConfigMap", Name: cm.GetName(), Namespace: cm.GetNamespace()}] = cm } } + + secrets, err := acdClients.secrets.List(ctx, metav1.ListOptions{}) + errors.CheckError(err) + for _, secret := range secrets.Items { + if isArgoCDSecret(secret) { + pruneObjects[kube.ResourceKey{Group: "", Kind: "Secret", Name: secret.GetName(), Namespace: secret.GetNamespace()}] = secret + } + } applications, err := acdClients.applications.List(ctx, metav1.ListOptions{}) errors.CheckError(err) for _, app := range applications.Items { From 803dd3dbcdb90845da7f87a431b5dd92d010c04c Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Mon, 3 Feb 2025 14:34:23 -0500 Subject: [PATCH 6/6] simplify Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> --- util/db/repository.go | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/util/db/repository.go b/util/db/repository.go index 0beffa47878c8..9ff1d7eeef9dc 100644 --- a/util/db/repository.go +++ b/util/db/repository.go @@ -168,21 +168,15 @@ func (db *db) ListWriteRepositories(ctx context.Context) ([]*v1alpha1.Repository } func (db *db) listRepositories(ctx context.Context, repoType *string, writeCreds bool) ([]*v1alpha1.Repository, error) { - // TODO It would be nice to check for duplicates between secret and legacy repositories and make it so that - // repositories from secrets overlay repositories from legacys. - - var repositories []*v1alpha1.Repository - var err error + var backend repositoryBackend if writeCreds { - repositories, err = db.repoWriteBackend().ListRepositories(ctx, repoType) - if err != nil { - return nil, err - } + backend = db.repoWriteBackend() } else { - repositories, err = db.repoBackend().ListRepositories(ctx, repoType) - if err != nil { - return nil, err - } + backend = db.repoBackend() + } + repositories, err := backend.ListRepositories(ctx, repoType) + if err != nil { + return nil, err } if err = db.enrichCredsToRepos(ctx, repositories); err != nil { return nil, err