Skip to content

Commit

Permalink
chore: restore early exit on build no-embed (#1731)
Browse files Browse the repository at this point in the history
## Description

When running a build using `--no-embed` the build should _not_ generate
the Pepr module as a secret as used during typical builds. As we were
working towards reducing statements in the build file a function was
created to handle the embed option and stop execution. This PR updates
the function to stop execution which was causing the command to write to
`stdErr`

While implementing this fix I encountered errors and needed to satisfy
eslint configs
```bash
src/cli/build.ts
  131:13  warning  Async arrow function has too many statements (21). Maximum allowed is 20  max-statements
  131:13  warning  Async arrow function has a complexity of 11. Maximum allowed is 10        complexity
```


## Related Issue

Fixes #1659 
<!-- or -->
Relates to #

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Other (security config, docs update, etc)

## Checklist before merging
- [x] Unit,
[Journey](https://github.com/defenseunicorns/pepr/tree/main/journey),
[E2E Tests](https://github.com/defenseunicorns/pepr-excellent-examples),
[docs](https://github.com/defenseunicorns/pepr/tree/main/docs),
[adr](https://github.com/defenseunicorns/pepr/tree/main/adr) added or
updated as needed
- [x] [Contributor Guide
Steps](https://docs.pepr.dev/main/contribute/#submitting-a-pull-request)
followed

---------

Signed-off-by: Case Wylie <[email protected]>
Co-authored-by: Barrett <[email protected]>
Co-authored-by: Sam Mayer <[email protected]>
  • Loading branch information
3 people authored Jan 28, 2025
1 parent 677fa26 commit ec32a8e
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 105 deletions.
4 changes: 2 additions & 2 deletions integration/cli/build.noembed.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ describe("build", () => {
const argz = [`--no-embed`].join(" ");
const build = await pepr.cli(testModule, { cmd: `pepr build ${argz}` });
expect(build.exitcode).toBe(0);
expect(build.stderr.join("").trim()).toContain("Error: Cannot find module");
expect(build.stdout.join("").trim()).toContain("");
expect(build.stderr.join("").trim()).toContain("");
expect(build.stdout.join("").trim()).toContain("Module built successfully at");

packageJson = await resource.fromFile(`${testModule}/package.json`);
uuid = packageJson.pepr.uuid;
Expand Down
41 changes: 28 additions & 13 deletions src/cli/build.helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,34 @@ import { generateAllYaml } from "../lib/assets/yaml/generateAllYaml";
import { webhookConfigGenerator } from "../lib/assets/webhooks";
import { generateZarfYamlGeneric } from "../lib/assets/yaml/generateZarfYaml";

interface ImageOptions {
customImage?: string;
registryInfo?: string;
peprVersion?: string;
registry?: string;
}
/**
* Assign image string
* @param imageOptions CLI options for image
* @returns image string
*/
export function assignImage(imageOptions: ImageOptions): string {
const { customImage, registryInfo, peprVersion, registry } = imageOptions;
if (customImage) {
return customImage;
}

if (registryInfo) {
return `${registryInfo}/custom-pepr-controller:${peprVersion}`;
}

if (registry) {
return checkIronBankImage(registry, "", peprVersion!);
}

return "";
}

export type Reloader = (opts: BuildResult<BuildOptions>) => void | Promise<void>;
/**
* Determine the RBAC mode based on the CLI options and the module's config
Expand Down Expand Up @@ -114,19 +142,6 @@ export async function handleCustomImageBuild(
}
}

/**
* Disables embedding of deployment files into output module
* @param embed
* @param path
* @returns
*/
export function handleEmbedding(embed: boolean, path: string): void {
if (!embed) {
console.info(`✅ Module built successfully at ${path}`);
return;
}
}

/**
* Check if the capability names are valid
* @param capabilities The capabilities to check
Expand Down
75 changes: 48 additions & 27 deletions src/cli/build.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
import {
determineRbacMode,
handleCustomOutputDir,
handleEmbedding,
handleValidCapabilityNames,
handleCustomImageBuild,
checkIronBankImage,
validImagePullSecret,
assignImage,
} from "./build.helpers";

import { createDirectoryIfNotExists } from "../lib/filesystemService";
import { expect, describe, it, jest, beforeEach } from "@jest/globals";
import { createDockerfile } from "../lib/included-files";
Expand All @@ -29,6 +30,52 @@ jest.mock("../lib/filesystemService", () => ({
createDirectoryIfNotExists: jest.fn(),
}));

describe("assignImage", () => {
const mockPeprVersion = "1.0.0";

it("should return the customImage if provided", () => {
const result = assignImage({
customImage: "pepr:dev",
registryInfo: "docker.io/defenseunicorns",
peprVersion: mockPeprVersion,
registry: "my-registry",
});
expect(result).toBe("pepr:dev");
});

it("should return registryInfo with custom-pepr-controller and peprVersion if customImage is not provided", () => {
const result = assignImage({
customImage: "",
registryInfo: "docker.io/defenseunicorns",
peprVersion: mockPeprVersion,
registry: "my-registry",
});
expect(result).toBe(`docker.io/defenseunicorns/custom-pepr-controller:1.0.0`);
});

it("should return IronBank image if registry is provided and others are not", () => {
const result = assignImage({
customImage: "",
registryInfo: "",
peprVersion: mockPeprVersion,
registry: "Iron Bank",
});
expect(result).toBe(
`registry1.dso.mil/ironbank/opensource/defenseunicorns/pepr/controller:v${mockPeprVersion}`,
);
});

it("should return an empty string if none of the conditions are met", () => {
const result = assignImage({
customImage: "",
registryInfo: "",
peprVersion: "",
registry: "",
});
expect(result).toBe("");
});
});

describe("determineRbacMode", () => {
it("should allow CLI options to overwrite module config", () => {
const opts = { rbacMode: "admin" };
Expand Down Expand Up @@ -171,32 +218,6 @@ describe("handleCustomImageBuild", () => {
expect(mockedExecSync).not.toHaveBeenCalled();
});
});
describe("handleEmbedding", () => {
const consoleInfoSpy = jest.spyOn(console, "info").mockImplementation(() => {});

beforeEach(() => {
jest.clearAllMocks();
});

it("should log success message if embed is false", () => {
const embed = false;
const path = "test/path";

handleEmbedding(embed, path);

expect(consoleInfoSpy).toHaveBeenCalledWith(`✅ Module built successfully at ${path}`);
});

it("should not log success message if embed is true", () => {
const embed = true;
const path = "test/path";

handleEmbedding(embed, path);

expect(consoleInfoSpy).not.toHaveBeenCalled();
});
});

describe("handleValidCapabilityNames", () => {
const mockExit = jest.spyOn(process, "exit").mockImplementation(() => {
return undefined as never;
Expand Down
126 changes: 63 additions & 63 deletions src/cli/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,21 @@ import { RootCmd } from "./root";
import { Option } from "commander";
import { parseTimeout } from "../lib/helpers";
import { peprFormat } from "./format";
import { ModuleConfig } from "../lib/core/module";
import {
watchForChanges,
determineRbacMode,
handleEmbedding,
assignImage,
handleCustomOutputDir,
handleValidCapabilityNames,
handleCustomImageBuild,
checkIronBankImage,
validImagePullSecret,
generateYamlAndWriteToDisk,
} from "./build.helpers";
import { ModuleConfig } from "../lib/core/module";

const peprTS = "pepr.ts";
let outputDir: string = "dist";
export type Reloader = (opts: BuildResult<BuildOptions>) => void | Promise<void>;

export type PeprNestedFields = Pick<
ModuleConfig,
| "uuid"
Expand Down Expand Up @@ -64,7 +62,7 @@ type BuildModuleReturn = {
path: string;
cfg: PeprConfig;
uuid: string;
} | void;
};

export default function (program: RootCmd): void {
program
Expand Down Expand Up @@ -134,68 +132,70 @@ export default function (program: RootCmd): void {

// Build the module
const buildModuleResult = await buildModule(undefined, opts.entryPoint, opts.embed);
if (buildModuleResult?.cfg && buildModuleResult.path && buildModuleResult.uuid) {
const { cfg, path, uuid } = buildModuleResult;
// Files to include in controller image for WASM support
const { includedFiles } = cfg.pepr;

let image = opts.customImage || "";

// Check if there is a custom timeout defined
if (opts.timeout !== undefined) {
cfg.pepr.webhookTimeout = opts.timeout;
}

if (opts.registryInfo !== undefined) {
console.info(`Including ${includedFiles.length} files in controller image.`);

// for journey test to make sure the image is built
image = `${opts.registryInfo}/custom-pepr-controller:${cfg.pepr.peprVersion}`;

// only actually build/push if there are files to include
await handleCustomImageBuild(includedFiles, cfg.pepr.peprVersion, cfg.description, image);
}

// If building without embedding, exit after building
handleEmbedding(opts.embed, path);

// set the image version if provided
opts.version ? (cfg.pepr.peprVersion = opts.version) : null;

// Generate a secret for the module
const assets = new Assets(
{
...cfg.pepr,
appVersion: cfg.version,
description: cfg.description,
alwaysIgnore: {
namespaces: cfg.pepr.alwaysIgnore?.namespaces,
},
// Can override the rbacMode with the CLI option
rbacMode: determineRbacMode(opts, cfg),
},
path,
opts.withPullSecret === "" ? [] : [opts.withPullSecret],
);

// If registry is set to Iron Bank, use Iron Bank image
image = checkIronBankImage(opts.registry, image, cfg.pepr.peprVersion);
const { cfg, path, uuid } = buildModuleResult!;
const image = assignImage({
customImage: opts.customImage,
registryInfo: opts.registryInfo,
peprVersion: cfg.pepr.peprVersion,
registry: opts.registry,
});

// Check if there is a custom timeout defined
if (opts.timeout !== undefined) {
cfg.pepr.webhookTimeout = opts.timeout;
}

// if image is a custom image, use that instead of the default
image !== "" ? (assets.image = image) : null;
if (opts.registryInfo !== undefined) {
console.info(`Including ${cfg.pepr.includedFiles.length} files in controller image.`);
// for journey test to make sure the image is built

// Ensure imagePullSecret is valid
validImagePullSecret(opts.withPullSecret);
// only actually build/push if there are files to include
await handleCustomImageBuild(
cfg.pepr.includedFiles,
cfg.pepr.peprVersion,
cfg.description,
image,
);
}

handleValidCapabilityNames(assets.capabilities);
await generateYamlAndWriteToDisk({
uuid,
outputDir,
imagePullSecret: opts.withPullSecret,
zarf: opts.zarf,
assets,
});
// If building without embedding, exit after building
if (!opts.embed) {
console.info(`✅ Module built successfully at ${path}`);
return;
}
// set the image version if provided
opts.version ? (cfg.pepr.peprVersion = opts.version) : null;

// Generate a secret for the module
const assets = new Assets(
{
...cfg.pepr,
appVersion: cfg.version,
description: cfg.description,
alwaysIgnore: {
namespaces: cfg.pepr.alwaysIgnore?.namespaces,
},
// Can override the rbacMode with the CLI option
rbacMode: determineRbacMode(opts, cfg),
},
path,
opts.withPullSecret === "" ? [] : [opts.withPullSecret],
);

image !== "" ? (assets.image = image) : null;

// Ensure imagePullSecret is valid
validImagePullSecret(opts.withPullSecret);

handleValidCapabilityNames(assets.capabilities);
await generateYamlAndWriteToDisk({
uuid,
outputDir,
imagePullSecret: opts.withPullSecret,
zarf: opts.zarf,
assets,
});
});
}

Expand Down Expand Up @@ -253,7 +253,7 @@ export async function buildModule(
reloader?: Reloader,
entryPoint = peprTS,
embed = true,
): Promise<BuildModuleReturn> {
): Promise<BuildModuleReturn | void> {
try {
const { cfg, modulePath, path, uuid } = await loadModule(entryPoint);

Expand Down

0 comments on commit ec32a8e

Please sign in to comment.