Allow only empty StorageID

treeverse · Jan 17, 2025 · 298bc0d · 298bc0d
1 parent 989a388
commit 298bc0d
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 25 deletions.
diff --git a/pkg/catalog/catalog.go b/pkg/catalog/catalog.go
@@ -453,6 +453,7 @@ func (c *Catalog) CreateRepository(ctx context.Context, repository string, stora
 	branchID := graveler.BranchID(branch)
 	if err := validator.Validate([]validator.ValidateArg{
 		{Name: "name", Value: repositoryID, Fn: graveler.ValidateRepositoryID},
+		{Name: "storageID", Value: storageIdentifier, Fn: graveler.ValidateStorageID},
 		{Name: "storageNamespace", Value: storageNS, Fn: graveler.ValidateStorageNamespace},
 	}); err != nil {
 		return nil, err
@@ -481,6 +482,7 @@ func (c *Catalog) CreateBareRepository(ctx context.Context, repository string, s
 	branchID := graveler.BranchID(defaultBranchID)
 	if err := validator.Validate([]validator.ValidateArg{
 		{Name: "name", Value: repositoryID, Fn: graveler.ValidateRepositoryID},
+		{Name: "storageID", Value: storageIdentifier, Fn: graveler.ValidateStorageID},
 		{Name: "storageNamespace", Value: storageNS, Fn: graveler.ValidateStorageNamespace},
 	}); err != nil {
 		return nil, err

diff --git a/pkg/catalog/catalog_test.go b/pkg/catalog/catalog_test.go
@@ -72,11 +72,11 @@ func TestCatalog_ListRepositories(t *testing.T) {
 	// prepare data tests
 	now := time.Now()
 	gravelerData := []*graveler.RepositoryRecord{
-		{RepositoryID: "re", Repository: &graveler.Repository{StorageID: "storage", StorageNamespace: "storageNS1", CreationDate: now, DefaultBranchID: "main1"}},
-		{RepositoryID: "repo1", Repository: &graveler.Repository{StorageID: "storage1", StorageNamespace: "storageNS2", CreationDate: now, DefaultBranchID: "main2"}},
-		{RepositoryID: "repo2", Repository: &graveler.Repository{StorageID: "storage1", StorageNamespace: "storageNS3", CreationDate: now, DefaultBranchID: "main3"}},
-		{RepositoryID: "repo22", Repository: &graveler.Repository{StorageID: "storage1", StorageNamespace: "storageNS4", CreationDate: now, DefaultBranchID: "main4"}},
-		{RepositoryID: "repo23", Repository: &graveler.Repository{StorageID: "storage1", StorageNamespace: "storageNS5", CreationDate: now, DefaultBranchID: "main5"}},
+		{RepositoryID: "re", Repository: &graveler.Repository{StorageID: "", StorageNamespace: "storageNS1", CreationDate: now, DefaultBranchID: "main1"}},
+		{RepositoryID: "repo1", Repository: &graveler.Repository{StorageID: "", StorageNamespace: "storageNS2", CreationDate: now, DefaultBranchID: "main2"}},
+		{RepositoryID: "repo2", Repository: &graveler.Repository{StorageID: "", StorageNamespace: "storageNS3", CreationDate: now, DefaultBranchID: "main3"}},
+		{RepositoryID: "repo22", Repository: &graveler.Repository{StorageID: "", StorageNamespace: "storageNS4", CreationDate: now, DefaultBranchID: "main4"}},
+		{RepositoryID: "repo23", Repository: &graveler.Repository{StorageID: "", StorageNamespace: "storageNS5", CreationDate: now, DefaultBranchID: "main5"}},
 		{RepositoryID: "repo3", Repository: &graveler.Repository{StorageID: "", StorageNamespace: "storageNS6", CreationDate: now, DefaultBranchID: "main6"}},
 	}
 	type args struct {
@@ -101,11 +101,11 @@ func TestCatalog_ListRepositories(t *testing.T) {
 				searchString: "",
 			},
 			want: []*catalog.Repository{
-				{Name: "re", StorageID: "storage", StorageNamespace: "storageNS1", DefaultBranch: "main1", CreationDate: now},
-				{Name: "repo1", StorageID: "storage1", StorageNamespace: "storageNS2", DefaultBranch: "main2", CreationDate: now},
-				{Name: "repo2", StorageID: "storage1", StorageNamespace: "storageNS3", DefaultBranch: "main3", CreationDate: now},
-				{Name: "repo22", StorageID: "storage1", StorageNamespace: "storageNS4", DefaultBranch: "main4", CreationDate: now},
-				{Name: "repo23", StorageID: "storage1", StorageNamespace: "storageNS5", DefaultBranch: "main5", CreationDate: now},
+				{Name: "re", StorageID: "", StorageNamespace: "storageNS1", DefaultBranch: "main1", CreationDate: now},
+				{Name: "repo1", StorageID: "", StorageNamespace: "storageNS2", DefaultBranch: "main2", CreationDate: now},
+				{Name: "repo2", StorageID: "", StorageNamespace: "storageNS3", DefaultBranch: "main3", CreationDate: now},
+				{Name: "repo22", StorageID: "", StorageNamespace: "storageNS4", DefaultBranch: "main4", CreationDate: now},
+				{Name: "repo23", StorageID: "", StorageNamespace: "storageNS5", DefaultBranch: "main5", CreationDate: now},
 				{Name: "repo3", StorageID: "", StorageNamespace: "storageNS6", DefaultBranch: "main6", CreationDate: now},
 			},
 			wantHasMore: false,
@@ -120,7 +120,7 @@ func TestCatalog_ListRepositories(t *testing.T) {
 				searchString: "",
 			},
 			want: []*catalog.Repository{
-				{Name: "re", StorageID: "storage", StorageNamespace: "storageNS1", DefaultBranch: "main1", CreationDate: now},
+				{Name: "re", StorageID: "", StorageNamespace: "storageNS1", DefaultBranch: "main1", CreationDate: now},
 			},
 			wantHasMore: true,
 			wantErr:     false,
@@ -134,7 +134,7 @@ func TestCatalog_ListRepositories(t *testing.T) {
 				searchString: "",
 			},
 			want: []*catalog.Repository{
-				{Name: "repo1", StorageID: "storage1", StorageNamespace: "storageNS2", DefaultBranch: "main2", CreationDate: now},
+				{Name: "repo1", StorageID: "", StorageNamespace: "storageNS2", DefaultBranch: "main2", CreationDate: now},
 			},
 			wantHasMore: true,
 			wantErr:     false,
@@ -148,8 +148,8 @@ func TestCatalog_ListRepositories(t *testing.T) {
 				searchString: "",
 			},
 			want: []*catalog.Repository{
-				{Name: "repo2", StorageID: "storage1", StorageNamespace: "storageNS3", DefaultBranch: "main3", CreationDate: now},
-				{Name: "repo22", StorageID: "storage1", StorageNamespace: "storageNS4", DefaultBranch: "main4", CreationDate: now},
+				{Name: "repo2", StorageID: "", StorageNamespace: "storageNS3", DefaultBranch: "main3", CreationDate: now},
+				{Name: "repo22", StorageID: "", StorageNamespace: "storageNS4", DefaultBranch: "main4", CreationDate: now},
 			},
 			wantHasMore: true,
 			wantErr:     false,
@@ -163,7 +163,7 @@ func TestCatalog_ListRepositories(t *testing.T) {
 				searchString: "",
 			},
 			want: []*catalog.Repository{
-				{Name: "repo1", StorageID: "storage1", StorageNamespace: "storageNS2", DefaultBranch: "main2", CreationDate: now},
+				{Name: "repo1", StorageID: "", StorageNamespace: "storageNS2", DefaultBranch: "main2", CreationDate: now},
 			},
 			wantHasMore: true,
 			wantErr:     false,
@@ -177,7 +177,7 @@ func TestCatalog_ListRepositories(t *testing.T) {
 				searchString: "",
 			},
 			want: []*catalog.Repository{
-				{Name: "repo23", StorageID: "storage1", StorageNamespace: "storageNS5", DefaultBranch: "main5", CreationDate: now},
+				{Name: "repo23", StorageID: "", StorageNamespace: "storageNS5", DefaultBranch: "main5", CreationDate: now},
 				{Name: "repo3", StorageID: "", StorageNamespace: "storageNS6", DefaultBranch: "main6", CreationDate: now},
 			},
 			wantHasMore: false,
@@ -192,9 +192,9 @@ func TestCatalog_ListRepositories(t *testing.T) {
 				searchString: "o2",
 			},
 			want: []*catalog.Repository{
-				{Name: "repo2", StorageID: "storage1", StorageNamespace: "storageNS3", DefaultBranch: "main3", CreationDate: now},
-				{Name: "repo22", StorageID: "storage1", StorageNamespace: "storageNS4", DefaultBranch: "main4", CreationDate: now},
-				{Name: "repo23", StorageID: "storage1", StorageNamespace: "storageNS5", DefaultBranch: "main5", CreationDate: now},
+				{Name: "repo2", StorageID: "", StorageNamespace: "storageNS3", DefaultBranch: "main3", CreationDate: now},
+				{Name: "repo22", StorageID: "", StorageNamespace: "storageNS4", DefaultBranch: "main4", CreationDate: now},
+				{Name: "repo23", StorageID: "", StorageNamespace: "storageNS5", DefaultBranch: "main5", CreationDate: now},
 			},
 			wantHasMore: false,
 			wantErr:     false,
@@ -208,8 +208,8 @@ func TestCatalog_ListRepositories(t *testing.T) {
 				searchString: "o2",
 			},
 			want: []*catalog.Repository{
-				{Name: "repo2", StorageID: "storage1", StorageNamespace: "storageNS3", DefaultBranch: "main3", CreationDate: now},
-				{Name: "repo22", StorageID: "storage1", StorageNamespace: "storageNS4", DefaultBranch: "main4", CreationDate: now},
+				{Name: "repo2", StorageID: "", StorageNamespace: "storageNS3", DefaultBranch: "main3", CreationDate: now},
+				{Name: "repo22", StorageID: "", StorageNamespace: "storageNS4", DefaultBranch: "main4", CreationDate: now},
 			},
 			wantHasMore: true,
 			wantErr:     false,
@@ -223,7 +223,7 @@ func TestCatalog_ListRepositories(t *testing.T) {
 				searchString: "o2",
 			},
 			want: []*catalog.Repository{
-				{Name: "repo23", StorageID: "storage1", StorageNamespace: "storageNS5", DefaultBranch: "main5", CreationDate: now},
+				{Name: "repo23", StorageID: "", StorageNamespace: "storageNS5", DefaultBranch: "main5", CreationDate: now},
 			},
 			wantHasMore: false,
 			wantErr:     false,
@@ -237,8 +237,8 @@ func TestCatalog_ListRepositories(t *testing.T) {
 				searchString: "o2",
 			},
 			want: []*catalog.Repository{
-				{Name: "repo22", StorageID: "storage1", StorageNamespace: "storageNS4", DefaultBranch: "main4", CreationDate: now},
-				{Name: "repo23", StorageID: "storage1", StorageNamespace: "storageNS5", DefaultBranch: "main5", CreationDate: now},
+				{Name: "repo22", StorageID: "", StorageNamespace: "storageNS4", DefaultBranch: "main4", CreationDate: now},
+				{Name: "repo23", StorageID: "", StorageNamespace: "storageNS5", DefaultBranch: "main5", CreationDate: now},
 			},
 			wantHasMore: false,
 			wantErr:     false,
@@ -871,7 +871,7 @@ func createPrepareUncommittedTestScenario(t *testing.T, repositoryID string, num
 	repository := &graveler.RepositoryRecord{
 		RepositoryID: graveler.RepositoryID(repositoryID),
 		Repository: &graveler.Repository{
-			StorageID:        "storage",
+			StorageID:        "",
 			StorageNamespace: graveler.StorageNamespace("mem://" + repositoryID),
 			CreationDate:     time.Now(),
 			DefaultBranchID:  "main",

diff --git a/pkg/graveler/errors.go b/pkg/graveler/errors.go
@@ -30,6 +30,7 @@ var (
 	ErrInvalidTagID                 = fmt.Errorf("tag id: %w", ErrInvalidValue)
 	ErrInvalid                      = errors.New("validation error")
 	ErrInvalidType                  = fmt.Errorf("invalid type: %w", ErrInvalid)
+	ErrInvalidStorageID             = fmt.Errorf("storage id: %w", ErrInvalidValue)
 	ErrInvalidRepositoryID          = fmt.Errorf("repository id: %w", ErrInvalidValue)
 	ErrRequiredValue                = fmt.Errorf("required value: %w", ErrInvalid)
 	ErrCommitNotFound               = fmt.Errorf("commit %w", ErrNotFound)

diff --git a/pkg/graveler/validate.go b/pkg/graveler/validate.go
@@ -7,6 +7,19 @@ import (
 	"github.com/treeverse/lakefs/pkg/validator"
 )
 
+func ValidateStorageID(v interface{}) error {
+	s, ok := v.(StorageID)
+	if !ok {
+		panic(ErrInvalidType)
+	}
+
+	// TODO (gilo): this should somehow be validated in the context of the block storage
+	if len(s) != 0 {
+		return ErrInvalidStorageID
+	}
+	return nil
+}
+
 func ValidateStorageNamespace(v interface{}) error {
 	s, ok := v.(StorageNamespace)
 	if !ok {