UPSTREAM: <carry>:Update ArifactView enum to allow pipelines artifact…

…s api to have optional content disposition

Signed-off-by: VaniHaripriya <[email protected]>

backend/api/v2beta1/artifacts.proto

@@ -55,6 +55,10 @@ message GetArtifactRequest {
 
     // Server responses include download_url
     DOWNLOAD = 2;
+
+    // Server response includes a signed URL,
+    // allowing in-browser rendering or preview of the artifact.
+    RENDER = 3;
   }
 
   // Optional. Set to "DOWNLOAD" to included a signed URL with
@@ -137,4 +141,7 @@ message Artifact {
   // and the error message is returned. Client has the flexibility of choosing
   // how to handle the error. This is especially useful when calling ListArtifacts.
   google.rpc.Status error = 11;
+  // Optional Output. Specifies a signed URL that can be used to
+  // render this Artifact directly from its store.
+  string render_url = 12;
 }

backend/api/v2beta1/swagger/artifacts.swagger.json

@@ -107,14 +107,15 @@
           },
           {
             "name": "view",
-            "description": "Optional. Set to \"DOWNLOAD\" to included a signed URL with\nan expiry (default 15 seconds, unless configured other wise).\nThis URL can be used to download the Artifact directly from\nthe Artifact's storage provider. Set to \"BASIC\" to exclude\nthe download_url from server responses, thus preventing the\ncreation of any signed url.\nDefaults to BASIC.\n\n - ARTIFACT_VIEW_UNSPECIFIED: Not specified, equivalent to BASIC.\n - BASIC: Server responses excludes download_url\n - DOWNLOAD: Server responses include download_url",
+            "description": "Optional. Set to \"DOWNLOAD\" to included a signed URL with\nan expiry (default 15 seconds, unless configured other wise).\nThis URL can be used to download the Artifact directly from\nthe Artifact's storage provider. Set to \"BASIC\" to exclude\nthe download_url from server responses, thus preventing the\ncreation of any signed url.\nDefaults to BASIC.\n\n - ARTIFACT_VIEW_UNSPECIFIED: Not specified, equivalent to BASIC.\n - BASIC: Server responses excludes download_url\n - DOWNLOAD: Server responses include download_url\n - RENDER: Server response includes a signed URL,\nallowing in-browser rendering or preview of the artifact.",
             "in": "query",
             "required": false,
             "type": "string",
             "enum": [
               "ARTIFACT_VIEW_UNSPECIFIED",
               "BASIC",
-              "DOWNLOAD"
+              "DOWNLOAD",
+              "RENDER"
             ],
             "default": "ARTIFACT_VIEW_UNSPECIFIED"
           }
@@ -131,10 +132,11 @@
       "enum": [
         "ARTIFACT_VIEW_UNSPECIFIED",
         "BASIC",
-        "DOWNLOAD"
+        "DOWNLOAD",
+        "RENDER"
       ],
       "default": "ARTIFACT_VIEW_UNSPECIFIED",
-      "title": "- ARTIFACT_VIEW_UNSPECIFIED: Not specified, equivalent to BASIC.\n - BASIC: Server responses excludes download_url\n - DOWNLOAD: Server responses include download_url"
+      "description": " - ARTIFACT_VIEW_UNSPECIFIED: Not specified, equivalent to BASIC.\n - BASIC: Server responses excludes download_url\n - DOWNLOAD: Server responses include download_url\n - RENDER: Server response includes a signed URL,\nallowing in-browser rendering or preview of the artifact."
     },
     "ListArtifactRequestField": {
       "type": "string",
@@ -253,6 +255,10 @@
         "error": {
           "$ref": "#/definitions/googlerpcStatus",
           "description": "In case any error happens retrieving an artifact field, only artifact ID\nand the error message is returned. Client has the flexibility of choosing\nhow to handle the error. This is especially useful when calling ListArtifacts."
+        },
+        "render_url": {
+          "type": "string",
+          "description": "Optional Output. Specifies a signed URL that can be used to\nrender this Artifact directly from its store."
         }
       }
     },

backend/src/apiserver/resource/resource_manager.go

@@ -2052,6 +2052,14 @@ func (r *ResourceManager) GetSignedUrl(bucketConfig *objectstore.Config, secret
 	return signedUrl, nil
 }
 
