Merge pull request #12080 from ivangonzalezacuna/feature/vault-plugin-improvements

[Vault Plugin] Add requested improvements

plugins/vault-backend/api-report.md

@@ -12,13 +12,6 @@ import { TaskRunner } from '@backstage/backend-tasks';
 // @public
 export function createRouter(options: RouterOptions): express.Router;
 
-// @public
-export type RenewTokenResponse = {
-  auth: {
-    client_token: string;
-  };
-};
-
 // @public
 export interface RouterOptions {
   // (undocumented)
@@ -33,7 +26,7 @@ export interface RouterOptions {
 export interface VaultApi {
   getFrontendSecretsUrl(): string;
   listSecrets(secretPath: string): Promise<VaultSecret[]>;
-  renewToken?(): Promise<boolean>;
+  renewToken?(): Promise<void>;
 }
 
 // @public
@@ -45,7 +38,6 @@ export class VaultBuilder {
   enableTokenRenew(schedule?: TaskRunner): Promise<this>;
   // (undocumented)
   protected readonly env: VaultEnvironment;
-  protected renewToken(vaultClient: VaultClient): Promise<void>;
   setVaultClient(vaultClient: VaultClient): this;
 }
 
@@ -62,7 +54,7 @@ export class VaultClient implements VaultApi {
   // (undocumented)
   listSecrets(secretPath: string): Promise<VaultSecret[]>;
   // (undocumented)
-  renewToken(): Promise<boolean>;
+  renewToken(): Promise<void>;
 }
 
 // @public
@@ -82,12 +74,5 @@ export type VaultSecret = {
   editUrl: string;
 };
 
-// @public
-export type VaultSecretList = {
-  data: {
-    keys: string[];
-  };
-};
-
 // (No @packageDocumentation comment for this package)
 ```

plugins/vault-backend/config.d.ts

@@ -19,6 +19,7 @@ export interface Config {
   vault?: {
     /**
      * The baseUrl for your Vault instance.
+     * @visibility frontend
      */
     baseUrl: string;
 

plugins/vault-backend/package.json

@@ -46,6 +46,7 @@
     "express": "^4.17.1",
     "express-promise-router": "^4.1.0",
     "helmet": "^5.0.2",
+    "p-limit": "^3.1.0",
     "winston": "^3.7.2",
     "yn": "^5.0.0"
   },
@@ -54,7 +55,7 @@
     "@types/compression": "^1.7.2",
     "@types/supertest": "^2.0.8",
     "msw": "^0.42.0",
-    "supertest": "^4.0.2"
+    "supertest": "^6.1.6"
   },
   "files": [
     "dist",

plugins/vault-backend/src/service/VaultBuilder.tsx

@@ -45,9 +45,6 @@ export type VaultBuilderReturn = {
  * @public
  */
 export class VaultBuilder {
-  // private vaultTokenRefreshInterval: Duration = Duration.fromObject({
-  //   minutes: 60,
-  // });
   private vaultClient?: VaultClient;
 
   /**
@@ -118,26 +115,12 @@ export class VaultBuilder {
       fn: async () => {
         this.env.logger.info('Renewing Vault token');
         const vaultClient = this.vaultClient ?? new VaultClient(this.env);
-        await this.renewToken(vaultClient);
+        await vaultClient.renewToken();
       },
     });
     return this;
   }
 
-  /**
-   * Renews the token for vault using a defined client.
-   *
-   * @param vaultClient - The vault client used to renew the token
-   */
-  protected async renewToken(vaultClient: VaultClient) {
-    const result = await vaultClient.renewToken();
-    if (!result) {
-      this.env.logger.warn('Error renewing vault token');
-    } else {
-      this.env.logger.info('Vault token renewed');
-    }
-  }
-
   /**
    * Builds the backend routes for Vault.
    *
@@ -159,7 +142,7 @@ export class VaultBuilder {
       }
 
       const secrets = await vaultClient.listSecrets(path);
-      res.json(secrets);
+      res.json({ items: secrets });
     });
 
     return router;

plugins/vault-backend/src/service/vaultApi.test.ts

@@ -93,8 +93,7 @@ describe('VaultApi', () => {
   it('should return success token renew', async () => {
     setupHandlers();
     const api = new VaultClient({ config });
-    const apiRenew = await api.renewToken();
-    expect(apiRenew).toBeTruthy();
+    expect(await api.renewToken()).toBe(undefined);
   });
 
   it('should render frontend url', () => {

plugins/vault-backend/src/service/vaultApi.ts

@@ -15,12 +15,14 @@
  */
 
 import { Config } from '@backstage/config';
+import { NotFoundError } from '@backstage/errors';
 import fetch from 'cross-fetch';
+import plimit from 'p-limit';
 import { getVaultConfig, VaultConfig } from '../config';
 
 /**
  * Object received as a response from the Vault API when fetching secrets
- * @public
+ * @internal
  */
 export type VaultSecretList = {
   data: {
@@ -40,9 +42,8 @@ export type VaultSecret = {
 
 /**
  * Object received as response when the token is renewed using the Vault API
- * @public
  */
-export type RenewTokenResponse = {
+type RenewTokenResponse = {
   auth: {
     client_token: string;
   };
@@ -63,10 +64,10 @@ export interface VaultApi {
    */
   listSecrets(secretPath: string): Promise<VaultSecret[]>;
   /**
-   * Optional, to renew the token used to list the secrets. Returns true
-   * if the action was successfull, false otherwise.
+   * Optional, to renew the token used to list the secrets. Throws an
+   * error if the token renewal went wrong.
    */
-  renewToken?(): Promise<boolean>;
+  renewToken?(): Promise<void>;
 }
 
 /**
@@ -75,6 +76,7 @@ export interface VaultApi {
  */
 export class VaultClient implements VaultApi {
   private vaultConfig: VaultConfig;
+  private readonly limit = plimit(5);
 
   constructor({ config }: { config: Config }) {
     this.vaultConfig = getVaultConfig(config);
@@ -84,7 +86,7 @@ export class VaultClient implements VaultApi {
     path: string,
     query: { [key in string]: any },
     method: string = 'GET',
-  ): Promise<T | undefined> {
+  ): Promise<T> {
     const url = new URL(path, this.vaultConfig.baseUrl);
     const response = await fetch(
       `${url.toString()}?${new URLSearchParams(query).toString()}`,
@@ -96,15 +98,14 @@ export class VaultClient implements VaultApi {
         },
       },
     );
-    if (response.status === 200) {
+    if (response.ok) {
       return (await response.json()) as T;
+    } else if (response.status === 404) {
+      throw new NotFoundError(`No secrets found in path '${path}'`);
     }
-    return undefined;
-  }
-
-  private isFolder(secretName: string): boolean {
-    const regex = /^.*\/$/gm;
-    return regex.test(secretName);
+    throw new Error(
+      `Unexpected error while fetching secrets from path '${path}'`,
+    );
   }
 
   getFrontendSecretsUrl(): string {
@@ -116,17 +117,19 @@ export class VaultClient implements VaultApi {
       this.vaultConfig.kvVersion === 2
         ? `v1/${this.vaultConfig.secretEngine}/metadata/${secretPath}`
         : `v1/${this.vaultConfig.secretEngine}/${secretPath}`;
-    const result = await this.callApi<VaultSecretList>(listUrl, { list: true });
-    if (!result) {
-      return [];
-    }
+    const result = await this.limit(() =>
+      this.callApi<VaultSecretList>(listUrl, { list: true }),
+    );
 
     const secrets: VaultSecret[] = [];
+
     await Promise.all(
       result.data.keys.map(async secret => {
-        if (this.isFolder(secret)) {
+        if (secret.endsWith('/')) {
           secrets.push(
-            ...(await this.listSecrets(`${secretPath}/${secret.slice(0, -1)}`)),
+            ...(await this.limit(() =>
+              this.listSecrets(`${secretPath}/${secret.slice(0, -1)}`),
+            )),
           );
         } else {
           secrets.push({
@@ -141,17 +144,13 @@ export class VaultClient implements VaultApi {
     return secrets;
   }
 
-  async renewToken(): Promise<boolean> {
+  async renewToken(): Promise<void> {
     const result = await this.callApi<RenewTokenResponse>(
       'v1/auth/token/renew-self',
       {},
       'POST',
     );
-    if (!result) {
-      return false;
-    }
 
     this.vaultConfig.token = result.auth.client_token;
-    return true;
   }
 }

plugins/vault/api-report.md

@@ -5,13 +5,36 @@
 ```ts
 /// <reference types="react" />
 
+import { ApiRef } from '@backstage/core-plugin-api';
 import { BackstagePlugin } from '@backstage/core-plugin-api';
+import { Entity } from '@backstage/catalog-model';
 
 // @public
 export const EntityVaultCard: () => JSX.Element;
 
+// @public
+export function isVaultAvailable(entity: Entity): boolean;
+
+// @public (undocumented)
+export const VAULT_SECRET_PATH_ANNOTATION = 'vault.io/secrets-path';
+
+// @public
+export interface VaultApi {
+  listSecrets(secretPath: string): Promise<VaultSecret[]>;
+}
+
+// @public (undocumented)
+export const vaultApiRef: ApiRef<VaultApi>;
+
 // @public
 export const vaultPlugin: BackstagePlugin<{}, {}>;
 
+// @public
+export type VaultSecret = {
+  name: string;
+  showUrl: string;
+  editUrl: string;
+};
+
 // (No @packageDocumentation comment for this package)
 ```

plugins/vault/package.json

@@ -37,6 +37,7 @@
     "@backstage/catalog-model": "^1.0.3",
     "@backstage/core-components": "^0.9.5",
     "@backstage/core-plugin-api": "^1.0.3",
+    "@backstage/errors": "^1.0.0",
     "@backstage/plugin-catalog-react": "^1.1.1",
     "@backstage/theme": "^0.2.15",
     "@material-ui/core": "^4.12.2",

plugins/vault/src/api.test.ts

@@ -27,18 +27,20 @@ describe('api', () => {
   const mockBaseUrl = 'https://api-vault.com/api/vault';
   const discoveryApi = UrlPatternDiscovery.compile(mockBaseUrl);
 
-  const mockSecretsResult: VaultSecret[] = [
-    {
-      name: 'secret::one',
-      editUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/edit/test/success/secret::one`,
-      showUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/show/test/success/secret::one`,
-    },
-    {
-      name: 'secret::two',
-      editUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/edit/test/success/secret::two`,
-      showUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/show/test/success/secret::two`,
-    },
-  ];
+  const mockSecretsResult: { items: VaultSecret[] } = {
+    items: [
+      {
+        name: 'secret::one',
+        editUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/edit/test/success/secret::one`,
+        showUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/show/test/success/secret::one`,
+      },
+      {
+        name: 'secret::two',
+        editUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/edit/test/success/secret::two`,
+        showUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/show/test/success/secret::two`,
+      },
+    ],
+  };
 
   const setupHandlers = () => {
     server.use(
@@ -47,31 +49,33 @@ describe('api', () => {
         if (path === 'test/success') {
           return res(ctx.json(mockSecretsResult));
         } else if (path === 'test/error') {
-          return res(ctx.json([]));
+          return res(ctx.json({ items: [] }));
         }
         return res(ctx.status(400));
       }),
+      rest.get(`${mockBaseUrl}/v1/secrets/`, (_req, res, ctx) => {
+        return res(ctx.json(mockSecretsResult));
+      }),
     );
   };
 
   it('should return secrets', async () => {
     setupHandlers();
     const api = new VaultClient({ discoveryApi });
-    const secrets = await api.listSecrets('test/success');
-    expect(secrets).toEqual(mockSecretsResult);
+    expect(await api.listSecrets('test/success')).toEqual(
+      mockSecretsResult.items,
+    );
   });
 
   it('should return empty secret list', async () => {
     setupHandlers();
     const api = new VaultClient({ discoveryApi });
     expect(await api.listSecrets('test/error')).toEqual([]);
-    expect(await api.listSecrets('')).toEqual([]);
   });
 
-  it('should return no secrets', async () => {
+  it('should return all the secrets if no path defined', async () => {
     setupHandlers();
     const api = new VaultClient({ discoveryApi });
-    const secrets = await api.listSecrets('');
-    expect(secrets).toEqual([]);
+    expect(await api.listSecrets('')).toEqual(mockSecretsResult.items);
   });
 });

plugins/vault/src/api.ts

@@ -14,21 +14,41 @@
  * limitations under the License.
  */
 import { DiscoveryApi, createApiRef } from '@backstage/core-plugin-api';
+import { NotFoundError } from '@backstage/errors';
 
+/**
+ * @public
+ */
 export const vaultApiRef = createApiRef<VaultApi>({
   id: 'plugin.vault.service',
 });
 
+/**
+ * Object containing the secret name and some links.
+ * @public
+ */
 export type VaultSecret = {
   name: string;
   showUrl: string;
   editUrl: string;
 };
 
+/**
+ * Interface for the VaultApi.
+ * @public
+ */
 export interface VaultApi {
+  /**
+   * Returns a list of secrets used to show in a table.
+   * @param secretPath - The path where the secrets are stored in Vault
+   */
   listSecrets(secretPath: string): Promise<VaultSecret[]>;
 }
 
+/**
+ * Default implementation of the VaultApi.
+ * @public
+ */
 export class VaultClient implements VaultApi {
   private readonly discoveryApi: DiscoveryApi;
 
@@ -39,7 +59,7 @@ export class VaultClient implements VaultApi {
   private async callApi<T>(
     path: string,
     query: { [key in string]: any },
-  ): Promise<T | undefined> {
+  ): Promise<T> {
     const apiUrl = `${await this.discoveryApi.getBaseUrl('vault')}`;
     const response = await fetch(
       `${apiUrl}/${path}?${new URLSearchParams(query).toString()}`,
@@ -49,23 +69,21 @@ export class VaultClient implements VaultApi {
         },
       },
     );
-    if (response.status === 200) {
+    if (response.ok) {
       return (await response.json()) as T;
+    } else if (response.status === 404) {
+      throw new NotFoundError(`No secrets found in path '${path}'`);
     }
-    return undefined;
+    throw new Error(
+      `Unexpected error while fetching secrets from path '${path}'`,
+    );
   }
 
   async listSecrets(secretPath: string): Promise<VaultSecret[]> {
-    if (secretPath === '') {
-      return [];
-    }
-    const result = await this.callApi<VaultSecret[]>(
+    const result = await this.callApi<{ items: VaultSecret[] }>(
       `v1/secrets/${encodeURIComponent(secretPath)}`,
       {},
     );
-    if (!result) {
-      return [];
-    }
-    return result;
+    return result.items;
   }
 }

plugins/vault/src/components/EntityVaultTable/EntityVaultTable.test.tsx

@@ -68,18 +68,20 @@ describe('EntityVaultTable', () => {
     },
   };
 
-  const mockSecretsResult: VaultSecret[] = [
-    {
-      name: 'secret::one',
-      editUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/edit/test/success/secret::one`,
-      showUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/show/test/success/secret::one`,
-    },
-    {
-      name: 'secret::two',
-      editUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/edit/test/success/secret::two`,
-      showUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/show/test/success/secret::two`,
-    },
-  ];
+  const mockSecretsResult: { items: VaultSecret[] } = {
+    items: [
+      {
+        name: 'secret::one',
+        editUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/edit/test/success/secret::one`,
+        showUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/show/test/success/secret::one`,
+      },
+      {
+        name: 'secret::two',
+        editUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/edit/test/success/secret::two`,
+        showUrl: `${mockBaseUrl}/ui/vault/secrets/secrets/show/test/success/secret::two`,
+      },
+    ],
+  };
 
   const setupHandlers = () => {
     server.use(

plugins/vault/src/components/EntityVaultTable/EntityVaultTable.tsx

@@ -15,7 +15,7 @@
  */
 import React from 'react';
 import { Entity } from '@backstage/catalog-model';
-import { Table, TableColumn } from '@backstage/core-components';
+import { Link, Table, TableColumn } from '@backstage/core-components';
 import { useApi } from '@backstage/core-plugin-api';
 import { Box, Typography } from '@material-ui/core';
 import Edit from '@material-ui/icons/Edit';
@@ -27,7 +27,7 @@ import { VAULT_SECRET_PATH_ANNOTATION } from '../../constants';
 
 export const vaultSecretPath = (entity: Entity) => {
   const secretPath =
-    entity.metadata.annotations?.[VAULT_SECRET_PATH_ANNOTATION] ?? '';
+    entity.metadata.annotations?.[VAULT_SECRET_PATH_ANNOTATION];
 
   return { secretPath };
 };
@@ -35,6 +35,11 @@ export const vaultSecretPath = (entity: Entity) => {
 export const EntityVaultTable = ({ entity }: { entity: Entity }) => {
   const vaultApi = useApi(vaultApiRef);
   const { secretPath } = vaultSecretPath(entity);
+  if (!secretPath) {
+    throw Error(
+      `The secret path is undefined. Please, define the annotation ${VAULT_SECRET_PATH_ANNOTATION}`,
+    );
+  }
   const { value, loading, error } = useAsync(async (): Promise<
     VaultSecret[]
   > => {
@@ -51,22 +56,22 @@ export const EntityVaultTable = ({ entity }: { entity: Entity }) => {
     return {
       secret: secret.name,
       view: (
-        <a
+        <Link
           aria-label="View"
           title={`View ${secret.name}`}
-          href={secret.showUrl}
+          to={secret.showUrl}
         >
           <Visibility style={{ fontSize: 16 }} />
-        </a>
+        </Link>
       ),
       edit: (
-        <a
+        <Link
           aria-label="Edit"
           title={`Edit ${secret.name}`}
-          href={secret.editUrl}
+          to={secret.editUrl}
         >
           <Edit style={{ fontSize: 16 }} />
-        </a>
+        </Link>
       ),
     };
   });

plugins/vault/src/conditions.ts

@@ -16,6 +16,12 @@
 import { Entity } from '@backstage/catalog-model';
 import { VAULT_SECRET_PATH_ANNOTATION } from './constants';
 
+/**
+ * Checks if an entity contains the annotation for Vault.
+ * @param entity - The entity to check if the annotation exists
+ * @returns If the annotation exists or not
+ * @public
+ */
 export function isVaultAvailable(entity: Entity): boolean {
   return Boolean(
     entity.metadata.annotations?.hasOwnProperty(VAULT_SECRET_PATH_ANNOTATION),

plugins/vault/src/constants.ts

@@ -13,4 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
+/**
+ * @public
+ */
 export const VAULT_SECRET_PATH_ANNOTATION = 'vault.io/secrets-path';

plugins/vault/src/index.ts

@@ -14,3 +14,7 @@
  * limitations under the License.
  */
 export { vaultPlugin, EntityVaultCard } from './plugin';
+export { isVaultAvailable } from './conditions';
+export { VAULT_SECRET_PATH_ANNOTATION } from './constants';
+export { vaultApiRef } from './api';
+export type { VaultApi, VaultSecret } from './api';