Skip to content

Fix CRD parameterization: package naming, Value path, and array flattening#4261

Draft
guineveresaenger wants to merge 7 commits intomasterfrom
fix-crd-parameterization
Draft

Fix CRD parameterization: package naming, Value path, and array flattening#4261
guineveresaenger wants to merge 7 commits intomasterfrom
fix-crd-parameterization

Conversation

@guineveresaenger
Copy link
Copy Markdown
Contributor

Summary

Fixes three issues blocking end-to-end pulumi package add kubernetes -- -c crds.yaml usage:

  • Derive package name from CRD groups instead of hardcoding "mycrd". Adds -n/--name flag for explicit naming, falls back to first CRD group name (e.g., gateway.networking.k8s.iogateway-networking), or "crds" as default.
  • Implement Parameterize(Value) so subsequent pulumi up runs can reconstruct the CRD schema from saved state in Pulumi.yaml. Previously this was a stub returning nil.
  • Flatten array-of-objects in OpenAPI specs. flattenOpenAPI only handled direct object properties, missing objects nested inside arrays (e.g., spec.listeners, spec.rules[].backendRefs). These ended up as untyped StringMapArrayInput instead of typed structs like GatewaySpecListenersArgs.

Tested end-to-end with Gateway API CRDs — pulumi package add generates typed SDKs and pulumi up successfully creates resources.

Known limitations

  • Go module path collision: The generated SDK reuses the base k8s provider's module path, so base k8s resources can't be used alongside CRD resources in the same Go program. This is a Go codegen issue in pulumi/pulumi and will be resolved by extension parameterization.
  • Hyphenated field names: CRD fields with hyphens aren't normalized to valid identifiers. Tracked as a TODO.

Test plan

  • Unit tests for derivePackageName, stripK8sSuffix, parseCrdArgs
  • Unit tests for flattenOpenAPI with array-of-objects and nested arrays
  • End-to-end: pulumi package add kubernetes -- -v 1.2.1 -c gateway-api-crds.yaml → typed Go SDK → pulumi up

🤖 Generated with Claude Code

guineveresaenger and others added 4 commits March 23, 2026 13:59
Replace the hardcoded "mycrd" package name with a name derived from the
CRD API groups. Users can also specify a name explicitly via -n/--name.

- Add -n/--name flag to parseCrdArgs
- derivePackageName() strips k8s suffixes (.k8s.io, .x-k8s.io, .io) and
  joins group prefixes with hyphens (e.g. gateway.networking.k8s.io →
  gateway-networking)
- Falls back to "crds" if no groups can be extracted
- Add tests for parseCrdArgs, derivePackageName, and stripK8sSuffix

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…t runs

The engine calls Parameterize(Value) on subsequent `pulumi up` runs to
reconstruct the CRD schema from the parameters persisted in Pulumi.yaml.
Previously this was a stub returning nil, meaning parameterized CRD
programs would fail on any run after the initial `pulumi package add`.

The Value message carries Name, Version, and the serialized OpenAPI spec
(stored as the Parameter field in ParameterizationSpec during the Args
path). We deserialize it and regenerate the Pulumi schema via the same
generateSchema() function used by the Args path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
flattenOpenAPI only handled direct object properties, missing objects
nested inside arrays (e.g., spec.listeners, spec.rules[].backendRefs).
These array-of-object fields ended up as generic StringMapArrayInput in
the generated SDK, losing all type safety.

Now when a property is an array whose items schema is an object with
properties, the inner object is hoisted to a top-level definition and
the items schema is replaced with a $ref — the same treatment direct
object properties already received. This works recursively, so deeply
nested structures like rules[].matches[].path are also flattened.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 30, 2026

Codecov Report

❌ Patch coverage is 20.00000% with 108 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.73%. Comparing base (d4759d6) to head (1179bfe).
⚠️ Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
provider/pkg/provider/provider_parameterize.go 21.87% 58 Missing and 17 partials ⚠️
provider/pkg/gen/schema.go 0.00% 17 Missing ⚠️
provider/pkg/gen/typegen.go 35.29% 7 Missing and 4 partials ⚠️
provider/pkg/provider/provider.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4261      +/-   ##
==========================================
+ Coverage   44.47%   44.73%   +0.25%     
==========================================
  Files          93       93              
  Lines       10393    10478      +85     
==========================================
+ Hits         4622     4687      +65     
- Misses       5241     5261      +20     
  Partials      530      530              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@guineveresaenger guineveresaenger requested a review from a team March 30, 2026 21:58

func TestFlattenOpenAPINestedArrayOfObjects(t *testing.T) {
// Simulate rules[].backendRefs[] — nested array-of-objects inside array-of-objects.
sw := &spec.Swagger{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(minor/nitpick) I wonder if it's possible creating the nested objects using temporary variables to have this flatter (so it's a bit more legible for humans).

Copy link
Copy Markdown
Member

@pose pose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (even if not an SME)!

@pose
Copy link
Copy Markdown
Member

pose commented Apr 1, 2026

@claude please review this PR as well 🙏 Specially, around k8s and comprehensive Golang aspects (which are not my forte).

@guineveresaenger
Copy link
Copy Markdown
Contributor Author

/review-again

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@guineveresaenger
Copy link
Copy Markdown
Contributor Author

/review-again

@guineveresaenger
Copy link
Copy Markdown
Contributor Author

/review-again

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No actionable issues found after checking for bugs and CLAUDE.md compliance (no CLAUDE.md files exist in this repo).

Reviewed the three core changes:

  • Package naming: derivePackageName / stripK8sSuffix logic is correct; suffix ordering (.x-k8s.io before .k8s.io before .io) is intentional and tested.
  • parameterizeRequestValue: Correctly deserializes from ParameterizationSpec.Parameter bytes (set in generateSchema via marshaledOpenAPISchema). The serialization contract between the Args and Value paths is sound.
  • flattenOpenAPI array-of-objects: The shallow copy + shared Properties map pattern is correct — the parent's Items.Schema is replaced with a new $ref schema before the hoisted definition is processed, so no aliasing corruption occurs. Recursive flattening (e.g., rules[].backendRefs[]) is confirmed by TestFlattenOpenAPINestedArrayOfObjects.

The prior open nitpick on test readability (line 313) was addressed by commit 9874ea0ac.

Reviewed by Internal PR Re-Review (Slash Command)

To install this agentic workflow, run

gh aw add pulumi-labs/gh-aw-internal/.github/workflows/gh-aw-pr-rereview.md@734ef41746387a6818fd8ac3e619c9fd81ac6957

@guineveresaenger guineveresaenger marked this pull request as draft April 10, 2026 16:53
Thread packageName through gen.PulumiSchema and gen/typegen.go so that
parameterized CRD packages get distinct token prefixes, import paths,
and schema metadata from the start — no post-processing rewrite needed.

This lets a program import both the base @pulumi/kubernetes SDK and a
CRD-generated SDK (e.g., gateway-networking) side by side without Go
module path collisions.

Also teach the provider runtime to accept resource tokens with the
parameterized package prefix by checking crdSchemas.hasPackage() in
gvkFromURN, so that resources like
gateway-networking:gateway.networking.k8s.io/v1:Gateway are handled
correctly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants