diff --git a/api/v1/codebase_types.go b/api/v1/codebase_types.go index 7649139c..72e42556 100644 --- a/api/v1/codebase_types.go +++ b/api/v1/codebase_types.go @@ -1,9 +1,11 @@ package v1 import ( + "fmt" "strconv" "strings" + corev1 "k8s.io/api/core/v1" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -129,6 +131,24 @@ type CodebaseSpec struct { // +optional // +kubebuilder:default:=true Private bool `json:"private"` + + // CloneRepositoryCredentials contains reference to secret with credentials used to clone repository. + // +nullable + // +optional + CloneRepositoryCredentials *CloneRepositoryCredentials `json:"cloneRepositoryCredentials,omitempty"` +} + +type CloneRepositoryCredentials struct { + // SecretRef is a reference to secret that contains credentials for cloning repository. + // The secret must contain "username" and "password" keys. + // +required + SecretRef corev1.LocalObjectReference `json:"secretRef"` + + // ClearSecretAfterUse indicates whether the secret should be deleted after use. + // For backward compatibility, the default value is true. + // +optional + // +kubebuilder:default:=true + ClearSecretAfterUse bool `json:"clearSecretAfterUse"` } // GetProjectID returns project id from GitUrlPath codebase spec. It removes the leading slash. @@ -141,6 +161,16 @@ func (in *CodebaseSpec) IsVersionTypeSemver() bool { return in.Versioning.Type == VersioningTypeSemver || in.Versioning.Type == VersioningTypeEDP } +func (in *Codebase) GetCloneRepositoryCredentialSecret() string { + if in.Spec.CloneRepositoryCredentials != nil && in.Spec.CloneRepositoryCredentials.SecretRef.Name != "" { + return in.Spec.CloneRepositoryCredentials.SecretRef.Name + } + + // Fallback to generate secret name for backward compatibility. + // Deprecated: Fallback for backward compatibility. Will be removed after new field is fully adopted. + return fmt.Sprintf("repository-codebase-%s-temp", in.Name) +} + type ActionType string const ( diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 64177428..a2803ee0 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -103,6 +103,22 @@ func (in *CDStageDeployStatus) DeepCopy() *CDStageDeployStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CloneRepositoryCredentials) DeepCopyInto(out *CloneRepositoryCredentials) { + *out = *in + out.SecretRef = in.SecretRef +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CloneRepositoryCredentials. +func (in *CloneRepositoryCredentials) DeepCopy() *CloneRepositoryCredentials { + if in == nil { + return nil + } + out := new(CloneRepositoryCredentials) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Codebase) DeepCopyInto(out *Codebase) { *out = *in @@ -412,6 +428,11 @@ func (in *CodebaseSpec) DeepCopyInto(out *CodebaseSpec) { *out = new(string) **out = **in } + if in.CloneRepositoryCredentials != nil { + in, out := &in.CloneRepositoryCredentials, &out.CloneRepositoryCredentials + *out = new(CloneRepositoryCredentials) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CodebaseSpec. diff --git a/config/crd/bases/v2.edp.epam.com_codebases.yaml b/config/crd/bases/v2.edp.epam.com_codebases.yaml index c1fea16a..1ad2996b 100644 --- a/config/crd/bases/v2.edp.epam.com_codebases.yaml +++ b/config/crd/bases/v2.edp.epam.com_codebases.yaml @@ -74,6 +74,36 @@ spec: default: tekton description: A name of tool which should be used as CI. type: string + cloneRepositoryCredentials: + description: CloneRepositoryCredentials contains reference to secret + with credentials used to clone repository. + nullable: true + properties: + clearSecretAfterUse: + default: true + description: |- + ClearSecretAfterUse indicates whether the secret should be deleted after use. + For backward compatibility, the default value is true. + type: boolean + secretRef: + description: |- + SecretRef is a reference to secret that contains credentials for cloning repository. + The secret must contain "username" and "password" keys. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + required: + - secretRef + type: object commitMessagePattern: nullable: true type: string diff --git a/controllers/codebase/service/chain/checkout_branch.go b/controllers/codebase/service/chain/checkout_branch.go index f89c2b0c..03fbd184 100644 --- a/controllers/codebase/service/chain/checkout_branch.go +++ b/controllers/codebase/service/chain/checkout_branch.go @@ -4,33 +4,14 @@ import ( "context" "fmt" - k8sErrors "k8s.io/apimachinery/pkg/api/errors" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" + codebaseutil "github.com/epam/edp-codebase-operator/v2/pkg/codebase" gitproviderv2 "github.com/epam/edp-codebase-operator/v2/pkg/git/v2" - "github.com/epam/edp-codebase-operator/v2/pkg/util" ) -func GetRepositoryCredentialsIfExists(cb *codebaseApi.Codebase, c client.Client) (userName, password *string, err error) { - if cb.Spec.Repository == nil { - return nil, nil, nil - } - - secret := fmt.Sprintf("repository-codebase-%v-temp", cb.Name) - - repositoryUsername, repositoryPassword, err := util.GetVcsBasicAuthConfig(c, cb.Namespace, secret) - if err != nil { - return nil, nil, fmt.Errorf("failed to fetch VCS auth config: %w", err) - } - - userName = &repositoryUsername - password = &repositoryPassword - - return -} - func CheckoutBranch( ctx context.Context, branchName string, @@ -59,15 +40,14 @@ func CheckoutBranch( } case "clone": - user, password, err := GetRepositoryCredentialsIfExists(cb, c) - if err != nil && !k8sErrors.IsNotFound(err) { - return err + user, password, _, err := codebaseutil.GetRepositoryCredentialsIfExists(ctx, cb, c) + if err != nil { + return fmt.Errorf("failed to get repository credentials for checkout (clone strategy): %w", err) } - cfg := gitproviderv2.Config{} - if user != nil && password != nil { - cfg.Username = *user - cfg.Token = *password + cfg := gitproviderv2.Config{ + Username: user, + Token: password, } if err := gitProviderFactory(cfg).Checkout(ctx, repoContext.WorkDir, branchName, true); err != nil { diff --git a/controllers/codebase/service/chain/checkout_branch_test.go b/controllers/codebase/service/chain/checkout_branch_test.go index 371be406..6a158f88 100644 --- a/controllers/codebase/service/chain/checkout_branch_test.go +++ b/controllers/codebase/service/chain/checkout_branch_test.go @@ -24,63 +24,6 @@ const ( fakeName = "fake-name" ) -func TestGetRepositoryCredentialsIfExists_ShouldPass(t *testing.T) { - c := &codebaseApi.Codebase{ - ObjectMeta: metaV1.ObjectMeta{ - Name: "fake-name", - Namespace: fakeNamespace, - }, - Spec: codebaseApi.CodebaseSpec{ - Repository: &codebaseApi.Repository{ - Url: "repo", - }, - }, - } - s := &coreV1.Secret{ - ObjectMeta: metaV1.ObjectMeta{ - Name: "repository-codebase-fake-name-temp", - Namespace: fakeNamespace, - }, - Data: map[string][]byte{ - "username": []byte("user"), - "password": []byte("pass"), - }, - } - scheme := runtime.NewScheme() - scheme.AddKnownTypes(coreV1.SchemeGroupVersion, s) - scheme.AddKnownTypes(codebaseApi.GroupVersion, c) - fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(s, c).Build() - u, p, err := GetRepositoryCredentialsIfExists(c, fakeCl) - assert.Equal(t, u, util.GetStringP("user")) - assert.Equal(t, p, util.GetStringP("pass")) - assert.NoError(t, err) -} - -func TestGetRepositoryCredentialsIfExists_ShouldFail(t *testing.T) { - c := &codebaseApi.Codebase{ - ObjectMeta: metaV1.ObjectMeta{ - Name: "fake-name", - Namespace: fakeNamespace, - }, - Spec: codebaseApi.CodebaseSpec{ - Repository: &codebaseApi.Repository{ - Url: "repo", - }, - }, - } - - scheme := runtime.NewScheme() - scheme.AddKnownTypes(codebaseApi.GroupVersion, c) - fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(c).Build() - - _, _, err := GetRepositoryCredentialsIfExists(c, fakeCl) - assert.Error(t, err) - - if !strings.Contains(err.Error(), "failed to get secret repository-codebase-fake-name-temp") { - t.Fatalf("wrong error returned: %s", err.Error()) - } -} - func TestCheckoutBranch_ShouldFailOnGetCurrentBranchName(t *testing.T) { c := &codebaseApi.Codebase{ ObjectMeta: metaV1.ObjectMeta{ @@ -236,6 +179,11 @@ func TestCheckoutBranch_ShouldPassForCloneStrategy(t *testing.T) { Url: "repo", }, Strategy: codebaseApi.Import, + CloneRepositoryCredentials: &codebaseApi.CloneRepositoryCredentials{ + SecretRef: coreV1.LocalObjectReference{ + Name: "repository-creds", + }, + }, }, } gs := &codebaseApi.GitServer{ @@ -252,7 +200,7 @@ func TestCheckoutBranch_ShouldPassForCloneStrategy(t *testing.T) { } s := &coreV1.Secret{ ObjectMeta: metaV1.ObjectMeta{ - Name: "repository-codebase-fake-name-temp", + Name: "repository-creds", Namespace: fakeNamespace, }, Data: map[string][]byte{ diff --git a/controllers/codebase/service/chain/cleaner.go b/controllers/codebase/service/chain/cleaner.go index b371fca3..4c550dfb 100644 --- a/controllers/codebase/service/chain/cleaner.go +++ b/controllers/codebase/service/chain/cleaner.go @@ -2,6 +2,7 @@ package chain import ( "context" + "errors" "fmt" v1 "k8s.io/api/core/v1" @@ -39,39 +40,46 @@ func (h *Cleaner) ServeRequest(ctx context.Context, codebase *codebaseApi.Codeba } func (h *Cleaner) clean(ctx context.Context, codebase *codebaseApi.Codebase) error { - log := ctrl.LoggerFrom(ctx) - - s := fmt.Sprintf("repository-codebase-%v-temp", codebase.Name) + var errs []error - if err := h.deleteSecret(ctx, s, codebase.Namespace); err != nil { - return fmt.Errorf("failed to delete secret %v: %w", s, err) + if err := h.deleteSecret(ctx, codebase); err != nil { + errs = append(errs, err) } wd := util.GetWorkDir(codebase.Name, codebase.Namespace) - log.Info("Deleting work directory", "directory", wd) + ctrl.LoggerFrom(ctx).Info("Deleting work directory", "directory", wd) + + if err := deleteWorkDirectory(wd); err != nil { + errs = append(errs, err) + } - return deleteWorkDirectory(wd) + return errors.Join(errs...) } -func (h *Cleaner) deleteSecret(ctx context.Context, secretName, namespace string) error { +func (h *Cleaner) deleteSecret(ctx context.Context, codebase *codebaseApi.Codebase) error { + if codebase.Spec.CloneRepositoryCredentials != nil && !codebase.Spec.CloneRepositoryCredentials.ClearSecretAfterUse { + return nil + } + + secretName := codebase.GetCloneRepositoryCredentialSecret() log := ctrl.LoggerFrom(ctx).WithValues("secret", secretName) - log.Info("Deleting secret") + log.Info("Deleting secret with repository credentials for clone") if err := h.client.Delete(ctx, &v1.Secret{ ObjectMeta: metaV1.ObjectMeta{ Name: secretName, - Namespace: namespace, + Namespace: codebase.Namespace, }, }); err != nil { if k8sErrors.IsNotFound(err) { - log.Info("Secret is not found. Skip deleting") + log.Info("Secret not found. Skipping deletion") return nil } - return fmt.Errorf("failed to Delete 'Secret' resource %q: %w", secretName, err) + return fmt.Errorf("failed to delete secret with repository credentials for clone: %w", err) } log.Info("Secret was deleted") diff --git a/controllers/codebase/service/chain/cleaner_test.go b/controllers/codebase/service/chain/cleaner_test.go index 11062bb0..aa9bcce7 100644 --- a/controllers/codebase/service/chain/cleaner_test.go +++ b/controllers/codebase/service/chain/cleaner_test.go @@ -10,111 +10,269 @@ import ( coreV1 "k8s.io/api/core/v1" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" + "github.com/epam/edp-codebase-operator/v2/pkg/util" ) -func TestCleaner_ShouldPass(t *testing.T) { - ctx := context.Background() - - dir, err := os.MkdirTemp("/tmp", "codebase") - require.NoError(t, err, "failed to create temp directory for testing") +func TestCleaner_ServeRequest(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, coreV1.AddToScheme(scheme)) + require.NoError(t, codebaseApi.AddToScheme(scheme)) - defer func() { - err = os.RemoveAll(dir) - require.NoError(t, err) - }() + secretName := "test-secret" - t.Setenv("WORKING_DIR", dir) + tests := []struct { + name string + codebase *codebaseApi.Codebase + prepare func(t *testing.T, testDir string) + wantErr require.ErrorAssertionFunc + objects []client.Object + verify func(t *testing.T, cl client.Client, testDir string) + }{ + { + name: "successfully deletes secret and work directory when ClearSecretAfterUse is true", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: fakeName, + Namespace: fakeNamespace, + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: &codebaseApi.CloneRepositoryCredentials{ + SecretRef: coreV1.LocalObjectReference{ + Name: secretName, + }, + ClearSecretAfterUse: true, + }, + }, + }, + prepare: prepareWorkDir, + wantErr: require.NoError, + objects: []client.Object{ + &coreV1.Secret{ + ObjectMeta: metaV1.ObjectMeta{ + Name: secretName, + Namespace: fakeNamespace, + }, + }, + }, + verify: func(t *testing.T, cl client.Client, testDir string) { + // Secret should be deleted + secret := &coreV1.Secret{} + err := cl.Get(context.Background(), client.ObjectKey{Name: secretName, Namespace: fakeNamespace}, secret) + assert.Error(t, err, "secret should be deleted") - c := &codebaseApi.Codebase{ - ObjectMeta: metaV1.ObjectMeta{ - Name: fakeName, - Namespace: fakeNamespace, + verifyWorkDirDeleted(t) + }, }, - } - ssh := &coreV1.Secret{ - ObjectMeta: metaV1.ObjectMeta{ - Name: "repository-codebase-fake-name-temp", - Namespace: fakeNamespace, + { + name: "successfully completes when secret not found but ClearSecretAfterUse is true", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: fakeName, + Namespace: fakeNamespace, + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: &codebaseApi.CloneRepositoryCredentials{ + SecretRef: coreV1.LocalObjectReference{ + Name: secretName, + }, + ClearSecretAfterUse: true, + }, + }, + }, + prepare: prepareWorkDir, + wantErr: require.NoError, + objects: []client.Object{}, + verify: func(t *testing.T, cl client.Client, testDir string) { + verifyWorkDirDeleted(t) + }, }, - } - - scheme := runtime.NewScheme() - scheme.AddKnownTypes(coreV1.SchemeGroupVersion, ssh) - scheme.AddKnownTypes(codebaseApi.GroupVersion, c) - - fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(c, ssh).Build() + { + name: "deletes fallback secret when CloneRepositoryCredentials is nil", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: fakeName, + Namespace: fakeNamespace, + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: nil, + }, + }, + prepare: prepareWorkDir, + wantErr: require.NoError, + objects: []client.Object{ + &coreV1.Secret{ + ObjectMeta: metaV1.ObjectMeta{ + // Fallback secret should be deleted even when CloneRepositoryCredentials is nil + Name: "repository-codebase-fake-name-temp", + Namespace: fakeNamespace, + }, + }, + }, + verify: func(t *testing.T, cl client.Client, testDir string) { + // Fallback secret should be deleted + secret := &coreV1.Secret{} + err := cl.Get(context.Background(), client.ObjectKey{ + Name: "repository-codebase-fake-name-temp", + Namespace: fakeNamespace, + }, secret) + assert.Error(t, err, "fallback secret should be deleted") - cl := NewCleaner(fakeCl) - - err = cl.ServeRequest(ctx, c) - assert.NoError(t, err) -} - -func TestCleaner_ShouldNotFailedIfSecretNotFound(t *testing.T) { - ctx := context.Background() - - dir, err := os.MkdirTemp("/tmp", "codebase") - require.NoError(t, err, "failed to create temp directory for testing") - - defer func() { - err = os.RemoveAll(dir) - require.NoError(t, err) - }() + verifyWorkDirDeleted(t) + }, + }, + { + name: "skips secret deletion when ClearSecretAfterUse is false", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: fakeName, + Namespace: fakeNamespace, + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: &codebaseApi.CloneRepositoryCredentials{ + SecretRef: coreV1.LocalObjectReference{ + Name: secretName, + }, + ClearSecretAfterUse: false, + }, + }, + }, + prepare: prepareWorkDir, + wantErr: require.NoError, + objects: []client.Object{ + &coreV1.Secret{ + ObjectMeta: metaV1.ObjectMeta{ + Name: secretName, + Namespace: fakeNamespace, + }, + }, + }, + verify: func(t *testing.T, cl client.Client, testDir string) { + // Secret should still exist + secret := &coreV1.Secret{} + err := cl.Get(context.Background(), client.ObjectKey{Name: secretName, Namespace: fakeNamespace}, secret) + assert.NoError(t, err, "secret should still exist") - t.Setenv("WORKING_DIR", dir) + verifyWorkDirDeleted(t) + }, + }, + { + name: "uses GetCloneRepositoryCredentialSecret to get secret name", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: fakeName, + Namespace: fakeNamespace, + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: &codebaseApi.CloneRepositoryCredentials{ + SecretRef: coreV1.LocalObjectReference{ + Name: "", // Empty name should trigger fallback + }, + ClearSecretAfterUse: true, + }, + }, + }, + prepare: prepareWorkDir, + wantErr: require.NoError, + objects: []client.Object{ + &coreV1.Secret{ + ObjectMeta: metaV1.ObjectMeta{ + // Fallback secret name generated by GetCloneRepositoryCredentialSecret + Name: "repository-codebase-fake-name-temp", + Namespace: fakeNamespace, + }, + }, + }, + verify: func(t *testing.T, cl client.Client, testDir string) { + // Fallback secret should be deleted + secret := &coreV1.Secret{} + err := cl.Get(context.Background(), client.ObjectKey{ + Name: "repository-codebase-fake-name-temp", + Namespace: fakeNamespace, + }, secret) + assert.Error(t, err, "fallback secret should be deleted") - c := &codebaseApi.Codebase{ - ObjectMeta: metaV1.ObjectMeta{ - Name: fakeName, - Namespace: fakeNamespace, + verifyWorkDirDeleted(t) + }, + }, + { + name: "successfully completes when CloneRepositoryCredentials is nil and fallback secret not found", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: fakeName, + Namespace: fakeNamespace, + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: nil, + }, + }, + prepare: prepareWorkDir, + wantErr: require.NoError, + objects: []client.Object{}, + verify: func(t *testing.T, cl client.Client, testDir string) { + verifyWorkDirDeleted(t) + }, + }, + { + name: "deletes work directory even when work directory doesn't exist", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: fakeName, + Namespace: fakeNamespace, + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: nil, + }, + }, + prepare: func(t *testing.T, testDir string) { + t.Helper() + t.Setenv(util.WorkDirEnv, testDir) + // Don't create work directory - it doesn't exist + }, + wantErr: require.NoError, + objects: []client.Object{}, + verify: func(t *testing.T, cl client.Client, testDir string) { + verifyWorkDirDeleted(t) + }, }, } - ssh := &coreV1.Secret{} - - scheme := runtime.NewScheme() - scheme.AddKnownTypes(coreV1.SchemeGroupVersion, ssh) - scheme.AddKnownTypes(codebaseApi.GroupVersion, c) - - fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(c, ssh).Build() - - cl := NewCleaner(fakeCl) - - err = cl.ServeRequest(ctx, c) - assert.NoError(t, err) -} -func TestCleaner_ShouldFail(t *testing.T) { - ctx := context.Background() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testDir := t.TempDir() + tt.prepare(t, testDir) - dir, err := os.MkdirTemp("/tmp", "codebase") - require.NoError(t, err, "failed to create temp directory for testing") + fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objects...).Build() + cl := NewCleaner(fakeCl) - defer func() { - err = os.RemoveAll(dir) - require.NoError(t, err) - }() + err := cl.ServeRequest(context.Background(), tt.codebase) + tt.wantErr(t, err) - t.Setenv("WORKING_DIR", dir) - - c := &codebaseApi.Codebase{ - ObjectMeta: metaV1.ObjectMeta{ - Name: fakeName, - Namespace: fakeNamespace, - }, + if tt.verify != nil { + tt.verify(t, fakeCl, testDir) + } + }) } +} - scheme := runtime.NewScheme() - require.NoError(t, codebaseApi.AddToScheme(scheme)) - - fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(c).Build() +func prepareWorkDir(t *testing.T, testDir string) { + t.Helper() + t.Setenv(util.WorkDirEnv, testDir) - cl := NewCleaner(fakeCl) + // Create work directory structure + workDir := util.GetWorkDir(fakeName, fakeNamespace) + require.NoError(t, os.MkdirAll(workDir, 0755)) +} - err = cl.ServeRequest(ctx, c) +func verifyWorkDirDeleted(t *testing.T) { + t.Helper() - require.Error(t, err) - assert.Contains(t, err.Error(), "failed to delete secret repository-codebase-fake-name-temp") + // Work directory should be deleted + // Note: util.GetWorkDir() uses the WORKING_DIR env var set by prepareWorkDir + workDir := util.GetWorkDir(fakeName, fakeNamespace) + _, err := os.Stat(workDir) + require.ErrorIs(t, err, os.ErrNotExist, "work directory should be deleted") } diff --git a/controllers/codebase/service/chain/put_project.go b/controllers/codebase/service/chain/put_project.go index 5846da8d..7eeb027a 100644 --- a/controllers/codebase/service/chain/put_project.go +++ b/controllers/codebase/service/chain/put_project.go @@ -8,12 +8,12 @@ import ( "slices" "strconv" - k8sErrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" + codebaseutil "github.com/epam/edp-codebase-operator/v2/pkg/codebase" "github.com/epam/edp-codebase-operator/v2/pkg/gerrit" gitproviderv2 "github.com/epam/edp-codebase-operator/v2/pkg/git/v2" "github.com/epam/edp-codebase-operator/v2/pkg/gitprovider" @@ -384,7 +384,7 @@ func (h *PutProject) setDefaultBranch( func (h *PutProject) tryToCloneRepo( ctx context.Context, repoUrl string, - repositoryUsername, repositoryPassword *string, + repositoryUsername, repositoryPassword string, repoContext *GitRepositoryContext, ) error { log := ctrl.LoggerFrom(ctx).WithValues("dest", repoContext.WorkDir, "repoUrl", repoUrl) @@ -397,15 +397,10 @@ func (h *PutProject) tryToCloneRepo( return nil } - config := gitproviderv2.Config{} - - if repositoryUsername != nil && repositoryPassword != nil { - config = gitproviderv2.Config{} - config.Username = *repositoryUsername - config.Token = *repositoryPassword - } - - gitProvider := h.gitProviderFactory(config) + gitProvider := h.gitProviderFactory(gitproviderv2.Config{ + Username: repositoryUsername, + Token: repositoryPassword, + }) if err := gitProvider.Clone(ctx, repoUrl, repoContext.WorkDir); err != nil { return fmt.Errorf("failed to clone repository: %w", err) } @@ -482,22 +477,19 @@ func (h *PutProject) notEmptyProjectProvisioning(ctx context.Context, codebase * return fmt.Errorf("failed to build repo url: %w", err) } - repu, repp, err := GetRepositoryCredentialsIfExists(codebase, h.k8sClient) - // we are ok if no credentials is found, assuming this is a public repo - if err != nil && !k8sErrors.IsNotFound(err) { - return fmt.Errorf("failed to get repository credentials: %w", err) + repu, repp, exists, err := codebaseutil.GetRepositoryCredentialsIfExists(ctx, codebase, h.k8sClient) + if err != nil { + return fmt.Errorf("failed to get repository credentials for project provisioning: %w", err) } - // Check permissions if credentials exist - if repu != nil && repp != nil { - tempConfig := gitproviderv2.Config{ - Username: *repu, - Token: *repp, - } - tempProvider := h.gitProviderFactory(tempConfig) + if exists { + cloneGitProvider := h.gitProviderFactory(gitproviderv2.Config{ + Username: repu, + Token: repp, + }) - if err := tempProvider.CheckPermissions(ctx, repoUrl); err != nil { - return fmt.Errorf("failed to get access to the repository %v for user %v: %w", repoUrl, *repu, err) + if err := cloneGitProvider.CheckPermissions(ctx, repoUrl); err != nil { + return fmt.Errorf("failed to get access to the repository %s for user %s: %w", repoUrl, repu, err) } } diff --git a/deploy-templates/crds/v2.edp.epam.com_codebases.yaml b/deploy-templates/crds/v2.edp.epam.com_codebases.yaml index c1fea16a..1ad2996b 100644 --- a/deploy-templates/crds/v2.edp.epam.com_codebases.yaml +++ b/deploy-templates/crds/v2.edp.epam.com_codebases.yaml @@ -74,6 +74,36 @@ spec: default: tekton description: A name of tool which should be used as CI. type: string + cloneRepositoryCredentials: + description: CloneRepositoryCredentials contains reference to secret + with credentials used to clone repository. + nullable: true + properties: + clearSecretAfterUse: + default: true + description: |- + ClearSecretAfterUse indicates whether the secret should be deleted after use. + For backward compatibility, the default value is true. + type: boolean + secretRef: + description: |- + SecretRef is a reference to secret that contains credentials for cloning repository. + The secret must contain "username" and "password" keys. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + required: + - secretRef + type: object commitMessagePattern: nullable: true type: string diff --git a/docs/api.md b/docs/api.md index 5b0f3bdb..ed508a66 100644 --- a/docs/api.md +++ b/docs/api.md @@ -803,6 +803,13 @@ Selected branch will become a default branch for a new codebase (e.g. master, ma Default: tekton
false + + cloneRepositoryCredentials + object + + CloneRepositoryCredentials contains reference to secret with credentials used to clone repository.
+ + false commitMessagePattern string @@ -915,6 +922,78 @@ Selected branch will become a default branch for a new codebase (e.g. master, ma +### Codebase.spec.cloneRepositoryCredentials +[↩ Parent](#codebasespec) + + + +CloneRepositoryCredentials contains reference to secret with credentials used to clone repository. + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
secretRefobject + SecretRef is a reference to secret that contains credentials for cloning repository. +The secret must contain "username" and "password" keys.
+
true
clearSecretAfterUseboolean + ClearSecretAfterUse indicates whether the secret should be deleted after use. +For backward compatibility, the default value is true.
+
+ Default: true
+
false
+ + +### Codebase.spec.cloneRepositoryCredentials.secretRef +[↩ Parent](#codebasespecclonerepositorycredentials) + + + +SecretRef is a reference to secret that contains credentials for cloning repository. +The secret must contain "username" and "password" keys. + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
namestring + Name of the referent. +This field is effectively required, but due to backwards compatibility is +allowed to be empty. Instances of this type with an empty value here are +almost certainly wrong. +More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+
+ Default:
+
false
+ + ### Codebase.spec.repository [↩ Parent](#codebasespec) diff --git a/pkg/codebase/creds.go b/pkg/codebase/creds.go new file mode 100644 index 00000000..fa446b43 --- /dev/null +++ b/pkg/codebase/creds.go @@ -0,0 +1,45 @@ +package codebase + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" +) + +// GetRepositoryCredentialsIfExists retrieves the repository credentials from the secret +// referenced in the codebase's CloneRepositoryCredentials field. +func GetRepositoryCredentialsIfExists( + ctx context.Context, + codebase *codebaseApi.Codebase, + k8sClient client.Client, +) (userName, password string, exists bool, err error) { + secret := &corev1.Secret{} + + err = k8sClient.Get(ctx, types.NamespacedName{ + Namespace: codebase.Namespace, + Name: codebase.GetCloneRepositoryCredentialSecret(), + }, secret) + if err != nil { + if k8serrors.IsNotFound(err) { + return "", "", false, nil + } + + return "", "", false, fmt.Errorf("failed to get secret with clone repository credentials: %w", err) + } + + if len(secret.Data["username"]) == 0 { + return "", "", false, fmt.Errorf("username key is not defined in secret %s", secret.Name) + } + + if len(secret.Data["password"]) == 0 { + return "", "", false, fmt.Errorf("password key is not defined in secret %s", secret.Name) + } + + return string(secret.Data["username"]), string(secret.Data["password"]), true, nil +} diff --git a/pkg/codebase/creds_test.go b/pkg/codebase/creds_test.go new file mode 100644 index 00000000..08574969 --- /dev/null +++ b/pkg/codebase/creds_test.go @@ -0,0 +1,275 @@ +package codebase_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" + "github.com/epam/edp-codebase-operator/v2/pkg/codebase" +) + +func TestGetRepositoryCredentialsIfExists(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, codebaseApi.AddToScheme(scheme)) + + tests := []struct { + name string + codebase *codebaseApi.Codebase + objects []runtime.Object + wantUser string + wantPass string + exists bool + wantErr require.ErrorAssertionFunc + }{ + { + name: "credentials exist and are valid", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-codebase", + Namespace: "test-namespace", + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: &codebaseApi.CloneRepositoryCredentials{ + SecretRef: corev1.LocalObjectReference{ + Name: "test-secret", + }, + }, + }, + }, + objects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + }, + Data: map[string][]byte{ + "username": []byte("test-user"), + "password": []byte("test-pass"), + }, + }, + }, + wantUser: "test-user", + wantPass: "test-pass", + exists: true, + wantErr: require.NoError, + }, + { + name: "cloneRepositoryCredentials is nil - uses fallback secret name", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-codebase", + Namespace: "test-namespace", + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: nil, + }, + }, + objects: []runtime.Object{}, + wantUser: "", + wantPass: "", + exists: false, + wantErr: require.NoError, + }, + { + name: "secret name is empty - uses fallback secret name", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-codebase", + Namespace: "test-namespace", + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: &codebaseApi.CloneRepositoryCredentials{ + SecretRef: corev1.LocalObjectReference{ + Name: "", + }, + }, + }, + }, + objects: []runtime.Object{}, + wantUser: "", + wantPass: "", + exists: false, + wantErr: require.NoError, + }, + { + name: "secret does not exist (NotFound)", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-codebase", + Namespace: "test-namespace", + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: &codebaseApi.CloneRepositoryCredentials{ + SecretRef: corev1.LocalObjectReference{ + Name: "non-existent-secret", + }, + }, + }, + }, + objects: []runtime.Object{}, + wantUser: "", + wantPass: "", + exists: false, + wantErr: require.NoError, + }, + { + name: "username is missing in secret", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-codebase", + Namespace: "test-namespace", + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: &codebaseApi.CloneRepositoryCredentials{ + SecretRef: corev1.LocalObjectReference{ + Name: "test-secret", + }, + }, + }, + }, + objects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + }, + Data: map[string][]byte{ + "password": []byte("test-pass"), + }, + }, + }, + wantUser: "", + wantPass: "", + exists: false, + wantErr: func(t require.TestingT, err error, msgAndArgs ...any) { + require.ErrorContains(t, err, "username key is not defined in secret") + }, + }, + { + name: "username is empty in secret", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-codebase", + Namespace: "test-namespace", + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: &codebaseApi.CloneRepositoryCredentials{ + SecretRef: corev1.LocalObjectReference{ + Name: "test-secret", + }, + }, + }, + }, + objects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + }, + Data: map[string][]byte{ + "username": []byte(""), + "password": []byte("test-pass"), + }, + }, + }, + wantUser: "", + wantPass: "", + exists: false, + wantErr: func(t require.TestingT, err error, msgAndArgs ...any) { + require.ErrorContains(t, err, "username key is not defined in secret") + }, + }, + { + name: "password is missing in secret", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-codebase", + Namespace: "test-namespace", + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: &codebaseApi.CloneRepositoryCredentials{ + SecretRef: corev1.LocalObjectReference{ + Name: "test-secret", + }, + }, + }, + }, + objects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + }, + Data: map[string][]byte{ + "username": []byte("test-user"), + }, + }, + }, + wantUser: "", + wantPass: "", + exists: false, + wantErr: func(t require.TestingT, err error, msgAndArgs ...any) { + require.ErrorContains(t, err, "password key is not defined in secret") + }, + }, + { + name: "password is empty in secret", + codebase: &codebaseApi.Codebase{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-codebase", + Namespace: "test-namespace", + }, + Spec: codebaseApi.CodebaseSpec{ + CloneRepositoryCredentials: &codebaseApi.CloneRepositoryCredentials{ + SecretRef: corev1.LocalObjectReference{ + Name: "test-secret", + }, + }, + }, + }, + objects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + }, + Data: map[string][]byte{ + "username": []byte("test-user"), + "password": []byte(""), + }, + }, + }, + wantUser: "", + wantPass: "", + exists: false, + wantErr: func(t require.TestingT, err error, msgAndArgs ...any) { + require.ErrorContains(t, err, "password key is not defined in secret") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tt.objects...).Build() + + gotUser, gotPass, gotExists, err := codebase.GetRepositoryCredentialsIfExists( + context.Background(), + tt.codebase, + fakeClient, + ) + + tt.wantErr(t, err) + assert.Equal(t, tt.wantUser, gotUser) + assert.Equal(t, tt.wantPass, gotPass) + assert.Equal(t, tt.exists, gotExists) + }) + } +} diff --git a/pkg/util/helper.go b/pkg/util/helper.go index 48743f59..316c7873 100644 --- a/pkg/util/helper.go +++ b/pkg/util/helper.go @@ -15,7 +15,7 @@ func GetStringP(val string) *string { } func GetWorkDir(codebaseName, namespace string) string { - value, ok := os.LookupEnv("WORKING_DIR") + value, ok := os.LookupEnv(WorkDirEnv) if !ok { return fmt.Sprintf("/home/codebase-operator/edp/%v/%v/%v/%v", namespace, codebaseName, "templates", codebaseName) } diff --git a/pkg/util/platform.go b/pkg/util/platform.go index d42504b6..8890672a 100644 --- a/pkg/util/platform.go +++ b/pkg/util/platform.go @@ -20,29 +20,6 @@ const ( debugModeEnvVar = "DEBUG_MODE" ) -func GetVcsBasicAuthConfig(c client.Client, namespace, secretName string) (userName, password string, err error) { - log.Info("Start getting secret", "name", secretName) - - secret := &coreV1.Secret{} - - err = c.Get(context.TODO(), types.NamespacedName{ - Namespace: namespace, - Name: secretName, - }, secret) - if err != nil { - return "", "", fmt.Errorf("failed to get secret %v: %w", secretName, err) - } - - if len(secret.Data["username"]) == 0 || len(secret.Data["password"]) == 0 { - return "", "", fmt.Errorf("username/password keys are not defined in Secret %v ", secretName) - } - - userName = string(secret.Data["username"]) - password = string(secret.Data["password"]) - - return -} - func GetGitServer(c client.Client, name, namespace string) (*model.GitServer, error) { gitReq, err := getGitServerCR(c, name, namespace) if err != nil { diff --git a/pkg/util/platform_test.go b/pkg/util/platform_test.go index 578a61de..6e4894d4 100644 --- a/pkg/util/platform_test.go +++ b/pkg/util/platform_test.go @@ -57,73 +57,6 @@ func TestGetSecret_ShouldFail(t *testing.T) { } } -func TestGetVcsBasicAuthConfig_ShouldPass(t *testing.T) { - secret := &coreV1.Secret{ - ObjectMeta: metaV1.ObjectMeta{ - Name: "stub-name", - Namespace: "stub-namespace", - }, - Data: map[string][]byte{ - "username": []byte("user"), - "password": []byte("pass"), - }, - } - scheme := runtime.NewScheme() - scheme.AddKnownTypes(coreV1.SchemeGroupVersion, secret) - fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(secret).Build() - - u, p, err := GetVcsBasicAuthConfig(fakeCl, "stub-namespace", "stub-name") - assert.Equal(t, u, "user") - assert.Equal(t, p, "pass") - assert.NoError(t, err) -} - -func TestGetVcsBasicAuthConfig_ShouldFail(t *testing.T) { - s := &coreV1.Secret{ - ObjectMeta: metaV1.ObjectMeta{ - Name: "stub-name", - Namespace: "stub-namespace", - }, - } - - scheme := runtime.NewScheme() - scheme.AddKnownTypes(coreV1.SchemeGroupVersion, s) - fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(s).Build() - - u, p, err := GetVcsBasicAuthConfig(fakeCl, "stub-namespace", "non-existing-stub-name") - assert.Equal(t, u, "") - assert.Equal(t, p, "") - assert.Error(t, err) - - if !strings.Contains(err.Error(), "secrets \"non-existing-stub-name\" not found") { - t.Fatalf("wrong error returned: %s", err.Error()) - } -} - -func TestGetVcsBasicAuthConfig_ShouldFailIfUsernameOrPasswordIsNotDefined(t *testing.T) { - secret := &coreV1.Secret{ - ObjectMeta: metaV1.ObjectMeta{ - Name: "stub-name", - Namespace: "stub-namespace", - }, - Data: map[string][]byte{ - "username": []byte("user"), - }, - } - scheme := runtime.NewScheme() - scheme.AddKnownTypes(coreV1.SchemeGroupVersion, secret) - fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(secret).Build() - - u, p, err := GetVcsBasicAuthConfig(fakeCl, "stub-namespace", "stub-name") - assert.Equal(t, u, "") - assert.Equal(t, p, "") - assert.Error(t, err) - - if !strings.Contains(err.Error(), "username/password keys are not defined in Secret stub-name") { - t.Fatalf("wrong error returned: %s", err.Error()) - } -} - func TestGetGitServer_ShouldPass(t *testing.T) { gs := &codebaseApi.GitServer{ ObjectMeta: metaV1.ObjectMeta{