+func (r *ResourceManager) GetSignedUrlWithoutContentDisposition(bucketConfig *objectstore.Config, secret *corev1.Secret, expirySeconds time.Duration, artifactURI string) (string, error) {
+	signedUrl, err := r.objectStore.GetSignedUrlWithoutContentDisposition(bucketConfig, secret, expirySeconds, artifactURI)
+	if err != nil {
+		return "", err
+	}
+	return signedUrl, nil
+}
+
 // GetObjectSize retrieves the size of the Artifact's object in bytes.
 func (r *ResourceManager) GetObjectSize(bucketConfig *objectstore.Config, secret *corev1.Secret, artifactURI string) (int64, error) {
 	size, err := r.objectStore.GetObjectSize(bucketConfig, secret, artifactURI)

backend/src/apiserver/resource/resource_manager_test.go

@@ -87,6 +87,10 @@ func (m *FakeBadObjectStore) GetSignedUrl(bucketConfig *objectstore.Config, secr
 	return "", util.NewInternalServerError(errors.New("Error"), "bad object store")
 }
 
+func (m *FakeBadObjectStore) GetSignedUrlWithoutContentDisposition(bucketConfig *objectstore.Config, secret *corev1.Secret, expirySeconds time.Duration, artifactURI string) (string, error) {
+	return "", util.NewInternalServerError(errors.New("Error"), "bad object store")
+}
+
 func (m *FakeBadObjectStore) GetObjectSize(bucketConfig *objectstore.Config, secret *corev1.Secret, artifactURI string) (int64, error) {
 	return 0, util.NewInternalServerError(errors.New("Error"), "bad object store")
 }

backend/src/apiserver/server/artifact_server.go

@@ -141,7 +141,7 @@ func (s *ArtifactServer) ListArtifacts(ctx context.Context, r *apiv2beta1.ListAr
 		if err1 != nil || bucketConfig == nil {
 			return nil, util.NewInternalServerError(fmt.Errorf("failed to retrieve session info error: %v", err1), artifactId)
 		}
-		artifactResp, err1 := s.generateResponseArtifact(ctx, artifact, bucketConfig, namespace, false)
+		artifactResp, err1 := s.generateResponseArtifact(ctx, artifact, bucketConfig, namespace, false, false)
 		if err1 != nil {
 			return nil, util.NewInternalServerError(fmt.Errorf("encountered error parsing artifact: %v", err), artifactId)
 		}
@@ -186,12 +186,15 @@ func (s *ArtifactServer) GetArtifact(ctx context.Context, r *apiv2beta1.GetArtif
 		return nil, util.Wrap(err, "Failed to authorize the request")
 	}
 
-	downloadURL := false
-	if r.GetView() == apiv2beta1.GetArtifactRequest_DOWNLOAD {
+	downloadURL, renderURL := false, false
+
+	switch r.GetView() {
+	case apiv2beta1.GetArtifactRequest_DOWNLOAD:
 		downloadURL = true
+	case apiv2beta1.GetArtifactRequest_RENDER:
+		renderURL = true
 	}
-
-	artifactResp, err := s.generateResponseArtifact(ctx, artifact, sessionInfo, namespace, downloadURL)
+	artifactResp, err := s.generateResponseArtifact(ctx, artifact, sessionInfo, namespace, downloadURL, renderURL)
 	if err != nil {
 		return nil, util.NewInternalServerError(fmt.Errorf("encountered error parsing artifact: %v", err), r.ArtifactId)
 	}
@@ -212,6 +215,7 @@ func (s *ArtifactServer) generateResponseArtifact(
 	bucketConfig *objectstore.Config,
 	namespace string,
 	includeShareUrl bool,
+	includeRenderUrl bool,
 ) (*apiv2beta1.Artifact, error) {
 	params, err := objectstore.StructuredS3Params(bucketConfig.SessionInfo.Params)
 	if err != nil {
@@ -251,6 +255,15 @@ func (s *ArtifactServer) generateResponseArtifact(
 		artifactResp.DownloadUrl = shareUrl
 	}
 
+	if includeRenderUrl {
+		expiry := time.Second * time.Duration(common.GetSignedURLExpiryTimeSeconds())
+		renderUrl, err := s.resourceManager.GetSignedUrlWithoutContentDisposition(bucketConfig, secret, expiry, *artifact.Uri)
+		if err != nil {
+			return nil, err
+		}
+		artifactResp.RenderUrl = renderUrl
+	}
+
 	return artifactResp, nil
 }
 

backend/src/apiserver/server/artifact_server_test.go

@@ -16,12 +16,13 @@ package server
 
 import (
 	"context"
-	apiv2beta1 "github.com/kubeflow/pipelines/backend/api/v2beta1/go_client"
-	"github.com/stretchr/testify/require"
-	"google.golang.org/protobuf/types/known/timestamppb"
 	"strings"
 	"testing"
 	"time"
+
+	apiv2beta1 "github.com/kubeflow/pipelines/backend/api/v2beta1/go_client"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
 func TestGetArtifacts(t *testing.T) {
@@ -76,6 +77,26 @@ func TestGetArtifacts(t *testing.T) {
 				LastUpdatedAt:   timestamppb.New(time.Unix(2, 0)),
 			},
 		},
+		{
+			name: "Fetch artifact 2",
+			artifactRequest: &apiv2beta1.GetArtifactRequest{
+				ArtifactId: "2",
+				View:       apiv2beta1.GetArtifactRequest_RENDER,
+			},
+			expectedArtifact: &apiv2beta1.Artifact{
+				ArtifactId:      "2",
+				StorageProvider: "s3",
+				StoragePath:     "pipeline/some-pipeline-id/task/key0",
+				Uri:             "s3://test-bucket/pipeline/some-pipeline-id/task/key0",
+				DownloadUrl:     "dummy-render-url", // defined in object_store_fake
+				Namespace:       "test-namespace",
+				ArtifactType:    "1",
+				ArtifactSize:    123,
+
+				CreatedAt:     timestamppb.New(time.Unix(1, 0)),
+				LastUpdatedAt: timestamppb.New(time.Unix(1, 0)),
+			},
+		},
 		{
 			name: "Fetch artifact 1 - View unspecified",
 			artifactRequest: &apiv2beta1.GetArtifactRequest{

backend/src/apiserver/storage/object_store.go

@@ -41,6 +41,7 @@ type ObjectStoreInterface interface {
 	GetFromYamlFile(o interface{}, filePath string) error
 	GetPipelineKey(pipelineId string) string
 	GetSignedUrl(bucketConfig *objectstore.Config, secret *v1.Secret, expirySeconds time.Duration, artifactURI string) (string, error)
+	GetSignedUrlWithoutContentDisposition(bucketConfig *objectstore.Config, secret *v1.Secret, expirySeconds time.Duration, artifactURI string) (string, error)
 	GetObjectSize(bucketConfig *objectstore.Config, secret *v1.Secret, artifactURI string) (int64, error)
 }
 
@@ -151,6 +152,26 @@ func (m *MinioObjectStore) GetSignedUrl(bucketConfig *objectstore.Config, secret
 	return signedUrl.String(), nil
 }
 
+func (m *MinioObjectStore) GetSignedUrlWithoutContentDisposition(bucketConfig *objectstore.Config, secret *v1.Secret, expirySeconds time.Duration, artifactURI string) (string, error) {
+	s3Client, err := buildClientFromConfig(bucketConfig, secret)
+	if err != nil {
+		return "", err
+	}
+
+	key, err := objectstore.ArtifactKeyFromURI(artifactURI)
+	if err != nil {
+		return "", err
+	}
+	reqParams := make(url.Values)
+	reqParams.Set("response-content-disposition", "inline")
+	signedUrl, err := s3Client.Presign("GET", bucketConfig.BucketName, key, expirySeconds, reqParams)
+	if err != nil {
+		return "", util.Wrap(err, "Failed to generate signed url")
+	}
+
+	return signedUrl.String(), nil
+}
+
 // GetObjectSize Retrieves the Size of the object in bytes.
 // Return zero with no error if artifact URI does not exist.
 func (m *MinioObjectStore) GetObjectSize(bucketConfig *objectstore.Config, secret *v1.Secret, artifactURI string) (int64, error) {

backend/src/apiserver/storage/object_store_fake.go

@@ -15,9 +15,10 @@
 package storage
 
 import (
-	"github.com/kubeflow/pipelines/backend/src/v2/objectstore"
-	"k8s.io/api/core/v1"
 	"time"
+
+	"github.com/kubeflow/pipelines/backend/src/v2/objectstore"
+	v1 "k8s.io/api/core/v1"
 )
 
 type fakeMinioObjectStore struct {
@@ -52,6 +53,10 @@ func (m *fakeMinioObjectStore) GetSignedUrl(*objectstore.Config, *v1.Secret, tim
 	return "dummy-signed-url", nil
 }
 
+func (m *fakeMinioObjectStore) GetSignedUrlWithoutContentDisposition(*objectstore.Config, *v1.Secret, time.Duration, string) (string, error) {
+	return "dummy-render-url", nil
+}
+
 func (m *fakeMinioObjectStore) GetObjectSize(*objectstore.Config, *v1.Secret, string) (int64, error) {
 	return 123, nil
 }