Skip to content

chore: enable terraform provisioners in e2e by default #13134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -222,5 +222,7 @@
"go.testFlags": ["-short", "-coverpkg=./..."],
// We often use a version of TypeScript that's ahead of the version shipped
// with VS Code.
"typescript.tsdk": "./site/node_modules/typescript/lib"
"typescript.tsdk": "./site/node_modules/typescript/lib",
// Playwright tests in VSCode will open a browser to live "view" the test.
"playwright.reuseBrowser": true
Comment on lines -225 to +227
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required to run a "live" debugger in VSCode with the chrome window open the whole time. Otherwise the window keeps opening and closing for each snapshot step.

}
4 changes: 4 additions & 0 deletions site/e2e/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ export const requireEnterpriseTests = Boolean(
);
export const enterpriseLicense = process.env.CODER_E2E_ENTERPRISE_LICENSE ?? "";

// Disabling terraform tests is optional for environments without Docker + Terraform.
// By default, we opt into these tests.
export const requireTerraformTests = !process.env.CODER_E2E_DISABLE_TERRAFORM;
Comment on lines +42 to +44
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to include this option if the Terraform stuff ever gets a bit messy. Right now for example docker containers are leaky.


// Fake experiments to verify that site presents them as enabled.
export const e2eFakeExperiment1 = "e2e-fake-experiment-1";
export const e2eFakeExperiment2 = "e2e-fake-experiment-2";
52 changes: 40 additions & 12 deletions site/e2e/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
enterpriseLicense,
prometheusPort,
requireEnterpriseTests,
requireTerraformTests,
} from "./constants";
import { expectUrl } from "./expectUrl";
import {
Expand All @@ -43,6 +44,11 @@ export function requiresEnterpriseLicense() {
test.skip(!enterpriseLicense);
}

// requireTerraformProvisioner by default is enabled.
export function requireTerraformProvisioner() {
test.skip(!requireTerraformTests);
}

// createWorkspace creates a workspace for a template.
// It does not wait for it to be running, but it does navigate to the page.
export const createWorkspace = async (
Expand Down Expand Up @@ -149,25 +155,46 @@ export const verifyParameters = async (
}
};

// StarterTemplates are ids of starter templates that can be used in place of
// the responses payload. These starter templates will require real provisioners.
export enum StarterTemplates {
STARTER_DOCKER = "docker",
}
Comment on lines +158 to +162
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add ways to add more to the templates later. For now, just using the starter template is easier.


function isStarterTemplate(
input: EchoProvisionerResponses | StarterTemplates | undefined,
): input is StarterTemplates {
if (!input) {
return false;
}
return typeof input === "string";
}

// createTemplate navigates to the /templates/new page and uploads a template
// with the resources provided in the responses argument.
export const createTemplate = async (
page: Page,
responses?: EchoProvisionerResponses,
responses?: EchoProvisionerResponses | StarterTemplates,
): Promise<string> => {
// Required to have templates submit their provisioner type as echo!
await page.addInitScript({
content: "window.playwright = true",
});
let path = "/templates/new";
if (isStarterTemplate(responses)) {
path += `?exampleId=${responses}`;
} else {
// The form page will read this value and use it as the default type.
path += "?provisioner_type=echo";
}

await page.goto("/templates/new", { waitUntil: "domcontentloaded" });
await page.goto(path, { waitUntil: "domcontentloaded" });
await expectUrl(page).toHavePathName("/templates/new");

await page.getByTestId("file-upload").setInputFiles({
buffer: await createTemplateVersionTar(responses),
mimeType: "application/x-tar",
name: "template.tar",
});
if (!isStarterTemplate(responses)) {
await page.getByTestId("file-upload").setInputFiles({
buffer: await createTemplateVersionTar(responses),
mimeType: "application/x-tar",
name: "template.tar",
});
}

const name = randomName();
await page.getByLabel("Name *").fill(name);
await page.getByTestId("form-submit").click();
Expand Down Expand Up @@ -868,6 +895,7 @@ export async function openTerminalWindow(
page: Page,
context: BrowserContext,
workspaceName: string,
agentName: string = "dev",
): Promise<Page> {
// Wait for the web terminal to open in a new tab
const pagePromise = context.waitForEvent("page");
Expand All @@ -879,7 +907,7 @@ export async function openTerminalWindow(
// isn't POSIX compatible, such as Fish.
const commandQuery = `?command=${encodeURIComponent("/usr/bin/env bash")}`;
await expectUrl(terminal).toHavePathName(
`/@admin/${workspaceName}.dev/terminal`,
`/@admin/${workspaceName}.${agentName}/terminal`,
);
await terminal.goto(`/@admin/${workspaceName}.dev/terminal${commandQuery}`);

Expand Down
40 changes: 38 additions & 2 deletions site/e2e/playwright.config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { defineConfig } from "@playwright/test";
import { execSync } from "child_process";
import * as path from "path";
import {
coderMain,
Expand All @@ -7,13 +8,47 @@ import {
e2eFakeExperiment1,
e2eFakeExperiment2,
gitAuth,
requireTerraformTests,
} from "./constants";

export const wsEndpoint = process.env.CODER_E2E_WS_ENDPOINT;

// This is where auth cookies are stored!
export const storageState = path.join(__dirname, ".auth.json");

// If running terraform tests, verify the requirements exist in the
// environment.
//
// These execs will throw an error if the status code is non-zero.
// So if both these work, then we can launch terraform provisioners.
let hasTerraform = false;
let hasDocker = false;
try {
execSync("terraform --version");
hasTerraform = true;
} catch {
/* empty */
}

try {
execSync("docker --version");
hasDocker = true;
} catch {
/* empty */
}

if (!hasTerraform || !hasDocker) {
const msg =
"Terraform provisioners require docker & terraform binaries to function. \n" +
(hasTerraform
? ""
: "\tThe `terraform` executable is not present in the runtime environment.\n") +
(hasDocker
? ""
: "\tThe `docker` executable is not present in the runtime environment.\n");
throw new Error(msg);
}

const localURL = (port: number, path: string): string => {
return `http://localhost:${port}${path}`;
};
Expand Down Expand Up @@ -54,13 +89,14 @@ export default defineConfig({
`go run -tags embed ${coderMain} server`,
"--global-config $(mktemp -d -t e2e-XXXXXXXXXX)",
`--access-url=http://localhost:${coderPort}`,
`--http-address=localhost:${coderPort}`,
`--http-address=0.0.0.0:${coderPort}`,
"--in-memory",
"--telemetry=false",
"--dangerous-disable-rate-limits",
"--provisioner-daemons 10",
// TODO: Enable some terraform provisioners
"--provisioner-types=echo",
`--provisioner-types=echo${requireTerraformTests ? ",terraform" : ""}`,
`--provisioner-daemons=10`,
"--web-terminal-renderer=dom",
"--pprof-enable",
]
Expand Down
42 changes: 42 additions & 0 deletions site/e2e/tests/createWorkspace.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { test, expect } from "@playwright/test";
import {
StarterTemplates,
createTemplate,
createWorkspace,
echoResponsesWithParameters,
openTerminalWindow,
requireTerraformProvisioner,
verifyParameters,
} from "../helpers";
import { beforeCoderTest } from "../hooks";
Expand Down Expand Up @@ -147,3 +150,42 @@ test("create workspace with disable_param search params", async ({ page }) => {
await expect(page.getByLabel(/First parameter/i)).toBeDisabled();
await expect(page.getByLabel(/Second parameter/i)).toBeDisabled();
});

test("create docker workspace", async ({ context, page }) => {
test.skip(
true,
"creating docker containers is currently leaky. They are not cleaned up when the tests are over.",
);
Comment on lines +155 to +158
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to resolve this in another PR. Terraform provisioners are now working, just need to do some cleanup before I allow this test into the mainstage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracked here: #13163

requireTerraformProvisioner();
const template = await createTemplate(page, StarterTemplates.STARTER_DOCKER);

const workspaceName = await createWorkspace(page, template);

// The workspace agents must be ready before we try to interact with the workspace.
await page.waitForSelector(
`//div[@role="status"][@data-testid="agent-status-ready"]`,
{
state: "visible",
},
);

// Wait for the terminal button to be visible, and click it.
const terminalButton =
"//a[@data-testid='terminal'][normalize-space()='Terminal']";
await page.waitForSelector(terminalButton, {
state: "visible",
});

const terminal = await openTerminalWindow(
page,
context,
workspaceName,
"main",
);
await terminal.waitForSelector(
`//textarea[contains(@class,"xterm-helper-textarea")]`,
{
state: "visible",
},
);
});
14 changes: 14 additions & 0 deletions site/src/pages/CreateTemplatePage/CreateTemplateForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ import { useFormik } from "formik";
import camelCase from "lodash/camelCase";
import capitalize from "lodash/capitalize";
import type { FC } from "react";
import { useSearchParams } from "react-router-dom";
import * as Yup from "yup";
import type {
ProvisionerJobLog,
ProvisionerType,
Template,
TemplateExample,
TemplateVersionVariable,
Expand Down Expand Up @@ -50,6 +52,7 @@ export interface CreateTemplateData {
parameter_values_by_name?: Record<string, string>;
user_variable_values?: VariableValue[];
allow_everyone_group_access: boolean;
provisioner_type: ProvisionerType;
}

const validationSchema = Yup.object({
Expand Down Expand Up @@ -81,23 +84,31 @@ const defaultInitialValues: CreateTemplateData = {
allow_user_autostart: false,
allow_user_autostop: false,
allow_everyone_group_access: true,
provisioner_type: "terraform",
};

type GetInitialValuesParams = {
fromExample?: TemplateExample;
fromCopy?: Template;
variables?: TemplateVersionVariable[];
allowAdvancedScheduling: boolean;
searchParams: URLSearchParams;
};

const getInitialValues = ({
fromExample,
fromCopy,
allowAdvancedScheduling,
variables,
searchParams,
}: GetInitialValuesParams) => {
let initialValues = defaultInitialValues;

// Will assume the query param has a valid ProvisionerType, as this query param is only used
// in testing.
defaultInitialValues.provisioner_type =
(searchParams.get("provisioner_type") as ProvisionerType) || "terraform";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd kind of rather leave this as a string, because we're not doing any validation on it, so it could be anything

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean make provisioner_type a string?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the .get here returns string | undefined, and the || narrows it to string, but I think the cast here narrow it more than it should, basically. we don't actually know that the returned value is a valid ProvisionerType here because we didn't check!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is correct. The type of the defaultInitialValues.provisioner_type is ProvisionerType which I would like to keep. Essentially I think adding validation is probably the best approach, but since this is a hidden query param only used in testing, I just skipped it for now. The BE will return a validation error if someone discovers this and tries to do something unsupported.


if (!allowAdvancedScheduling) {
initialValues = {
...initialValues,
Expand Down Expand Up @@ -164,6 +175,7 @@ export type CreateTemplateFormProps = (
};

export const CreateTemplateForm: FC<CreateTemplateFormProps> = (props) => {
const [searchParams] = useSearchParams();
const {
onCancel,
onSubmit,
Expand All @@ -176,13 +188,15 @@ export const CreateTemplateForm: FC<CreateTemplateFormProps> = (props) => {
allowAdvancedScheduling,
variablesSectionRef,
} = props;

const form = useFormik<CreateTemplateData>({
initialValues: getInitialValues({
allowAdvancedScheduling,
fromExample:
"starterTemplate" in props ? props.starterTemplate : undefined,
fromCopy: "copiedTemplate" in props ? props.copiedTemplate : undefined,
variables,
searchParams,
}),
validationSchema,
onSubmit,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export const DuplicateTemplateView: FC<CreateTemplatePageViewProps> = ({
version: firstVersionFromFile(
templateVersionQuery.data!.job.file_id,
formData.user_variable_values,
formData.provisioner_type,
),
template: newTemplate(formData),
});
Expand Down
1 change: 1 addition & 0 deletions site/src/pages/CreateTemplatePage/UploadTemplateView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export const UploadTemplateView: FC<CreateTemplatePageViewProps> = ({
version: firstVersionFromFile(
uploadedFile!.hash,
formData.user_variable_values,
formData.provisioner_type,
),
template: newTemplate(formData),
});
Expand Down
15 changes: 7 additions & 8 deletions site/src/pages/CreateTemplatePage/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type {
CreateTemplateVersionRequest,
Entitlements,
ProvisionerType,
TemplateExample,
Expand All @@ -7,10 +8,6 @@ import type {
import { calculateAutostopRequirementDaysValue } from "utils/schedule";
import type { CreateTemplateData } from "./CreateTemplateForm";

const provisioner: ProvisionerType =
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Playwright needs to use a different provisioner type!
typeof (window as any).playwright !== "undefined" ? "echo" : "terraform";

export const newTemplate = (formData: CreateTemplateData) => {
const { autostop_requirement_days_of_week, autostop_requirement_weeks } =
formData;
Expand Down Expand Up @@ -56,10 +53,11 @@ export const getFormPermissions = (entitlements: Entitlements) => {
export const firstVersionFromFile = (
fileId: string,
variables: VariableValue[] | undefined,
) => {
provisionerType: ProvisionerType,
): CreateTemplateVersionRequest => {
return {
storage_method: "file" as const,
provisioner: provisioner,
provisioner: provisionerType,
user_variable_values: variables,
file_id: fileId,
tags: {},
Expand All @@ -69,10 +67,11 @@ export const firstVersionFromFile = (
export const firstVersionFromExample = (
example: TemplateExample,
variables: VariableValue[] | undefined,
) => {
): CreateTemplateVersionRequest => {
return {
storage_method: "file" as const,
provisioner: provisioner,
// All starter templates are for the terraform provisioner type.
provisioner: "terraform",
user_variable_values: variables,
example_id: example.id,
tags: {},
Expand Down
Loading