Skip to content
Open
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
3 changes: 2 additions & 1 deletion packages/shared/src/cli/commands/doctor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ wiring):
`databricks.yml` binding (the env var would never be injected).
- `BUNDLE_REF_MISSING` — a `${resources.<type>.<key>.*}` binding references a
resource not declared in the bundle (deploy would fail).
- `ENV_UNWIRED` — a plugin needs an env var no `app.yaml` entry provides, so it
- `ENV_UNWIRED` — a plugin needs an env var no `app.yaml` entry provides
(neither `valueFrom` nor a literal `value`), so it
won't be set in the environment of the deployed app. Error for a required
resource (a guaranteed break), warning for an optional one.

Expand Down
23 changes: 23 additions & 0 deletions packages/shared/src/cli/commands/doctor/bundle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,29 @@ describe("readBundleInfo", () => {
"sql-warehouse",
);
});

it("counts valueFrom and literal value entries as wired; a null value isn't", () => {
const dir = tmp({
"databricks.yml": BUNDLE,
"app.yaml": `${APP_YAML} - name: DATABRICKS_GENIE_SPACE_ID
value: 01f0abc
- name: EMPTY_LITERAL
value: ""
- name: NULL_VALUE
value:
`,
});
dirs.push(dir);
const info = readBundleInfo(dir);
expect([...info.wiredEnvVars].sort()).toEqual([
"DATABRICKS_GENIE_SPACE_ID",
"DATABRICKS_JOB_REPORT",
"DATABRICKS_WAREHOUSE_ID",
"EMPTY_LITERAL",
]);
// A literal has no binding to join, so it stays out of envToBinding.
expect(info.envToBinding.has("DATABRICKS_GENIE_SPACE_ID")).toBe(false);
});
});

describe("originForEnvVars", () => {
Expand Down
29 changes: 25 additions & 4 deletions packages/shared/src/cli/commands/doctor/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ export interface BundleInfo {
bindings: Map<string, BundleBinding>;
/** `app.yaml` env var name → binding name (from `valueFrom`). */
envToBinding: Map<string, string>;
/** `app.yaml` env var names the platform sets on deploy: via `valueFrom` or a
* literal `value`. */
wiredEnvVars: Set<string>;
/** Declared bundle resource keys, as `"<type>.<key>"`, for ref-integrity. */
declaredResources: Set<string>;
/** True when a `databricks.yml` was found and parsed. */
Expand All @@ -63,7 +66,7 @@ interface BundleDoc {
};
}
interface AppYamlDoc {
env?: Array<{ name?: string; valueFrom?: string }>;
env?: Array<{ name?: string; valueFrom?: string; value?: unknown }>;
}

/** Classifies a binding by its typed sub-key and origin: scanning its field
Expand Down Expand Up @@ -126,11 +129,18 @@ export function readBundleInfo(
): BundleInfo {
const bindings = new Map<string, BundleBinding>();
const envToBinding = new Map<string, string>();
const wiredEnvVars = new Set<string>();
const declaredResources = new Set<string>();

const doc = readYaml<BundleDoc>(path.resolve(cwd, bundleFile));
if (!doc) {
return { bindings, envToBinding, declaredResources, present: false };
return {
bindings,
envToBinding,
wiredEnvVars,
declaredResources,
present: false,
};
}

for (const [type, group] of Object.entries(doc.resources ?? {})) {
Expand All @@ -154,12 +164,23 @@ export function readBundleInfo(

const appYaml = readYaml<AppYamlDoc>(path.resolve(cwd, appYamlFile));
for (const entry of appYaml?.env ?? []) {
if (entry?.name && entry.valueFrom) {
if (!entry?.name) continue;
if (entry.valueFrom) {
envToBinding.set(entry.name, entry.valueFrom);
wiredEnvVars.add(entry.name);
} else if (entry.value != null) {
// A literal `value: ""` still sets the var; a bare `value:` (null) doesn't.
wiredEnvVars.add(entry.name);
}
}

return { bindings, envToBinding, declaredResources, present: true };
return {
bindings,
envToBinding,
wiredEnvVars,
declaredResources,
present: true,
};
}

/**
Expand Down
18 changes: 18 additions & 0 deletions packages/shared/src/cli/commands/doctor/checks-wiring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ function info(overrides: Partial<BundleInfo> = {}): BundleInfo {
return {
bindings: new Map(),
envToBinding: new Map(),
wiredEnvVars: new Set(),
declaredResources: new Set(),
present: true,
...overrides,
Expand Down Expand Up @@ -90,6 +91,7 @@ describe("checkWiring", () => {
info({
// app.yaml provides one env, bindings exist for it.
envToBinding: new Map([["DATABRICKS_WAREHOUSE_ID", "sql-warehouse"]]),
wiredEnvVars: new Set(["DATABRICKS_WAREHOUSE_ID"]),
bindings: new Map([
[
"sql-warehouse",
Expand Down Expand Up @@ -137,6 +139,22 @@ describe("checkWiring", () => {
);
});

it("treats a literal app.yaml `value:` entry as wired", () => {
// `{ name: X, value: <id> }` sets the var on deploy with no binding, so it's
// neither unwired nor an unbound valueFrom.
const findings = checkWiring(
info({ wiredEnvVars: new Set(["DATABRICKS_GENIE_SPACE_ID"]) }),
[
target({
type: "genie_space",
required: true,
envVars: ["DATABRICKS_GENIE_SPACE_ID"],
}),
],
);
expect(findings).toEqual([]);
});

it("marks an optional resource's unwired var as optional in the detail", () => {
const findings = checkWiring(info({ present: true }), [
target({ required: false, envVars: ["DATABRICKS_MISSING"] }),
Expand Down
6 changes: 4 additions & 2 deletions packages/shared/src/cli/commands/doctor/checks-wiring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
* `name` — otherwise the platform has nothing to inject and the env var the
* plugin reads stays empty at runtime;
* - every bundle-managed `${resources.<type>.<key>.*}` binding must reference a
* resource actually declared in the bundle — otherwise deploy fails.
* resource actually declared in the bundle — otherwise deploy fails;
* - every env var a used plugin reads must have an `app.yaml` entry, either
* `valueFrom` or a literal `value` — otherwise it's unset once deployed.
*
* Runs regardless of auth; these are declaration problems, not connectivity.
*/
Expand Down Expand Up @@ -59,7 +61,7 @@ export function checkWiring(
// it's a warning.
for (const target of targets) {
for (const envVar of target.envVars) {
if (!info.envToBinding.has(envVar)) {
if (!info.wiredEnvVars.has(envVar)) {
findings.push({
status: target.required ? "error" : "warn",
code: "ENV_UNWIRED",
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/src/cli/commands/doctor/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ export async function runDoctor(options: DoctorOptions): Promise<DoctorReport> {
// the deploy wiring so it doesn't advise fixing what's already correct.
const configCtx: ConfigCheckContext = {
envFile: options.envFile ?? DEFAULT_ENV_FILE,
wiredEnvVars: new Set(bundle.envToBinding.keys()),
wiredEnvVars: bundle.wiredEnvVars,
};

const setup = checkSetup(cwd, options);
Expand Down
Loading