Skip to content

feat: [AH-2353]: Updated storage layer initiation for default bucket #88

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abhinavcode wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: [AH-2353]: Updated storage layer initiation for default bucket #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
77b8e72
[AH-2353]: Updated storage layer initiation for default bucket
arvind-choudhary-h Feb 20, 2026
7506407
[AH-2353]: Created copy function in GCS
arvind-choudhary-h Feb 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions registry/app/api/wire.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import (
storagedriver "github.com/harness/gitness/registry/app/driver"
"github.com/harness/gitness/registry/app/driver/factory"
"github.com/harness/gitness/registry/app/driver/filesystem"
"github.com/harness/gitness/registry/app/driver/gcs"
"github.com/harness/gitness/registry/app/driver/s3-aws"
"github.com/harness/gitness/registry/app/pkg"
"github.com/harness/gitness/registry/app/pkg/base"
Expand Down Expand Up @@ -86,18 +87,26 @@ func DefaultStorageProvider(ctx context.Context, c *types.Config) (storagedriver
var d storagedriver.StorageDriver
var err error

if c.Registry.Storage.StorageType == "filesystem" {
switch c.Registry.Storage.StorageType {
case "filesystem":
filesystem.Register(ctx)
d, err = factory.Create(ctx, "filesystem", config.GetFilesystemParams(c))
if err != nil {
log.Fatal().Stack().Err(err).Msgf("")
panic(err)
}
} else {
case "gcs":
gcs.Register(ctx)
d, err = factory.Create(ctx, "gcs", config.GetGCSStorageParameters(c))
if err != nil {
log.Error().Stack().Err(err).Msg("failed to init gcs Blob storage")
panic(err)
}
default:
s3.Register(ctx)
d, err = factory.Create(ctx, "s3aws", config.GetS3StorageParameters(c))
if err != nil {
log.Error().Stack().Err(err).Msg("failed to init s3 Blob storage ")
log.Error().Stack().Err(err).Msg("failed to init s3 Blob storage")
panic(err)
}
}
Comment on lines +90 to 112

@coderabbitai coderabbitai Bot Feb 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Unknown StorageType values silently fall through to S3.

If someone mistypes the storage type (e.g., "gc" instead of "gcs"), the default branch will quietly initialize S3 storage instead of surfacing a configuration error. Consider adding an explicit "s3aws" case and making default return an error for unrecognized values.

Suggested change 
 switch c.Registry.Storage.StorageType {
 case "filesystem":
     filesystem.Register(ctx)
     d, err = factory.Create(ctx, "filesystem", config.GetFilesystemParams(c))
     if err != nil {
         log.Fatal().Stack().Err(err).Msgf("")
         panic(err)
     }
 case "gcs":
     gcs.Register(ctx)
     d, err = factory.Create(ctx, "gcs", config.GetGCSStorageParameters(c))
     if err != nil {
         log.Error().Stack().Err(err).Msg("failed to init gcs Blob storage")
         panic(err)
     }
-default:
+case "s3aws":
     s3.Register(ctx)
     d, err = factory.Create(ctx, "s3aws", config.GetS3StorageParameters(c))
     if err != nil {
         log.Error().Stack().Err(err).Msg("failed to init s3 Blob storage")
         panic(err)
     }
+default:
+    return nil, fmt.Errorf("unsupported registry storage type: %q", c.Registry.Storage.StorageType)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
switch c.Registry.Storage.StorageType {
case "filesystem":
filesystem.Register(ctx)
d, err = factory.Create(ctx, "filesystem", config.GetFilesystemParams(c))
if err != nil {
log.Fatal().Stack().Err(err).Msgf("")
panic(err)
}
} else {
case "gcs":
gcs.Register(ctx)
d, err = factory.Create(ctx, "gcs", config.GetGCSStorageParameters(c))
if err != nil {
log.Error().Stack().Err(err).Msg("failed to init gcs Blob storage")
panic(err)
}
default:
s3.Register(ctx)
d, err = factory.Create(ctx, "s3aws", config.GetS3StorageParameters(c))
if err != nil {
log.Error().Stack().Err(err).Msg("failed to init s3 Blob storage ")
log.Error().Stack().Err(err).Msg("failed to init s3 Blob storage")
panic(err)
}
}
switch c.Registry.Storage.StorageType {
case "filesystem":
filesystem.Register(ctx)
d, err = factory.Create(ctx, "filesystem", config.GetFilesystemParams(c))
if err != nil {
log.Fatal().Stack().Err(err).Msgf("")
panic(err)
}
case "gcs":
gcs.Register(ctx)
d, err = factory.Create(ctx, "gcs", config.GetGCSStorageParameters(c))
if err != nil {
log.Error().Stack().Err(err).Msg("failed to init gcs Blob storage")
panic(err)
}
case "s3aws":
s3.Register(ctx)
d, err = factory.Create(ctx, "s3aws", config.GetS3StorageParameters(c))
if err != nil {
log.Error().Stack().Err(err).Msg("failed to init s3 Blob storage")
panic(err)
}
default:
return nil, fmt.Errorf("unsupported registry storage type: %q", c.Registry.Storage.StorageType)
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@registry/app/api/wire.go` around lines 90 - 112, The switch on
c.Registry.Storage.StorageType currently treats any unknown value as S3; add an
explicit "s3aws" case that calls s3.Register(ctx) and sets d, err =
factory.Create(ctx, "s3aws", config.GetS3StorageParameters(c)) (mirroring the
existing S3 logic) and change the switch default to surface a configuration
error instead of initializing S3 (e.g., return or log.Fatalf with a clear
message including the invalid c.Registry.Storage.StorageType); update callers or
function signature if needed so the function returns an error rather than
silently panicking or defaulting.

Expand Down
30 changes: 26 additions & 4 deletions registry/app/driver/gcs/gcs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ import (
)

const (
driverName = "gcs"
dummyProjectID = "<unknown>"
driverName = "gcs"

minChunkSize = 256 * 1024
defaultChunkSize = 16 * 1024 * 1024
Expand Down Expand Up @@ -88,6 +87,11 @@ func init() {
factory.Register(driverName, &gcsDriverFactory{})
}

// TODO: figure-out why init is not called automatically
func Register(ctx context.Context) {
log.Ctx(ctx).Info().Msgf("registering gcs driver")
}

// gcsDriverFactory implements the factory.StorageDriverFactory interface.
type gcsDriverFactory struct{}

Expand All @@ -105,6 +109,7 @@ var _ storagedriver.StorageDriver = &driver{}
// Objects are stored at absolute keys in the provided bucket.
type driver struct {
client *http.Client
gcs *storage.Client
bucket *storage.BucketHandle
email string
privateKey []byte
Expand Down Expand Up @@ -256,6 +261,7 @@ func New(_ context.Context, params driverParameters) (storagedriver.StorageDrive
return nil, fmt.Errorf("invalid chunksize: %d is not a positive multiple of %d", params.chunkSize, minChunkSize)
}
d := &driver{
gcs: params.gcs,
bucket: params.gcs.Bucket(params.bucket),
rootDirectory: rootDirectory,
email: params.email,
Expand Down Expand Up @@ -932,6 +938,22 @@ func (d *driver) keyToPath(key string) string {
return "/" + strings.Trim(strings.TrimPrefix(key, d.rootDirectory), "/")
}

func (d *driver) CopyObject(_ context.Context, _, _, _ string) error {
return fmt.Errorf("not yet implemented")
func (d *driver) CopyObject(ctx context.Context, srcKey, destBucket, destKey string) error {
src := d.bucket.Object(d.pathToKey(srcKey))

dst := d.gcs.Bucket(destBucket).Object(destKey)

copier := dst.CopierFrom(src)
copier.ContentType = blobContentType

_, err := copier.Run(ctx)
if err != nil {
var status *googleapi.Error
if errors.As(err, &status) && status.Code == http.StatusNotFound {
return storagedriver.PathNotFoundError{Path: srcKey}
}
return fmt.Errorf("copy %q to %q/%q: %w", srcKey, destBucket, destKey, err)
}

return nil
}
Comment on lines +941 to 959

@coderabbitai coderabbitai Bot Feb 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Asymmetric path handling: srcKey is resolved via pathToKey but destKey is used verbatim.

d.pathToKey(srcKey) prepends rootDirectory to the source, but destKey is passed directly to d.gcs.Bucket(destBucket).Object(destKey) without any root-directory prefix. If the caller isn't aware of this inconsistency, the destination object will end up at an unexpected path. Either document this contract clearly or apply consistent path resolution.

Additionally, consider whether this method should verify that destBucket is non-empty before proceeding, to avoid a confusing GCS error.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@registry/app/driver/gcs/gcs.go` around lines 941 - 959, CopyObject currently
resolves the source key with d.pathToKey but uses destKey verbatim; make path
handling consistent by applying d.pathToKey to the destination (e.g., use
d.gcs.Bucket(destBucket).Object(d.pathToKey(destKey))) so destination objects
live under the same rootDirectory, and add a pre-check that destBucket is
non-empty (return a clear error if empty) before creating the destination
object; update any related error messages in CopyObject to reference the
resolved destination key.

6 changes: 6 additions & 0 deletions registry/config/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,9 @@ func GetFilesystemParams(c *types.Config) map[string]any {
props["rootdirectory"] = c.Registry.Storage.FileSystemStorage.RootDirectory
return props
}

func GetGCSStorageParameters(c *types.Config) map[string]any {
gcsProperties := make(map[string]any)
gcsProperties["bucket"] = c.Registry.Storage.GCSStorage.Bucket
return gcsProperties
}
Comment on lines +49 to +53

@coderabbitai coderabbitai Bot Feb 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Pass rootdirectory (once added to config) for namespace isolation.

This helper should forward the rootdirectory parameter (and any other newly-added GCS config fields) to the driver, matching the pattern used by GetS3StorageParameters and GetFilesystemParams. Currently all registry objects will be written directly under the bucket root.

Suggested change (once config is updated) 
 func GetGCSStorageParameters(c *types.Config) map[string]any {
 	gcsProperties := make(map[string]any)
 	gcsProperties["bucket"] = c.Registry.Storage.GCSStorage.Bucket
+	gcsProperties["rootdirectory"] = c.Registry.Storage.GCSStorage.RootDirectory
 	return gcsProperties
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@registry/config/helper.go` around lines 49 - 53, GetGCSStorageParameters
currently only returns the bucket and omits the rootdirectory (and any other new
GCS options), causing all objects to be written to the bucket root; update
GetGCSStorageParameters to read c.Registry.Storage.GCSStorage.RootDirectory (and
any other new fields) and include them in the returned map (e.g., set
"rootdirectory" => c.Registry.Storage.GCSStorage.RootDirectory) so the GCS
driver receives the same namespace/isolation params as GetS3StorageParameters
and GetFilesystemParams.

8 changes: 7 additions & 1 deletion types/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ type Config struct {
Registry struct {
Enable bool `envconfig:"GITNESS_REGISTRY_ENABLED" default:"true"`
Storage struct {
// StorageType defines the type of storage to use for the registry. Options are: `filesystem`, `s3aws`
// StorageType defines the type of storage to use for the registry. Options are: `filesystem`, `s3aws`, `gcs`
StorageType string `envconfig:"GITNESS_REGISTRY_STORAGE_TYPE" default:"filesystem"`

// FileSystemStorage defines the configuration for the filesystem storage if StorageType is `filesystem`.
Expand Down Expand Up @@ -544,6 +544,12 @@ type Config struct {
Redirect bool `envconfig:"GITNESS_REGISTRY_S3_STORAGE_REDIRECT" default:"false"`
Provider string `envconfig:"GITNESS_REGISTRY_S3_PROVIDER" default:"cloudflare"`
}

// GCSStorage defines the configuration for the GCS storage if StorageType is `gcs`.
// Authentication is handled via workload identity (google.DefaultTokenSource).
GCSStorage struct {
Bucket string `envconfig:"GITNESS_REGISTRY_GCS_BUCKET"`
}
Comment on lines +548 to +552

@coderabbitai coderabbitai Bot Feb 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

GCSStorage config is missing RootDirectory (and other driver parameters).

Both S3Storage and FileSystemStorage expose a RootDirectory field to isolate registry data under a path prefix. The GCS driver's FromParameters also supports rootdirectory, chunksize, maxconcurrency, keyfile, and credentials, but none of these are configurable here. At minimum, RootDirectory should be added for parity and to avoid storing objects at the bucket root with no namespace isolation.

Suggested minimal addition 
 GCSStorage struct {
-    Bucket string `envconfig:"GITNESS_REGISTRY_GCS_BUCKET"`
+    Bucket        string `envconfig:"GITNESS_REGISTRY_GCS_BUCKET"`
+    RootDirectory string `envconfig:"GITNESS_REGISTRY_GCS_ROOT_DIRECTORY"`
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// GCSStorage defines the configuration for the GCS storage if StorageType is `gcs`.
// Authentication is handled via workload identity (google.DefaultTokenSource).
GCSStorage struct {
Bucket string `envconfig:"GITNESS_REGISTRY_GCS_BUCKET"`
}
// GCSStorage defines the configuration for the GCS storage if StorageType is `gcs`.
// Authentication is handled via workload identity (google.DefaultTokenSource).
GCSStorage struct {
Bucket string `envconfig:"GITNESS_REGISTRY_GCS_BUCKET"`
RootDirectory string `envconfig:"GITNESS_REGISTRY_GCS_ROOT_DIRECTORY"`
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@types/config.go` around lines 548 - 552, The GCSStorage struct is missing
RootDirectory (and other driver params) so registry objects would be stored at
the bucket root; update the GCSStorage struct to add at least a RootDirectory
string field (matching envconfig tag pattern like
GITNESS_REGISTRY_GCS_ROOT_DIRECTORY) and consider adding optional fields
supported by the driver such as ChunkSize, MaxConcurrency, KeyFile, and
Credentials to mirror S3Storage/FileSystemStorage; ensure the struct field names
and envconfig tags align with how FromParameters expects keys so FromParameters
can map rootdirectory, chunksize, maxconcurrency, keyfile, and credentials into
the GCSStorage config.

}

HTTP struct {
Expand Down