Add get-by-name to artifact get (#1990)

* Add SQL search for artifact versions by artifact name * Add gRPC call to get artifact by name * artifact get: Get artifact by name * Reduce complexity of artifact_get's main command * use cobra to mark either of name,id required * Fix URL collision for artifact get
mindersec · Dec 21, 2023 · 8156178 · 8156178
1 parent c228baa
commit 8156178
Show file tree

Hide file tree

Showing 12 changed files with 2,734 additions and 2,069 deletions.
diff --git a/cmd/cli/app/artifact/artifact_get.go b/cmd/cli/app/artifact/artifact_get.go
@@ -17,14 +17,15 @@ package artifact
 
 import (
 	"context"
+	"errors"
 	"fmt"
-	"os"
 	"strings"
 	"time"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"google.golang.org/grpc"
+	"google.golang.org/protobuf/reflect/protoreflect"
 
 	"github.com/stacklok/minder/cmd/cli/app"
 	"github.com/stacklok/minder/internal/util"
@@ -48,6 +49,7 @@ func getCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn)
 	project := viper.GetString("project")
 	tag := viper.GetString("tag")
 	artifactID := viper.GetString("id")
+	artifactName := viper.GetString("name")
 	latestVersions := viper.GetInt32("versions")
 	format := viper.GetString("output")
 
@@ -61,33 +63,27 @@ func getCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn)
 		return cli.MessageAndError(fmt.Sprintf("Output format %s not supported", format), fmt.Errorf("invalid argument"))
 	}
 
