diff --git a/artifactory_test.go b/artifactory_test.go index 5cb271e04..ed1d98da9 100644 --- a/artifactory_test.go +++ b/artifactory_test.go @@ -2801,6 +2801,120 @@ func TestArtifactoryDeleteByProps(t *testing.T) { cleanArtifactoryTest() } +// TestArtifactoryDeleteCountsPartialSuccess tests that delete correctly handles +// a mix of successful and failed deletions using CLI's DeleteFiles API. +// Uses permission-based failures (403) to reliably test partial success scenarios. +// - Files in RtRepo1: user has delete permission → success (204) +// - Files in RtRepo2: user has NO delete permission → failure (403) +func TestArtifactoryDeleteCountsPartialSuccess(t *testing.T) { + initArtifactoryTest(t, "") + + // Test user credentials + testUser := "delete-test-user" + testPassword := "DeleteTest123!" + + // Step 1: Create a test user + err := tests.CreateUserWithPassword(serverDetails, testUser, testPassword) + assert.NoError(t, err, "Failed to create test user") + t.Logf("Created test user: %s", testUser) + + // Cleanup user at the end + defer func() { + _ = tests.DeleteUser(serverDetails, testUser) + t.Logf("Cleaned up test user: %s", testUser) + }() + + // Step 2: Create permission target - user can delete from RtRepo1 but NOT from RtRepo2 + permissionName := "delete-partial-test-perm" + err = tests.CreatePermissionTarget(serverDetails, permissionName, tests.RtRepo1, testUser, []string{"read", "write", "delete"}) + assert.NoError(t, err, "Failed to create permission target for RtRepo1") + t.Logf("Created permission target: %s (allows delete on %s)", permissionName, tests.RtRepo1) + + // Cleanup permission at the end + defer func() { + _ = tests.DeletePermissionTarget(serverDetails, permissionName) + t.Logf("Cleaned up permission target: %s", permissionName) + }() + + // Step 3: Upload files as admin to both repos + // 3 files to RtRepo1 (will succeed with delete) + allowedFiles := []string{"f1/a1.in", "f2/a2.in", "f3/a3.in"} + sourceFilesAllowed := []string{"testdata/a/a1.in", "testdata/a/a2.in", "testdata/a/a3.in"} + for i, f := range allowedFiles { + runRt(t, "upload", sourceFilesAllowed[i], tests.RtRepo1+"/delete-partial/"+f) + } + t.Logf("Uploaded %d files to %s (delete allowed)", len(allowedFiles), tests.RtRepo1) + + // 2 files to RtRepo2 (will fail with 403) + deniedFiles := []string{"f4/b1.in", "f5/b2.in"} + sourceFilesDenied := []string{"testdata/a/b/b1.in", "testdata/a/b/b2.in"} + for i, f := range deniedFiles { + runRt(t, "upload", sourceFilesDenied[i], tests.RtRepo2+"/delete-partial/"+f) + } + t.Logf("Uploaded %d files to %s (delete denied)", len(deniedFiles), tests.RtRepo2) + + // Step 4: Create server details for the test user + testUserServerDetails := tests.CreateServerDetailsWithCredentials(serverDetails, testUser, testPassword) + + // Step 5: Build list of all artifact paths (from both repos) + var artifactPaths []string + for _, f := range allowedFiles { + artifactPaths = append(artifactPaths, tests.RtRepo1+"/delete-partial/"+f) + } + for _, f := range deniedFiles { + artifactPaths = append(artifactPaths, tests.RtRepo2+"/delete-partial/"+f) + } + t.Logf("Attempting to delete %d paths with limited user: %v", len(artifactPaths), artifactPaths) + + // Step 6: Delete using the test user's credentials + // - 3 files from RtRepo1 → should succeed (204) + // - 2 files from RtRepo2 → should fail (403) + totalSuccess, totalFail, err := tests.DeleteFilesByPathsUsingCli(testUserServerDetails, artifactPaths) + // Error IS expected because some files return 403 + t.Logf("Delete result: success=%d, fail=%d, err=%v", totalSuccess, totalFail, err) + + // Step 7: Verify counts + expectedSuccess := len(allowedFiles) + expectedFail := len(deniedFiles) + t.Logf("Expected: successCount=%d, failCount=%d", expectedSuccess, expectedFail) + t.Logf("Actual: successCount=%d, failCount=%d", totalSuccess, totalFail) + assert.Equal(t, expectedSuccess, totalSuccess, + "Should have %d successes (files from %s), got %d", expectedSuccess, tests.RtRepo1, totalSuccess) + assert.Equal(t, expectedFail, totalFail, + "Should have %d failures (files from %s - 403), got %d", expectedFail, tests.RtRepo2, totalFail) + + // Step 8: Verify files from RtRepo1 are deleted + searchSpec := spec.NewBuilder(). + Pattern(tests.RtRepo1 + "/delete-partial/*"). + Recursive(true). + BuildSpec() + searchCmd := generic.NewSearchCommand() + searchCmd.SetServerDetails(serverDetails).SetSpec(searchSpec) + reader, err := searchCmd.Search() + assert.NoError(t, err) + repo1Count, err := reader.Length() + assert.NoError(t, err) + readerCloseAndAssert(t, reader) + assert.Equal(t, 0, repo1Count, "All files from %s should be deleted", tests.RtRepo1) + + // Step 9: Verify files from RtRepo2 still exist (delete failed) + searchSpec2 := spec.NewBuilder(). + Pattern(tests.RtRepo2 + "/delete-partial/*"). + Recursive(true). + BuildSpec() + searchCmd2 := generic.NewSearchCommand() + searchCmd2.SetServerDetails(serverDetails).SetSpec(searchSpec2) + reader2, err := searchCmd2.Search() + assert.NoError(t, err) + repo2Count, err := reader2.Length() + assert.NoError(t, err) + readerCloseAndAssert(t, reader2) + assert.Equal(t, len(deniedFiles), repo2Count, "Files from %s should still exist (delete was denied)", tests.RtRepo2) + + // Cleanup + cleanArtifactoryTest() +} + func TestArtifactoryMultipleFileSpecsUpload(t *testing.T) { initArtifactoryTest(t, "") specFile, err := tests.CreateSpec(tests.UploadMultipleFileSpecs) diff --git a/go.mod b/go.mod index 9d2f7c0ca..bdb966365 100644 --- a/go.mod +++ b/go.mod @@ -19,7 +19,7 @@ require ( github.com/jfrog/build-info-go v1.13.1-0.20260107080257-82671efa69a2 github.com/jfrog/gofrog v1.7.6 github.com/jfrog/jfrog-cli-application v1.0.2-0.20251231144110-a68c3ac11c7a - github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20260107090044-56a45e5c560e + github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20260113000842-12090b43088f github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260106204841-744f3f71817b github.com/jfrog/jfrog-cli-evidence v0.8.3-0.20251225153025-9d8ac181d615 github.com/jfrog/jfrog-cli-platform-services v1.10.1-0.20251205121610-171eb9b0000e @@ -285,7 +285,7 @@ replace github.com/gfleury/go-bitbucket-v1 => github.com/gfleury/go-bitbucket-v1 //replace github.com/jfrog/jfrog-cli-core/v2 => ../jfrog-cli-core // replace github.com/jfrog/jfrog-cli-artifactory => github.com/fluxxBot/jfrog-cli-artifactory v0.0.0-20260105073552-ae4f86048a11 -// + //replace github.com/jfrog/build-info-go => github.com/fluxxBot/build-info-go v1.10.10-0.20260105070825-d3f36f619ba5 // //replace github.com/jfrog/jfrog-cli-core/v2 => github.com/fluxxBot/jfrog-cli-core/v2 v2.58.1-0.20260105065921-c6488910f44c diff --git a/go.sum b/go.sum index 00de23985..70c91df11 100644 --- a/go.sum +++ b/go.sum @@ -1216,8 +1216,8 @@ github.com/jfrog/jfrog-apps-config v1.0.1 h1:mtv6k7g8A8BVhlHGlSveapqf4mJfonwvXYL github.com/jfrog/jfrog-apps-config v1.0.1/go.mod h1:8AIIr1oY9JuH5dylz2S6f8Ym2MaadPLR6noCBO4C22w= github.com/jfrog/jfrog-cli-application v1.0.2-0.20251231144110-a68c3ac11c7a h1:XoJ3w2AFi7zniimALNK3idw9bzY9MwB/FM45TMgxYAY= github.com/jfrog/jfrog-cli-application v1.0.2-0.20251231144110-a68c3ac11c7a/go.mod h1:xum2HquWO5uExa/A7MQs3TgJJVEeoqTR+6Z4mfBr1Xw= -github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20260107090044-56a45e5c560e h1:+qB6eWbzeSOh5i6Pc0sC9arG8r5f6GLZm722jDyQ6nI= -github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20260107090044-56a45e5c560e/go.mod h1:U/1q7jEO0YGSAWZEZiEmo0lZHI48xBorsFuL/F8C1fU= +github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20260113000842-12090b43088f h1:pTUm8bp2vjjKCZI8hqJgIrwEc6veb9FU4hSd7ly7zBs= +github.com/jfrog/jfrog-cli-artifactory v0.8.1-0.20260113000842-12090b43088f/go.mod h1:U/1q7jEO0YGSAWZEZiEmo0lZHI48xBorsFuL/F8C1fU= github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260106204841-744f3f71817b h1:gGGmYXuYvcNns1BnLQI13lC+pgMxrmenx+ramtolQuA= github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260106204841-744f3f71817b/go.mod h1:+Hnaikp/xCSPD/q7txxRy4Zc0wzjW/usrCSf+6uONSQ= github.com/jfrog/jfrog-cli-evidence v0.8.3-0.20251225153025-9d8ac181d615 h1:y5an0bojHL00ipHP1QuBUrVcP+XK+yZHHOJ/r1I0RUM= diff --git a/utils/tests/utils.go b/utils/tests/utils.go index 2245a5562..dea01a798 100644 --- a/utils/tests/utils.go +++ b/utils/tests/utils.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "math/rand" + "net/http" "os" "path/filepath" "regexp" @@ -34,9 +35,11 @@ import ( coreTests "github.com/jfrog/jfrog-cli-core/v2/utils/tests" "github.com/jfrog/jfrog-cli/utils/summary" "github.com/jfrog/jfrog-client-go/artifactory/services" - "github.com/jfrog/jfrog-client-go/artifactory/services/utils" + serviceutils "github.com/jfrog/jfrog-client-go/artifactory/services/utils" "github.com/jfrog/jfrog-client-go/auth" + "github.com/jfrog/jfrog-client-go/http/jfroghttpclient" clientutils "github.com/jfrog/jfrog-client-go/utils" + "github.com/jfrog/jfrog-client-go/utils/io/content" "github.com/jfrog/jfrog-client-go/utils/io/fileutils" "github.com/jfrog/jfrog-client-go/utils/log" "github.com/stretchr/testify/assert" @@ -237,6 +240,175 @@ func DeleteFiles(deleteSpec *spec.SpecFiles, serverDetails *config.ServerDetails return deleteCommand.DeleteFiles(reader) } +// DeleteFileDirect deletes a single file by its full Artifactory path (e.g., "repo/path/file.txt") +// This calls the DELETE API directly WITHOUT searching first. +// Returns: +// - success=true, err=nil: File was deleted (HTTP 204) +// - success=false, err=nil: File not found (HTTP 404) +// - success=false, err!=nil: Other error occurred +func DeleteFileDirect(serverDetails *config.ServerDetails, artifactoryPath string) (success bool, err error) { + // Create auth config + artAuth, err := serverDetails.CreateArtAuthConfig() + if err != nil { + return false, err + } + + // Build the full delete URL + deleteUrl := clientutils.AddTrailingSlashIfNeeded(artAuth.GetUrl()) + artifactoryPath + + // Create HTTP client + client, err := jfroghttpclient.JfrogClientBuilder(). + SetInsecureTls(serverDetails.InsecureTls). + AppendPreRequestInterceptor(artAuth.RunPreRequestFunctions). + Build() + if err != nil { + return false, err + } + + // Create HTTP client details with auth + httpClientsDetails := artAuth.CreateHttpClientDetails() + + // Send DELETE request directly + resp, body, err := client.SendDelete(deleteUrl, nil, &httpClientsDetails) + if err != nil { + return false, err + } + + // Check response status + switch resp.StatusCode { + case http.StatusNoContent: + // Successfully deleted + return true, nil + case http.StatusNotFound: + // File not found + return false, nil + default: + // Other error + return false, fmt.Errorf("unexpected status %d: %s", resp.StatusCode, string(body)) + } +} + +// DeleteFilesByPathsUsingCli deletes files by their explicit paths using the CLI's DeleteCommand.DeleteFiles API. +// This bypasses the search phase and directly calls delete on the provided paths. +// Returns successCount, failedCount, err - where failedCount includes 404 errors. +func DeleteFilesByPathsUsingCli(serverDetails *config.ServerDetails, artifactoryPaths []string) (successCount, failedCount int, err error) { + // Create a ContentWriter to build a reader with the file paths + writer, err := content.NewContentWriter(content.DefaultKey, true, false) + if err != nil { + return 0, 0, err + } + + // Write each path as a ResultItem + for _, artifactPath := range artifactoryPaths { + // Parse the path into repo/path/name + parts := strings.SplitN(artifactPath, "/", 2) + repo := parts[0] + pathAndName := "" + name := "" + if len(parts) > 1 { + pathAndName = parts[1] + // Split into path and name + lastSlash := strings.LastIndex(pathAndName, "/") + if lastSlash >= 0 { + pathAndName = pathAndName[:lastSlash] + name = artifactPath[strings.LastIndex(artifactPath, "/")+1:] + } else { + name = pathAndName + pathAndName = "." + } + } + item := serviceutils.ResultItem{ + Repo: repo, + Path: pathAndName, + Name: name, + Type: "file", + } + writer.Write(item) + } + + err = writer.Close() + if err != nil { + return 0, 0, err + } + + // Create a reader from the writer's file + reader := content.NewContentReader(writer.GetFilePath(), content.DefaultKey) + defer func() { + closeErr := reader.Close() + if err == nil { + err = closeErr + } + }() + + // Create DeleteCommand and call DeleteFiles + deleteCmd := generic.NewDeleteCommand() + deleteCmd.SetServerDetails(serverDetails) + + return deleteCmd.DeleteFiles(reader) +} + +// CreateUserWithPassword creates a new user with the specified password. +func CreateUserWithPassword(serverDetails *config.ServerDetails, username, password string) error { + servicesManager, err := artUtils.CreateServiceManager(serverDetails, -1, 0, false) + if err != nil { + return err + } + adminFalse := false + userParams := services.NewUserParams() + userParams.UserDetails.Name = username + userParams.UserDetails.Email = username + "@test.com" + userParams.UserDetails.Password = password + userParams.UserDetails.Admin = &adminFalse + return servicesManager.CreateUser(userParams) +} + +// DeleteUser deletes a user. +func DeleteUser(serverDetails *config.ServerDetails, username string) error { + servicesManager, err := artUtils.CreateServiceManager(serverDetails, -1, 0, false) + if err != nil { + return err + } + return servicesManager.DeleteUser(username) +} + +// CreatePermissionTarget creates a permission target giving the specified user permissions on the specified repo. +func CreatePermissionTarget(serverDetails *config.ServerDetails, permName, repoKey, username string, actions []string) error { + servicesManager, err := artUtils.CreateServiceManager(serverDetails, -1, 0, false) + if err != nil { + return err + } + params := services.NewPermissionTargetParams() + params.Name = permName + params.Repo = &services.PermissionTargetSection{ + Repositories: []string{repoKey}, + Actions: &services.Actions{ + Users: map[string][]string{ + username: actions, + }, + }, + } + return servicesManager.CreatePermissionTarget(params) +} + +// DeletePermissionTarget deletes a permission target. +func DeletePermissionTarget(serverDetails *config.ServerDetails, permName string) error { + servicesManager, err := artUtils.CreateServiceManager(serverDetails, -1, 0, false) + if err != nil { + return err + } + return servicesManager.DeletePermissionTarget(permName) +} + +// CreateServerDetailsWithCredentials creates a copy of server details with different credentials. +func CreateServerDetailsWithCredentials(original *config.ServerDetails, username, password string) *config.ServerDetails { + newDetails := *original + newDetails.SetUser(username) + newDetails.SetPassword(password) + // Clear access token if using username/password + newDetails.SetAccessToken("") + return &newDetails +} + // This function makes no assertion, caller is responsible to assert as needed. func GetBuildInfo(serverDetails *config.ServerDetails, buildName, buildNumber string) (pbi *buildinfo.PublishedBuildInfo, found bool, err error) { servicesManager, err := artUtils.CreateServiceManager(serverDetails, -1, 0, false) @@ -614,7 +786,7 @@ func CreateSpec(fileName string) (string, error) { return searchFilePath, err } -func ConvertSliceToMap(props []utils.Property) map[string][]string { +func ConvertSliceToMap(props []serviceutils.Property) map[string][]string { propsMap := make(map[string][]string) for _, item := range props { propsMap[item.Key] = append(propsMap[item.Key], item.Value)