Skip to content

Commit

Permalink
Integrate discovery with composer (#6042)
Browse files Browse the repository at this point in the history
* Added runtime command discovery

* Resolved comments

* Added error case to analyse codebase method

* Updated install command

* Reorganized tests and removed unwated promise.resolve stmt

* Added review changes on install command and node version string array

* Changes to node.ts to include additional condions on run script

* Added code comments to types

* Integrate discovery with composer

* Minor modification

* Minor changes

* Minor changes

* Resolved code commits

* Added undefied to return if no cmd

* Added undefied to return if no cmd

* Added frameworkhook interface

* code comments

* format error msg

* format error msg

* format error msg

* format error msg

* Code comments

* Fixed imports and compose command

* Fix bugs.

* Update base image for node runtime.

* bug fix

* bug fix

* Remove hooks

---------

Co-authored-by: Daniel Young Lee <danielylee@google.com>
  • Loading branch information
svnsairam and taeold authored Jun 28, 2023
1 parent c131e4f commit c0a59a5
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 74 deletions.
6 changes: 4 additions & 2 deletions src/commands/internaltesting-frameworks-compose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,20 @@ import { logger } from "../logger";
import { Mode, SUPPORTED_MODES } from "../frameworks/compose/driver";
import { compose } from "../frameworks/compose";
import { FirebaseError } from "../error";
import { LocalFileSystem } from "../frameworks/compose/discover/filesystem";
import { frameworkSpecs } from "../frameworks/compose/discover/frameworkSpec";

export const command = new Command("internaltesting:frameworks:compose")
.option("-m, --mode <mode>", "Composer mode (local or docker)", "local")
.description("compose framework in current directory")
.action((options: Options) => {
.action(async (options: Options) => {
const mode = options.mode as string;
if (!(SUPPORTED_MODES as unknown as string[]).includes(mode)) {
throw new FirebaseError(
`Unsupported mode ${mode}. Supported modes are [${SUPPORTED_MODES.join(", ")}]`
);
}
const bundle = compose(mode as Mode);
const bundle = await compose(mode as Mode, new LocalFileSystem("."), frameworkSpecs);
logger.info(JSON.stringify(bundle, null, 2));
return {};
});
2 changes: 1 addition & 1 deletion src/frameworks/compose/discover/filesystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { FileSystem } from "./types";
import { pathExists, readFile } from "fs-extra";
import * as path from "path";
import { FirebaseError } from "../../../error";
import { logger } from "../../../../src/logger";
import { logger } from "../../../logger";

/**
* Find files or read file contents present in the directory.
Expand Down
59 changes: 37 additions & 22 deletions src/frameworks/compose/discover/index.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,43 @@
import { AppSpec } from "../interfaces";
import { Runtime, FileSystem, FrameworkSpec, RuntimeSpec } from "./types";
import { NodejsRuntime } from "./runtime/node";
import { FirebaseError } from "../../../error";

const supportedRuntimes: Runtime[] = [new NodejsRuntime()];

/**
* Discover framework in the given project directory
* Discover the best matching runtime specs for the application.
*/
export function discover(): AppSpec {
return {
baseImage: "us-docker.pkg.dev/firestack-build/test/run:latest",
environmentVariables: {
NODE_ENV: "PRODUCTION",
},
installCommand: "npm install",
buildCommand: "npm run build",
startCommand: "npm run start",
export async function discover(
fs: FileSystem,
allFrameworkSpecs: FrameworkSpec[]
): Promise<RuntimeSpec> {
try {
let discoveredRuntime = undefined;
for (const runtime of supportedRuntimes) {
if (await runtime.match(fs)) {
if (!discoveredRuntime) {
discoveredRuntime = runtime;
} else {
throw new FirebaseError(
`Conflit occurred as multiple runtimes ${discoveredRuntime.getRuntimeName()}, ${runtime.getRuntimeName()} are discovered in the application.`
);
}
}
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
afterInstall: (b) => {
console.log("HOOK: AFTER INSTALL");
return { ...b, version: "v1alpha", notes: "afterInstall" };
},
if (!discoveredRuntime) {
throw new FirebaseError(
`Unable to determine the specific runtime for the application. The supported runtime options include ${supportedRuntimes
.map((x) => x.getRuntimeName())
.join(" , ")}.`
);
}
const runtimeSpec = await discoveredRuntime.analyseCodebase(fs, allFrameworkSpecs);

// eslint-disable-next-line @typescript-eslint/no-unused-vars
afterBuild(b) {
console.log("HOOK: AFTER BUILD");
return { ...b, version: "v1alpha", notes: "afterBuild" };
},
};
return runtimeSpec;
} catch (error: any) {
throw new FirebaseError(
`Failed to identify required specifications to execute the application: ${error}`
);
}
}
7 changes: 3 additions & 4 deletions src/frameworks/compose/discover/runtime/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { frameworkMatcher } from "../frameworkMatcher";
import { LifecycleCommands } from "../types";
import { Command } from "../types";
import { FirebaseError } from "../../../../error";
import { logger } from "../../../../../src/logger";
import { logger } from "../../../../logger";
import { conjoinOptions } from "../../../utils";

export interface PackageJSON {
Expand All @@ -23,7 +23,6 @@ const YARN_LOCK = "yarn.lock";

export class NodejsRuntime implements Runtime {
private readonly runtimeRequiredFiles: string[] = [PACKAGE_JSON];
private readonly contentCache: Record<string, boolean> = {};

// Checks if the codebase is using Node as runtime.
async match(fs: FileSystem): Promise<boolean | null> {
Expand All @@ -41,7 +40,7 @@ export class NodejsRuntime implements Runtime {
getNodeImage(engine: Record<string, string> | undefined): string {
// If no version is mentioned explicitly, assuming application is compatible with latest version.
if (!engine || !engine.node) {
return `node:${supportedNodeVersions[supportedNodeVersions.length - 1]}-slim`;
return "us-docker.pkg.dev/firestack-build/test/run";
}
const versionNumber = engine.node;

Expand All @@ -54,7 +53,7 @@ export class NodejsRuntime implements Runtime {
);
}

return `node:${versionNumber}-slim`;
return "us-docker.pkg.dev/firestack-build/test/run";
}

async getPackageManager(fs: FileSystem): Promise<PackageManager> {
Expand Down
17 changes: 17 additions & 0 deletions src/frameworks/compose/discover/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { AppBundle } from "../interfaces";

export interface FileSystem {
exists(file: string): Promise<boolean>;
read(file: string): Promise<string | null>;
Expand Down Expand Up @@ -77,4 +79,19 @@ export interface RuntimeSpec {
// The runtime has detected a command that should always be run irrespective of
// the framework (e.g. the "build" script always wins in Node)
detectedCommands?: LifecycleCommands;

environmentVariables?: Record<string, string>;

// Framework authors can execute framework-specific code using hooks at different stages of Frameworks API build process.
frameworkHooks?: FrameworkHooks;
}

export interface FrameworkHooks {
// Programmatic hook with access to filesystem and nodejs API to inspect the workspace.
// Primarily intended to gather hints relevant to the build.
afterInstall?: (b: AppBundle) => AppBundle;

// Programmatic hook with access to filesystem and nodejs API to inspect the build artifacts.
// Primarily intended to informs what assets should be deployed.
afterBuild?: (b: AppBundle) => AppBundle;
}
33 changes: 19 additions & 14 deletions src/frameworks/compose/driver/docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import * as fs from "node:fs";
import * as path from "node:path";
import * as spawn from "cross-spawn";

import { AppBundle, AppSpec, Driver, Hook } from "../interfaces";
import { AppBundle, Driver, Hook } from "../interfaces";
import { BUNDLE_PATH, genHookScript } from "./hooks";
import { RuntimeSpec } from "../discover/types";

const ADAPTER_SCRIPTS_PATH = "./.firebase/adapters" as const;

Expand Down Expand Up @@ -98,7 +99,7 @@ export class DockerfileBuilder {
export class DockerDriver implements Driver {
private dockerfileBuilder;

constructor(readonly spec: AppSpec) {
constructor(readonly spec: RuntimeSpec) {
this.dockerfileBuilder = new DockerfileBuilder();
this.dockerfileBuilder.from(spec.baseImage, "base").user("firebase");
}
Expand Down Expand Up @@ -148,21 +149,25 @@ export class DockerDriver implements Driver {
}

install(): void {
this.dockerfileBuilder
.fromLastStage(DOCKER_STAGE_INSTALL)
.workdir("/home/firebase/app")
.envs(this.spec.environmentVariables || {})
.copyForFirebase("package.json", ".")
.run(this.spec.installCommand);
this.buildStage(DOCKER_STAGE_INSTALL, ".");
if (this.spec.installCommand) {
this.dockerfileBuilder
.fromLastStage(DOCKER_STAGE_INSTALL)
.workdir("/home/firebase/app")
.envs(this.spec.environmentVariables || {})
.copyForFirebase("package.json", ".")
.run(this.spec.installCommand);
this.buildStage(DOCKER_STAGE_INSTALL, ".");
}
}

build(): void {
this.dockerfileBuilder
.fromLastStage(DOCKER_STAGE_BUILD)
.copyForFirebase(".", ".")
.run(this.spec.buildCommand);
this.buildStage(DOCKER_STAGE_BUILD, ".");
if (this.spec.detectedCommands?.build) {
this.dockerfileBuilder
.fromLastStage(DOCKER_STAGE_BUILD)
.copyForFirebase(".", ".")
.run(this.spec.detectedCommands.build.cmd);
this.buildStage(DOCKER_STAGE_BUILD, ".");
}
}

export(bundle: AppBundle): void {
Expand Down
5 changes: 3 additions & 2 deletions src/frameworks/compose/driver/index.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import { AppSpec, Driver } from "../interfaces";
import { Driver } from "../interfaces";
import { LocalDriver } from "./local";
import { DockerDriver } from "./docker";
import { RuntimeSpec } from "../discover/types";

export const SUPPORTED_MODES = ["local", "docker"] as const;
export type Mode = (typeof SUPPORTED_MODES)[number];

/**
* Returns the driver that provides the execution context for the composer.
*/
export function getDriver(mode: Mode, app: AppSpec): Driver {
export function getDriver(mode: Mode, app: RuntimeSpec): Driver {
if (mode === "local") {
return new LocalDriver(app);
} else if (mode === "docker") {
Expand Down
17 changes: 11 additions & 6 deletions src/frameworks/compose/driver/local.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import * as fs from "node:fs";
import * as spawn from "cross-spawn";

import { AppBundle, AppSpec, Hook, Driver } from "../interfaces";
import { AppBundle, Hook, Driver } from "../interfaces";
import { BUNDLE_PATH, genHookScript } from "./hooks";
import { RuntimeSpec } from "../discover/types";

export class LocalDriver implements Driver {
constructor(readonly spec: AppSpec) {}
constructor(readonly spec: RuntimeSpec) {}

private execCmd(cmd: string, args: string[]) {
const ret = spawn.sync(cmd, args, {
Expand All @@ -18,13 +19,17 @@ export class LocalDriver implements Driver {
}

install(): void {
const [cmd, ...args] = this.spec.installCommand.split(" ");
this.execCmd(cmd, args);
if (this.spec.installCommand) {
const [cmd, ...args] = this.spec.installCommand.split(" ");
this.execCmd(cmd, args);
}
}

build(): void {
const [cmd, ...args] = this.spec.buildCommand.split(" ");
this.execCmd(cmd, args);
if (this.spec.detectedCommands?.build) {
const [cmd, ...args] = this.spec.detectedCommands.build.cmd.split(" ");
this.execCmd(cmd, args);
}
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
Expand Down
21 changes: 13 additions & 8 deletions src/frameworks/compose/index.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,36 @@
import { AppBundle } from "./interfaces";
import { getDriver, Mode } from "./driver";
import { discover } from "./discover";
import { FrameworkSpec, FileSystem } from "./discover/types";

/**
* Run composer in the specified execution context.
*/
export function compose(mode: Mode): AppBundle {
export async function compose(
mode: Mode,
fs: FileSystem,
allFrameworkSpecs: FrameworkSpec[]
): Promise<AppBundle> {
let bundle: AppBundle = { version: "v1alpha" };
const spec = discover();
const spec = await discover(fs, allFrameworkSpecs);
const driver = getDriver(mode, spec);

if (spec.startCommand) {
if (spec.detectedCommands?.run) {
bundle.server = {
start: {
cmd: spec.startCommand.split(" "),
cmd: spec.detectedCommands.run.cmd.split(" "),
},
};
}

driver.install();
if (spec.afterInstall) {
bundle = driver.execHook(bundle, spec.afterInstall);
if (spec.frameworkHooks?.afterInstall) {
bundle = driver.execHook(bundle, spec.frameworkHooks.afterInstall);
}

driver.build();
if (spec.afterBuild) {
bundle = driver.execHook(bundle, spec.afterBuild);
if (spec.frameworkHooks?.afterBuild) {
bundle = driver.execHook(bundle, spec.frameworkHooks?.afterBuild);
}

if (bundle.server) {
Expand Down
16 changes: 3 additions & 13 deletions src/frameworks/compose/interfaces.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { RuntimeSpec } from "./discover/types";

export interface AppBundle {
version: "v1alpha";
server?: ServerConfig;
Expand All @@ -22,22 +24,10 @@ interface StartConfig {
runtime?: "nodejs18" | string;
}

export interface AppSpec {
baseImage: string;
packageManagerInstallCommand?: string;
environmentVariables?: Record<string, string>;
installCommand: string;
buildCommand: string;
startCommand: string;

afterInstall?: (b: AppBundle) => AppBundle;
afterBuild?: (b: AppBundle) => AppBundle;
}

export type Hook = (b: AppBundle) => AppBundle;

export class Driver {
constructor(readonly spec: AppSpec) {}
constructor(readonly spec: RuntimeSpec) {}

install(): void {
throw new Error("install() not implemented");
Expand Down
4 changes: 2 additions & 2 deletions src/test/frameworks/compose/discover/runtime/node.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe("NodejsRuntime", () => {
node: "18",
};
const actualImage = nodeJSRuntime.getNodeImage(version);
const expectedImage = "node:18-slim";
const expectedImage = "us-docker.pkg.dev/firestack-build/test/run";

expect(actualImage).to.deep.equal(expectedImage);
});
Expand Down Expand Up @@ -189,7 +189,7 @@ describe("NodejsRuntime", () => {
const actual = await nodeJSRuntime.analyseCodebase(fileSystem, allFrameworks);
const expected = {
id: "nodejs",
baseImage: "node:18-slim",
baseImage: "us-docker.pkg.dev/firestack-build/test/run",
packageManagerInstallCommand: undefined,
installCommand: "npm install",
detectedCommands: {
Expand Down

0 comments on commit c0a59a5

Please sign in to comment.