Address review items

- Fix versions in readme - Make expected test values readable - Support typed vars for testing
10 months ago · d06cd4a2e4
parent 4b0ac2a6fb
commit d06cd4a2e4
6 changed files with 103 additions and 69 deletions
--- a/jfrog-oauth/README.md
+++ b/jfrog-oauth/README.md
@ -17,7 +17,7 @@ Install the JF CLI and authenticate package managers with Artifactory using OAut
 ```tf
 module "jfrog" {
  source         = "registry.coder.com/modules/jfrog-oauth/coder"
-  version        = "2.0.0"
+  version        = "1.0.19"
  agent_id       = coder_agent.example.id
  jfrog_url      = "https://example.jfrog.io"
  username_field = "username" # If you are using GitHub to login to both Coder and Artifactory, use username_field = "username"
@ -45,7 +45,7 @@ Configure the Python pip package manager to fetch packages from Artifactory whil
 ```tf
 module "jfrog" {
  source         = "registry.coder.com/modules/jfrog-oauth/coder"
-  version        = "2.0.0"
+  version        = "1.0.19"
  agent_id       = coder_agent.example.id
  jfrog_url      = "https://example.jfrog.io"
  username_field = "email"
@ -73,7 +73,7 @@ The [JFrog extension](https://open-vsx.org/extension/JFrog/jfrog-vscode-extensio
 ```tf
 module "jfrog" {
  source                = "registry.coder.com/modules/jfrog-oauth/coder"
-  version               = "2.0.0"
+  version               = "1.0.19"
  agent_id              = coder_agent.example.id
  jfrog_url             = "https://example.jfrog.io"
  username_field        = "username" # If you are using GitHub to login to both Coder and Artifactory, use username_field = "username"
--- a/jfrog-oauth/main.test.ts
+++ b/jfrog-oauth/main.test.ts
@ -7,13 +7,25 @@ import {
 } from "../test";

 describe("jfrog-oauth", async () => {
+  type TestVariables = {
+    agent_id: string;
+    jfrog_url: string;
+    package_managers: string;
+
+    username_field?: string;
+    jfrog_server_id?: string;
+    external_auth_id?: string;
+    configure_code_server?: boolean;
+  };
+
  await runTerraformInit(import.meta.dir);

-  const fakeFrogHostAndPort = "localhost:8081";
-  const fakeFrogUrl = `http://${fakeFrogHostAndPort}`;
+  const fakeFrogApi = "localhost:8081/artifactory/api";
+  const fakeFrogUrl = "http://localhost:8081";
+  const user = "default";

  it("can run apply with required variables", async () => {
-    testRequiredVariables(import.meta.dir, {
+    testRequiredVariables<TestVariables>(import.meta.dir, {
      agent_id: "some-agent-id",
      jfrog_url: fakeFrogUrl,
      package_managers: "{}",
@ -21,7 +33,7 @@ describe("jfrog-oauth", async () => {
  });

  it("generates an npmrc with scoped repos", async () => {
-    const state = await runTerraformApply(import.meta.dir, {
+    const state = await runTerraformApply<TestVariables>(import.meta.dir, {
      agent_id: "some-agent-id",
      jfrog_url: fakeFrogUrl,
      package_managers: JSON.stringify({
@ -30,13 +42,13 @@ describe("jfrog-oauth", async () => {
    });
    const coderScript = findResourceInstance(state, "coder_script");
    const npmrcStanza = `cat << EOF > ~/.npmrc
-email=default@example.com
-registry=${fakeFrogUrl}/artifactory/api/npm/global
-//${fakeFrogHostAndPort}/artifactory/api/npm/global/:_authToken=
-@foo:registry=${fakeFrogUrl}/artifactory/api/npm/foo
-//${fakeFrogHostAndPort}/artifactory/api/npm/foo/:_authToken=
-@bar:registry=${fakeFrogUrl}/artifactory/api/npm/bar
-//${fakeFrogHostAndPort}/artifactory/api/npm/bar/:_authToken=
+email=${user}@example.com
+registry=http://${fakeFrogApi}/npm/global
+//${fakeFrogApi}/npm/global/:_authToken=
+@foo:registry=http://${fakeFrogApi}/npm/foo
+//${fakeFrogApi}/npm/foo/:_authToken=
+@bar:registry=http://${fakeFrogApi}/npm/bar
+//${fakeFrogApi}/npm/bar/:_authToken=

 EOF`;
    expect(coderScript.script).toContain(npmrcStanza);
@ -49,7 +61,7 @@ EOF`;
  });

  it("generates a pip config with extra-indexes", async () => {
-    const state = await runTerraformApply(import.meta.dir, {
+    const state = await runTerraformApply<TestVariables>(import.meta.dir, {
      agent_id: "some-agent-id",
      jfrog_url: fakeFrogUrl,
      package_managers: JSON.stringify({
@ -59,10 +71,10 @@ EOF`;
    const coderScript = findResourceInstance(state, "coder_script");
    const pipStanza = `cat << EOF > ~/.pip/pip.conf
 [global]
-index-url = https://default:@${fakeFrogHostAndPort}/artifactory/api/pypi/global/simple
+index-url = https://${user}:@${fakeFrogApi}/pypi/global/simple
 extra-index-url =
-    https://default:@${fakeFrogHostAndPort}/artifactory/api/pypi/foo/simple
-    https://default:@${fakeFrogHostAndPort}/artifactory/api/pypi/bar/simple
+    https://${user}:@${fakeFrogApi}/pypi/foo/simple
+    https://${user}:@${fakeFrogApi}/pypi/bar/simple

 EOF`;
    expect(coderScript.script).toContain(pipStanza);
@ -75,7 +87,7 @@ EOF`;
  });

  it("registers multiple docker repos", async () => {
-    const state = await runTerraformApply(import.meta.dir, {
+    const state = await runTerraformApply<TestVariables>(import.meta.dir, {
      agent_id: "some-agent-id",
      jfrog_url: fakeFrogUrl,
      package_managers: JSON.stringify({
@ -83,9 +95,9 @@ EOF`;
      }),
    });
    const coderScript = findResourceInstance(state, "coder_script");
-    const dockerStanza = `register_docker "foo.jfrog.io"
-register_docker "bar.jfrog.io"
-register_docker "baz.jfrog.io"`;
+    const dockerStanza = ["foo", "bar", "baz"]
+      .map((r) => `register_docker "${r}.jfrog.io"`)
+      .join("\n");
    expect(coderScript.script).toContain(dockerStanza);
    expect(coderScript.script).toContain(
      'if [ -z "YES" ]; then\n  not_configured docker',
@ -93,7 +105,7 @@ register_docker "baz.jfrog.io"`;
  });

  it("sets goproxy with multiple repos", async () => {
-    const state = await runTerraformApply(import.meta.dir, {
+    const state = await runTerraformApply<TestVariables>(import.meta.dir, {
      agent_id: "some-agent-id",
      jfrog_url: fakeFrogUrl,
      package_managers: JSON.stringify({
@ -101,7 +113,9 @@ register_docker "baz.jfrog.io"`;
      }),
    });
    const proxyEnv = findResourceInstance(state, "coder_env", "goproxy");
-    const proxies = `https://default:@${fakeFrogHostAndPort}/artifactory/api/go/foo,https://default:@${fakeFrogHostAndPort}/artifactory/api/go/bar,https://default:@${fakeFrogHostAndPort}/artifactory/api/go/baz`;
+    const proxies = ["foo", "bar", "baz"]
+      .map((r) => `https://${user}:@${fakeFrogApi}/go/${r}`)
+      .join(",");
    expect(proxyEnv["value"]).toEqual(proxies);

    const coderScript = findResourceInstance(state, "coder_script");
--- a/jfrog-token/README.md
+++ b/jfrog-token/README.md
@ -15,7 +15,7 @@ Install the JF CLI and authenticate package managers with Artifactory using Arti
 ```tf
 module "jfrog" {
  source                   = "registry.coder.com/modules/jfrog-token/coder"
-  version                  = "2.0.0"
+  version                  = "1.0.19"
  agent_id                 = coder_agent.example.id
  jfrog_url                = "https://XXXX.jfrog.io"
  artifactory_access_token = var.artifactory_access_token
@ -42,7 +42,7 @@ For detailed instructions, please see this [guide](https://coder.com/docs/v2/lat
 ```tf
 module "jfrog" {
  source                   = "registry.coder.com/modules/jfrog-token/coder"
-  version                  = "2.0.0"
+  version                  = "1.0.19"
  agent_id                 = coder_agent.example.id
  jfrog_url                = "https://YYYY.jfrog.io"
  artifactory_access_token = var.artifactory_access_token # An admin access token
@ -75,7 +75,7 @@ The [JFrog extension](https://open-vsx.org/extension/JFrog/jfrog-vscode-extensio
 ```tf
 module "jfrog" {
  source                   = "registry.coder.com/modules/jfrog-token/coder"
-  version                  = "2.0.0"
+  version                  = "1.0.19"
  agent_id                 = coder_agent.example.id
  jfrog_url                = "https://XXXX.jfrog.io"
  artifactory_access_token = var.artifactory_access_token
@ -95,7 +95,7 @@ data "coder_workspace" "me" {}

 module "jfrog" {
  source                   = "registry.coder.com/modules/jfrog-token/coder"
-  version                  = "2.0.0"
+  version                  = "1.0.19"
  agent_id                 = coder_agent.example.id
  jfrog_url                = "https://XXXX.jfrog.io"
  artifactory_access_token = var.artifactory_access_token
--- a/jfrog-token/main.test.ts
+++ b/jfrog-token/main.test.ts
@ -7,8 +7,24 @@ import {
  runTerraformApply,
  testRequiredVariables,
 } from "../test";
+import { Test } from "test";

 describe("jfrog-token", async () => {
+  type TestVariables = {
+    agent_id: string;
+    jfrog_url: string;
+    artifactory_access_token: string;
+    package_managers: string;
+
+    token_description?: string;
+    check_license?: boolean;
+    refreshable?: boolean;
+    expires_in?: number;
+    username_field?: string;
+    jfrog_server_id?: string;
+    configure_code_server?: boolean;
+  };
+
  await runTerraformInit(import.meta.dir);

  // Run a fake JFrog server so the provider can initialize
@ -34,11 +50,13 @@ describe("jfrog-token", async () => {
    port: 0,
  });

-  const fakeFrogHostAndPort = `${fakeFrogHost.hostname}:${fakeFrogHost.port}`;
-  const fakeFrogUrl = `http://${fakeFrogHostAndPort}`;
+  const fakeFrogApi = `${fakeFrogHost.hostname}:${fakeFrogHost.port}/artifactory/api`;
+  const fakeFrogUrl = `http://${fakeFrogHost.hostname}:${fakeFrogHost.port}`;
+  const user = "default";
+  const token = "xxx";

  it("can run apply with required variables", async () => {
-    testRequiredVariables(import.meta.dir, {
+    testRequiredVariables<TestVariables>(import.meta.dir, {
      agent_id: "some-agent-id",
      jfrog_url: fakeFrogUrl,
      artifactory_access_token: "XXXX",
@ -47,7 +65,7 @@ describe("jfrog-token", async () => {
  });

  it("generates an npmrc with scoped repos", async () => {
-    const state = await runTerraformApply(import.meta.dir, {
+    const state = await runTerraformApply<TestVariables>(import.meta.dir, {
      agent_id: "some-agent-id",
      jfrog_url: fakeFrogUrl,
      artifactory_access_token: "XXXX",
@ -57,13 +75,13 @@ describe("jfrog-token", async () => {
    });
    const coderScript = findResourceInstance(state, "coder_script");
    const npmrcStanza = `cat << EOF > ~/.npmrc
-email=default@example.com
-registry=${fakeFrogUrl}/artifactory/api/npm/global
-//${fakeFrogHostAndPort}/artifactory/api/npm/global/:_authToken=xxx
-@foo:registry=${fakeFrogUrl}/artifactory/api/npm/foo
-//${fakeFrogHostAndPort}/artifactory/api/npm/foo/:_authToken=xxx
-@bar:registry=${fakeFrogUrl}/artifactory/api/npm/bar
-//${fakeFrogHostAndPort}/artifactory/api/npm/bar/:_authToken=xxx
+email=${user}@example.com
+registry=http://${fakeFrogApi}/npm/global
+//${fakeFrogApi}/npm/global/:_authToken=xxx
+@foo:registry=http://${fakeFrogApi}/npm/foo
+//${fakeFrogApi}/npm/foo/:_authToken=xxx
+@bar:registry=http://${fakeFrogApi}/npm/bar
+//${fakeFrogApi}/npm/bar/:_authToken=xxx

 EOF`;
    expect(coderScript.script).toContain(npmrcStanza);
@ -76,7 +94,7 @@ EOF`;
  });

  it("generates a pip config with extra-indexes", async () => {
-    const state = await runTerraformApply(import.meta.dir, {
+    const state = await runTerraformApply<TestVariables>(import.meta.dir, {
      agent_id: "some-agent-id",
      jfrog_url: fakeFrogUrl,
      artifactory_access_token: "XXXX",
@ -87,10 +105,10 @@ EOF`;
    const coderScript = findResourceInstance(state, "coder_script");
    const pipStanza = `cat << EOF > ~/.pip/pip.conf
 [global]
-index-url = https://default:xxx@${fakeFrogHostAndPort}/artifactory/api/pypi/global/simple
+index-url = https://${user}:${token}@${fakeFrogApi}/pypi/global/simple
 extra-index-url =
-    https://default:xxx@${fakeFrogHostAndPort}/artifactory/api/pypi/foo/simple
-    https://default:xxx@${fakeFrogHostAndPort}/artifactory/api/pypi/bar/simple
+    https://${user}:${token}@${fakeFrogApi}/pypi/foo/simple
+    https://${user}:${token}@${fakeFrogApi}/pypi/bar/simple

 EOF`;
    expect(coderScript.script).toContain(pipStanza);
@ -103,7 +121,7 @@ EOF`;
  });

  it("registers multiple docker repos", async () => {
-    const state = await runTerraformApply(import.meta.dir, {
+    const state = await runTerraformApply<TestVariables>(import.meta.dir, {
      agent_id: "some-agent-id",
      jfrog_url: fakeFrogUrl,
      artifactory_access_token: "XXXX",
@ -112,9 +130,9 @@ EOF`;
      }),
    });
    const coderScript = findResourceInstance(state, "coder_script");
-    const dockerStanza = `register_docker "foo.jfrog.io"
-register_docker "bar.jfrog.io"
-register_docker "baz.jfrog.io"`;
+    const dockerStanza = ["foo", "bar", "baz"]
+      .map((r) => `register_docker "${r}.jfrog.io"`)
+      .join("\n");
    expect(coderScript.script).toContain(dockerStanza);
    expect(coderScript.script).toContain(
      'if [ -z "YES" ]; then\n  not_configured docker',
@ -122,7 +140,7 @@ register_docker "baz.jfrog.io"`;
  });

  it("sets goproxy with multiple repos", async () => {
-    const state = await runTerraformApply(import.meta.dir, {
+    const state = await runTerraformApply<TestVariables>(import.meta.dir, {
      agent_id: "some-agent-id",
      jfrog_url: fakeFrogUrl,
      artifactory_access_token: "XXXX",
@ -131,7 +149,9 @@ register_docker "baz.jfrog.io"`;
      }),
    });
    const proxyEnv = findResourceInstance(state, "coder_env", "goproxy");
-    const proxies = `https://default:xxx@${fakeFrogHostAndPort}/artifactory/api/go/foo,https://default:xxx@${fakeFrogHostAndPort}/artifactory/api/go/bar,https://default:xxx@${fakeFrogHostAndPort}/artifactory/api/go/baz`;
+    const proxies = ["foo", "bar", "baz"]
+      .map((r) => `https://${user}:${token}@${fakeFrogApi}/go/${r}`)
+      .join(",");
    expect(proxyEnv["value"]).toEqual(proxies);

    const coderScript = findResourceInstance(state, "coder_script");
--- a/terraform_validate.sh
+++ b/terraform_validate.sh
@ -4,25 +4,25 @@ set -euo pipefail

 # Function to run terraform init and validate in a directory
 run_terraform() {
-    local dir="$1"
-    echo "Running terraform init and validate in $dir"
-    pushd "$dir"
-    terraform init -upgrade
-    terraform validate
-    popd
+  local dir="$1"
+  echo "Running terraform init and validate in $dir"
+  pushd "$dir"
+  terraform init -upgrade
+  terraform validate
+  popd
 }

 # Main script
 main() {
-    # Get the directory of the script
-    script_dir=$(dirname "$(readlink -f "$0")")
+  # Get the directory of the script
+  script_dir=$(dirname "$(readlink -f "$0")")

-    # Get all subdirectories in the repository
-    subdirs=$(find "$script_dir" -mindepth 1 -maxdepth 1 -type d -not -name ".*" | sort)
+  # Get all subdirectories in the repository
+  subdirs=$(find "$script_dir" -mindepth 1 -maxdepth 1 -type d -not -name ".*" | sort)

-    for dir in $subdirs; do
-        run_terraform "$dir"
-    done
+  for dir in $subdirs; do
+    run_terraform "$dir"
+  done
 }

 # Run the main script
--- a/test.ts
+++ b/test.ts
@ -108,6 +108,8 @@ export interface TerraformState {
  resources: [TerraformStateResource, ...TerraformStateResource[]];
 }

+type TerraformVariables = Record<string, JsonValue>;
+
 export interface CoderScriptAttributes {
  script: string;
  agent_id: string;
@ -145,9 +147,9 @@ export const findResourceInstance = <T extends string>(
 * Creates a test-case for each variable provided and ensures that the apply
 * fails without it.
 */
-export const testRequiredVariables = <TVars extends Record<string, string>>(
+export const testRequiredVariables = <TVars extends TerraformVariables>(
  dir: string,
-  vars: TVars,
+  vars: Readonly<TVars>,
 ) => {
  // Ensures that all required variables are provided.
  it("required variables", async () => {
@ -158,7 +160,7 @@ export const testRequiredVariables = <TVars extends Record<string, string>>(
  varNames.forEach((varName) => {
    // Ensures that every variable provided is required!
    it("missing variable " + varName, async () => {
-      const localVars: Record<string, string> = {};
+      const localVars: TerraformVariables = {};
      varNames.forEach((otherVarName) => {
        if (otherVarName !== varName) {
          localVars[otherVarName] = vars[otherVarName];
@ -187,11 +189,9 @@ export const testRequiredVariables = <TVars extends Record<string, string>>(
 * fine to run in parallel with other instances of this function, as it uses a
 * random state file.
 */
-export const runTerraformApply = async <
-  TVars extends Readonly<Record<string, string | boolean>>,
->(
+export const runTerraformApply = async <TVars extends TerraformVariables>(
  dir: string,
-  vars: TVars,
+  vars: Readonly<TVars>,
  env?: Record<string, string>,
 ): Promise<TerraformState> => {
  const stateFile = `${dir}/${crypto.randomUUID()}.tfstate`;