Consult artifact repository in artifact check

Artifacts may be downloaded in the start task from repository A, but
should be uploaded to repository B in the finish task. In this case,
Contains erroneously returned true before this change. Contains should
only return true if the artifacts were downloaded from the same
repository earlier. When the repositories differ, the task must fetch a
manifest from the target repository before checking the manifest.

CHANGELOG.md

@@ -12,6 +12,7 @@ listed in the changelog.
 
 ### Fixed
 
+- Artifacts may not be uploaded to target repository when the target repository differs from the source repository ([#715](https://github.com/opendevstack/ods-pipeline/pull/715))
 - Gradle build script changes twice into working dir ([#705](https://github.com/opendevstack/ods-pipeline/issues/705))
 
 ## [0.13.1] - 2023-06-05

cmd/finish/main.go

@@ -213,19 +213,27 @@ func uploadArtifacts(
 	if err != nil {
 		return err
 	}
+	if am.Repository != nexusRepository {
+		logger.Infof("Artifacts were downloaded from %q but should be uploaded to %q. Checking for existing artifacts in %q before proceeding ...", am.Repository, nexusRepository, nexusRepository)
+		group := pipelinectxt.ArtifactGroupBase(ctxt)
+		am, err = pipelinectxt.DownloadGroup(nexusClient, nexusRepository, group, "", logger)
+		if err != nil {
+			return err
+		}
+	}
 	for artifactsSubDir, files := range artifactsMap {
 		for _, filename := range files {
-			if am.Contains(artifactsSubDir, filename) {
-				logger.Infof("Artifact %s is already present in Nexus repository %s.", filename, nexusRepository)
+			if am.Contains(nexusRepository, artifactsSubDir, filename) {
+				logger.Infof("Artifact %q is already present in Nexus repository %q.", filename, nexusRepository)
 			} else {
 				nexusGroup := artifactGroup(ctxt, artifactsSubDir, opts)
 				localFile := filepath.Join(checkoutDir, pipelinectxt.ArtifactsPath, artifactsSubDir, filename)
-				logger.Infof("Uploading %s to Nexus repository %s, group %s ...", localFile, nexusRepository, nexusGroup)
+				logger.Infof("Uploading %q to Nexus repository %q, group %q ...", localFile, nexusRepository, nexusGroup)
 				link, err := nexusClient.Upload(nexusRepository, nexusGroup, localFile)
 				if err != nil {
 					return err
 				}
-				logger.Infof("Successfully uploaded %s to %s", localFile, link)
+				logger.Infof("Successfully uploaded %q to %s", localFile, link)
 			}
 		}
 	}

cmd/finish/main_test.go

@@ -28,33 +28,35 @@ func TestUploadArtifacts(t *testing.T) {
 		Repository:   "my-repo",
 		GitCommitSHA: "8d351a10fb428c0c1239530256e21cf24f136e73",
 	}
-	t.Log("Write dummy artifact")
+	t.Logf("Write dummy image-digest artifact to %q\n", pipelinectxt.ArtifactsPath)
 	artifactsDir := filepath.Join(tempWorkingDir, pipelinectxt.ArtifactsPath)
 	err = writeArtifactFile(artifactsDir, pipelinectxt.ImageDigestsDir, "foo.json")
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	err = uploadArtifacts(logger, nexusClient, nexusRepo, tempWorkingDir, ctxt, options{aggregateTasksStatus: "Succeeded"})
-	if err != nil {
+	t.Logf("Upload dummy image-digest artifact to %q on PR failure\n", nexusRepo)
+	opts := options{pipelineRunName: "pipelineRun", aggregateTasksStatus: "Failed"}
+	if err := uploadArtifacts(logger, nexusClient, nexusRepo, tempWorkingDir, ctxt, opts); err != nil {
 		t.Fatal(err)
 	}
 	if len(nexusClient.Artifacts[nexusRepo]) != 1 {
 		t.Fatalf("want 1 uploaded file, got: %v", nexusClient.Artifacts[nexusRepo])
 	}
-	wantFile := "/my-project/my-repo/8d351a10fb428c0c1239530256e21cf24f136e73/image-digests/foo.json"
+	wantFile := "/my-project/my-repo/8d351a10fb428c0c1239530256e21cf24f136e73/failed-pipelineRun-artifacts/image-digests/foo.json"
 	if !nexusRepoContains(nexusClient.Artifacts[nexusRepo], wantFile) {
 		t.Fatalf("want: %s, got: %s", wantFile, nexusClient.Artifacts[nexusRepo][0])
 	}
 
-	err = uploadArtifacts(logger, nexusClient, nexusRepo, tempWorkingDir, ctxt, options{pipelineRunName: "pipelineRun", aggregateTasksStatus: "Failed"})
-	if err != nil {
+	t.Logf("Upload dummy image-digest artifact to %q on PR success\n", nexusRepo)
+	opts = options{pipelineRunName: "pipelineRun", aggregateTasksStatus: "Succeeded"}
+	if err := uploadArtifacts(logger, nexusClient, nexusRepo, tempWorkingDir, ctxt, opts); err != nil {
 		t.Fatal(err)
 	}
 	if len(nexusClient.Artifacts[nexusRepo]) != 2 {
-		t.Fatalf("expected one additional file upload, got: %v", nexusClient.Artifacts[nexusRepo])
+		t.Fatalf("expected two artifacts in repo, got: %v", nexusClient.Artifacts[nexusRepo])
 	}
-	wantFile = "/my-project/my-repo/8d351a10fb428c0c1239530256e21cf24f136e73/failed-pipelineRun-artifacts/image-digests/foo.json"
+	wantFile = "/my-project/my-repo/8d351a10fb428c0c1239530256e21cf24f136e73/image-digests/foo.json"
 	if !nexusRepoContains(nexusClient.Artifacts[nexusRepo], wantFile) {
 		t.Fatalf("want: %s, got: %s", wantFile, nexusClient.Artifacts[nexusRepo][1])
 	}
@@ -109,7 +111,7 @@ func TestHandleArtifacts(t *testing.T) {
 	}
 	t.Log("Write empty artifacts manifest for subrepository")
 	subrepoArtifactsDir := filepath.Join(subrepoDir, pipelinectxt.ArtifactsPath)
-	err = writeArtifactsManifest(subrepoArtifactsDir)
+	err = writeArtifactsManifest("nexus-repo", subrepoArtifactsDir)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -156,7 +158,7 @@ func prepareTempWorkingDir(nexusRepo string) (string, func(), error) {
 	}
 	cleanup = func() { os.RemoveAll(tempWorkingDir) }
 	artifactsDir := filepath.Join(tempWorkingDir, pipelinectxt.ArtifactsPath)
-	err = writeArtifactsManifest(artifactsDir)
+	err = writeArtifactsManifest("nexus-repo", artifactsDir)
 	if err != nil {
 		return tempWorkingDir, cleanup, err
 	}
@@ -190,9 +192,7 @@ func writeArtifactFile(artifactsDir, subdir, filename string) error {
 }
 
 // writeArtifactsManifest writes an artigact manifest JSON file into artifactsDir.
-func writeArtifactsManifest(artifactsDir string) error {
-	am := &pipelinectxt.ArtifactsManifest{
-		Artifacts: []pipelinectxt.ArtifactInfo{},
-	}
+func writeArtifactsManifest(repository, artifactsDir string) error {
+	am := pipelinectxt.NewArtifactsManifest(repository)
 	return pipelinectxt.WriteJsonArtifact(am, artifactsDir, pipelinectxt.ArtifactsManifestFilename)
 }

cmd/start/main.go

@@ -232,7 +232,7 @@ func main() {
 }
 
 func writeEmptyArtifactManifests(subrepoContexts []*pipelinectxt.ODSContext) error {
-	emptyManifest := &pipelinectxt.ArtifactsManifest{Artifacts: []pipelinectxt.ArtifactInfo{}}
+	emptyManifest := pipelinectxt.NewArtifactsManifest("")
 	err := pipelinectxt.WriteJsonArtifact(emptyManifest, pipelinectxt.ArtifactsPath, pipelinectxt.ArtifactsManifestFilename)
 	if err != nil {
 		return fmt.Errorf("write repo empty manifest: %w", err)

pkg/pipelinectxt/artifacts.go

@@ -44,6 +44,9 @@ const (
 
 // ArtifactsManifest represents all downloaded artifacts.
 type ArtifactsManifest struct {
+	// Repository is the artifact repository from which the manifests were downloaded.
+	Repository string `json:"repository"`
+	// Artifacts lists all artifacts downloaded.
 	Artifacts []ArtifactInfo `json:"artifacts"`
 }
 
@@ -54,6 +57,11 @@ type ArtifactInfo struct {
 	Name      string `json:"name"`
 }
 
+// NewArtifactsManifest returns a new ArtifactsManifest instance.
+func NewArtifactsManifest(repository string, artifacts ...ArtifactInfo) *ArtifactsManifest {
+	return &ArtifactsManifest{Repository: repository, Artifacts: artifacts}
+}
+
 // ReadArtifactsManifestFromFile reads an artifact manifest from given filename or errors.
 func ReadArtifactsManifestFromFile(filename string) (*ArtifactsManifest, error) {
 	body, err := os.ReadFile(filename)
@@ -72,7 +80,10 @@ func ReadArtifactsManifestFromFile(filename string) (*ArtifactsManifest, error)
 }
 
 // Contains checks whether given directory/name is already present in repository.
-func (am *ArtifactsManifest) Contains(directory, name string) bool {
+func (am *ArtifactsManifest) Contains(repository, directory, name string) bool {
+	if am.Repository != repository {
+		return false
+	}
 	for _, a := range am.Artifacts {
 		if a.Directory == directory && a.Name == name {
 			return true
@@ -203,46 +214,53 @@ func ArtifactGroup(ctxt *ODSContext, subdir string) string {
 
 // DownloadGroup searches given repositories in order for assets in given group.
 // As soon as one repository has any asset in the group, the search is stopped
-// and all fond artifacts are downloaded into artifactsDir.
+// and all found artifacts are downloaded into artifactsDir.
 // An artifacts manifest is returned describing the downloaded files.
 // When none of the given repositories contains any artifacts under the group,
 // no artifacts are downloaded and no error is returned.
-func DownloadGroup(nexusClient nexus.ClientInterface, repository string, group, artifactsDir string, logger logging.LeveledLoggerInterface) (*ArtifactsManifest, error) {
+// If artifactsDir is an empty string, the searched files are not downloaded but
+// the artifacts are still recorded in the returned manifest.
+func DownloadGroup(
+	nexusClient nexus.ClientInterface,
+	repository, group, artifactsDir string,
+	logger logging.LeveledLoggerInterface) (*ArtifactsManifest, error) {
 	// We want to target all artifacts underneath the group, hence the trailing '*'.
 	nexusSearchGroup := fmt.Sprintf("%s/*", group)
-	am := &ArtifactsManifest{
-		Artifacts: []ArtifactInfo{},
-	}
+	am := NewArtifactsManifest(repository)
 	urls, err := searchForAssets(nexusClient, nexusSearchGroup, repository, logger)
 	if err != nil {
 		return nil, err
 	}
 
+	if artifactsDir == "" {
+		logger.Debugf("Artifacts will not be downloaded but only added to the manifest ...")
+	}
+
 	for _, s := range urls {
 		u, err := url.Parse(s)
 		if err != nil {
 			return nil, err
 		}
-		urlPathParts := strings.Split(u.Path, fmt.Sprintf("%s/", group))
-		if len(urlPathParts) != 2 {
-			return nil, fmt.Errorf("unexpected URL path (must contain two parts after group '%s'): %s", group, u.Path)
+		_, fileWithSubPath, ok := strings.Cut(u.Path, fmt.Sprintf("%s/", group)) // e.g. "pipeline-runs/foo-zh9gt0.json"
+		if !ok {
+			return nil, fmt.Errorf("unexpected URL path (must contain group '%s'): %s", group, u.Path)
 		}
-		fileWithSubPath := urlPathParts[1] // e.g. "pipeline-runs/foo-zh9gt0.json"
 		if !strings.Contains(fileWithSubPath, "/") {
 			return nil, fmt.Errorf("unexpected URL path (must contain a subfolder after the commit SHA): %s", fileWithSubPath)
 		}
 		aritfactName := path.Base(fileWithSubPath) // e.g. "pipeline-runs"
 		artifactType := path.Dir(fileWithSubPath)  // e.g. "foo-zh9gt0.json"
-		artifactsSubPath := filepath.Join(artifactsDir, artifactType)
-		if _, err := os.Stat(artifactsSubPath); os.IsNotExist(err) {
-			if err := os.MkdirAll(artifactsSubPath, 0755); err != nil {
-				return nil, fmt.Errorf("failed to create directory: %s, error: %w", artifactsSubPath, err)
+		if artifactsDir != "" {
+			artifactsSubPath := filepath.Join(artifactsDir, artifactType)
+			if _, err := os.Stat(artifactsSubPath); os.IsNotExist(err) {
+				if err := os.MkdirAll(artifactsSubPath, 0755); err != nil {
+					return nil, fmt.Errorf("failed to create directory: %s, error: %w", artifactsSubPath, err)
+				}
+			}
+			outfile := filepath.Join(artifactsDir, fileWithSubPath)
+			if _, err := nexusClient.Download(s, outfile); err != nil {
+				return nil, err
 			}
-		}
-		outfile := filepath.Join(artifactsDir, fileWithSubPath)
-		_, err = nexusClient.Download(s, outfile)
-		if err != nil {
-			return nil, err
 		}
 		am.Artifacts = append(am.Artifacts, ArtifactInfo{
 			URL:       s,
@@ -261,9 +279,9 @@ func searchForAssets(nexusClient nexus.ClientInterface, searchGroup string, repo
 		return nil, err
 	}
 	if len(urls) > 0 {
-		logger.Infof("Found artifacts in repository %s inside group %s ...", repository, searchGroup)
+		logger.Infof("Found artifacts in repository %q inside group %q ...", repository, searchGroup)
 		return urls, nil
 	}
-	logger.Infof("No artifacts found in repository %s inside group %s.", repository, searchGroup)
+	logger.Infof("No artifacts found in repository %q inside group %q.", repository, searchGroup)
 	return []string{}, nil
 }

pkg/pipelinectxt/artifacts_test.go

@@ -118,6 +118,84 @@ func TestDownloadGroup(t *testing.T) {
 	}
 }
 
+func TestContains(t *testing.T) {
+	tests := map[string]struct {
+		manifest *ArtifactsManifest
+		repo     string
+		dir      string
+		name     string
+		want     bool
+	}{
+		"different repo": {
+			manifest: &ArtifactsManifest{
+				Repository: "a",
+				Artifacts: []ArtifactInfo{
+					{
+						Directory: "b",
+						Name:      "c",
+					},
+				},
+			},
+			repo: "x",
+			dir:  "b",
+			name: "c",
+			want: false,
+		},
+		"same repo, different dir": {
+			manifest: &ArtifactsManifest{
+				Repository: "a",
+				Artifacts: []ArtifactInfo{
+					{
+						Directory: "b",
+						Name:      "c",
+					},
+				},
+			},
+			repo: "a",
+			dir:  "x",
+			name: "c",
+			want: false,
+		},
+		"same repo, same dir, different name": {
+			manifest: &ArtifactsManifest{
+				Repository: "a",
+				Artifacts: []ArtifactInfo{
+					{
+						Directory: "b",
+						Name:      "c",
+					},
+				},
+			},
+			repo: "a",
+			dir:  "b",
+			name: "x",
+			want: false,
+		},
+		"match": {
+			manifest: &ArtifactsManifest{
+				Repository: "a",
+				Artifacts: []ArtifactInfo{
+					{
+						Directory: "b",
+						Name:      "c",
+					},
+				},
+			},
+			repo: "a",
+			dir:  "b",
+			name: "c",
+			want: true,
+		},
+	}
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			if tc.manifest.Contains(tc.repo, tc.dir, tc.name) != tc.want {
+				t.Errorf("Want %q to contain=%v, but did not", name, tc.want)
+			}
+		})
+	}
+}
+
 func findArtifact(url string, artifacts []ArtifactInfo) *ArtifactInfo {
 	for _, a := range artifacts {
 		if a.URL == url {

test/tasks/ods-finish_test.go

@@ -59,14 +59,13 @@ func TestTaskODSFinish(t *testing.T) {
 					); err != nil {
 						t.Fatal(err)
 					}
-					am := pipelinectxt.ArtifactsManifest{
-						Artifacts: []pipelinectxt.ArtifactInfo{
-							{
-								Directory: pipelinectxt.CodeCoveragesDir,
-								Name:      "coverage.out",
-							},
+					am := pipelinectxt.NewArtifactsManifest(
+						nexus.TestTemporaryRepository,
+						pipelinectxt.ArtifactInfo{
+							Directory: pipelinectxt.CodeCoveragesDir,
+							Name:      "coverage.out",
 						},
-					}
+					)
 					if err := pipelinectxt.WriteJsonArtifact(
 						am,
 						filepath.Join(wsDir, pipelinectxt.ArtifactsPath),
@@ -87,7 +86,7 @@ func TestTaskODSFinish(t *testing.T) {
 					checkBuildStatus(t, bitbucketClient, ctxt.ODS.GitCommitSHA, bitbucket.BuildStatusSuccessful)
 					checkArtifactsAreInNexus(t, ctxt, nexus.TestTemporaryRepository)
 
-					wantLogMsg := "Artifact coverage.out is already present in Nexus repository"
+					wantLogMsg := "Artifact \"coverage.out\" is already present in Nexus repository"
 					if !strings.Contains(string(ctxt.CollectedLogs), wantLogMsg) {
 						t.Fatalf("Want:\n%s\n\nGot:\n%s", wantLogMsg, string(ctxt.CollectedLogs))
 					}