Skip to content

fix: locate Terraform entrypoint file #16753

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.

Sign up for GitHub

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 2 commits into from
Feb 28, 2025
Merged

fix: locate Terraform entrypoint file #16753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bcdad65
fix: locate Terraform entrypoint file
mtojek Feb 28, 2025
86761b0
sorting
mtojek Feb 28, 2025
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
129 changes: 128 additions & 1 deletion site/src/pages/TemplateVersionEditorPage/TemplateVersionEditorPage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@ import {
waitForLoaderToBeRemoved,
} from "testHelpers/renderHelpers";
import { server } from "testHelpers/server";
import type { FileTree } from "utils/filetree";
import type { MonacoEditorProps } from "./MonacoEditor";
import { Language } from "./PublishTemplateVersionDialog";
import TemplateVersionEditorPage from "./TemplateVersionEditorPage";
import TemplateVersionEditorPage, {
findEntrypointFile,
} from "./TemplateVersionEditorPage";

const { API } = apiModule;

Expand Down Expand Up @@ -409,3 +412,127 @@ function renderEditorPage(queryClient: QueryClient) {
</AppProviders>,
);
}

describe("Find entrypoint", () => {
it("empty tree", () => {
const ft: FileTree = {};
const mainFile = findEntrypointFile(ft);
expect(mainFile).toBeUndefined();
});
it("flat structure, main.tf in root", () => {
const ft: FileTree = {
"aaa.tf": "hello",
"bbb.tf": "world",
"main.tf": "foobar",
"nnn.tf": "foobaz",
};

const mainFile = findEntrypointFile(ft);
expect(mainFile).toBe("main.tf");
});
it("flat structure, no main.tf", () => {
const ft: FileTree = {
"aaa.tf": "hello",
"bbb.tf": "world",
"ccc.tf": "foobaz",
"nnn.tf": "foobaz",
};

const mainFile = findEntrypointFile(ft);
expect(mainFile).toBe("nnn.tf");
});
it("with dirs, single main.tf", () => {
const ft: FileTree = {
"aaa-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
},
"bbb-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
},
"main.tf": "foobar",
"nnn.tf": "foobaz",
};

const mainFile = findEntrypointFile(ft);
expect(mainFile).toBe("main.tf");
});
it("with dirs, multiple main.tf's", () => {
const ft: FileTree = {
"aaa-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
"main.tf": "foobar",
},
"bbb-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
"main.tf": "foobar",
},
"ccc-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
},
"main.tf": "foobar",
"nnn.tf": "foobaz",
"zzz-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
"main.tf": "foobar",
},
};

const mainFile = findEntrypointFile(ft);
expect(mainFile).toBe("main.tf");
});
it("with dirs, multiple main.tf, no main.tf in root", () => {
const ft: FileTree = {
"aaa-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
"main.tf": "foobar",
},
"bbb-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
"main.tf": "foobar",
},
"ccc-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
},
"nnn.tf": "foobaz",
"zzz-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
"main.tf": "foobar",
},
};

const mainFile = findEntrypointFile(ft);
expect(mainFile).toBe("aaa-dir/main.tf");
});
it("with dirs, multiple main.tf, unordered file tree", () => {
const ft: FileTree = {
"ccc-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
"main.tf": "foobar",
},
"aaa-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
"main.tf": "foobar",
},
"zzz-dir": {
"aaa.tf": "hello",
"bbb.tf": "world",
"main.tf": "foobar",
},
};

const mainFile = findEntrypointFile(ft);
expect(mainFile).toBe("aaa-dir/main.tf");
});
});
29 changes: 26 additions & 3 deletions site/src/pages/TemplateVersionEditorPage/TemplateVersionEditorPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export const TemplateVersionEditorPage: FC = () => {
// File navigation
// It can be undefined when a selected file is deleted
const activePath: string | undefined =
searchParams.get("path") ?? findInitialFile(fileTree ?? {});
searchParams.get("path") ?? findEntrypointFile(fileTree ?? {});
Copy link
Member

@Parkreiner Parkreiner Feb 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Do you know what happens if searchParams.get returns an empty string or a string that isn't a valid filename?

That would short-circuit the nullish expression and prevent findEntrypointFile from ever being called, and as far as I can tell, activePath gets fed directly into the editor without any additional data massaging

Copy link
Member Author

Choose a reason for hiding this comment

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

nice catch! let me look into this 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

#16757

const onActivePathChange = (path: string | undefined) => {
if (path) {
searchParams.set("path", path);
Expand Down Expand Up @@ -357,10 +357,33 @@ const publishVersion = async (options: {
return Promise.all(publishActions);
};

const findInitialFile = (fileTree: FileTree): string | undefined => {
const defaultMainTerraformFile = "main.tf";

// findEntrypointFile function locates the entrypoint file to open in the Editor.
// It browses the filetree following these steps:
// 1. If "main.tf" exists in root, return it.
// 2. Traverse through sub-directories.
// 3. If "main.tf" exists in a sub-directory, skip further browsing, and return the path.
// 4. If "main.tf" was not found, return the last reviewed "".tf" file.
Copy link
Member

Choose a reason for hiding this comment

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

With the sorting in place now, I feel like it would've helped to clarify what "last reviewed" means, especially to help make it clear that the tests are actually deterministic

export const findEntrypointFile = (fileTree: FileTree): string | undefined => {
let initialFile: string | undefined;

traverse(fileTree, (content, filename, path) => {
if (Object.keys(fileTree).find((key) => key === defaultMainTerraformFile)) {
return defaultMainTerraformFile;
}

let skip = false;
traverse(fileTree, (_, filename, path) => {
if (skip) {
return;
}

if (filename === defaultMainTerraformFile) {
initialFile = path;
skip = true;
return;
}
Comment on lines +381 to +385
Copy link
Member

Choose a reason for hiding this comment

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

What is the order in which traverse() will traverse the nodes of fileTree? Are child paths sorted prior to being added to the frontier? In other words, if you have both a/main.tf and z/main.tf, which will get chosen? I know you have a test above, but is the output dependant on the ordering of the nodes? Is it just "first found wins"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me expand traverse() with sorting capabilities.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

nice 👍


if (filename.endsWith(".tf")) {
initialFile = path;
}
Expand Down
2 changes: 1 addition & 1 deletion site/src/utils/filetree.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,6 @@ test("traverse() go trough all the file tree files", () => {
traverse(fileTree, (_content, _filename, fullPath) => {
filePaths.push(fullPath);
});
const expectedFilePaths = ["main.tf", "images", "images/java.Dockerfile"];
const expectedFilePaths = ["images", "images/java.Dockerfile", "main.tf"];
expect(filePaths).toEqual(expectedFilePaths);
});
4 changes: 3 additions & 1 deletion site/src/utils/filetree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ export const traverse = (
) => void,
parent?: string,
) => {
for (const [filename, content] of Object.entries(fileTree)) {
for (const [filename, content] of Object.entries(fileTree).sort(([a], [b]) =>
a.localeCompare(b),
)) {
const fullPath = parent ? `${parent}/${filename}` : filename;
callback(content, filename, fullPath);
if (typeof content === "object") {
Expand Down
Loading