fix(provider-generator): emit provider functions into a "functions" submodule

[jsteinich]

Related issue

No filed issue — found by CI on #398, which raises the Terraform test ceiling to 1.16.1. #398 is deliberately on hold until this lands.

Description

Generated Python bindings for any provider declaring provider-defined functions are unimportable:

File ".../imports/kubernetes/provider/__init__.py", line 40, in <module>
    from .functions import (
ModuleNotFoundError: No module named 'imports.kubernetes.provider.functions'

Root cause: a jsii-pacmak bug, triggered by our emitted layout

jsii-pacmak/lib/targets/python/type-name.ts#relativeImportPath decides "is the target submodule a child of me?" with a bare string-prefix test and no .-boundary check:

if (toPkg.startsWith(fromPkg)) {
  return `.${toPkg.substring(fromPkg.length + 1)}`;   // from A.B to A.B.C === .C
}

The provider class lives in the provider jsii submodule and imports the functions wrapper from a sibling submodule. Named provider-functions, that sibling's Python name is <provider>.provider_functions — which string-prefixes <provider>.provider. pacmak therefore treats the sibling as a child and emits from .functions import …, naming <provider>.provider.functions, 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 is affected. Go (.../edge/providerfunctions), Java (imports.edgeprovider.provider_functions) and C# (Providers/Edge/ProviderFunctions) reference the sibling by fully qualified name and never compute a relative path — verified in the generated edge-provider bindings.

Already fixed upstream — in a pacmak we don't use yet

jsii-pacmak stopped emitting relative cross-submodule imports in 1.136.0, switching to absolute LazyImport references. relativeImportPath still exists in 1.140.0 but is dead code. Verified by inspecting each published tarball:

jsii-pacmak	relativeImportPath call sites	LazyImport refs
1.128.0 (this repo's pin)	1	0
1.130.0 / 1.133.0 / 1.135.0	1	0
1.136.0 → 1.140.0	0	7

This is why prebuilt providers were never affected, despite being generated against a modern Terraform that emits functions. cdktn-provider-time@14.0.1 on PyPI ships both a provider/ and a provider_functions/ submodule — the exact prefix-colliding pair — yet its provider/__init__.py contains import cdktn_provider_time.provider_functions as _provider_functions_1fc2c0c2, an absolute import, with no broken relative one. It was built with jsii 5.9.51 and a post-1.136 pacmak. Only cdktn get run from this repo's pinned 1.128.0 produces the broken output.

So there are two ways to fix this: rename the folder (this PR), or upgrade jsii-pacmak past 1.136.0 — which #373 already proposes (1.128.0 → 1.139.0+) as part of the jsii 6.0 migration.

I went with the rename because it is small, independent, and unblocks #398 now, whereas #373 is a coordinated jsii/TypeScript/constructs upgrade. The rename also stays worthwhile after #373: the two submodule names become prefix-disjoint regardless of which pacmak is in use, so the layout stops depending on an upstream implementation detail. If you would rather wait for #373 and drop this, that is a reasonable call — the regression test would need rethinking, since it replays relativeImportPath deliberately.

Why this layer

The real defect is upstream. Fixing it there means patching a bundled dependency that also ships inside cdktn-cli. The generator owns the emitted layout and is the only thing that has to change: two emit sites plus a documented constant, PROVIDER_FUNCTIONS_FOLDER_NAME, carrying the "must not start with provider" rationale at the point where someone would otherwise rename it back.

The change is invisible to users — the functions are reached through the provider.functions getter, and nothing in examples/, test/ or docs/ referenced the old submodule name.

Worth reporting upstream to aws/jsii separately; the fix there is a .-boundary check in relativeImportPath.

Why it was never caught

Terraform only emits a functions section in terraform providers schema -json from 1.8 onward, and CI's ceiling was 1.6.5, so no CI job could produce a schema that reaches this codepath. That is exactly the gap #337 was filed about. test/python/edge/test.ts is also describe.skip'd, which is why #311's cross-language compile coverage did not catch it either.

Testing

New test in packages/@cdktn/provider-generator/src/get/__tests__/generator/provider-functions.test.ts. Rather than asserting the folder name — a change-detector that would be re-broken by the same rename — it copies pacmak's relativeImportPath verbatim, reads the provider→functions submodule pair back out of the generated index.ts, computes the import specifier pacmak would write, and asserts it resolves to the emitted functions submodule.

Confirmed to be a real regression guard: with PROVIDER_FUNCTIONS_FOLDER_NAME reverted to "provider-functions", the suite fails 8 of 15; with the fix it passes.

Also verified:

• @cdktn/provider-generator: 22 suites / 107 tests / 101 snapshots pass (re-run after rebasing onto fix(provider-generator): satisfy module provider configuration aliases on get #383). 7 snapshots updated, all the providerFunctions→functions export line.
• Edge bindings rebuild: post-fix python/edge/functions/ exists and provider/__init__.py has from ..functions import …. An AST walk resolving every relative import in the generated Python: 22 checked, 0 unresolvable; the same walk pre-fix flags exactly one.
• End-to-end cdktn get against the real hashicorp/time@0.14.1 provider (which declares provider functions) with targetLanguage: PYTHON — real schema → jsii → pacmak → Python. Produces a imports/time/functions/ sibling package and from ..functions import TimeProviderFunctions; all 8 relative imports resolve.

Not verified: @examples/python-documentation itself against kubernetes@~> 2.0, which needs a full pnpm package + example synth. The time run exercises the identical path with a real functions-bearing schema, and the pre-fix symptom matched the CI traceback's file and line exactly. #398's CI is the definitive check — that example is what fails there today.

Follow-ups, deliberately not here

• test/python/edge/test.ts is describe.skip'd; un-skipping it is how this class of bug gets caught in future.
• No guard exists against a provider that has both functions and a resource named <provider>_functions, which would collide on the functions folder.
• Real providers already produce prefix-related sibling submodules (e.g. instance / instance_state); they don't cross-reference today, so the pacmak bug stays latent, but it is one reference away from biting again.

Checklist

• I have updated the PR title to match CDKTN's style guide
• I have run the linter on my code locally
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation if applicable — n/a, the submodule name is an internal emit detail
• My changes generate no new warnings
• I have added tests that prove my fix is effective
• New and existing unit tests pass locally with my changes

🤖 Generated with Claude Code

[so0k]

small change, big impact.

Too bad my out-of-repo harnesses didn't really validate Python consumption of provider functions - the real issue is correctly being addressed now with raising the version ceiling of what we test of course

but if I build demo harnesses I should include JSII cross compiled library testing
https://github.com/sakul-learning/cdktn-provider-features-demo

[so0k]

Status after #395 / #398: no longer a blocker — the rename is now the risk

#395 (jsii-pacmak 1.140.0) merged, #398 was rebased on it and went fully green, including @examples/python-documentation @ tf1.16.1, and cdktn-cli@0.25.0-pre.11 ships it. So the bug this PR was opened for is fixed on main without the rename: in pacmak ≥1.136 relativeImportPath still exists but has no caller, and the generated bindings now emit _LazyImport("kubernetes.provider_functions"). (That also makes the upstream aws/jsii report moot — it's dead code there.)

What remains in this PR is the provider-functions → functions rename, and I'd push back on the "invisible to users / internal emit detail" framing. The jsii submodule name is a public FQN in every target. @cdktn/provider-time@14.0.1 on npm has @cdktn/provider-time.providerFunctions.TimeProviderFunctions in its .jsii; that is lib/provider-functions/ in TS, cdktn_provider_time.provider_functions in Python (which imports fine today under pacmak ≥1.136 — this PR would rename it out from under anyone using it), providerfunctions in Go, provider_functions in Java, ProviderFunctions in C#.

My own position is that the supported surface is provider.functions — the functions need the provider instance/alias to be invoked, so reaching for the submodule directly is the wrong way anyway, and it's undocumented. That is why "patch release" seemed defensible while this rename was the only way to unblock Python. Now that it isn't, there's no consumer benefit left to weigh against the risk, and the risk lands awkwardly: prebuilt provider versions track the upstream provider, so the rename would ship as a non-major bump, and their compat/jsii-diff task isn't wired in. If we ever do this, it should be a deliberate major with the submodule name documented — not a patch.

Worth keeping, re-scoped

1. Folder-level collision guard. This is a real gap on main today, not just after a rename. sanitizeClassOrNamespaceName reserves function (singular), license, version and the TypeScript keywords, and handles provider via resourceIsNamedProvider — but neither provider_functions nor functions is reserved. A resource or data source named <provider>_provider_functions currently writes into providers/<provider>/provider-functions/ on top of the functions submodule, silently. Reserving the functions submodule's base name there (→ _resource suffix, like provider) plus a test would close it. Note provider-functions is a far less likely resource name than functions, which is one more reason to keep the current name.

2. Regression coverage. The pacmak-replay test asserts against 1.128's relativeImportPath, which nothing calls in 1.140 — it would pass vacuously now. The durable check is a real Python import: test/python/edge/test.ts is still describe.skip'd (line 7 on main), which is exactly why Edge-provider schema: cross-language compile coverage for ephemeral resources, provider functions, write-only attributes #311's cross-language coverage never saw this; un-skipping it, or a small import imports.<provider>.provider smoke test in the python-documentation example, would have caught it and will catch the next one.

Suggest either re-scoping this PR to (1) + (2) and dropping the rename, or closing it and opening those two as small follow-ups. Happy with either.

[jsteinich]

Agreed on all of it — closing in favour of the re-scoped follow-ups.

You're right that the rename is the risk now rather than the fix. #395 landing makes it unnecessary, and "internal emit detail" was wrong of me: the jsii submodule name is a public FQN in every target, and renaming it as a non-major on packages whose versions track the upstream provider — with compat/jsii-diff not wired in — is not a trade worth making for zero consumer benefit. If it's ever done it should be a deliberate major with the name documented.

Split as suggested:

• fix(provider-generator): stop a provider_functions resource clobbering the functions submodule #403 — folder-level collision guard. Confirmed this is live on main today, independent of the rename: with a provider that declares functions plus an example_provider_functions resource, the resource overwrites provider-functions/index.ts and index.ts emits export * as providerFunctions twice, which doesn't compile. Reserved provider_functions alongside the existing provider case, with a fixture covering all three symptoms.
• test(tests): re-enable the Python edge integration test #404 — re-enables test/python/edge/test.ts, skipped since Dec 2022 pending chore(python): use builtins.str instead of just str in Dict keys aws/jsii#3866 and never lifted. Draft, because pipenv/rsync aren't available here so CI has to be the verification.

On the pacmak-replay test: agreed it would pass vacuously now, so it's dropped rather than carried over — #404 is the durable replacement. And you're right that the upstream aws/jsii report is moot; relativeImportPath still exists in 1.140 but has no caller.