Skip to content

Commit

Permalink
fix: handle repo creation when secret does not have proper label (#7617)
Browse files Browse the repository at this point in the history 
* fix: handle repo creation when secret does not have proper label

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* address code review comments with linters

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove gpg test data refactoring code

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
  • Loading branch information
@leoluz
leoluz committed Nov 5, 2021
1 parent 1431e04 commit b0278c4
Show file tree
Hide file tree
Showing 2 changed files with 155 additions and 33 deletions.
35 changes: 32 additions & 3 deletions util/db/repository_secrets.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package db

import (
"fmt"
"strings"

log "github.com/sirupsen/logrus"
Expand Down Expand Up @@ -31,11 +32,19 @@ func (s *secretsRepositoryBackend) CreateRepository(ctx context.Context, reposit
},
}

s.repositoryToSecret(repository, repositorySecret)
repositoryToSecret(repository, repositorySecret)

_, err := s.db.createSecret(ctx, repositorySecret)
if err != nil {
if apierr.IsAlreadyExists(err) {
hasLabel, err := s.hasRepoTypeLabel(secName)
if err != nil {
return nil, status.Errorf(codes.Internal, err.Error())
}
if !hasLabel {
msg := fmt.Sprintf("secret %q doesn't have the proper %q label: please fix the secret or delete it", secName, common.LabelKeySecretType)
return nil, status.Errorf(codes.InvalidArgument, msg)
}
return nil, status.Errorf(codes.AlreadyExists, "repository %q already exists", repository.Repo)
}
return nil, err
Expand All @@ -44,6 +53,26 @@ func (s *secretsRepositoryBackend) CreateRepository(ctx context.Context, reposit
return repository, s.db.settingsMgr.ResyncInformers()
}

// hasRepoTypeLabel will verify if a secret with the given name exists. If so it will check if
// the secret has the proper label argocd.argoproj.io/secret-type defined. Will return true if
// the label is found and false otherwise. Will return false if no secret is found with the given
// name.
func (s *secretsRepositoryBackend) hasRepoTypeLabel(secretName string) (bool, error) {
noCache := make(map[string]*corev1.Secret)
sec, err := s.db.getSecret(secretName, noCache)
if err != nil {
if apierr.IsNotFound(err) {
return false, nil
}
return false, err
}
_, ok := sec.GetLabels()[common.LabelKeySecretType]
if ok {
return true, nil
}
return false, nil
}

func (s *secretsRepositoryBackend) GetRepository(ctx context.Context, repoURL string) (*appsv1.Repository, error) {
secret, err := s.getRepositorySecret(repoURL)
if err != nil {
Expand Down Expand Up @@ -104,7 +133,7 @@ func (s *secretsRepositoryBackend) UpdateRepository(ctx context.Context, reposit
return nil, err
}

s.repositoryToSecret(repository, repositorySecret)
repositoryToSecret(repository, repositorySecret)

_, err = s.db.kubeclientset.CoreV1().Secrets(s.db.ns).Update(ctx, repositorySecret, metav1.UpdateOptions{})
if err != nil {
Expand Down Expand Up @@ -317,7 +346,7 @@ func (s *secretsRepositoryBackend) secretToRepository(secret *corev1.Secret) (*a
return repository, nil
}

func (s *secretsRepositoryBackend) repositoryToSecret(repository *appsv1.Repository, secret *corev1.Secret) {
func repositoryToSecret(repository *appsv1.Repository, secret *corev1.Secret) {
if secret.Data == nil {
secret.Data = make(map[string][]byte)
}
Expand Down
153 changes: 123 additions & 30 deletions util/db/repository_secrets_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,53 +6,146 @@ import (

"github.com/stretchr/testify/assert"
"golang.org/x/net/context"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/client-go/kubernetes/fake"
k8stesting "k8s.io/client-go/testing"

"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/settings"
)

func TestSecretsRepositoryBackend_CreateRepository(t *testing.T) {
clientset := getClientset(map[string]string{})
testee := &secretsRepositoryBackend{db: &db{
ns: testNamespace,
kubeclientset: clientset,
settingsMgr: settings.NewSettingsManager(context.TODO(), clientset, testNamespace),
}}

input := &appsv1.Repository{
type fixture struct {
clientSet *fake.Clientset
repoBackend *secretsRepositoryBackend
}
repo := &appsv1.Repository{
Name: "ArgoCD",
Repo: "git@github.com:argoproj/argo-cd.git",
Username: "someUsername",
Password: "somePassword",
InsecureIgnoreHostKey: false,
EnableLFS: true,
}

output, err := testee.CreateRepository(context.TODO(), input)
assert.NoError(t, err)
assert.Same(t, input, output)

secret, err := clientset.CoreV1().Secrets(testNamespace).Get(
context.TODO(),
RepoURLToSecretName(repoSecretPrefix, input.Repo),
metav1.GetOptions{},
)
assert.NotNil(t, secret)
assert.NoError(t, err)

assert.Equal(t, common.AnnotationValueManagedByArgoCD, secret.Annotations[common.AnnotationKeyManagedBy])
assert.Equal(t, common.LabelValueSecretTypeRepository, secret.Labels[common.LabelKeySecretType])

assert.Equal(t, input.Name, string(secret.Data["name"]))
assert.Equal(t, input.Repo, string(secret.Data["url"]))
assert.Equal(t, input.Username, string(secret.Data["username"]))
assert.Equal(t, input.Password, string(secret.Data["password"]))
assert.Equal(t, "", string(secret.Data["insecureIgnoreHostKey"]))
assert.Equal(t, strconv.FormatBool(input.EnableLFS), string(secret.Data["enableLfs"]))
setupWithK8sObjects := func(objects ...runtime.Object) *fixture {

clientset := getClientset(map[string]string{}, objects...)
settingsMgr := settings.NewSettingsManager(context.Background(), clientset, testNamespace)
repoBackend := &secretsRepositoryBackend{db: &db{
ns: testNamespace,
kubeclientset: clientset,
settingsMgr: settingsMgr,
}}
return &fixture{
clientSet: clientset,
repoBackend: repoBackend,
}
}
t.Run("will create repository successfully", func(t *testing.T) {
// given
t.Parallel()
f := setupWithK8sObjects()

// when
output, err := f.repoBackend.CreateRepository(context.Background(), repo)

// then
assert.NoError(t, err)
assert.Same(t, repo, output)

secret, err := f.clientSet.CoreV1().Secrets(testNamespace).Get(
context.TODO(),
RepoURLToSecretName(repoSecretPrefix, repo.Repo),
metav1.GetOptions{},
)
assert.NotNil(t, secret)
assert.NoError(t, err)

assert.Equal(t, common.AnnotationValueManagedByArgoCD, secret.Annotations[common.AnnotationKeyManagedBy])
assert.Equal(t, common.LabelValueSecretTypeRepository, secret.Labels[common.LabelKeySecretType])

assert.Equal(t, repo.Name, string(secret.Data["name"]))
assert.Equal(t, repo.Repo, string(secret.Data["url"]))
assert.Equal(t, repo.Username, string(secret.Data["username"]))
assert.Equal(t, repo.Password, string(secret.Data["password"]))
assert.Equal(t, "", string(secret.Data["insecureIgnoreHostKey"]))
assert.Equal(t, strconv.FormatBool(repo.EnableLFS), string(secret.Data["enableLfs"]))
})
t.Run("will return proper error if secret does not have expected label", func(t *testing.T) {
// given
t.Parallel()
secret := &corev1.Secret{}
repositoryToSecret(repo, secret)
delete(secret.Labels, common.LabelKeySecretType)
f := setupWithK8sObjects(secret)
f.clientSet.ReactionChain = nil
f.clientSet.AddReactor("create", "secrets", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
gr := schema.GroupResource{
Group: "v1",
Resource: "secrets",
}
return true, nil, k8serrors.NewAlreadyExists(gr, "already exists")
})

// when
output, err := f.repoBackend.CreateRepository(context.Background(), repo)

// then
assert.Error(t, err)
assert.Nil(t, output)
status, ok := status.FromError(err)
assert.True(t, ok)
assert.Equal(t, codes.InvalidArgument, status.Code())
})
t.Run("will return proper error if secret already exists", func(t *testing.T) {
// given
t.Parallel()
secName := RepoURLToSecretName(repoSecretPrefix, repo.Repo)
secret := &corev1.Secret{
TypeMeta: metav1.TypeMeta{
Kind: "Secret",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: secName,
Namespace: "default",
},
}
repositoryToSecret(repo, secret)
f := setupWithK8sObjects(secret)
f.clientSet.ReactionChain = nil
f.clientSet.WatchReactionChain = nil
f.clientSet.AddReactor("create", "secrets", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
gr := schema.GroupResource{
Group: "v1",
Resource: "secrets",
}
return true, nil, k8serrors.NewAlreadyExists(gr, "already exists")
})
watcher := watch.NewFakeWithChanSize(1, true)
watcher.Add(secret)
f.clientSet.AddWatchReactor("secrets", func(action k8stesting.Action) (handled bool, ret watch.Interface, err error) {
return true, watcher, nil
})

// when
output, err := f.repoBackend.CreateRepository(context.Background(), repo)

// then
assert.Error(t, err)
assert.Nil(t, output)
status, ok := status.FromError(err)
assert.True(t, ok)
assert.Equal(t, codes.AlreadyExists, status.Code())
})
}

func TestSecretsRepositoryBackend_GetRepository(t *testing.T) {
Expand Down

0 comments on commit b0278c4

Please sign in to comment.