-	// check artifact by name
-	art, err := client.GetArtifactById(ctx, &minderv1.GetArtifactByIdRequest{
-		Context:        &minderv1.Context{Provider: &provider, Project: &project},
-		Id:             artifactID,
-		LatestVersions: latestVersions,
-		Tag:            tag,
-	})
+	pbArt, art, versions, err := artifactGet(ctx, client, provider, project, artifactID, artifactName, latestVersions, tag)
 	if err != nil {
-		return cli.MessageAndError("Error getting artifact by id", err)
+		return cli.MessageAndError("Error getting artifact", err)
 	}
 
 	switch format {
 	case app.Table:
 		ta := table.New(table.Simple, "", []string{"ID", "Type", "Owner", "Name", "Repository", "Visibility", "Creation date"})
 		ta.AddRow([]string{
-			art.Artifact.ArtifactPk,
-			art.Artifact.Type,
-			art.Artifact.GetOwner(),
-			art.Artifact.GetName(),
-			art.Artifact.Repository,
-			art.Artifact.Visibility,
-			art.Artifact.CreatedAt.AsTime().Format(time.RFC3339),
+			art.ArtifactPk,
+			art.Type,
+			art.GetOwner(),
+			art.GetName(),
+			art.Repository,
+			art.Visibility,
+			art.CreatedAt.AsTime().Format(time.RFC3339),
 		})
 		ta.Render()
 
 		tv := table.New(table.Simple, "", []string{"ID", "Tags", "Signature", "Identity", "Creation date"})
-		for _, version := range art.Versions {
+		for _, version := range versions {
 			tv.AddRow([]string{
 				fmt.Sprintf("%d", version.VersionId),
 				strings.Join(version.Tags, ","),
@@ -98,13 +94,13 @@ func getCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn)
 		}
 		tv.Render()
 	case app.JSON:
-		out, err := util.GetJsonFromProto(art)
+		out, err := util.GetJsonFromProto(pbArt)
 		if err != nil {
 			return cli.MessageAndError("Error getting json from proto", err)
 		}
 		cmd.Println(out)
 	case app.YAML:
-		out, err := util.GetYamlFromProto(art)
+		out, err := util.GetYamlFromProto(pbArt)
 		if err != nil {
 			return cli.MessageAndError("Error getting yaml from proto", err)
 		}
@@ -114,6 +110,51 @@ func getCommand(ctx context.Context, cmd *cobra.Command, conn *grpc.ClientConn)
 	return nil
 }
 
+func artifactGet(
+	ctx context.Context,
+	client minderv1.ArtifactServiceClient,
+	provider string, project string,
+	artifactID string, artifactName string, latestVersions int32, tag string,
+) (pbArt protoreflect.ProtoMessage, art *minderv1.Artifact, versions []*minderv1.ArtifactVersion, err error) {
+
+	if artifactName != "" {
+		// check artifact by Name
+		artByName, errGet := client.GetArtifactByName(ctx, &minderv1.GetArtifactByNameRequest{
+			Context:        &minderv1.Context{Provider: &provider, Project: &project},
+			Name:           artifactName,
+			LatestVersions: latestVersions,
+			Tag:            tag,
+		})
+		if errGet != nil {
+			err = fmt.Errorf("error getting artifact by name: %w", errGet)
+			return
+		}
+		pbArt = artByName
+		art = artByName.GetArtifact()
+		versions = artByName.GetVersions()
+		return
+	} else if artifactID != "" {
+		// check artifact by ID
+		artById, errGet := client.GetArtifactById(ctx, &minderv1.GetArtifactByIdRequest{
+			Context:        &minderv1.Context{Provider: &provider, Project: &project},
+			Id:             artifactID,
+			LatestVersions: latestVersions,
+			Tag:            tag,
+		})
+		if errGet != nil {
+			err = fmt.Errorf("error getting artifact by id: %w", errGet)
+			return
+		}
+		pbArt = artById
+		art = artById.GetArtifact()
+		versions = artById.GetVersions()
+		return
+	}
+
+	err = errors.New("neither name nor ID set")
+	return
+}
+
 func getSignatureStatusText(sigVer *minderv1.SignatureVerification) string {
 	if !sigVer.IsSigned {
 		return "❌ not signed"
@@ -132,14 +173,14 @@ func init() {
 	// Flags
 	getCmd.Flags().StringP("output", "o", app.Table,
 		fmt.Sprintf("Output format (one of %s)", strings.Join(app.SupportedOutputFormats(), ",")))
+	getCmd.Flags().StringP("name", "n", "", "name of the artifact to get info from in the form repoOwner/repoName/artifactName")
 	getCmd.Flags().StringP("id", "i", "", "ID of the artifact to get info from")
 	getCmd.Flags().Int32P("versions", "v", 1, "Latest artifact versions to retrieve")
 	getCmd.Flags().StringP("tag", "", "", "Specific artifact tag to retrieve")
-	// Required
-	if err := getCmd.MarkFlagRequired("id"); err != nil {
-		getCmd.Printf("Error marking flag as required: %s", err)
-		os.Exit(1)
-	}
-	// Exclusive
+	// We allow searching by either versions or tags but not both. It's OK to not specify either, in which case
+	// we return all the versions and tags
 	getCmd.MarkFlagsMutuallyExclusive("versions", "tag")
+	// We allow searching by name or ID but not both. One of them must be specified.
+	getCmd.MarkFlagsMutuallyExclusive("name", "id")
+	getCmd.MarkFlagsOneRequired("name", "id")
 }
diff --git a/database/mock/store.go b/database/mock/store.go
diff --git a/database/query/artifacts.sql b/database/query/artifacts.sql
@@ -26,6 +26,13 @@ repositories.provider, repositories.project_id, repositories.repo_owner, reposit
 FROM artifacts INNER JOIN repositories ON repositories.id = artifacts.repository_id
 WHERE artifacts.id = $1;
 
+-- name: GetArtifactByName :one
+SELECT artifacts.id, artifacts.repository_id, artifacts.artifact_name, artifacts.artifact_type,
+       artifacts.artifact_visibility, artifacts.created_at,
+       repositories.provider, repositories.project_id, repositories.repo_owner, repositories.repo_name
+FROM artifacts INNER JOIN repositories ON repositories.id = artifacts.repository_id
+WHERE artifacts.artifact_name = $1 AND artifacts.repository_id = $2;
+
 -- name: ListArtifactsByRepoID :many
 SELECT * FROM artifacts
 WHERE repository_id = $1

diff --git a/docs/docs/ref/proto.md b/docs/docs/ref/proto.md
diff --git a/internal/controlplane/handlers_artifacts.go b/internal/controlplane/handlers_artifacts.go
@@ -64,6 +64,76 @@ func (s *Server) ListArtifacts(ctx context.Context, in *pb.ListArtifactsRequest)
 	return &pb.ListArtifactsResponse{Results: results}, nil
 }
 
+// GetArtifactByName gets an artifact by name
+// nolint:gocyclo
+func (s *Server) GetArtifactByName(ctx context.Context, in *pb.GetArtifactByNameRequest) (*pb.GetArtifactByNameResponse, error) {
+	// tag and latest versions cannot be set at same time
+	if in.Tag != "" && in.LatestVersions > 1 {
+		return nil, status.Errorf(codes.InvalidArgument, "tag and latest versions cannot be set at same time")
+	}
+
+	if in.LatestVersions < 1 || in.LatestVersions > 10 {
+		return nil, status.Errorf(codes.InvalidArgument, "latest versions must be between 1 and 10")
+	}
+
+	// get artifact versions
+	if in.LatestVersions <= 0 {
+		in.LatestVersions = 10
+	}
+
+	nameParts := strings.Split(in.Name, "/")
+	if len(nameParts) < 3 {
+		return nil, util.UserVisibleError(codes.InvalidArgument, "invalid artifact name user repoOwner/repoName/artifactName")
+	}
+
+	repo, err := s.store.GetRepositoryByRepoName(ctx, db.GetRepositoryByRepoNameParams{
+		Provider:  in.GetContext().GetProvider(),
+		RepoOwner: nameParts[0],
+		RepoName:  nameParts[1],
+	})
+	if err != nil {
+		if errors.Is(err, sql.ErrNoRows) {
+			return nil, util.UserVisibleError(codes.NotFound, "repository not found")
+		}
+		return nil, status.Errorf(codes.Unknown, "failed to get repository: %s", err)
+	}
+
+	// the artifact name is the rest of the parts
+	artifactName := strings.Join(nameParts[2:], "/")
+	artifact, err := s.store.GetArtifactByName(ctx, db.GetArtifactByNameParams{
+		RepositoryID: repo.ID,
+		ArtifactName: artifactName,
+	})
+	if err != nil {
+		if errors.Is(err, sql.ErrNoRows) {
+			return nil, status.Errorf(codes.NotFound, "artifact not found")
+		}
+		return nil, status.Errorf(codes.Unknown, "failed to get artifact: %s", err)
+	}
+
+	// check if user is authorized
+	if err := AuthorizedOnProject(ctx, artifact.ProjectID); err != nil {
+		return nil, err
+	}
+
+	pbVersions, err := getPbArtifactVersions(ctx, s.store, artifact.ID, in.Tag, in.LatestVersions)
+	if err != nil {
+		return nil, status.Errorf(codes.Unknown, "failed to get artifact versions: %s", err)
+	}
+
+	return &pb.GetArtifactByNameResponse{Artifact: &pb.Artifact{
+		ArtifactPk: artifact.ID.String(),
+		Owner:      artifact.RepoOwner,
+		Name:       artifact.ArtifactName,
+		Type:       artifact.ArtifactType,
+		Visibility: artifact.ArtifactVisibility,
+		Repository: artifact.RepoName,
+		CreatedAt:  timestamppb.New(artifact.CreatedAt),
+	},
+		Versions: pbVersions,
+	}, nil
+}
+
 // GetArtifactById gets an artifact by id
 // nolint:gocyclo
 func (s *Server) GetArtifactById(ctx context.Context, in *pb.GetArtifactByIdRequest) (*pb.GetArtifactByIdResponse, error) {
@@ -100,23 +170,40 @@ func (s *Server) GetArtifactById(ctx context.Context, in *pb.GetArtifactByIdRequ
 		in.LatestVersions = 10
 	}
 
-	var versions []db.ArtifactVersion
-	if in.Tag != "" {
-		versions, err = s.store.ListArtifactVersionsByArtifactIDAndTag(ctx,
-			db.ListArtifactVersionsByArtifactIDAndTagParams{ArtifactID: parsedArtifactID,
-				Tags:  sql.NullString{Valid: true, String: in.Tag},
-				Limit: sql.NullInt32{Valid: true, Int32: in.LatestVersions}})
-
-	} else {
-		versions, err = s.store.ListArtifactVersionsByArtifactID(ctx,
-			db.ListArtifactVersionsByArtifactIDParams{ArtifactID: parsedArtifactID,
-				Limit: sql.NullInt32{Valid: true, Int32: in.LatestVersions}})
-	}
-
+	pbVersions, err := getPbArtifactVersions(ctx, s.store, parsedArtifactID, in.Tag, in.LatestVersions)
 	if err != nil {
 		return nil, status.Errorf(codes.Unknown, "failed to get artifact versions: %s", err)
 	}
 
+	return &pb.GetArtifactByIdResponse{Artifact: &pb.Artifact{
+		ArtifactPk: artifact.ID.String(),
+		Owner:      artifact.RepoOwner,
+		Name:       artifact.ArtifactName,
+		Type:       artifact.ArtifactType,
+		Visibility: artifact.ArtifactVisibility,
+		Repository: artifact.RepoName,
+		CreatedAt:  timestamppb.New(artifact.CreatedAt),
+	},
+		Versions: pbVersions,
+	}, nil
+}
+
+func getDbArtifactVersions(
+	ctx context.Context, store db.Store, artifactID uuid.UUID, tag string, limit int32,
+) ([]db.ArtifactVersion, error) {
+	if tag != "" {
+		return store.ListArtifactVersionsByArtifactIDAndTag(ctx,
+			db.ListArtifactVersionsByArtifactIDAndTagParams{ArtifactID: artifactID,
+				Tags:  sql.NullString{Valid: true, String: tag},
+				Limit: sql.NullInt32{Valid: true, Int32: limit}})
+	}
+
+	return store.ListArtifactVersionsByArtifactID(ctx,
+		db.ListArtifactVersionsByArtifactIDParams{ArtifactID: artifactID,
+			Limit: sql.NullInt32{Valid: true, Int32: limit}})
+}
+
+func artifactVersionsDbToPb(versions []db.ArtifactVersion) ([]*pb.ArtifactVersion, error) {
 	final_versions := []*pb.ArtifactVersion{}
 	for _, version := range versions {
 		tags := []string{}
@@ -149,17 +236,18 @@ func (s *Server) GetArtifactById(ctx context.Context, in *pb.GetArtifactByIdRequ
 
 	}
 
-	return &pb.GetArtifactByIdResponse{Artifact: &pb.Artifact{
-		ArtifactPk: artifact.ID.String(),
-		Owner:      artifact.RepoOwner,
-		Name:       artifact.ArtifactName,
-		Type:       artifact.ArtifactType,
-		Visibility: artifact.ArtifactVisibility,
-		Repository: artifact.RepoName,
-		CreatedAt:  timestamppb.New(artifact.CreatedAt),
-	},
-		Versions: final_versions,
-	}, nil
+	return final_versions, nil
+}
+
+func getPbArtifactVersions(
+	ctx context.Context, store db.Store, artifactID uuid.UUID, tag string, limit int32,
+) ([]*pb.ArtifactVersion, error) {
+	versions, err := getDbArtifactVersions(ctx, store, artifactID, tag, limit)
+	if err != nil {
+		return nil, err
+	}
+
+	return artifactVersionsDbToPb(versions)
 }
 
 type artifactSource string