From c923a266f413bcde101a14d25363e48f81cb43aa Mon Sep 17 00:00:00 2001 From: Jon Steinich Date: Tue, 8 Sep 2026 07:59:39 -0500 Subject: [PATCH] fix(provider-generator): emit provider functions into a "functions" submodule Generated Python bindings for any provider that declares provider-defined functions (Terraform >= 1.8) were unimportable: File ".../imports/kubernetes/provider/__init__.py", line 40, in from .functions import ( ModuleNotFoundError: No module named 'imports.kubernetes.provider.functions' Root cause is in jsii-pacmak's Python target. Cross-submodule type references are rendered as relative imports computed by `lib/targets/python/type-name.ts#relativeImportPath`, which decides "is the target a child of me?" with a bare prefix test and no `.`-boundary check: if (toPkg.startsWith(fromPkg)) return `.${toPkg.substring(fromPkg.length + 1)}`; The provider class lives in the `provider` submodule and imports the functions wrapper from a sibling submodule. With the folder named `provider-functions`, that sibling's Python name is `.provider_functions`, which string-prefixes `.provider` - so pacmak treated it as a child and emitted `from .functions import ...`, naming a module that is never written. Renaming the emitted folder to `functions` keeps the two submodule names prefix-disjoint, so pacmak emits the correct `from ..functions import ...`. Only Python was affected: Go, Java and C# reference the sibling package by its fully qualified name and never compute a relative path. The regression test replays pacmak's own `relativeImportPath` over the emitted layout and asserts the import it would write resolves to the emitted functions submodule, rather than asserting the folder name. Co-Authored-By: Claude Opus 5 --- .../edge-provider-schema.test.ts.snap | 4 +- .../provider-functions.test.ts.snap | 10 +- .../generator/provider-functions.test.ts | 108 +++++++++++++++++- .../get/__tests__/generator/provider.test.ts | 5 +- .../get/generator/emitter/resource-emitter.ts | 12 +- .../models/provider-function-model.ts | 16 ++- .../get/generator/models/resource-model.ts | 2 +- .../src/get/generator/provider-generator.ts | 4 +- .../cdktn/src/functions/provider-function.ts | 2 +- 9 files changed, 140 insertions(+), 23 deletions(-) diff --git a/packages/@cdktn/provider-generator/src/__tests__/__snapshots__/edge-provider-schema.test.ts.snap b/packages/@cdktn/provider-generator/src/__tests__/__snapshots__/edge-provider-schema.test.ts.snap index 06c866f94..919413032 100644 --- a/packages/@cdktn/provider-generator/src/__tests__/__snapshots__/edge-provider-schema.test.ts.snap +++ b/packages/@cdktn/provider-generator/src/__tests__/__snapshots__/edge-provider-schema.test.ts.snap @@ -11,7 +11,7 @@ export * as setBlockResource from './set-block-resource/index'; export * as mapListResource from './map-list-resource/index'; export * as ephemeralCachedSecret from './ephemeral-cached-secret/index'; export * as provider from './provider/index'; -export * as providerFunctions from './provider-functions/index'; +export * as functions from './functions/index'; " `; @@ -27,7 +27,7 @@ Object.defineProperty(exports, 'setBlockResource', { get: function () { return r Object.defineProperty(exports, 'mapListResource', { get: function () { return require('./map-list-resource'); } }); Object.defineProperty(exports, 'ephemeralCachedSecret', { get: function () { return require('./ephemeral-cached-secret'); } }); Object.defineProperty(exports, 'provider', { get: function () { return require('./provider'); } }); -Object.defineProperty(exports, 'providerFunctions', { get: function () { return require('./provider-functions'); } }); +Object.defineProperty(exports, 'functions', { get: function () { return require('./functions'); } }); " `; diff --git a/packages/@cdktn/provider-generator/src/get/__tests__/generator/__snapshots__/provider-functions.test.ts.snap b/packages/@cdktn/provider-generator/src/get/__tests__/generator/__snapshots__/provider-functions.test.ts.snap index 7eabf38b2..6f664f67f 100644 --- a/packages/@cdktn/provider-generator/src/get/__tests__/generator/__snapshots__/provider-functions.test.ts.snap +++ b/packages/@cdktn/provider-generator/src/get/__tests__/generator/__snapshots__/provider-functions.test.ts.snap @@ -238,14 +238,14 @@ export class ExampleProviderFunctions { exports[`generate provider functions covering variadic parameters, primitive/list returns, and a 'default' parameter name matches the snapshot: provider-index 1`] = ` "// generated by cdktn get -export * as providerFunctions from './provider-functions/index'; +export * as functions from './functions/index'; " `; exports[`generate provider functions covering variadic parameters, primitive/list returns, and a 'default' parameter name matches the snapshot: provider-lazy-index 1`] = ` "// generated by cdktn get -Object.defineProperty(exports, 'providerFunctions', { get: function () { return require('./provider-functions'); } }); +Object.defineProperty(exports, 'functions', { get: function () { return require('./functions'); } }); " `; @@ -254,7 +254,7 @@ exports[`generate provider functions for the time provider (real terraform 1.15. "// generated by cdktn get export * as staticResource from './static-resource/index'; export * as provider from './provider/index'; -export * as providerFunctions from './provider-functions/index'; +export * as functions from './functions/index'; " `; @@ -263,7 +263,7 @@ exports[`generate provider functions for the time provider (real terraform 1.15. "// generated by cdktn get Object.defineProperty(exports, 'staticResource', { get: function () { return require('./static-resource'); } }); Object.defineProperty(exports, 'provider', { get: function () { return require('./provider'); } }); -Object.defineProperty(exports, 'providerFunctions', { get: function () { return require('./provider-functions'); } }); +Object.defineProperty(exports, 'functions', { get: function () { return require('./functions'); } }); " `; @@ -286,7 +286,7 @@ export interface TimeProviderConfig { readonly alias?: string; } -import { TimeProviderFunctions } from '../provider-functions/index'; +import { TimeProviderFunctions } from '../functions/index'; /** * Represents a {@link https://registry.terraform.io/providers/hashicorp/time/latest/docs time} */ diff --git a/packages/@cdktn/provider-generator/src/get/__tests__/generator/provider-functions.test.ts b/packages/@cdktn/provider-generator/src/get/__tests__/generator/provider-functions.test.ts index 97240eef7..e107f078a 100644 --- a/packages/@cdktn/provider-generator/src/get/__tests__/generator/provider-functions.test.ts +++ b/packages/@cdktn/provider-generator/src/get/__tests__/generator/provider-functions.test.ts @@ -7,12 +7,44 @@ import { assertNoFunctionsGetterCollision, buildProviderFunctionsModel, } from "../../generator/models/provider-function-model"; -import { CodeMaker } from "codemaker"; +import { CodeMaker, toSnakeCase } from "codemaker"; import { FunctionSignature } from "@cdktn/commons"; import { createTmpHelper } from "../util"; const tmp = createTmpHelper(); +/** + * Verbatim copy of jsii-pacmak's Python cross-submodule import path + * calculation (jsii-pacmak@1.128.0 + * `lib/targets/python/type-name.ts#relativeImportPath`), including the + * `startsWith` test that is missing a `.`-boundary check. Copied rather than + * approximated so the assertion below fails for exactly the layouts pacmak + * mis-renders. + */ +function pacmakRelativeImportPath(fromPkg: string, toPkg: string): string { + if (toPkg.startsWith(fromPkg)) { + return `.${toPkg.substring(fromPkg.length + 1)}`; + } + const fromPkgParent = fromPkg.substring(0, fromPkg.lastIndexOf(".")); + return `.${pacmakRelativeImportPath(fromPkgParent, toPkg)}`; +} + +/** + * Resolves a Python relative import specifier (`.x`, `..x`, ...) written + * inside the package `fromPkg` to the absolute module it names. One leading + * dot means "this package", each further dot climbs one level. + */ +function resolveRelativeImport(fromPkg: string, specifier: string): string { + const dots = /^\.*/.exec(specifier)![0].length; + const tail = specifier.slice(dots); + const segments = fromPkg.split("."); + const base = segments.slice(0, segments.length - (dots - 1)); + return [...base, ...(tail ? [tail] : [])].join("."); +} + +/** jsii's submodule name -> Python module name mapping. */ +const pythonModuleName = (submoduleName: string) => toSnakeCase(submoduleName); + test("generate provider functions for the time provider (real terraform 1.15.6 schema fragment)", async () => { const code = new CodeMaker(); const workdir = tmp("provider-functions.test"); @@ -26,7 +58,7 @@ test("generate provider functions for the time provider (real terraform 1.15.6 s await code.save(workdir); const providerFunctionsOutput = fs.readFileSync( - path.join(workdir, "providers/time/provider-functions/index.ts"), + path.join(workdir, "providers/time/functions/index.ts"), "utf-8", ); expect(providerFunctionsOutput).toMatchSnapshot("time-provider-functions"); @@ -50,6 +82,76 @@ test("generate provider functions for the time provider (real terraform 1.15.6 s expect(providerLazyIndex).toMatchSnapshot("provider-lazy-index"); }); +// Regression test for the Python bindings of a provider that declares +// provider-defined functions. +// +// The provider class lives in the `provider` jsii submodule and imports the +// functions wrapper class from a sibling submodule. jsii-pacmak renders that +// cross-submodule reference in Python as a *relative* import, computed by +// `relativeImportPath` - which decides "is the target a child of me?" with a +// bare `toPkg.startsWith(fromPkg)`, no `.`-boundary check. A sibling +// submodule whose Python name merely string-prefixes the importing one is +// therefore mistaken for a child, and pacmak emits an import of a module +// that was never written to disk. That is what a `provider-functions` folder +// did: submodule `.provider_functions` string-prefixes +// `.provider`, so `/provider/__init__.py` got +// `from .functions import ...` and importing the provider raised +// ModuleNotFoundError (Go/Java/C# are unaffected - they use fully qualified +// names and never compute a relative path). +// +// Rather than assert the folder name, this reproduces pacmak's own +// calculation over the emitted layout and checks the import it would write +// actually resolves to the emitted functions submodule. +test("the emitted layout makes jsii-pacmak's Python relative import from the provider submodule resolve to the functions submodule", async () => { + const code = new CodeMaker(); + const workdir = tmp("provider-functions-python-layout.test"); + const spec = JSON.parse( + fs.readFileSync( + path.join(__dirname, "fixtures", "provider-functions.test.fixture.json"), + "utf-8", + ), + ); + new TerraformProviderGenerator(code, spec).generateAll(); + await code.save(workdir); + + // The folder the provider class imports the functions wrapper from, read + // back out of the generated source instead of hard-coded. + const providerOutput = fs.readFileSync( + path.join(workdir, "providers/time/provider/index.ts"), + "utf-8", + ); + const importMatch = /from '\.\.\/([^/]+)\/index'/.exec(providerOutput); + expect(importMatch).not.toBeNull(); + const functionsFolder = importMatch![1]; + + // ...and it has to be a submodule the root index actually exports, or + // there would be no Python package for it at all. + const providerIndex = fs.readFileSync( + path.join(workdir, "providers/time/index.ts"), + "utf-8", + ); + const submodules = new Map( + [ + ...providerIndex.matchAll( + /export \* as (\w+) from '\.\/([^/]+)\/index'/g, + ), + ].map((m) => [m[2], m[1]]), + ); + expect(submodules.has("provider")).toBe(true); + expect(submodules.has(functionsFolder)).toBe(true); + + // Python module names of the two submodules, under the provider's own + // Python root package (`imports.` in a real project). + const root = "time"; + const providerPkg = `${root}.${pythonModuleName(submodules.get("provider")!)}`; + const functionsPkg = `${root}.${pythonModuleName( + submodules.get(functionsFolder)!, + )}`; + + const specifier = pacmakRelativeImportPath(providerPkg, functionsPkg); + expect(resolveRelativeImport(providerPkg, specifier)).toBe(functionsPkg); +}); + describe("generate provider functions covering variadic parameters, primitive/list returns, and a 'default' parameter name", () => { let providerFunctionsOutput: string; let providerIndex: string; @@ -72,7 +174,7 @@ describe("generate provider functions covering variadic parameters, primitive/li await code.save(workdir); providerFunctionsOutput = fs.readFileSync( - path.join(workdir, "providers/example/provider-functions/index.ts"), + path.join(workdir, "providers/example/functions/index.ts"), "utf-8", ); providerIndex = fs.readFileSync( diff --git a/packages/@cdktn/provider-generator/src/get/__tests__/generator/provider.test.ts b/packages/@cdktn/provider-generator/src/get/__tests__/generator/provider.test.ts index fc40679bf..119c3d845 100644 --- a/packages/@cdktn/provider-generator/src/get/__tests__/generator/provider.test.ts +++ b/packages/@cdktn/provider-generator/src/get/__tests__/generator/provider.test.ts @@ -3,6 +3,7 @@ import * as fs from "fs"; import * as path from "path"; import { TerraformProviderGenerator } from "../../generator/provider-generator"; +import { PROVIDER_FUNCTIONS_FOLDER_NAME } from "../../generator/models"; import { CodeMaker } from "codemaker"; import { createTmpHelper } from "../util"; @@ -28,7 +29,7 @@ test("generate provider", async () => { // aws has no provider-defined functions in this fixture - no getter, no // cross-directory import should be emitted. expect(output).not.toContain("public get functions()"); - expect(output).not.toContain("provider-functions"); + expect(output).not.toContain(`../${PROVIDER_FUNCTIONS_FOLDER_NAME}/index`); }); test("generate provider with only block_types", async () => { @@ -55,5 +56,5 @@ test("generate provider with only block_types", async () => { // elasticstack has no provider-defined functions in this fixture - no // getter, no cross-directory import should be emitted. expect(output).not.toContain("public get functions()"); - expect(output).not.toContain("provider-functions"); + expect(output).not.toContain(`../${PROVIDER_FUNCTIONS_FOLDER_NAME}/index`); }); diff --git a/packages/@cdktn/provider-generator/src/get/generator/emitter/resource-emitter.ts b/packages/@cdktn/provider-generator/src/get/generator/emitter/resource-emitter.ts index 7f0550b97..c37d28f9d 100644 --- a/packages/@cdktn/provider-generator/src/get/generator/emitter/resource-emitter.ts +++ b/packages/@cdktn/provider-generator/src/get/generator/emitter/resource-emitter.ts @@ -1,7 +1,7 @@ // Copyright (c) HashiCorp, Inc // SPDX-License-Identifier: MPL-2.0 import { CodeMaker } from "codemaker"; -import { ResourceModel } from "../models"; +import { PROVIDER_FUNCTIONS_FOLDER_NAME, ResourceModel } from "../models"; import { AttributesEmitter } from "./attributes-emitter"; import { sanitizedComment } from "../sanitized-comments"; @@ -19,12 +19,12 @@ export class ResourceEmitter { this.code.line(); if (resource.isProvider && resource.providerFunctionsModel) { - // Sibling-directory import: providers//provider/index.ts - // (the emitted file) and providers//provider-functions/ - // index.ts are siblings under providers//, so unlike the - // child-folder struct imports this one has to step up a level. + // Sibling-directory import: provider/index.ts and functions/index.ts + // are siblings under providers//, so unlike the child-folder + // struct imports this one steps up a level. The folder name is + // constrained - see PROVIDER_FUNCTIONS_FOLDER_NAME. this.code.line( - `import { ${resource.providerFunctionsModel.className} } from '../provider-functions/index${this.importExtension}';`, + `import { ${resource.providerFunctionsModel.className} } from '../${PROVIDER_FUNCTIONS_FOLDER_NAME}/index${this.importExtension}';`, ); } diff --git a/packages/@cdktn/provider-generator/src/get/generator/models/provider-function-model.ts b/packages/@cdktn/provider-generator/src/get/generator/models/provider-function-model.ts index 34076c40e..f6c42469d 100644 --- a/packages/@cdktn/provider-generator/src/get/generator/models/provider-function-model.ts +++ b/packages/@cdktn/provider-generator/src/get/generator/models/provider-function-model.ts @@ -95,9 +95,21 @@ export interface ProviderFunctionModel { readonly variadicParameter?: ProviderFunctionParameterModel; } +/** + * Folder (and jsii submodule) holding a provider's generated functions + * wrapper, emitted as a sibling of `provider/` under `providers//`. + * + * Must not start with `provider`: jsii-pacmak <= 1.135.0 computes Python + * cross-submodule imports with a prefix test that has no `.`-boundary check, + * so `provider_functions` reads as a child of `provider` and emits an import + * for a module that is never written. Fixed upstream in pacmak 1.136.0; this + * repo pins 1.128.0. + */ +export const PROVIDER_FUNCTIONS_FOLDER_NAME = "functions"; + /** * All provider-defined functions of a single provider, mapped to a single - * generated `providers//provider-functions/index.ts` file. + * generated `providers//functions/index.ts` file. */ export interface ProviderFunctionsModel { readonly providerName: string; @@ -843,7 +855,7 @@ export function assertNoFunctionsGetterCollision( } /** - * Builds the model for a provider's `provider-functions/index.ts` file from + * Builds the model for a provider's `functions/index.ts` file from * its provider schema `functions` map. Returns `undefined` when the provider * declares no functions - callers should skip emitting the file entirely. */ diff --git a/packages/@cdktn/provider-generator/src/get/generator/models/resource-model.ts b/packages/@cdktn/provider-generator/src/get/generator/models/resource-model.ts index 33e8beb69..dc8ed3fa1 100644 --- a/packages/@cdktn/provider-generator/src/get/generator/models/resource-model.ts +++ b/packages/@cdktn/provider-generator/src/get/generator/models/resource-model.ts @@ -47,7 +47,7 @@ export class ResourceModel { * Only set (by provider-generator.ts) when isProvider is true and the * provider schema declares provider-defined functions - drives whether * ResourceEmitter emits the memoized `functions` getter and its - * cross-directory import of the sibling provider-functions/index.ts file. + * cross-directory import of the sibling functions/index.ts file. */ public providerFunctionsModel?: ProviderFunctionsModel; public fileName: string; diff --git a/packages/@cdktn/provider-generator/src/get/generator/provider-generator.ts b/packages/@cdktn/provider-generator/src/get/generator/provider-generator.ts index b3d62eadd..05f62d85c 100644 --- a/packages/@cdktn/provider-generator/src/get/generator/provider-generator.ts +++ b/packages/@cdktn/provider-generator/src/get/generator/provider-generator.ts @@ -11,6 +11,7 @@ import { } from "@cdktn/commons"; import { FQPN, parseFQPN, ProviderName } from "@cdktn/provider-schema"; import { + PROVIDER_FUNCTIONS_FOLDER_NAME, ProviderFunctionsModel, ResourceModel, assertNoFunctionsGetterCollision, @@ -304,7 +305,8 @@ export class TerraformProviderGenerator { provider: ProviderName, model: ProviderFunctionsModel, ): string { - const filePath = `providers/${provider}/provider-functions/index.ts`; + // Folder name is constrained - see PROVIDER_FUNCTIONS_FOLDER_NAME. + const filePath = `providers/${provider}/${PROVIDER_FUNCTIONS_FOLDER_NAME}/index.ts`; this.code.openFile(filePath); this.code.line(`// generated from provider function schema`); this.code.line(); diff --git a/packages/cdktn/src/functions/provider-function.ts b/packages/cdktn/src/functions/provider-function.ts index 9453050d5..bae9762ce 100644 --- a/packages/cdktn/src/functions/provider-function.ts +++ b/packages/cdktn/src/functions/provider-function.ts @@ -5,7 +5,7 @@ import { anyValue, terraformFunction } from "./helpers"; /** * Runtime entry point invoked by generated provider function bindings - * (`providers//provider-functions/index.ts`). Generated code only + * (`providers//functions/index.ts`). Generated code only * imports the public `cdktn` package root, so this is the single chokepoint * through which every provider-defined function call flows. */