Merge pull request #715 from opendevstack/fix/artifacts-upload

Consult artifact repository in artifact check

Author: Michael Sauter

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))
 					}