Merge pull request #670 from HumairAK/RHOAIENG-8225

feat: add the ability to configure artifact download link expiry
opendatahub-io · Jul 17, 2024 · bd4b501 · bd4b501
2 parents 4c179f5 + 5a8177c
commit bd4b501
Show file tree

Hide file tree

Showing 17 changed files with 55 additions and 0 deletions.
diff --git a/api/v1alpha1/dspipeline_types.go b/api/v1alpha1/dspipeline_types.go
@@ -141,6 +141,13 @@ type APIServer struct {
 	// pipeline server and user executor pods
 	// +kubebuilder:validation:Optional
 	CABundleFileName string `json:"caBundleFileName"`
+
+	// The expiry time (seconds) for artifact download links when
+	// querying the dsp server via /apis/v2beta1/artifacts/{id}?share_url=true
+	// Default: 15
+	// +kubebuilder:default:=15
+	// +kubebuilder:validation:Optional
+	ArtifactSignedURLExpirySeconds *int `json:"artifactSignedURLExpirySeconds"`
 }
 
 type CABundle struct {

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/...ses/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml b/...ses/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml
@@ -67,6 +67,12 @@ spec:
                       name:
                         type: string
                     type: object
+                  artifactSignedURLExpirySeconds:
+                    default: 15
+                    description: 'The expiry time (seconds) for artifact download
+                      links when querying the dsp server via /apis/v2beta1/artifacts/{id}?share_url=true
+                      Default: 15'
+                    type: integer
                   autoUpdatePipelineDefaultVersion:
                     default: true
                     description: 'Default: true Deprecated: DSP V1 only, will be removed

diff --git a/config/internal/apiserver/default/deployment.yaml.tmpl b/config/internal/apiserver/default/deployment.yaml.tmpl
@@ -116,6 +116,8 @@ spec:
               value: "ds-pipeline-{{.Name}}.{{.Namespace}}.svc.cluster.local"
             - name: ML_PIPELINE_SERVICE_PORT_GRPC
               value: "8887"
+            - name: SIGNED_URL_EXPIRY_TIME_SECONDS
+              value: "{{.APIServer.ArtifactSignedURLExpirySeconds}}"
             {{ if (eq .DSPVersion "v2") }}
             ## Argo-Specific Env Vars ##
             - name: EXECUTIONTYPE

diff --git a/config/internal/apiserver/default/role_ds-pipeline.yaml.tmpl b/config/internal/apiserver/default/role_ds-pipeline.yaml.tmpl
@@ -16,6 +16,13 @@ rules:
       - get
       - list
       - delete
+  - apiGroups:
+      - ""
+    resources:
+      - secrets
+    verbs:
+      - get
+      - list
   - apiGroups:
       - argoproj.io
     resources:

diff --git a/config/samples/v2/dspa-simple/dspa_simple.yaml b/config/samples/v2/dspa-simple/dspa_simple.yaml
@@ -7,6 +7,9 @@ spec:
   apiServer:
     enableSamplePipeline: true
   objectStorage:
+    # Need to enable this for artifact download links to work
+    # i.e. for when requesting /apis/v2beta1/artifacts/{id}?share_url=true
+    enableExternalRoute: true
     minio:
       deploy: true
       image: 'quay.io/opendatahub/minio:RELEASE.2019-08-14T20-37-41Z-license-compliance'

diff --git a/controllers/config/defaults.go b/controllers/config/defaults.go
@@ -57,6 +57,8 @@ const (
 	DefaultDBSecretKey        = "password"
 	GeneratedDBPasswordLength = 12
 
+	DefaultSignedUrlExpiryTimeSeconds = 15
+
 	MariaDBName        = "mlpipeline"
 	MariaDBHostPrefix  = "mariadb"
 	MariaDBHostPort    = "3306"

diff --git a/controllers/dspipeline_params.go b/controllers/dspipeline_params.go
@@ -765,6 +765,11 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
 			sslCertDir := strings.Join(certDirectories, ":")
 			p.CustomSSLCertDir = &sslCertDir
 		}
+
+		if p.APIServer.ArtifactSignedURLExpirySeconds == nil {
+			expiry := config.DefaultSignedUrlExpiryTimeSeconds
+			p.APIServer.ArtifactSignedURLExpirySeconds = &expiry
+		}
 	}
 
 	if p.PersistenceAgent != nil {

diff --git a/controllers/testdata/declarative/case_0/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_0/expected/created/apiserver_deployment.yaml
@@ -79,6 +79,8 @@ spec:
               value: ds-pipeline-testdsp0.default.svc.cluster.local
             - name: ML_PIPELINE_SERVICE_PORT_GRPC
               value: "8887"
+            - name: SIGNED_URL_EXPIRY_TIME_SECONDS
+              value: "15"
             - name: EXECUTIONTYPE
               value: PipelineRun
             - name: CACHE_IMAGE

diff --git a/controllers/testdata/declarative/case_2/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_2/expected/created/apiserver_deployment.yaml
@@ -79,6 +79,8 @@ spec:
               value: ds-pipeline-testdsp2.default.svc.cluster.local
             - name: ML_PIPELINE_SERVICE_PORT_GRPC
               value: "8887"
+            - name: SIGNED_URL_EXPIRY_TIME_SECONDS
+              value: "15"
             - name: EXECUTIONTYPE
               value: PipelineRun
             - name: CACHE_IMAGE

diff --git a/controllers/testdata/declarative/case_3/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_3/expected/created/apiserver_deployment.yaml
@@ -79,6 +79,8 @@ spec:
               value: ds-pipeline-testdsp3.default.svc.cluster.local
             - name: ML_PIPELINE_SERVICE_PORT_GRPC
               value: "8887"
+            - name: SIGNED_URL_EXPIRY_TIME_SECONDS
+              value: "15"
             - name: EXECUTIONTYPE
               value: PipelineRun
             - name: CACHE_IMAGE

diff --git a/controllers/testdata/declarative/case_4/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_4/expected/created/apiserver_deployment.yaml
@@ -79,6 +79,8 @@ spec:
               value: ds-pipeline-testdsp4.default.svc.cluster.local
             - name: ML_PIPELINE_SERVICE_PORT_GRPC
               value: "8887"
+            - name: SIGNED_URL_EXPIRY_TIME_SECONDS
+              value: "15"
             - name: EXECUTIONTYPE
               value: PipelineRun
             - name: CACHE_IMAGE

diff --git a/controllers/testdata/declarative/case_5/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_5/expected/created/apiserver_deployment.yaml
@@ -83,6 +83,8 @@ spec:
               value: ds-pipeline-testdsp5.default.svc.cluster.local
             - name: ML_PIPELINE_SERVICE_PORT_GRPC
               value: "8887"
+            - name: SIGNED_URL_EXPIRY_TIME_SECONDS
+              value: "15"
             - name: EXECUTIONTYPE
               value: PipelineRun
             - name: CACHE_IMAGE

diff --git a/controllers/testdata/declarative/case_6/deploy/03_cr.yaml b/controllers/testdata/declarative/case_6/deploy/03_cr.yaml
@@ -5,6 +5,7 @@
 # MLMD grpc server mounts the dspa cert and passes it into grpc server
 # When a user provides a caBundleFileMountPath, it will be used to mount the ca bundle
 # When a user provides ca bundle configmapkey, it will be used instead of default one
+# When a user provides a ArtifactSignedURLExpirySeconds, it will be used instead of default
 apiVersion: datasciencepipelinesapplications.opendatahub.io/v1alpha1
 kind: DataSciencePipelinesApplication
 metadata:
@@ -19,6 +20,7 @@ spec:
     cABundle:
       configMapKey: user-ca-bundle.crt
       configMapName: user-ca-bundle
+    artifactSignedURLExpirySeconds: 20
   database:
     externalDB:
       host: testdbhost6

diff --git a/controllers/testdata/declarative/case_6/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_6/expected/created/apiserver_deployment.yaml
@@ -91,6 +91,8 @@ spec:
               value: ds-pipeline-testdsp6.default.svc.cluster.local
             - name: ML_PIPELINE_SERVICE_PORT_GRPC
               value: "8887"
+            - name: SIGNED_URL_EXPIRY_TIME_SECONDS
+              value: "20"
             - name: EXECUTIONTYPE
               value: Workflow
             - name: DB_DRIVER_NAME

diff --git a/controllers/testdata/declarative/case_7/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_7/expected/created/apiserver_deployment.yaml
@@ -83,6 +83,8 @@ spec:
               value: ds-pipeline-testdsp7.default.svc.cluster.local
             - name: ML_PIPELINE_SERVICE_PORT_GRPC
               value: "8887"
+            - name: SIGNED_URL_EXPIRY_TIME_SECONDS
+              value: "15"
             - name: EXECUTIONTYPE
               value: Workflow
             - name: DB_DRIVER_NAME

diff --git a/controllers/testdata/declarative/case_8/expected/created/apiserver_deployment.yaml b/controllers/testdata/declarative/case_8/expected/created/apiserver_deployment.yaml
@@ -91,6 +91,8 @@ spec:
               value: ds-pipeline-testdsp8.default.svc.cluster.local
             - name: ML_PIPELINE_SERVICE_PORT_GRPC
               value: "8887"
+            - name: SIGNED_URL_EXPIRY_TIME_SECONDS
+              value: "15"
             - name: EXECUTIONTYPE
               value: Workflow
             - name: DB_DRIVER_NAME