address comments

cmd/gitness/wire_gen.go

@@ -91,14 +91,14 @@ func initSystem(ctx context.Context, config *types.Config) (*server.System, erro
 	executionStore := database.ProvideExecutionStore(db)
 	pipelineStore := database.ProvidePipelineStore(db)
 	executionController := execution.ProvideController(db, authorizer, executionStore, pipelineStore, spaceStore)
+	secretStore := database.ProvideSecretStore(db)
+	spaceController := space.ProvideController(db, provider, pathUID, authorizer, pathStore, pipelineStore, secretStore, spaceStore, repoStore, principalStore, repoController, membershipStore)
+	pipelineController := pipeline.ProvideController(db, pathUID, pathStore, repoStore, authorizer, pipelineStore, spaceStore)
 	encrypter, err := database.ProvideEncryptor(databaseConfig)
 	if err != nil {
 		return nil, err
 	}
-	secretStore := database.ProvideSecretStore(encrypter, db)
+	secretController := secret.ProvideController(db, pathUID, pathStore, encrypter, secretStore, authorizer, spaceStore)
-	spaceController := space.ProvideController(db, provider, pathUID, authorizer, pathStore, pipelineStore, secretStore, spaceStore, repoStore, principalStore, repoController, membershipStore)
-	pipelineController := pipeline.ProvideController(db, pathUID, pathStore, repoStore, authorizer, pipelineStore, spaceStore)
-	secretController := secret.ProvideController(db, pathUID, pathStore, secretStore, authorizer, spaceStore)
 	pullReqStore := database.ProvidePullReqStore(db, principalInfoCache)
 	pullReqActivityStore := database.ProvidePullReqActivityStore(db, principalInfoCache)
 	codeCommentView := database.ProvideCodeCommentView(db)

internal/api/controller/secret/controller.go

@@ -5,6 +5,7 @@
 package secret
 
 import (
+	"github.com/harness/gitness/encrypt"
 	"github.com/harness/gitness/internal/auth/authz"
 	"github.com/harness/gitness/internal/store"
 	"github.com/harness/gitness/types/check"
@@ -16,6 +17,7 @@ type Controller struct {
 	db          *sqlx.DB
 	uidCheck    check.PathUID
 	pathStore   store.PathStore
+	encrypter   encrypt.Encrypter
 	secretStore store.SecretStore
 	authorizer  authz.Authorizer
 	spaceStore  store.SpaceStore
@@ -26,6 +28,7 @@ func NewController(
 	uidCheck check.PathUID,
 	authorizer authz.Authorizer,
 	pathStore store.PathStore,
+	encrypter encrypt.Encrypter,
 	secretStore store.SecretStore,
 	spaceStore store.SpaceStore,
 ) *Controller {
@@ -33,6 +36,7 @@ func NewController(
 		db:          db,
 		uidCheck:    uidCheck,
 		pathStore:   pathStore,
+		encrypter:   encrypter,
 		secretStore: secretStore,
 		authorizer:  authorizer,
 		spaceStore:  spaceStore,

internal/api/controller/secret/create.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/harness/gitness/encrypt"
 	apiauth "github.com/harness/gitness/internal/api/auth"
 	"github.com/harness/gitness/internal/api/usererror"
 	"github.com/harness/gitness/internal/auth"
@@ -66,6 +67,10 @@ func (c *Controller) Create(ctx context.Context, session *auth.Session, in *Crea
 			Updated:     now,
 			Version:     0,
 		}
+		secret, err = enc(c.encrypter, secret)
+		if err != nil {
+			return fmt.Errorf("could not encrypt secret: %w", err)
+		}
 		err = c.secretStore.Create(ctx, secret)
 		if err != nil {
 			return fmt.Errorf("secret creation failed: %w", err)
@@ -97,3 +102,31 @@ func (c *Controller) sanitizeCreateInput(in *CreateInput) error {
 
 	return nil
 }
+
+// helper function returns the same secret with encrypted data.
+func enc(encrypt encrypt.Encrypter, secret *types.Secret) (*types.Secret, error) {
+	if secret == nil {
+		return nil, fmt.Errorf("cannot encrypt a nil secret")
+	}
+	s := *secret
+	ciphertext, err := encrypt.Encrypt(secret.Data)
+	if err != nil {
+		return nil, err
+	}
+	s.Data = string(ciphertext)
+	return &s, nil
+}
+
+// helper function returns the same secret with decrypted data.
+func dec(encrypt encrypt.Encrypter, secret *types.Secret) (*types.Secret, error) {
+	if secret == nil {
+		return nil, fmt.Errorf("cannot decrypt a nil secret")
+	}
+	s := *secret
+	plaintext, err := encrypt.Decrypt([]byte(secret.Data))
+	if err != nil {
+		return nil, err
+	}
+	s.Data = plaintext
+	return &s, nil
+}

internal/api/controller/secret/delete.go

@@ -16,12 +16,12 @@ import (
 func (c *Controller) Delete(ctx context.Context, session *auth.Session, spaceRef string, uid string) error {
 	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
 	if err != nil {
-		return err
+		return fmt.Errorf("could not find space: %w", err)
 	}
 
 	err = apiauth.CheckSecret(ctx, c.authorizer, session, space.Path, uid, enum.PermissionSecretDelete)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to authorize: %w", err)
 	}
 	err = c.secretStore.DeleteByUID(ctx, space.ID, uid)
 	if err != nil {

internal/api/controller/secret/find.go

@@ -6,6 +6,7 @@ package secret
 
 import (
 	"context"
+	"fmt"
 
 	apiauth "github.com/harness/gitness/internal/api/auth"
 	"github.com/harness/gitness/internal/auth"
@@ -21,11 +22,19 @@ func (c *Controller) Find(
 ) (*types.Secret, error) {
 	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("could not find space: %w", err)
 	}
 	err = apiauth.CheckSecret(ctx, c.authorizer, session, space.Path, uid, enum.PermissionSecretView)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to authorize: %w", err)
 	}
-	return c.secretStore.FindByUID(ctx, space.ID, uid)
+	secret, err := c.secretStore.FindByUID(ctx, space.ID, uid)
+	if err != nil {
+		return nil, fmt.Errorf("could not find secret: %w", err)
+	}
+	secret, err = dec(c.encrypter, secret)
+	if err != nil {
+		return nil, fmt.Errorf("could not decrypt secret: %w", err)
+	}
+	return secret, nil
 }

internal/api/controller/secret/update.go

@@ -6,6 +6,7 @@ package secret
 
 import (
 	"context"
+	"fmt"
 
 	apiauth "github.com/harness/gitness/internal/api/auth"
 	"github.com/harness/gitness/internal/auth"
@@ -25,20 +26,21 @@ func (c *Controller) Update(
 	session *auth.Session,
 	spaceRef string,
 	uid string,
-	in *UpdateInput) (*types.Secret, error) {
+	in *UpdateInput,
+) (*types.Secret, error) {
 	space, err := c.spaceStore.FindByRef(ctx, spaceRef)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("could not find space: %w", err)
 	}
 
 	err = apiauth.CheckSecret(ctx, c.authorizer, session, space.Path, uid, enum.PermissionSecretEdit)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to authorize: %w", err)
 	}
 
 	secret, err := c.secretStore.FindByUID(ctx, space.ID, uid)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("could not find secret: %w", err)
 	}
 
 	return c.secretStore.UpdateOptLock(ctx, secret, func(original *types.Secret) error {
@@ -46,7 +48,11 @@ func (c *Controller) Update(
 			original.Description = in.Description
 		}
 		if in.Data != "" {
-			original.Data = in.Data // will get encrypted at db layer
+			data, err := c.encrypter.Encrypt(original.Data)
+			if err != nil {
+				return err
+			}
+			original.Data = string(data)
 		}
 		if in.UID != "" {
 			original.UID = in.UID

internal/api/controller/secret/wire.go

@@ -5,6 +5,7 @@
 package secret
 
 import (
+	"github.com/harness/gitness/encrypt"
 	"github.com/harness/gitness/internal/auth/authz"
 	"github.com/harness/gitness/internal/store"
 	"github.com/harness/gitness/types/check"
@@ -21,9 +22,10 @@ var WireSet = wire.NewSet(
 func ProvideController(db *sqlx.DB,
 	uidCheck check.PathUID,
 	pathStore store.PathStore,
+	encrypter encrypt.Encrypter,
 	secretStore store.SecretStore,
 	authorizer authz.Authorizer,
 	spaceStore store.SpaceStore,
 ) *Controller {
-	return NewController(db, uidCheck, authorizer, pathStore, secretStore, spaceStore)
+	return NewController(db, uidCheck, authorizer, pathStore, encrypter, secretStore, spaceStore)
 }

internal/api/controller/space/list_pipelines.go

@@ -35,21 +35,21 @@ func (c *Controller) ListPipelines(
 	var pipelines []types.Pipeline
 
 	err = dbtx.New(c.db).WithTx(ctx, func(ctx context.Context) (err error) {
-		var dbErr error
+		count, err = c.pipelineStore.Count(ctx, space.ID, pagination)
-		count, dbErr = c.pipelineStore.Count(ctx, space.ID, pagination)
+		if err != nil {
-		if dbErr != nil {
+			err = fmt.Errorf("failed to count child executions: %w", err)
-			return fmt.Errorf("failed to count child executions: %w", err)
+			return
 		}
 
-		pipelines, dbErr = c.pipelineStore.List(ctx, space.ID, pagination)
+		pipelines, err = c.pipelineStore.List(ctx, space.ID, pagination)
-		if dbErr != nil {
+		if err != nil {
-			return fmt.Errorf("failed to list child executions: %w", err)
+			err = fmt.Errorf("failed to count child executions: %w", err)
+			return
 		}
-
+		return
-		return dbErr
 	}, dbtx.TxDefaultReadOnly)
 	if err != nil {
-		return pipelines, count, err
+		return pipelines, count, fmt.Errorf("failed to list pipelines: %w", err)
 	}
 
 	return pipelines, count, nil

internal/api/controller/space/list_secrets.go

@@ -35,21 +35,21 @@ func (c *Controller) ListSecrets(
 	var secrets []types.Secret
 
 	err = dbtx.New(c.db).WithTx(ctx, func(ctx context.Context) (err error) {
-		var dbErr error
+		count, err = c.secretStore.Count(ctx, space.ID, pagination)
-		count, dbErr = c.secretStore.Count(ctx, space.ID, pagination)
+		if err != nil {
-		if dbErr != nil {
+			err = fmt.Errorf("failed to count child executions: %w", err)
-			return fmt.Errorf("failed to count child executions: %w", err)
+			return
 		}
 
-		secrets, dbErr = c.secretStore.List(ctx, space.ID, pagination)
+		secrets, err = c.secretStore.List(ctx, space.ID, pagination)
-		if dbErr != nil {
+		if err != nil {
-			return fmt.Errorf("failed to list child executions: %w", err)
+			err = fmt.Errorf("failed to list child executions: %w", err)
+			return
 		}
-
+		return
-		return dbErr
 	}, dbtx.TxDefaultReadOnly)
 	if err != nil {
-		return secrets, count, err
+		return secrets, count, fmt.Errorf("failed to list secrets: %w", err)
 	}
 
 	return secrets, count, nil

internal/api/handler/secret/delete.go

@@ -13,9 +13,6 @@ import (
 	"github.com/harness/gitness/internal/paths"
 )
 
-/*
- * Deletes a secret.
- */
 func HandleDelete(secretCtrl *secret.Controller) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		ctx := r.Context()

internal/api/handler/secret/update.go

@@ -14,9 +14,6 @@ import (
 	"github.com/harness/gitness/internal/paths"
 )
 
-/*
- * Updates an existing secret.
- */
 func HandleUpdate(secretCtrl *secret.Controller) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		ctx := r.Context()

internal/store/database.go

@@ -450,7 +450,7 @@ type (
 		Create(ctx context.Context, pipeline *types.Pipeline) error
 
 		// Update tries to update a pipeline in the datastore
-		Update(ctx context.Context, pipeline *types.Pipeline) (*types.Pipeline, error)
+		Update(ctx context.Context, pipeline *types.Pipeline) error
 
 		// List lists the pipelines present in a parent space ID in the datastore.
 		List(ctx context.Context, spaceID int64, pagination types.Pagination) ([]types.Pipeline, error)
@@ -490,7 +490,7 @@ type (
 			mutateFn func(secret *types.Secret) error) (*types.Secret, error)
 
 		// Update tries to update a secret.
-		Update(ctx context.Context, secret *types.Secret) (*types.Secret, error)
+		Update(ctx context.Context, secret *types.Secret) error
 
 		// Delete deletes a secret given an ID.
 		Delete(ctx context.Context, id int64) error
@@ -510,7 +510,7 @@ type (
 		Create(ctx context.Context, execution *types.Execution) error
 
 		// Update tries to update an execution.
-		Update(ctx context.Context, execution *types.Execution) (*types.Execution, error)
+		Update(ctx context.Context, execution *types.Execution) error
 
 		// UpdateOptLock updates the execution using the optimistic locking mechanism.
 		UpdateOptLock(ctx context.Context, exectuion *types.Execution,

internal/store/database/execution.go

@@ -33,10 +33,6 @@ type executionStore struct {
 }
 
 const (
-	executionQueryBase = `
-		SELECT` + executionColumns + `
-		FROM executions`
-
 	executionColumns = `
 		execution_id
 		,execution_pipeline_id
@@ -74,8 +70,26 @@ const (
 		,execution_updated
 		,execution_version
 	`
+)
 
-	executionInsertStmt = `
+// Find returns an execution given a pipeline ID and an execution number.
+func (s *executionStore) Find(ctx context.Context, pipelineID int64, executionNum int64) (*types.Execution, error) {
+	const findQueryStmt = `
+	SELECT` + executionColumns + `
+	FROM executions
+	WHERE execution_pipeline_id = $1 AND execution_number = $2`
+	db := dbtx.GetAccessor(ctx, s.db)
+
+	dst := new(types.Execution)
+	if err := db.GetContext(ctx, dst, findQueryStmt, pipelineID, executionNum); err != nil {
+		return nil, database.ProcessSQLErrorf(err, "Failed to find execution")
+	}
+	return dst, nil
+}
+
+// Create creates a new execution in the datastore.
+func (s *executionStore) Create(ctx context.Context, execution *types.Execution) error {
+	const executionInsertStmt = `
 	INSERT INTO executions (
 		execution_pipeline_id
 		,execution_repo_id
@@ -147,58 +161,6 @@ const (
 		,:execution_updated
 		,:execution_version
 	) RETURNING execution_id`
-
-	executionUpdateStmt = `
-	UPDATE executions
-	SET
-		execution_trigger = :execution_trigger
-		,execution_parent = :execution_parent
-		,execution_status = :execution_status
-		,execution_error = :execution_error
-		,execution_event = :execution_event
-		,execution_action = :execution_action
-		,execution_link = :execution_link
-		,execution_timestamp = :execution_timestamp
-		,execution_title = :execution_title
-		,execution_message = :execution_message
-		,execution_before = :execution_before
-		,execution_after = :execution_after
-		,execution_ref = :execution_ref
-		,execution_source_repo = :execution_source_repo
-		,execution_source = :execution_source
-		,execution_target = :execution_target
-		,execution_author = :execution_author
-		,execution_author_name = :execution_author_name
-		,execution_author_email = :execution_author_email
-		,execution_author_avatar = :execution_author_avatar
-		,execution_sender = :execution_sender
-		,execution_params = :execution_params
-		,execution_cron = :execution_cron
-		,execution_deploy = :execution_deploy
-		,execution_deploy_id = :execution_deploy_id
-		,execution_debug = :execution_debug
-		,execution_started = :execution_started
-		,execution_finished = :execution_finished
-		,execution_updated = :execution_updated
-		,execution_version = :execution_version
-	WHERE execution_id = :execution_id AND execution_version = :execution_version - 1`
-)
-
-// Find returns an execution given a pipeline ID and an execution number.
-func (s *executionStore) Find(ctx context.Context, pipelineID int64, executionNum int64) (*types.Execution, error) {
-	const findQueryStmt = executionQueryBase + `
-		WHERE execution_pipeline_id = $1 AND execution_number = $2`
-	db := dbtx.GetAccessor(ctx, s.db)
-
-	dst := new(types.Execution)
-	if err := db.GetContext(ctx, dst, findQueryStmt, pipelineID, executionNum); err != nil {
-		return nil, database.ProcessSQLErrorf(err, "Failed to find execution")
-	}
-	return dst, nil
-}
-
-// Create creates a new execution in the datastore.
-func (s *executionStore) Create(ctx context.Context, execution *types.Execution) error {
 	db := dbtx.GetAccessor(ctx, s.db)
 
 	query, arg, err := db.BindNamed(executionInsertStmt, execution)
@@ -214,7 +176,18 @@ func (s *executionStore) Create(ctx context.Context, execution *types.Execution)
 }
 
 // Update tries to update an execution in the datastore with optimistic locking.
-func (s *executionStore) Update(ctx context.Context, execution *types.Execution) (*types.Execution, error) {
+func (s *executionStore) Update(ctx context.Context, execution *types.Execution) error {
+	const executionUpdateStmt = `
+	UPDATE executions
+	SET
+		,execution_status = :execution_status
+		,execution_error = :execution_error
+		,execution_event = :execution_event
+		,execution_started = :execution_started
+		,execution_finished = :execution_finished
+		,execution_updated = :execution_updated
+		,execution_version = :execution_version
+	WHERE execution_id = :execution_id AND execution_version = :execution_version - 1`
 	updatedAt := time.Now()
 
 	execution.Version++
@@ -224,24 +197,24 @@ func (s *executionStore) Update(ctx context.Context, execution *types.Execution)
 
 	query, arg, err := db.BindNamed(executionUpdateStmt, execution)
 	if err != nil {
-		return nil, database.ProcessSQLErrorf(err, "Failed to bind execution object")
+		return database.ProcessSQLErrorf(err, "Failed to bind execution object")
 	}
 
 	result, err := db.ExecContext(ctx, query, arg...)
 	if err != nil {
-		return nil, database.ProcessSQLErrorf(err, "Failed to update execution")
+		return database.ProcessSQLErrorf(err, "Failed to update execution")
 	}
 
 	count, err := result.RowsAffected()
 	if err != nil {
-		return nil, database.ProcessSQLErrorf(err, "Failed to get number of updated rows")
+		return database.ProcessSQLErrorf(err, "Failed to get number of updated rows")
 	}
 
 	if count == 0 {
-		return nil, gitness_store.ErrVersionConflict
+		return gitness_store.ErrVersionConflict
 	}
 
-	return execution, nil
+	return nil
 }
 
 // UpdateOptLock updates the pipeline using the optimistic locking mechanism.
@@ -251,16 +224,12 @@ func (s *executionStore) UpdateOptLock(ctx context.Context,
 	for {
 		dup := *execution
 
-		fmt.Println(dup.Status)
-
 		err := mutateFn(&dup)
 		if err != nil {
 			return nil, err
 		}
 
-		fmt.Println("dup.Status after: ", dup.Status)
+		err = s.Update(ctx, &dup)
-
-		execution, err = s.Update(ctx, &dup)
 		if err == nil {
 			return &dup, nil
 		}

internal/store/database/migrate/ci/ci_migrations.sql

@@ -1,17 +1,17 @@
 CREATE TABLE IF NOT EXISTS pipelines (
-    pipeline_id INTEGER PRIMARY KEY AUTOINCREMENT,
+    pipeline_id INTEGER PRIMARY KEY AUTOINCREMENT
-    pipeline_description TEXT,
+    ,pipeline_description TEXT NOT NULL
-    pipeline_space_id INTEGER NOT NULL,
+    ,pipeline_space_id INTEGER NOT NULL
-    pipeline_uid TEXT NOT NULL,
+    ,pipeline_uid TEXT NOT NULL
-    pipeline_seq INTEGER NOT NULL DEFAULT 0,
+    ,pipeline_seq INTEGER NOT NULL DEFAULT 0
-    pipeline_repo_id INTEGER,
+    ,pipeline_repo_id INTEGER
-    pipeline_repo_type TEXT NOT NULL,
+    ,pipeline_repo_type TEXT NOT NULL
-    pipeline_repo_name TEXT,
+    ,pipeline_repo_name TEXT
-    pipeline_default_branch TEXT,
+    ,pipeline_default_branch TEXT
-    pipeline_config_path TEXT NOT NULL,
+    ,pipeline_config_path TEXT NOT NULL
-    pipeline_created INTEGER NOT NULL,
+    ,pipeline_created INTEGER NOT NULL
-    pipeline_updated INTEGER NOT NULL,
+    ,pipeline_updated INTEGER NOT NULL
-    pipeline_version INTEGER NOT NULL,
+    ,pipeline_version INTEGER NOT NULL
 
     -- Ensure unique combination of UID and ParentID
     UNIQUE (pipeline_space_id, pipeline_uid),
@@ -30,67 +30,67 @@ CREATE TABLE IF NOT EXISTS pipelines (
 );
 
 CREATE TABLE IF NOT EXISTS executions (
-    execution_id INTEGER PRIMARY KEY AUTOINCREMENT,
+    execution_id INTEGER PRIMARY KEY AUTOINCREMENT
-    execution_pipeline_id INTEGER NOT NULL,
+    ,execution_pipeline_id INTEGER NOT NULL
-    execution_repo_id INTEGER,
+    ,execution_repo_id INTEGER
-    execution_trigger TEXT,
+    ,execution_trigger TEXT
-    execution_number INTEGER NOT NULL,
+    ,execution_number INTEGER NOT NULL
-    execution_parent INTEGER,
+    ,execution_parent INTEGER
-    execution_status TEXT,
+    ,execution_status TEXT
-    execution_error TEXT,
+    ,execution_error TEXT
-    execution_event TEXT,
+    ,execution_event TEXT
-    execution_action TEXT,
+    ,execution_action TEXT
-    execution_link TEXT,
+    ,execution_link TEXT
-    execution_timestamp INTEGER,
+    ,execution_timestamp INTEGER
-    execution_title TEXT,
+    ,execution_title TEXT
-    execution_message TEXT,
+    ,execution_message TEXT
-    execution_before TEXT,
+    ,execution_before TEXT
-    execution_after TEXT,
+    ,execution_after TEXT
-    execution_ref TEXT,
+    ,execution_ref TEXT
-    execution_source_repo TEXT,
+    ,execution_source_repo TEXT
-    execution_source TEXT,
+    ,execution_source TEXT
-    execution_target TEXT,
+    ,execution_target TEXT
-    execution_author TEXT,
+    ,execution_author TEXT
-    execution_author_name TEXT,
+    ,execution_author_name TEXT
-    execution_author_email TEXT,
+    ,execution_author_email TEXT
-    execution_author_avatar TEXT,
+    ,execution_author_avatar TEXT
-    execution_sender TEXT,
+    ,execution_sender TEXT
-    execution_params TEXT,
+    ,execution_params TEXT
-    execution_cron TEXT,
+    ,execution_cron TEXT
-    execution_deploy TEXT,
+    ,execution_deploy TEXT
-    execution_deploy_id INTEGER,
+    ,execution_deploy_id INTEGER
-    execution_debug BOOLEAN NOT NULL DEFAULT 0,
+    ,execution_debug BOOLEAN NOT NULL DEFAULT 0
-    execution_started INTEGER,
+    ,execution_started INTEGER
-    execution_finished INTEGER,
+    ,execution_finished INTEGER
-    execution_created INTEGER NOT NULL,
+    ,execution_created INTEGER NOT NULL
-    execution_updated INTEGER NOT NULL,
+    ,execution_updated INTEGER NOT NULL
-    execution_version INTEGER NOT NULL,
+    ,execution_version INTEGER NOT NULL
 
     -- Ensure unique combination of pipeline ID and number
     UNIQUE (execution_pipeline_id, execution_number),
 
     -- Foreign key to pipelines table
-    CONSTRAINT fk_executions_pipeline_id FOREIGN KEY (execution_pipeline_id)
+    CONSTRAINT fk_executions_pipeline_id FOREIGN KEY (,execution_pipeline_id)
         REFERENCES pipelines (pipeline_id) MATCH SIMPLE
         ON UPDATE NO ACTION
         ON DELETE CASCADE
 
     -- Foreign key to repositories table
-    CONSTRAINT fk_executions_repo_id FOREIGN KEY (execution_repo_id)
+    CONSTRAINT fk_executions_repo_id FOREIGN KEY (,execution_repo_id)
         REFERENCES repositories (repo_id) MATCH SIMPLE
         ON UPDATE NO ACTION
         ON DELETE CASCADE
 );
 
 CREATE TABLE IF NOT EXISTS secrets (
-    secret_id INTEGER PRIMARY KEY AUTOINCREMENT,
+    secret_id INTEGER PRIMARY KEY AUTOINCREMENT
-    secret_uid TEXT NOT NULL,
+    ,secret_uid TEXT NOT NULL
-    secret_space_id INTEGER NOT NULL,
+    ,secret_space_id INTEGER NOT NULL
-    secret_description TEXT,
+    ,secret_description TEXT NOT NULL
-    secret_data BLOB NOT NULL,
+    ,secret_data BLOB NOT NULL
-    secret_created INTEGER NOT NULL,
+    ,secret_created INTEGER NOT NULL
-    secret_updated INTEGER NOT NULL,
+    ,secret_updated INTEGER NOT NULL
-    secret_version INTEGER NOT NULL,
+    ,secret_version INTEGER NOT NULL
 
     -- Ensure unique combination of space ID and UID
     UNIQUE (secret_space_id, secret_uid),

internal/store/database/pipeline.go

@@ -43,51 +43,6 @@ const (
 	,pipeline_updated
 	,pipeline_version
 	`
-
-	pipelineInsertStmt = `
-	INSERT INTO pipelines (
-		pipeline_description
-		,pipeline_space_id
-		,pipeline_uid
-		,pipeline_seq
-		,pipeline_repo_id
-		,pipeline_repo_type
-		,pipeline_repo_name
-		,pipeline_default_branch
-		,pipeline_config_path
-		,pipeline_created
-		,pipeline_updated
-		,pipeline_version
-	) VALUES (
-		:pipeline_description,
-		:pipeline_space_id,
-		:pipeline_uid,
-		:pipeline_seq,
-		:pipeline_repo_id,
-		:pipeline_repo_type,
-		:pipeline_repo_name,
-		:pipeline_default_branch,
-		:pipeline_config_path,
-		:pipeline_created,
-		:pipeline_updated,
-		:pipeline_version
-	) RETURNING pipeline_id`
-
-	pipelineUpdateStmt = `
-	UPDATE pipelines
-	SET
-		pipeline_description = :pipeline_description,
-		pipeline_space_id = :pipeline_space_id,
-		pipeline_uid = :pipeline_uid,
-		pipeline_seq = :pipeline_seq,
-		pipeline_repo_id = :pipeline_repo_id,
-		pipeline_repo_type = :pipeline_repo_type,
-		pipeline_repo_name = :pipeline_repo_name,
-		pipeline_default_branch = :pipeline_default_branch,
-		pipeline_config_path = :pipeline_config_path,
-		pipeline_updated = :pipeline_updated,
-		pipeline_version = :pipeline_version
-	WHERE pipeline_id = :pipeline_id AND pipeline_version = :pipeline_version - 1`
 )
 
 // NewPipelineStore returns a new PipelineStore.
@@ -129,6 +84,34 @@ func (s *pipelineStore) FindByUID(ctx context.Context, spaceID int64, uid string
 
 // Create creates a pipeline.
 func (s *pipelineStore) Create(ctx context.Context, pipeline *types.Pipeline) error {
+	const pipelineInsertStmt = `
+	INSERT INTO pipelines (
+		pipeline_description
+		,pipeline_space_id
+		,pipeline_uid
+		,pipeline_seq
+		,pipeline_repo_id
+		,pipeline_repo_type
+		,pipeline_repo_name
+		,pipeline_default_branch
+		,pipeline_config_path
+		,pipeline_created
+		,pipeline_updated
+		,pipeline_version
+	) VALUES (
+		:pipeline_description,
+		:pipeline_space_id,
+		:pipeline_uid,
+		:pipeline_seq,
+		:pipeline_repo_id,
+		:pipeline_repo_type,
+		:pipeline_repo_name,
+		:pipeline_default_branch,
+		:pipeline_config_path,
+		:pipeline_created,
+		:pipeline_updated,
+		:pipeline_version
+	) RETURNING pipeline_id`
 	db := dbtx.GetAccessor(ctx, s.db)
 
 	query, arg, err := db.BindNamed(pipelineInsertStmt, pipeline)
@@ -144,7 +127,18 @@ func (s *pipelineStore) Create(ctx context.Context, pipeline *types.Pipeline) er
 }
 
 // Update updates a pipeline.
-func (s *pipelineStore) Update(ctx context.Context, pipeline *types.Pipeline) (*types.Pipeline, error) {
+func (s *pipelineStore) Update(ctx context.Context, pipeline *types.Pipeline) error {
+	const pipelineUpdateStmt = `
+	UPDATE pipelines
+	SET
+		pipeline_description = :pipeline_description,
+		pipeline_uid = :pipeline_uid,
+		pipeline_seq = :pipeline_seq,
+		pipeline_default_branch = :pipeline_default_branch,
+		pipeline_config_path = :pipeline_config_path,
+		pipeline_updated = :pipeline_updated,
+		pipeline_version = :pipeline_version
+	WHERE pipeline_id = :pipeline_id AND pipeline_version = :pipeline_version - 1`
 	updatedAt := time.Now()
 
 	pipeline.Version++
@@ -154,24 +148,24 @@ func (s *pipelineStore) Update(ctx context.Context, pipeline *types.Pipeline) (*
 
 	query, arg, err := db.BindNamed(pipelineUpdateStmt, pipeline)
 	if err != nil {
-		return nil, database.ProcessSQLErrorf(err, "Failed to bind pipeline object")
+		return database.ProcessSQLErrorf(err, "Failed to bind pipeline object")
 	}
 
 	result, err := db.ExecContext(ctx, query, arg...)
 	if err != nil {
-		return nil, database.ProcessSQLErrorf(err, "Failed to update pipeline")
+		return database.ProcessSQLErrorf(err, "Failed to update pipeline")
 	}
 
 	count, err := result.RowsAffected()
 	if err != nil {
-		return nil, database.ProcessSQLErrorf(err, "Failed to get number of updated rows")
+		return database.ProcessSQLErrorf(err, "Failed to get number of updated rows")
 	}
 
 	if count == 0 {
-		return nil, gitness_store.ErrVersionConflict
+		return gitness_store.ErrVersionConflict
 	}
 
-	return pipeline, nil
+	return nil
 }
 
 // List lists all the pipelines present in a space.
@@ -219,7 +213,7 @@ func (s *pipelineStore) UpdateOptLock(ctx context.Context,
 			return nil, err
 		}
 
-		pipeline, err = s.Update(ctx, &dup)
+		err = s.Update(ctx, &dup)
 		if err == nil {
 			return &dup, nil
 		}
@@ -296,7 +290,7 @@ func (s *pipelineStore) IncrementSeqNum(ctx context.Context, pipeline *types.Pip
 	for {
 		var err error
 		pipeline.Seq++
-		pipeline, err = s.Update(ctx, pipeline)
+		err = s.Update(ctx, pipeline)
 		if err == nil {
 			return pipeline, nil
 		} else if !errors.Is(err, gitness_store.ErrVersionConflict) {

internal/store/database/secret.go

@@ -10,7 +10,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/harness/gitness/encrypt"
 	"github.com/harness/gitness/internal/store"
 	gitness_store "github.com/harness/gitness/store"
 	"github.com/harness/gitness/store/database"
@@ -38,8 +37,48 @@ const (
 	secret_updated,
 	secret_version
 	`
+)
 
-	secretInsertStmt = `
+// NewSecretStore returns a new SecretStore.
+func NewSecretStore(db *sqlx.DB) *secretStore {
+	return &secretStore{
+		db: db,
+	}
+}
+
+type secretStore struct {
+	db *sqlx.DB
+}
+
+// Find returns a secret given a secret ID.
+func (s *secretStore) Find(ctx context.Context, id int64) (*types.Secret, error) {
+	const findQueryStmt = secretQueryBase + `
+		WHERE secret_id = $1`
+	db := dbtx.GetAccessor(ctx, s.db)
+
+	dst := new(types.Secret)
+	if err := db.GetContext(ctx, dst, findQueryStmt, id); err != nil {
+		return nil, database.ProcessSQLErrorf(err, "Failed to find secret")
+	}
+	return dst, nil
+}
+
+// FindByUID returns a secret in a given space with a given UID.
+func (s *secretStore) FindByUID(ctx context.Context, spaceID int64, uid string) (*types.Secret, error) {
+	const findQueryStmt = secretQueryBase + `
+		WHERE secret_space_id = $1 AND secret_uid = $2`
+	db := dbtx.GetAccessor(ctx, s.db)
+
+	dst := new(types.Secret)
+	if err := db.GetContext(ctx, dst, findQueryStmt, spaceID, uid); err != nil {
+		return nil, database.ProcessSQLErrorf(err, "Failed to find secret")
+	}
+	return dst, nil
+}
+
+// Create creates a secret.
+func (s *secretStore) Create(ctx context.Context, secret *types.Secret) error {
+	const secretInsertStmt = `
 	INSERT INTO secrets (
 		secret_description,
 		secret_space_id,
@@ -57,67 +96,8 @@ const (
 		:secret_updated,
 		:secret_version
 	) RETURNING secret_id`
-
-	secretUpdateStmt = `
-	UPDATE secrets
-	SET
-		secret_description = :secret_description,
-		secret_space_id = :secret_space_id,
-		secret_uid = :secret_uid,
-		secret_data = :secret_data,
-		secret_updated = :secret_updated,
-		secret_version = :secret_version
-	WHERE secret_id = :secret_id AND secret_version = :secret_version - 1`
-)
-
-// NewSecretStore returns a new SecretStore.
-func NewSecretStore(enc encrypt.Encrypter, db *sqlx.DB) *secretStore {
-	return &secretStore{
-		db:  db,
-		enc: enc,
-	}
-}
-
-type secretStore struct {
-	db  *sqlx.DB
-	enc encrypt.Encrypter
-}
-
-// Find returns a secret given a secret ID.
-func (s *secretStore) Find(ctx context.Context, id int64) (*types.Secret, error) {
-	const findQueryStmt = secretQueryBase + `
-		WHERE secret_id = $1`
 	db := dbtx.GetAccessor(ctx, s.db)
 
-	dst := new(types.Secret)
-	if err := db.GetContext(ctx, dst, findQueryStmt, id); err != nil {
-		return nil, database.ProcessSQLErrorf(err, "Failed to find secret")
-	}
-	return dec(s.enc, dst)
-}
-
-// FindByUID returns a secret in a given space with a given UID.
-func (s *secretStore) FindByUID(ctx context.Context, spaceID int64, uid string) (*types.Secret, error) {
-	const findQueryStmt = secretQueryBase + `
-		WHERE secret_space_id = $1 AND secret_uid = $2`
-	db := dbtx.GetAccessor(ctx, s.db)
-
-	dst := new(types.Secret)
-	if err := db.GetContext(ctx, dst, findQueryStmt, spaceID, uid); err != nil {
-		return nil, database.ProcessSQLErrorf(err, "Failed to find secret")
-	}
-	return dec(s.enc, dst)
-}
-
-// Create creates a secret.
-func (s *secretStore) Create(ctx context.Context, secret *types.Secret) error {
-	db := dbtx.GetAccessor(ctx, s.db)
-
-	secret, err := enc(s.enc, secret)
-	if err != nil {
-		return err
-	}
-
 	query, arg, err := db.BindNamed(secretInsertStmt, secret)
 	if err != nil {
 		return database.ProcessSQLErrorf(err, "Failed to bind secret object")
@@ -130,7 +110,16 @@ func (s *secretStore) Create(ctx context.Context, secret *types.Secret) error {
 	return nil
 }
 
-func (s *secretStore) Update(ctx context.Context, secret *types.Secret) (*types.Secret, error) {
+func (s *secretStore) Update(ctx context.Context, secret *types.Secret) error {
+	const secretUpdateStmt = `
+	UPDATE secrets
+	SET
+		secret_description = :secret_description,
+		secret_uid = :secret_uid,
+		secret_data = :secret_data,
+		secret_updated = :secret_updated,
+		secret_version = :secret_version
+	WHERE secret_id = :secret_id AND secret_version = :secret_version - 1`
 	updatedAt := time.Now()
 
 	secret.Version++
@@ -138,31 +127,26 @@ func (s *secretStore) Update(ctx context.Context, secret *types.Secret) (*types.
 
 	db := dbtx.GetAccessor(ctx, s.db)
 
-	secret, err := enc(s.enc, secret)
-	if err != nil {
-		return nil, err
-	}
-
 	query, arg, err := db.BindNamed(secretUpdateStmt, secret)
 	if err != nil {
-		return nil, database.ProcessSQLErrorf(err, "Failed to bind secret object")
+		return database.ProcessSQLErrorf(err, "Failed to bind secret object")
 	}
 
 	result, err := db.ExecContext(ctx, query, arg...)
 	if err != nil {
-		return nil, database.ProcessSQLErrorf(err, "Failed to update secret")
+		return database.ProcessSQLErrorf(err, "Failed to update secret")
 	}
 
 	count, err := result.RowsAffected()
 	if err != nil {
-		return nil, database.ProcessSQLErrorf(err, "Failed to get number of updated rows")
+		return database.ProcessSQLErrorf(err, "Failed to get number of updated rows")
 	}
 
 	if count == 0 {
-		return nil, gitness_store.ErrVersionConflict
+		return gitness_store.ErrVersionConflict
 	}
 
-	return secret, nil
+	return nil
 }
 
 // UpdateOptLock updates the pipeline using the optimistic locking mechanism.
@@ -177,7 +161,7 @@ func (s *secretStore) UpdateOptLock(ctx context.Context,
 			return nil, err
 		}
 
-		secret, err = s.Update(ctx, &dup)
+		err = s.Update(ctx, &dup)
 		if err == nil {
 			return &dup, nil
 		}
@@ -276,31 +260,3 @@ func (s *secretStore) Count(ctx context.Context, parentID int64, filter types.Pa
 	}
 	return count, nil
 }
-
-// helper function returns the same secret with encrypted data.
-func enc(encrypt encrypt.Encrypter, secret *types.Secret) (*types.Secret, error) {
-	if secret == nil {
-		return nil, fmt.Errorf("cannot encrypt a nil secret")
-	}
-	s := *secret
-	ciphertext, err := encrypt.Encrypt(secret.Data)
-	if err != nil {
-		return nil, err
-	}
-	s.Data = string(ciphertext)
-	return &s, nil
-}
-
-// helper function returns the same secret with decrypted data.
-func dec(encrypt encrypt.Encrypter, secret *types.Secret) (*types.Secret, error) {
-	if secret == nil {
-		return nil, fmt.Errorf("cannot decrypt a nil secret")
-	}
-	s := *secret
-	plaintext, err := encrypt.Decrypt([]byte(secret.Data))
-	if err != nil {
-		return nil, err
-	}
-	s.Data = plaintext
-	return &s, nil
-}

internal/store/database/wire.go

@@ -105,8 +105,8 @@ func ProvidePipelineStore(db *sqlx.DB) store.PipelineStore {
 }
 
 // ProvideSecretStore provides a secret store.
-func ProvideSecretStore(enc encrypt.Encrypter, db *sqlx.DB) store.SecretStore {
+func ProvideSecretStore(db *sqlx.DB) store.SecretStore {
-	return NewSecretStore(enc, db)
+	return NewSecretStore(db)
 }
 
 // ProvideExecutionStore provides an execution store.

mocks/mock_client.go

@@ -8,9 +8,10 @@ import (
 	context "context"
 	reflect "reflect"
 
-	gomock "github.com/golang/mock/gomock"
 	user "github.com/harness/gitness/internal/api/controller/user"
 	types "github.com/harness/gitness/types"
+
+	gomock "github.com/golang/mock/gomock"
 )
 
 // MockClient is a mock of Client interface.

mocks/mock_store.go

@@ -8,9 +8,10 @@ import (
 	context "context"
 	reflect "reflect"
 
-	gomock "github.com/golang/mock/gomock"
 	types "github.com/harness/gitness/types"
 	enum "github.com/harness/gitness/types/enum"
+
+	gomock "github.com/golang/mock/gomock"
 )
 
 // MockPrincipalStore is a mock of PrincipalStore interface.