[FEATURE] Error codes for key vault lib (#535)

* [FEATURE] Error codes for key vault lib

* Update i18n.json

Author: dennisseah

src/commands/project/init.test.ts

@@ -119,7 +119,7 @@ describe("Test execute function", () => {
 
   it("negative test", async () => {
     jest.spyOn(init, "initialize").mockImplementation(() => {
-      throw new Error();
+      throw Error();
     });
     const exitFn = jest.fn();
     const randomDir = createTempDir();

src/lib/azure/keyvault.test.ts

@@ -1,15 +1,30 @@
+import { PipelineOptions } from "@azure/core-http";
 import { KeyVaultSecret } from "@azure/keyvault-secrets";
 import uuid from "uuid/v4";
 import { disableVerboseLogging, enableVerboseLogging } from "../../logger";
-import { getSecret, setSecret } from "./keyvault";
+import { getErrorMessage } from "../errorBuilder";
+import * as azurecredentials from "./azurecredentials";
+import { getClient, getSecret, setSecret } from "./keyvault";
 import * as keyvault from "./keyvault";
+import { TokenCredential } from "azure-storage";
 
 const keyVaultName = uuid();
 const mockedName = uuid();
 const secretValue = uuid();
 
-jest.spyOn(keyvault, "getClient").mockReturnValue(
-  Promise.resolve({
+jest.mock("@azure/keyvault-secrets", () => {
+  class MockClient {
+    constructor() {
+      return {};
+    }
+  }
+  return {
+    SecretClient: MockClient,
+  };
+});
+
+const mockGetClient = (): void => {
+  jest.spyOn(keyvault, "getClient").mockResolvedValueOnce({
     getSecret: async (): Promise<KeyVaultSecret> => {
       return {
         name: "test",
@@ -30,8 +45,8 @@ jest.spyOn(keyvault, "getClient").mockReturnValue(
       };
     },
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  } as any)
-);
+  } as any);
+};
 
 beforeAll(() => {
   enableVerboseLogging();
@@ -42,64 +57,64 @@ afterAll(() => {
 });
 
 describe("set secret", () => {
-  test("should fail when all arguments are not specified", async () => {
-    await expect(setSecret("", "", "")).rejects.toThrow();
+  test("negative test: missing values for name, key name and value.", async () => {
+    await expect(setSecret("", "", "")).rejects.toThrow(
+      getErrorMessage("azure-key-vault-set-secret-err")
+    );
   });
-  test("should create storage account", async () => {
-    try {
-      await setSecret(keyVaultName, mockedName, secretValue);
-    } catch (_) {
-      expect(true).toBe(false);
-    }
+  test("negative test: missing values for key name and value.", async () => {
+    await expect(setSecret("vault-name", "", "")).rejects.toThrow(
+      getErrorMessage("azure-key-vault-set-secret-err")
+    );
   });
-  test("negative test", async () => {
-    jest.spyOn(keyvault, "getClient").mockReturnValueOnce(
-      Promise.resolve({
-        setSecret: (): Promise<KeyVaultSecret> => {
-          throw new Error("fake error");
-        },
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      } as any)
+  test("negative test: missing key value.", async () => {
+    await expect(setSecret("vault-name", "key-name", "")).rejects.toThrow(
+      getErrorMessage("azure-key-vault-set-secret-err")
     );
-    try {
-      await setSecret(keyVaultName, mockedName, secretValue);
-      expect(true).toBe(false);
-    } catch (e) {
-      expect(e).toBeDefined();
-    }
+  });
+
+  test("positive test: should create storage account", async () => {
+    mockGetClient();
+    await setSecret(keyVaultName, mockedName, secretValue);
+  });
+  test("negative test: getclient failed", async () => {
+    jest.spyOn(keyvault, "getClient").mockResolvedValueOnce({
+      setSecret: (): Promise<KeyVaultSecret> => {
+        throw Error("fake error");
+      },
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    } as any);
+
+    await expect(
+      setSecret(keyVaultName, mockedName, secretValue)
+    ).rejects.toThrow(getErrorMessage("azure-key-vault-set-secret-err"));
   });
 });
 
 describe("get secret", () => {
-  test("should fail getting storage account key when arguments are not specified", async () => {
-    await expect(getSecret("", "")).rejects.toThrow();
+  test("negative test: missing values for name and key name.", async () => {
+    await expect(getSecret("", "")).rejects.toThrow(
+      getErrorMessage("azure-key-vault-get-secret-err")
+    );
   });
-  it("should get storage account key", async () => {
-    try {
-      const val = await getSecret(keyVaultName, mockedName);
-      expect(val).toBe("secretValue");
-    } catch (err) {
-      expect(true).toBe(false);
-    }
+  it("positive test: should get storage account key", async () => {
+    mockGetClient();
+    const val = await getSecret(keyVaultName, mockedName);
+    expect(val).toBe("secretValue");
   });
   it("negative test: secret not found", async () => {
-    jest.spyOn(keyvault, "getClient").mockReturnValueOnce(
-      Promise.resolve({
-        getSecret: (): Promise<KeyVaultSecret> => {
-          throw {
-            code: "SecretNotFound",
-            statusCode: 404,
-          };
-        },
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      } as any)
-    );
-    try {
-      const val = await getSecret(keyVaultName, mockedName);
-      expect(val).toBe(undefined);
-    } catch (err) {
-      expect(true).toBe(false);
-    }
+    jest.spyOn(keyvault, "getClient").mockResolvedValueOnce({
+      getSecret: (): Promise<KeyVaultSecret> => {
+        throw {
+          code: "SecretNotFound",
+          statusCode: 404,
+        };
+      },
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    } as any);
+
+    const val = await getSecret(keyVaultName, mockedName);
+    expect(val).toBe(undefined);
   });
   it("negative test: other errors", async () => {
     jest.spyOn(keyvault, "getClient").mockReturnValueOnce(
@@ -113,11 +128,27 @@ describe("get secret", () => {
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
       } as any)
     );
-    try {
-      await getSecret(keyVaultName, mockedName);
-      expect(true).toBe(false);
-    } catch (err) {
-      expect(err).toBeDefined();
-    }
+
+    await expect(getSecret(keyVaultName, mockedName)).rejects.toThrow(
+      getErrorMessage("azure-key-vault-get-secret-err")
+    );
+  });
+});
+
+describe("test getClient function", () => {
+  it("negative test: missing credential", async () => {
+    jest
+      .spyOn(azurecredentials, "getCredentials")
+      .mockRejectedValueOnce(new Error());
+    await expect(getClient(keyVaultName, {})).rejects.toThrow(
+      getErrorMessage("azure-key-vault-client-err")
+    );
+  });
+  it("positive test", async () => {
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    jest
+      .spyOn(azurecredentials, "getCredentials")
+      .mockResolvedValueOnce({} as any);
+    await getClient(keyVaultName, {});
   });
 });

src/lib/azure/keyvault.ts

@@ -1,36 +1,51 @@
 import { SecretClient } from "@azure/keyvault-secrets";
 import { logger } from "../../logger";
 import { AzureAccessOpts } from "../../types";
+import { build as buildError } from "../errorBuilder";
+import { errorStatusCode } from "../errorStatusCode";
 import { getCredentials } from "./azurecredentials";
 
 export const validateValues = (
   keyVaultName: string,
   secretName: string,
   secretValue?: string
 ): void => {
-  const errors: string[] = [];
   if (!keyVaultName) {
-    errors.push(`Invalid keyVaultName`);
+    throw buildError(
+      errorStatusCode.VALIDATION_ERR,
+      "azure-key-vault-missing-name"
+    );
   }
   if (!secretName) {
-    errors.push(`Invalid secretName`);
+    throw buildError(
+      errorStatusCode.VALIDATION_ERR,
+      "azure-key-vault-missing-secret-name"
+    );
   }
   if (secretValue !== undefined && !secretValue) {
-    errors.push(`Invalid secretValue`);
-  }
-  if (errors.length !== 0) {
-    throw Error(`\n${errors.join("\n")}`);
+    throw buildError(
+      errorStatusCode.VALIDATION_ERR,
+      "azure-key-vault-missing-secret-value"
+    );
   }
 };
 
 export const getClient = async (
   keyVaultName: string,
   opts: AzureAccessOpts
 ): Promise<SecretClient> => {
-  const url = `https://${keyVaultName}.vault.azure.net`;
-  const credentials = await getCredentials(opts);
-  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-  return new SecretClient(url, credentials!);
+  try {
+    const url = `https://${keyVaultName}.vault.azure.net`;
+    const credentials = await getCredentials(opts);
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    return new SecretClient(url, credentials!);
+  } catch (err) {
+    throw buildError(
+      errorStatusCode.AZURE_KEY_VAULT_ERR,
+      "azure-key-vault-client-err",
+      err
+    );
+  }
 };
 
 /**
@@ -48,17 +63,20 @@ export const setSecret = async (
   secretValue: string,
   opts: AzureAccessOpts = {}
 ): Promise<void> => {
-  validateValues(keyVaultName, secretName, secretValue);
-  const messageWithNoValue = `secret ${secretName} in key vault ${keyVaultName}`;
-
   try {
+    validateValues(keyVaultName, secretName, secretValue);
+    const messageWithNoValue = `secret ${secretName} in key vault ${keyVaultName}`;
+
     const client = await getClient(keyVaultName, opts);
     logger.debug(`Setting ${messageWithNoValue}`);
     await client.setSecret(secretName, secretValue);
     logger.debug(`Setting ${messageWithNoValue} is complete`);
   } catch (err) {
-    logger.error(`Unable to set ${messageWithNoValue}. \n ${err}`);
-    throw err;
+    throw buildError(
+      errorStatusCode.AZURE_KEY_VAULT_ERR,
+      "azure-key-vault-set-secret-err",
+      err
+    );
   }
 };
 
@@ -75,10 +93,10 @@ export const getSecret = async (
   secretName: string,
   opts: AzureAccessOpts = {}
 ): Promise<string | undefined> => {
-  validateValues(keyVaultName, secretName);
-
-  const message = `secret ${secretName} from key vault ${keyVaultName}`;
   try {
+    validateValues(keyVaultName, secretName);
+    const message = `secret ${secretName} from key vault ${keyVaultName}`;
+
     const client = await getClient(keyVaultName, opts);
     logger.debug(`Getting ${message}`);
     const latestSecret = await client.getSecret(secretName);
@@ -88,7 +106,10 @@ export const getSecret = async (
     if (err.code === "SecretNotFound" && err.statusCode === 404) {
       return undefined;
     }
-    logger.error(`Unable to read ${message}. \n ${err}`);
-    throw err;
+    throw buildError(
+      errorStatusCode.AZURE_KEY_VAULT_ERR,
+      "azure-key-vault-get-secret-err",
+      err
+    );
   }
 };

src/lib/errorStatusCode.ts

@@ -15,6 +15,7 @@ export enum errorStatusCode {
   PIPELINE_ERR = 1500,
   COMMANDER_ERR = 1600,
   AZURE_SUBSCRIPTION_ERR = 1700,
+  AZURE_KEY_VAULT_ERR = 1800,
   AZURE_CLI_ERR = 1900,
   AZURE_STORAGE_OP_ERR = 2000,
   AZURE_RESOURCE_GROUP_ERR = 2500,

src/lib/i18n.json

@@ -301,6 +301,7 @@
     "validation-err-storage-key-vault-hyphen": "Storage Key Vault was invalid as it cannot contain consecutive hyphens.",
     "validation-err-storage-key-vault-length": "Storage Key Vault was invalid as it has to be between 3 and 24 characters long.",
     "validation-err-storage-access-key-missing": "Storage Access Key was missing. Provide it.",
+    "validation-err-rfc1123-compliant": "Invalid {0} '{1}' was provided. Must be RFC1123 compliant and match regex: {2}",
 
     "service-endpoint-err-validation": "Missing mandatory values ({0}) for service endpoint.",
     "service-endpoint-err-create-params": "Could not create parameters for service endpoint.",
@@ -317,6 +318,13 @@
     "azure-client-get-build-client-err": "Could not get build client. Check the Azure DevOps credential",
     "azure-client-get-task-agent-client-err": "Could not get task agent client. Check the Azure DevOps credential.",
 
+    "azure-key-vault-missing-name": "Missing key vault name. Provide it",
+    "azure-key-vault-missing-secret-name": "Missing key vault secret name. Provide it",
+    "azure-key-vault-missing-secret-value": "Missing key vault secret value. Provide it",
+    "azure-key-vault-client-err": "Could not get Azure key vault client. Check your Azure credentials.",
+    "azure-key-vault-set-secret-err": "Could not set secret in Azure key vault client.",
+    "azure-key-vault-get-secret-err": "Could not get secret from Azure key vault client.",
+
     "bedrock-yaml-ring-set-default-not-found": "Could not set default ring because ring {0} is not defined in {1}.",
     "bedrock-yaml-ring-remove-not-found": "Could not remove ring because ring {0} is not defined in {1}.",
     "bedrock-yaml-ring-remove-default": "Could not remove ring because ring {0} was set to isDefault. Set another default ring with 'spk ring set-default' first before attempting to delete",

src/lib/net/dns.ts

@@ -5,6 +5,9 @@
 // segments.
 ////////////////////////////////////////////////////////////////////////////////
 
+import { build as buildError } from "../errorBuilder";
+import { errorStatusCode } from "../errorStatusCode";
+
 export const validDnsRegex = /^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$/;
 
 /**
@@ -54,8 +57,9 @@ export function assertIsValid(
   dns: string
 ): asserts dns is string {
   if (!isValid(dns)) {
-    throw Error(
-      `Invalid ${fieldName} '${dns}' provided. Must be RFC1123 compliant and match regex: ${validDnsRegex}`
-    );
+    throw buildError(errorStatusCode.VALIDATION_ERR, {
+      errorKey: "validation-err-rfc1123-compliant",
+      values: [fieldName, dns, validDnsRegex.toString()],
+    });
   }
 }