[Frontend] GKE metadata endpoint should fail when fetched response is…

… not ok (kubeflow#3218) * [Frontend] Add mock data for gke metadata * Fix GKE metadata endpoint when request is not ok
Jeffwan · Dec 9, 2020 · eb69ff9 · eb69ff9
1 parent 0cf01ad
commit eb69ff9
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 19 deletions.
diff --git a/frontend/mock-backend/mock-api-middleware.ts b/frontend/mock-backend/mock-api-middleware.ts
@@ -605,6 +605,20 @@ export default (app: express.Application) => {
     res.send(true);
   });
 
+  // Uncomment this instead to test 404 endpoints.
+  // app.get('/system/cluster-name', (_, res) => {
+  //   res.status(404).send('404 Not Found');
+  // });
+  // app.get('/system/project-id', (_, res) => {
+  //   res.status(404).send('404 Not Found');
+  // });
+  app.get('/system/cluster-name', (_, res) => {
+    res.send('mock-cluster-name');
+  });
+  app.get('/system/project-id', (_, res) => {
+    res.send('mock-project-id');
+  });
+
   app.all(v1beta1Prefix + '*', (req, res) => {
     res.status(404).send('Bad request endpoint.');
   });

diff --git a/frontend/server/app.test.ts b/frontend/server/app.test.ts
@@ -373,59 +373,75 @@ describe('UIServer apis', () => {
 
   describe('/system', () => {
     describe('/cluster-name', () => {
+      beforeEach(() => {
+        mockedK8sHelper.isInCluster = true;
+      });
       it('responds with cluster name data from gke metadata', done => {
         mockedFetch.mockImplementationOnce((url: string, _opts: any) =>
           url === 'http://metadata/computeMetadata/v1/instance/attributes/cluster-name'
-            ? Promise.resolve({ text: () => Promise.resolve('test-cluster') })
+            ? Promise.resolve({ ok: true, text: () => Promise.resolve('test-cluster') })
             : Promise.reject('Unexpected request'),
         );
-        mockedK8sHelper.isInCluster = true;
-        const configs = loadConfigs(argv, {});
-        app = new UIServer(configs);
+        app = new UIServer(loadConfigs(argv, {}));
 
         const request = requests(app.start());
         request
           .get('/system/cluster-name')
           .expect('Content-Type', 'text/html; charset=utf-8')
           .expect(200, 'test-cluster', done);
       });
+      it('responds with 500 status code if corresponding endpoint is not ok', done => {
+        mockedFetch.mockImplementationOnce((url: string, _opts: any) =>
+          url === 'http://metadata/computeMetadata/v1/instance/attributes/cluster-name'
+            ? Promise.resolve({ ok: false, text: () => Promise.resolve('404 not found') })
+            : Promise.reject('Unexpected request'),
+        );
+        app = new UIServer(loadConfigs(argv, {}));
+
+        const request = requests(app.start());
+        request.get('/system/cluster-name').expect(500, 'Failed fetching GKE cluster name', done);
+      });
       it('responds with endpoint disabled if DISABLE_GKE_METADATA env is true', done => {
         const configs = loadConfigs(argv, { DISABLE_GKE_METADATA: 'true' });
         app = new UIServer(configs);
 
         const request = requests(app.start());
         request
           .get('/system/cluster-name')
-          .expect('Content-Type', 'text/html; charset=utf-8')
           .expect(500, 'GKE metadata endpoints are disabled.', done);
       });
     });
     describe('/project-id', () => {
+      beforeEach(() => {
+        mockedK8sHelper.isInCluster = true;
+      });
       it('responds with project id data from gke metadata', done => {
         mockedFetch.mockImplementationOnce((url: string, _opts: any) =>
           url === 'http://metadata/computeMetadata/v1/project/project-id'
-            ? Promise.resolve({ text: () => Promise.resolve('test-project') })
+            ? Promise.resolve({ ok: true, text: () => Promise.resolve('test-project') })
             : Promise.reject('Unexpected request'),
         );
-        mockedK8sHelper.isInCluster = true;
-        const configs = loadConfigs(argv, {});
-        app = new UIServer(configs);
+        app = new UIServer(loadConfigs(argv, {}));
 
         const request = requests(app.start());
-        request
-          .get('/system/project-id')
-          .expect('Content-Type', 'text/html; charset=utf-8')
-          .expect(200, 'test-project', done);
+        request.get('/system/project-id').expect(200, 'test-project', done);
+      });
+      it('responds with 500 status code if metadata request is not ok', done => {
+        mockedFetch.mockImplementationOnce((url: string, _opts: any) =>
+          url === 'http://metadata/computeMetadata/v1/project/project-id'
+            ? Promise.resolve({ ok: false, text: () => Promise.resolve('404 not found') })
+            : Promise.reject('Unexpected request'),
+        );
+        app = new UIServer(loadConfigs(argv, {}));
+
+        const request = requests(app.start());
+        request.get('/system/project-id').expect(500, 'Failed fetching GKE project id', done);
       });
       it('responds with endpoint disabled if DISABLE_GKE_METADATA env is true', done => {
-        const configs = loadConfigs(argv, { DISABLE_GKE_METADATA: 'true' });
-        app = new UIServer(configs);
+        app = new UIServer(loadConfigs(argv, { DISABLE_GKE_METADATA: 'true' }));
 
         const request = requests(app.start());
-        request
-          .get('/system/project-id')
-          .expect('Content-Type', 'text/html; charset=utf-8')
-          .expect(500, 'GKE metadata endpoints are disabled.', done);
+        request.get('/system/project-id').expect(500, 'GKE metadata endpoints are disabled.', done);
       });
     });
   });

diff --git a/frontend/server/handlers/gke-metadata.ts b/frontend/server/handlers/gke-metadata.ts
@@ -37,6 +37,10 @@ const clusterNameHandler: Handler = async (_, res) => {
     'http://metadata/computeMetadata/v1/instance/attributes/cluster-name',
     { headers: { 'Metadata-Flavor': 'Google' } },
   );
+  if (!response.ok) {
+    res.status(500).send('Failed fetching GKE cluster name');
+    return;
+  }
   res.send(await response.text());
 };
 
@@ -56,5 +60,9 @@ const projectIdHandler: Handler = async (_, res) => {
   const response = await fetch('http://metadata/computeMetadata/v1/project/project-id', {
     headers: { 'Metadata-Flavor': 'Google' },
   });
+  if (!response.ok) {
+    res.status(500).send('Failed fetching GKE project id');
+    return;
+  }
   res.send(await response.text());
 };