correct artifact preview behavior in UI

frontend/server/app.ts

@@ -143,6 +143,7 @@ function createUIServer(options: UIConfigs) {
       artifactsConfigs: options.artifacts,
       useParameter: false,
       tryExtract: true,
+      options: options,
     }),
   );
   // /artifacts/ endpoint downloads the artifact as is, it does not try to unzip or untar.
@@ -158,6 +159,7 @@ function createUIServer(options: UIConfigs) {
       artifactsConfigs: options.artifacts,
       useParameter: true,
       tryExtract: false,
+      options: options,
     }),
   );
 

frontend/server/configs.ts

@@ -120,6 +120,7 @@ export function loadConfigs(argv: string[], env: ProcessEnv): UIConfigs {
      * e.g. a valid header value for default values can be like `accounts.google.com:[email protected]`.
      */
     KUBEFLOW_USERID_PREFIX = 'accounts.google.com:',
+    FRONTEND_SERVER_NAMESPACE = 'kubeflow',
   } = env;
 
   return {
@@ -187,6 +188,7 @@ export function loadConfigs(argv: string[], env: ProcessEnv): UIConfigs {
           : asBool(HIDE_SIDENAV),
       port,
       staticDir,
+      serverNamespace: FRONTEND_SERVER_NAMESPACE,
     },
     viewer: {
       tensorboard: {
@@ -263,6 +265,8 @@ export interface ServerConfigs {
   apiVersion2Prefix: string;
   deployment: Deployments;
   hideSideNav: boolean;
+  // Namespace where the server is deployed
+  serverNamespace: string;
 }
 export interface GkeMetadataConfigs {
   disabled: boolean;

frontend/server/handlers/artifacts.ts

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 import fetch from 'node-fetch';
-import { AWSConfigs, HttpConfigs, MinioConfigs, ProcessEnv } from '../configs';
+import { AWSConfigs, HttpConfigs, MinioConfigs, ProcessEnv, UIConfigs } from '../configs';
 import { Client as MinioClient } from 'minio';
 import { PreviewStream, findFileOnPodVolume, parseJSONString } from '../utils';
 import { createMinioClient, getObjectStream } from '../minio-helper';
@@ -80,6 +80,7 @@ export function getArtifactsHandler({
   artifactsConfigs,
   useParameter,
   tryExtract,
+  options,
 }: {
   artifactsConfigs: {
     aws: AWSConfigs;
@@ -89,15 +90,18 @@ export function getArtifactsHandler({
   };
   tryExtract: boolean;
   useParameter: boolean;
+  options: UIConfigs;
 }): Handler {
   const { aws, http, minio, allowedDomain } = artifactsConfigs;
   return async (req, res) => {
     const source = useParameter ? req.params.source : req.query.source;
     const bucket = useParameter ? req.params.bucket : req.query.bucket;
     const key = useParameter ? req.params[0] : req.query.key;
-    const { peek = 0, providerInfo = '', namespace = '' } = req.query as Partial<
-      ArtifactsQueryStrings
-    >;
+    const {
+      peek = 0,
+      providerInfo = '',
+      namespace = options.server.serverNamespace,
+    } = req.query as Partial<ArtifactsQueryStrings>;
     if (!source) {
       res.status(500).send('Storage source is missing from artifact request');
       return;

frontend/server/integration-tests/artifact-get.test.ts

@@ -184,8 +184,11 @@ describe('/artifacts', () => {
         });
     });
 
-    it('responds error when source is s3, and creds are sourced from Provider Configs, but no namespace is provided', done => {
+    it('responds with artifact if source is AWS S3, and creds are sourced from Provider Configs, and uses default kubeflow namespace when no namespace is provided', done => {
       const mockedGetK8sSecret: jest.Mock = getK8sSecret as any;
+      mockedGetK8sSecret.mockResolvedValue('somevalue');
+      const mockedMinioClient: jest.Mock = minio.Client as any;
+      const namespace = 'kubeflow';
       const configs = loadConfigs(argv, {});
       app = new UIServer(configs);
       const request = requests(app.start());
@@ -203,18 +206,90 @@ describe('/artifacts', () => {
       };
       request
         .get(
-          `/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt$&providerInfo=${JSON.stringify(
+          `/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&providerInfo=${JSON.stringify(
+            providerInfo,
+          )}`,
+        )
+        .expect(200, artifactContent, err => {
+          expect(mockedMinioClient).toBeCalledWith({
+            accessKey: 'somevalue',
+            endPoint: 's3.amazonaws.com',
+            port: undefined,
+            region: 'us-east-2',
+            secretKey: 'somevalue',
+            useSSL: undefined,
+          });
+          expect(mockedMinioClient).toBeCalledTimes(1);
+          expect(mockedGetK8sSecret).toBeCalledTimes(2);
+          expect(mockedGetK8sSecret).toHaveBeenNthCalledWith(
+            1,
+            'aws-s3-creds',
+            'AWS_ACCESS_KEY_ID',
+            `${namespace}`,
+          );
+          expect(mockedGetK8sSecret).toHaveBeenNthCalledWith(
+            2,
+            'aws-s3-creds',
+            'AWS_SECRET_ACCESS_KEY',
+            `${namespace}`,
+          );
+          expect(mockedGetK8sSecret).toBeCalledTimes(2);
+          done(err);
+        });
+    });
+
+    it('responds with artifact if source is AWS S3, and creds are sourced from Provider Configs, and uses default namespace when no namespace is provided, as specified in ENV', done => {
+      const mockedGetK8sSecret: jest.Mock = getK8sSecret as any;
+      mockedGetK8sSecret.mockResolvedValue('somevalue');
+      const mockedMinioClient: jest.Mock = minio.Client as any;
+      const namespace = 'notkubeflow';
+      const configs = loadConfigs(argv, { FRONTEND_SERVER_NAMESPACE: namespace });
+      app = new UIServer(configs);
+      const request = requests(app.start());
+      const providerInfo = {
+        Params: {
+          accessKeyKey: 'AWS_ACCESS_KEY_ID',
+          disableSSL: 'false',
+          endpoint: 's3.amazonaws.com',
+          fromEnv: 'false',
+          region: 'us-east-2',
+          secretKeyKey: 'AWS_SECRET_ACCESS_KEY',
+          secretName: 'aws-s3-creds',
+        },
+        Provider: 's3',
+      };
+      request
+        .get(
+          `/artifacts/get?source=s3&bucket=ml-pipeline&key=hello%2Fworld.txt&providerInfo=${JSON.stringify(
             providerInfo,
           )}`,
         )
-        .expect(
-          500,
-          'Failed to initialize Minio Client for S3 Provider: Error: Artifact Store provider given, but no namespace provided.',
-          err => {
-            expect(mockedGetK8sSecret).toBeCalledTimes(0);
-            done(err);
-          },
-        );
+        .expect(200, artifactContent, err => {
+          expect(mockedMinioClient).toBeCalledWith({
+            accessKey: 'somevalue',
+            endPoint: 's3.amazonaws.com',
+            port: undefined,
+            region: 'us-east-2',
+            secretKey: 'somevalue',
+            useSSL: undefined,
+          });
+          expect(mockedMinioClient).toBeCalledTimes(1);
+          expect(mockedGetK8sSecret).toBeCalledTimes(2);
+          expect(mockedGetK8sSecret).toHaveBeenNthCalledWith(
+            1,
+            'aws-s3-creds',
+            'AWS_ACCESS_KEY_ID',
+            `${namespace}`,
+          );
+          expect(mockedGetK8sSecret).toHaveBeenNthCalledWith(
+            2,
+            'aws-s3-creds',
+            'AWS_SECRET_ACCESS_KEY',
+            `${namespace}`,
+          );
+          expect(mockedGetK8sSecret).toBeCalledTimes(2);
+          done(err);
+        });
     });
 
     it('responds with artifact if source is s3-compatible, and creds are sourced from Provider Configs', done => {

frontend/server/minio-helper.ts

@@ -78,7 +78,7 @@ export async function createMinioClient(
   }
 
   // If using s3 and sourcing credentials from environment (currently only aws is supported)
-  if (providerType === 's3' && (!config.accessKey || !config.secretKey)) {
+  if (providerType === 's3' && !(config.accessKey && config.secretKey)) {
     // AWS S3 with credentials from provider chain
     if (isAWSS3Endpoint(config.endPoint)) {
       try {
@@ -168,7 +168,11 @@ async function parseS3ProviderInfo(
     config.useSSL = undefined;
   } else {
     if (providerInfo.Params.endpoint) {
-      const parseEndpoint = new URL(providerInfo.Params.endpoint);
+      const url = providerInfo.Params.endpoint;
+      // this is a bit of a hack to add support for endpoints without a protocol (required by WHATWG URL standard)
+      // example: <ip>:<port> format. In general should expect most endpoints to provide a protocol as serviced
+      // by the backend
+      const parseEndpoint = new URL(url.startsWith('http') ? url : `https://${url}`);
       const host = parseEndpoint.hostname;
       const port = parseEndpoint.port;
       config.endPoint = host;

manifests/kustomize/base/pipeline/ml-pipeline-ui-deployment.yaml

@@ -48,6 +48,10 @@ spec:
                 key: secretkey
           - name: ALLOW_CUSTOM_VISUALIZATIONS
             value: "true"
+          - name: FRONTEND_SERVER_NAMESPACE
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.namespace
         readinessProbe:
           exec:
             command: