-
Notifications
You must be signed in to change notification settings - Fork 61
feat(config): implement global config to wire up config command #1789
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
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| import { | ||
| type DeepPartial, | ||
| type GlobalConfig, | ||
| type GlobalConfigAccessor, | ||
| type GlobalConfigFileData, | ||
| type ReadWriteJson, | ||
| } from "./types"; | ||
| import type { Logger } from "../logging"; | ||
| import z from "zod"; | ||
| import { globalConfigFileSchema } from "./types"; | ||
| import { DEFAULT_GLOBAL_CONFIG, applyOverrides } from "./config"; | ||
|
|
||
| type DefaultGlobalConfigAccessorConfig = { | ||
| logger: Logger; | ||
| json: ReadWriteJson; | ||
| filePath: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Implements {@link GlobalConfigAccessor} accepting overrides from the given file. | ||
| * | ||
| * @param config - The logger and json datasource to be used by the config. . | ||
| * @returns A {@link GlobalConfigAccessor} instance. | ||
| */ | ||
| export class DefaultGlobalConfigAccessor implements GlobalConfigAccessor { | ||
| private cachedConfig: GlobalConfig | undefined; | ||
| private readonly json: ReadWriteJson; | ||
| private readonly filePath: string; | ||
| private readonly logger: Logger; | ||
|
|
||
| constructor(config: DefaultGlobalConfigAccessorConfig) { | ||
| this.json = config.json; | ||
| this.filePath = config.filePath; | ||
| this.logger = config.logger.child({ configFilePath: this.filePath }); | ||
| this.logger.info(`creating global config accessor`); | ||
| } | ||
|
|
||
| public async get(): Promise<GlobalConfig> { | ||
| const logger = this.logger.child({ method: "get" }); | ||
| logger.debug("reading global config"); | ||
|
|
||
| if (this.cachedConfig) return this.cachedConfig; | ||
| logger.debug("no config cached, reading from source"); | ||
|
|
||
| const configFileData = await this.readConfigFile(); | ||
|
|
||
| // if no installationId is present, generate one and merge it into the file data | ||
| if (!configFileData.installationId) { | ||
| configFileData.installationId = DEFAULT_GLOBAL_CONFIG.installationId; | ||
| this.logger.info(`no installationId found, persisting one`); | ||
|
|
||
| try { | ||
| await this.writeToConfigFile(configFileData); | ||
| } catch (e) { | ||
| const error = e instanceof Error ? e : new Error(String(e)); | ||
| this.logger | ||
| .child({ errorName: error.name, errorMessage: error.message }) | ||
| .warn(`failed to write initial config file data`); | ||
| // best effort | ||
| } | ||
| } | ||
|
|
||
| this.cachedConfig = applyOverrides(DEFAULT_GLOBAL_CONFIG, configFileData); | ||
| return this.cachedConfig; | ||
| } | ||
|
|
||
| public async set(newConfig: GlobalConfig): Promise<GlobalConfig> { | ||
| this.logger.child({ newConfig, method: "set" }).debug("writing global config"); | ||
|
|
||
| const configDiff = diff(newConfig, DEFAULT_GLOBAL_CONFIG); | ||
| await this.writeToConfigFile(configDiff); | ||
| this.cachedConfig = newConfig; | ||
| return this.cachedConfig; | ||
| } | ||
|
|
||
| private async writeToConfigFile(data: GlobalConfigFileData): Promise<GlobalConfigFileData> { | ||
| const dataParseResult = globalConfigFileSchema.safeParse(data); | ||
| if (!dataParseResult.success) { | ||
| // TODO: mark this as a client-source error. | ||
| throw new TypeError(z.prettifyError(dataParseResult.error)); | ||
| } | ||
|
|
||
| await this.json.write(this.filePath, dataParseResult.data); | ||
| return data; | ||
| } | ||
|
|
||
| private async readConfigFile(): Promise<GlobalConfigFileData> { | ||
| try { | ||
| return await this.json.read(this.filePath, globalConfigFileSchema); | ||
| } catch (e) { | ||
| if (isFileNotFoundError(e)) return {}; | ||
|
|
||
| const error = e instanceof Error ? e : new Error(String(e)); | ||
| this.logger | ||
| .child({ errorName: error.name, errorMessage: error.message }) | ||
| .warn(`failed to read global config file`); | ||
| throw e; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function isFileNotFoundError(e: unknown): boolean { | ||
| return e instanceof Error && "code" in e && e.code === "ENOENT"; | ||
| } | ||
|
|
||
| /** Recursively diffs two objects, comparing leaf values by reference equality. Returns only the fields in a that differ from b */ | ||
| function diff<T extends Record<string, unknown>>(a: T, b: T): DeepPartial<T> { | ||
| const result: Record<string, unknown> = {}; | ||
|
|
||
| for (const key of Object.keys(a)) { | ||
| const aVal = a[key]; | ||
| const bVal = b[key]; | ||
|
|
||
| if (isRecord(aVal) && isRecord(bVal)) { | ||
| const nested = diff(aVal, bVal); | ||
| if (Object.keys(nested).length > 0) { | ||
| result[key] = nested; | ||
| } | ||
| } else if (aVal !== bVal) { | ||
| result[key] = aVal; | ||
| } | ||
| } | ||
|
|
||
| return result as DeepPartial<T>; | ||
| } | ||
|
|
||
| function isRecord(value: unknown): value is Record<string, unknown> { | ||
| return typeof value === "object" && value !== null && !Array.isArray(value); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import type { DeepPartial, GlobalConfig } from "./types"; | ||
|
|
||
| /** | ||
| * Default values for the global config. Includes a unique installationId for each process. | ||
| */ | ||
| export const DEFAULT_GLOBAL_CONFIG: GlobalConfig = { | ||
| telemetry: { | ||
| enabled: true, | ||
| audit: false, | ||
| endpoint: "https://telemetry.agentcore.aws.dev", | ||
| }, | ||
| installationId: crypto.randomUUID(), | ||
| }; | ||
|
|
||
| /** | ||
| * Applies the given overrides from a partial config on top of the provided defaults and returns the merged result. | ||
| */ | ||
| export function applyOverrides( | ||
| defaults: GlobalConfig, | ||
| overrides: DeepPartial<GlobalConfig>, | ||
| ): GlobalConfig { | ||
| return { | ||
| telemetry: { | ||
| enabled: overrides.telemetry?.enabled ?? defaults.telemetry.enabled, | ||
| audit: overrides.telemetry?.audit ?? defaults.telemetry.audit, | ||
| endpoint: overrides.telemetry?.endpoint ?? defaults.telemetry.endpoint, | ||
| }, | ||
| installationId: overrides.installationId ?? defaults.installationId, | ||
| }; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| export { type GlobalConfigAccessor, type GlobalConfig, type ReadWriteJson } from "./types"; | ||
| export { DefaultGlobalConfigAccessor } from "./accessor"; | ||
| export { FsReadWriteJson } from "./json"; | ||
| export { DEFAULT_GLOBAL_CONFIG } from "./config"; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import z from "zod"; | ||
| import { mkdir, readFile, writeFile } from "fs/promises"; | ||
| import type { Logger } from "../logging"; | ||
| import type { ReadWriteJson } from "./types"; | ||
| import { dirname } from "path"; | ||
|
|
||
| type ReadWriteJsonConfig = { | ||
| logger: Logger; | ||
| }; | ||
|
|
||
| // TODO: attach telemetry metadata to this error class. | ||
| class DeserializationError extends Error { | ||
| constructor(path: string, options?: { cause?: unknown }) { | ||
| super(`Failed to deserialize JSON at "${path}"`, options); | ||
| this.name = "DeserializationError"; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Implements {@link ReadWriteJson} through node fs. | ||
| * | ||
| * @param config - logger | ||
| */ | ||
| export class FsReadWriteJson implements ReadWriteJson { | ||
| private readonly logger: Logger; | ||
|
|
||
| constructor(config: ReadWriteJsonConfig) { | ||
| this.logger = config.logger; | ||
| } | ||
|
|
||
| private async readJsonFile(filePath: string): Promise<unknown> { | ||
| const raw = await readFile(filePath, "utf8"); | ||
|
|
||
| try { | ||
| return JSON.parse(raw); | ||
| } catch (e) { | ||
| const error = e instanceof Error ? e : new Error(String(e)); | ||
| this.logger | ||
| .child({ filePath, errorName: error.name, errorMessage: error.message }) | ||
| .error(`failed to parse json file`); | ||
| throw new DeserializationError(filePath, { cause: e }); | ||
| } | ||
| } | ||
|
|
||
| public async read<TSchema extends z.ZodType>( | ||
| filePath: string, | ||
| schema: TSchema, | ||
| ): Promise<z.infer<TSchema>> { | ||
| const data = await this.readJsonFile(filePath); | ||
| const parseResult = schema.safeParse(data); | ||
|
|
||
| if (!parseResult.success) { | ||
| this.logger | ||
| .child({ | ||
| filePath, | ||
| errorName: parseResult.error.name, | ||
| errorMessage: parseResult.error.message, | ||
| }) | ||
| .error(`failed to validate parsed json file`); | ||
| throw new DeserializationError(filePath, { cause: parseResult.error }); | ||
| } | ||
|
|
||
| return parseResult.data; | ||
| } | ||
|
|
||
| public async write<TData extends object>(filePath: string, data: TData): Promise<TData> { | ||
| const contents = JSON.stringify(data, undefined, 2); | ||
| await mkdir(dirname(filePath), { recursive: true }); | ||
| await writeFile(filePath, contents); | ||
| return data; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import z from "zod"; | ||
|
|
||
| /** Recursively marks all fields of a given shape as required */ | ||
| export type DeepRequired<T> = { | ||
| [P in keyof T]-?: DeepRequired<T[P]>; | ||
| }; | ||
|
|
||
| /** Recursively marks all fields of a given shape as optional */ | ||
| export type DeepPartial<T> = { [P in keyof T]?: DeepPartial<T[P]> }; | ||
|
|
||
| /** | ||
| * Schema for the global config file. All fields should be optional with defaults defined. | ||
| */ | ||
| export const globalConfigFileSchema = z.object({ | ||
| telemetry: z | ||
| .object({ | ||
| enabled: z.boolean().optional(), | ||
| endpoint: z.string().optional(), | ||
| audit: z.boolean().optional(), | ||
| }) | ||
| .optional(), | ||
| installationId: z.uuid().optional(), | ||
| }); | ||
|
|
||
| /** The raw shape stored on disk for overriding defaults. */ | ||
| export type GlobalConfigFileData = z.infer<typeof globalConfigFileSchema>; | ||
|
|
||
| /** The fully resolved config after applying defaults — all fields required. */ | ||
| export type GlobalConfig = DeepRequired<GlobalConfigFileData>; | ||
|
|
||
| /** Manages access to a set of configuration values for the CLI */ | ||
| export interface GlobalConfigAccessor { | ||
| /** Returns the current global config, with defaults applied. */ | ||
| get(): Promise<GlobalConfig>; | ||
| /** Validates and persists a new config. Throws on invalid shape. */ | ||
| set(newConfig: GlobalConfig): Promise<GlobalConfig>; | ||
| } | ||
|
|
||
| export interface ReadWriteJson { | ||
| /** Reads data from the given file */ | ||
| read<TSchema extends z.ZodType>(filePath: string, schema: TSchema): Promise<z.infer<TSchema>>; | ||
| /** Writes data to the given file */ | ||
| write<TData extends object>(filePath: string, data: TData): Promise<TData>; | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be in its own
jsonpackage. We can handle that in a follow up.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, I figure we pull it out once we have another consumer.