diff --git a/pkg/chartmuseum/server/multitenant/api.go b/pkg/chartmuseum/server/multitenant/api.go index ac3f667a..7ba6035f 100644 --- a/pkg/chartmuseum/server/multitenant/api.go +++ b/pkg/chartmuseum/server/multitenant/api.go @@ -197,7 +197,12 @@ func (server *MultiTenantServer) uploadProvenanceFile(log cm_logger.LoggingFn, r func (server *MultiTenantServer) checkStorageLimit(repo string, filename string, force bool) (bool, error) { if server.MaxStorageObjects > 0 { - allObjects, err := server.StorageBackend.ListObjects(repo) + // Ensure proper prefix for tenant repos by adding trailing slash + prefix := repo + if repo != "" { + prefix = repo + "/" + } + allObjects, err := server.StorageBackend.ListObjects(prefix) if err != nil { return false, err } @@ -246,7 +251,12 @@ func (server *MultiTenantServer) PutWithLimit(ctx *gin.Context, log cm_logger.Lo defer server.ChartLimits.Unlock() // clean the oldest chart(both index and storage) // storage cache first - objs, err := server.StorageBackend.ListObjects(repo) + // Ensure proper prefix for tenant repos by adding trailing slash + prefix := repo + if repo != "" { + prefix = repo + "/" + } + objs, err := server.StorageBackend.ListObjects(prefix) if err != nil { return err } diff --git a/pkg/chartmuseum/server/multitenant/cache.go b/pkg/chartmuseum/server/multitenant/cache.go index 60fdfb9b..2bcda164 100644 --- a/pkg/chartmuseum/server/multitenant/cache.go +++ b/pkg/chartmuseum/server/multitenant/cache.go @@ -198,7 +198,12 @@ func (server *MultiTenantServer) fetchChartsInStorage(log cm_logger.LoggingFn, r log(cm_logger.DebugLevel, "Fetching chart list from storage", "repo", repo, ) - allObjects, err := server.StorageBackend.ListObjects(repo) + // Ensure proper prefix for tenant repos by adding trailing slash + prefix := repo + if repo != "" { + prefix = repo + "/" + } + allObjects, err := server.StorageBackend.ListObjects(prefix) if err != nil { return []cm_storage.Object{}, err } diff --git a/pkg/chartmuseum/server/multitenant/server_test.go b/pkg/chartmuseum/server/multitenant/server_test.go index 02dfa8a3..c87f4bdf 100644 --- a/pkg/chartmuseum/server/multitenant/server_test.go +++ b/pkg/chartmuseum/server/multitenant/server_test.go @@ -18,6 +18,7 @@ package multitenant import ( "bytes" + "errors" "fmt" "io" "mime/multipart" @@ -39,6 +40,8 @@ import ( "sigs.k8s.io/yaml" ) +var TenantPrefixFixTestError = errors.New("tenant prefix test error") + var maxUploadSize = 1024 * 1024 * 20 // These are generated from scripts/setup-test-environment.sh @@ -1359,3 +1362,126 @@ func (suite *MultiTenantServerTestSuite) getBodyWithMultipartFormFiles(fields [] func TestMultiTenantServerTestSuite(t *testing.T) { suite.Run(t, new(MultiTenantServerTestSuite)) } + +var depthPrefixes = map[int]string{ + 0: "", // depth 0: charts in root + 1: "org1", // depth 1: org/charts + 2: "org1/team1", // depth 2: org/team/charts + 3: "org1/team1/repo1", // depth 3: org/team/repo/charts +} + +// TenantPrefixTestSuite tests that tenant prefixes are properly isolated +// across different depth configurations. It uses real chart files and local +// storage to validate the prefix-handling behavior. +type TenantPrefixTestSuite struct { + suite.Suite + StorageBackend storage.Backend + Logger *cm_logger.Logger + TempDirectory string +} + +func (suite *TenantPrefixTestSuite) SetupSuite() { + // Create test logger + logger, err := cm_logger.NewLogger(cm_logger.LoggerOptions{ + Debug: true, + }) + suite.Nil(err, "no error creating logger") + suite.Logger = logger + + // Create temporary directory + timestamp := time.Now().Format("20060102150405") + suite.TempDirectory = fmt.Sprintf("../../../../.test/chartmuseum-tenant-prefix/%s", timestamp) + err = os.MkdirAll(suite.TempDirectory, os.ModePerm) + suite.Nil(err, "no error creating temp directory") + + // Create test directories for each depth + for depth, path := range depthPrefixes { + if path != "" { + err = os.MkdirAll(pathutil.Join(suite.TempDirectory, path), os.ModePerm) + suite.Nil(err, fmt.Sprintf("no error creating directory for depth %d", depth)) + } + + // Copy test charts into each directory + testFiles := []struct { + src string + dest string + }{ + {testTarballPath, pathutil.Join(suite.TempDirectory, path, "mychart-0.1.0.tgz")}, + {testTarballPath, pathutil.Join(suite.TempDirectory, path, "mychart-0.2.0.tgz")}, + } + + for _, tf := range testFiles { + srcFile, err := os.Open(tf.src) + suite.Nil(err, fmt.Sprintf("no error opening %s", tf.src)) + defer srcFile.Close() + + destFile, err := os.Create(tf.dest) + suite.Nil(err, fmt.Sprintf("no error creating %s", tf.dest)) + defer destFile.Close() + + _, err = io.Copy(destFile, srcFile) + suite.Nil(err, fmt.Sprintf("no error copying %s to %s", tf.src, tf.dest)) + } + } + + // Create storage backend pointing to temp directory + suite.StorageBackend = storage.NewLocalFilesystemBackend(suite.TempDirectory) +} + +func (suite *TenantPrefixTestSuite) TearDownSuite() { + os.RemoveAll(suite.TempDirectory) +} + +// TestTenantPrefixSeparation tests that tenant prefixes are properly isolated +// across different depth configurations (0-3) +func (suite *TenantPrefixTestSuite) TestTenantPrefixSeparation() { + for _, depth := range []int{0, 1, 2, 3} { + suite.Run(fmt.Sprintf("depth%d", depth), func() { + log := suite.Logger.ContextLoggingFn(&gin.Context{}) + + router := cm_router.NewRouter(cm_router.RouterOptions{ + Logger: suite.Logger, + Depth: depth, + }) + + server, err := NewMultiTenantServer(MultiTenantServerOptions{ + Logger: suite.Logger, + Router: router, + StorageBackend: suite.StorageBackend, + TimestampTolerance: time.Second * 0, + EnableAPI: true, + ChartPostFormFieldName: "chart", + ProvPostFormFieldName: "prov", + CacheInterval: time.Second * 0, + }) + suite.Nil(err, "no error creating server") + + // Test fetchChartsInStorage which handles prefix isolation + prefix := depthPrefixes[depth] + objects, err := server.fetchChartsInStorage(log, prefix) + suite.Nil(err, "no error fetching charts") + + // Should find exactly 2 objects + suite.Equal(2, len(objects), fmt.Sprintf("should find exactly 2 objects for depth %d", depth)) + + // Verify the paths of the returned objects + foundPaths := map[string]bool{} + for _, obj := range objects { + foundPaths[obj.Path] = true + } + + // Storage backend strips the prefix, so we always expect bare filenames + expectedPath1 := "mychart-0.1.0.tgz" + expectedPath2 := "mychart-0.2.0.tgz" + + suite.True(foundPaths[expectedPath1], + fmt.Sprintf("should include %s", expectedPath1)) + suite.True(foundPaths[expectedPath2], + fmt.Sprintf("should include %s", expectedPath2)) + }) + } +} + +func TestTenantPrefixTestSuite(t *testing.T) { + suite.Run(t, new(TenantPrefixTestSuite)) +}