Skip to content
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

Enable parameterization RPC calls to generate Pulumi schemas #3187

Merged
merged 8 commits into from
Sep 12, 2024

Conversation

rquitales
Copy link
Member

@rquitales rquitales commented Aug 22, 2024

Proposed Changes

This PR introduces the ability to generate Pulumi schemas and SDKs from user-specified CRDs using the new Parameterize RPC method for the provider.

Although full support for generating and utilizing SDKs is currently blocked by on-going upstream implementation #17059, this PR ensures that valid schemas/SDKs can still be generated and used later once the upstream work is landed.

As there are some hard-coded logic (within the GetSchema RPC method), this PR will be merged to a feature branch until all hard-coded logic can be removed.

Detailed Changes

  • Implements the Parameterize RPC method for the provider, allowing CRD manifests to be processed via pulumi package get-schema.
  • Updates the existing schema generation logic to be reusable by other packages.
  • Supports converting CRD manifests into valid Pulumi schemas with typed nested objects.
  • Fixes a bug in schemagen that could result in malformed package names.
  • Resolves outstanding issues with schema/SDK generation in crd2pulumi (except for field names with hyphens).

Unresolved Issues

  • Generating valid SDKs for CRDs that contain field names with hyphens is still not supported. This will require enhanced parameterization to map between logical SDK field names and their physical Kubernetes counterparts.

Example CRD manifest that can't generate valid SDKs:

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: crontabs.stable.example.com
spec:
  group: stable.example.com
  versions:
    - name: v1
      served: true
      storage: true
      schema:
        openAPIV3Schema:
          type: object
          properties:
            spec:
              type: object
              properties:
                cronSpec:
                  type: string
                image:
                  type: string
                replicas:
                  type: integer
                test-hyphen-field:
                  type: string

Related Issues

Closes #3121
Closes #3122
Closes #3149

@rquitales rquitales marked this pull request as draft August 22, 2024 23:41
@rquitales rquitales force-pushed the rquitales/crd-generation-new branch 4 times, most recently from b1418e6 to 9f71690 Compare September 6, 2024 16:37
@pulumi pulumi deleted a comment from github-actions bot Sep 6, 2024
@rquitales rquitales force-pushed the rquitales/crd-generation-new branch from c3531bb to 4278a33 Compare September 6, 2024 16:43
@rquitales rquitales changed the base branch from master to crd-generation September 6, 2024 16:44
@rquitales rquitales marked this pull request as ready for review September 6, 2024 17:00
@rquitales rquitales force-pushed the rquitales/crd-generation-new branch from 4278a33 to fea1a62 Compare September 6, 2024 17:16
@rquitales rquitales changed the title [WIP] Enable parameterization RPC calls to generate Pulumi schemas Enable parameterization RPC calls to generate Pulumi schemas Sep 6, 2024
@pulumi pulumi deleted a comment from github-actions bot Sep 6, 2024
Copy link

github-actions bot commented Sep 6, 2024

Does the PR have any schema changes?

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

@rquitales rquitales changed the base branch from crd-generation to master September 6, 2024 19:57
Copy link

codecov bot commented Sep 6, 2024

Codecov Report

Attention: Patch coverage is 15.20913% with 223 lines in your changes missing coverage. Please review.

Project coverage is 38.65%. Comparing base (730d9d8) to head (39a2826).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
provider/pkg/provider/provider_parameterize.go 3.10% 156 Missing ⚠️
provider/pkg/gen/schema.go 0.00% 30 Missing ⚠️
provider/pkg/gen/typegen.go 0.00% 19 Missing ⚠️
provider/pkg/gen/dotnet.go 0.00% 10 Missing ⚠️
provider/pkg/provider/provider.go 0.00% 7 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3187      +/-   ##
==========================================
- Coverage   39.34%   38.65%   -0.69%     
==========================================
  Files          82       84       +2     
  Lines        9631     9853     +222     
==========================================
+ Hits         3789     3809      +20     
- Misses       5481     5682     +201     
- Partials      361      362       +1     

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

Copy link
Contributor

@blampe blampe left a comment

Choose a reason for hiding this comment

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

Absence of SDK changes makes me comfortable merging this into master. I do wonder if it would be possible to smoke test some of the CRD flattening, what do you think?

return fmt.Errorf("case mapping for %q already exists", name)
}

// Ensure the pascal case name is actually pascal case, otherwise capitalize the first letter.
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want to handle the "vN" prefixes in a generic way for consistency (e.g. v4gamma10 → V4Gamma10). Something like https://github.com/rossmacarthur/cases would probably work, or some home-grown code if we don't want to get too generic. This isn't blocking though!

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to use a forked copy (with tests) of Pulumi's cgstrings.ModifyStringAroundDelimeter which accepts a regex delimiter instead of a fixed string for this purpose.

Comment on lines +54 to +57
var PascalCaseMapping = CaseMapping{
mapping: map[string]string{
Copy link
Contributor

Choose a reason for hiding this comment

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

Would the user eventually be able to provide their own mapping via the parameterization? I could see this being a feature request.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that is the intention! Punting the actual implementation for this util we have more clarity on the entrypoints for this user journey.

provider/pkg/gen/typegen.go Outdated Show resolved Hide resolved
continue
}
// Defaults are not pruned here, but before being served.
sw, err := builder.BuildOpenAPIV2(crd, v.Name, builder.Options{V2: true, StripValueValidation: false, StripNullable: false, AllowNonStructural: true})
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we prefer v2 over v3?

Copy link
Member Author

Choose a reason for hiding this comment

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

Schemagen currently parses an OpenAPI v2 swagger spec (

SWAGGER_URL ?= https://github.com/kubernetes/kubernetes/raw/${KUBE_VERSION}/api/openapi-spec/swagger.json
). I didn't look too deep, but refactoring schemagen to handle v3 will likely be a larger effort. Since there are no plans for K8s to deprecate serving swagger specs, I'm deferring that work for now.

@rquitales
Copy link
Member Author

Absence of SDK changes makes me comfortable merging this into master. I do wonder if it would be possible to smoke test some of the CRD flattening, what do you think?

I'm intending to have the existing crd2pulumi tool take a dependency on these schemagen changes. Testing the e2e flows for this requires some hard-coding, so I'll be moving those to crd2pulumi for downstream testing for now so that these changes can be landed safely into master. I'm still in the process of reworking the unit tests for these changes.

This commit implements an empty Parameterize method to enable generating types for user specified CRD manifests.
To be able to use the existing schemagen that the k8s provider uses, we'll need to convert any CRD manifest to their served OpenAPI swagger spec. We'll be relying on the existing stable schemagen logic of p-k and deprecating the schemagen logic in crd2pulumi.

chore: go mod tidy
@rquitales rquitales force-pushed the rquitales/crd-generation-new branch from 764527f to 5885307 Compare September 9, 2024 21:15
@rquitales rquitales added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Sep 9, 2024
@rquitales rquitales force-pushed the rquitales/crd-generation-new branch from 5885307 to a99cb08 Compare September 11, 2024 17:02
We need to refactor our schemagen logic to be consumed as a library to be usable by the Parameterize method.
We'll be storing an encoded version of the OpenAPI specs for the user's CRDs within the package schema. This can then be reconstructed during runtime to teach the p-k provider how logical types should relate back to their physical k8s objects.

Fix linting
A yaml manifest for Kubernetes could contain multiple documents/CRDs. We need to iteratively convert all CRDs provided, not just the first document in a yaml manifest.
This commit solves the challenge of generating nested types by modifying the generated OpenAPI spec, rather than attempting to solve this in the schemagen logic. This has the added benefit of not needing a significant overhaul of the schemagen logic.
@rquitales rquitales force-pushed the rquitales/crd-generation-new branch from a99cb08 to 39a2826 Compare September 11, 2024 17:22
@rquitales rquitales merged commit ace60fd into master Sep 12, 2024
19 checks passed
@rquitales rquitales deleted the rquitales/crd-generation-new branch September 12, 2024 18:17
lumiere-bot bot referenced this pull request in coolguy1771/home-ops Sep 16, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@pulumi/kubernetes](https://pulumi.com)
([source](https://github.com/pulumi/pulumi-kubernetes)) |
dependencies | minor | [`4.17.1` ->
`4.18.1`](https://renovatebot.com/diffs/npm/@pulumi%2fkubernetes/4.17.1/4.18.1)
|

---

### Release Notes

<details>
<summary>pulumi/pulumi-kubernetes (@&#8203;pulumi/kubernetes)</summary>

###
[`v4.18.1`](https://github.com/pulumi/pulumi-kubernetes/blob/HEAD/CHANGELOG.md#4181-September-13-2024)

[Compare
Source](https://github.com/pulumi/pulumi-kubernetes/compare/v4.18.0...v4.18.1)

##### Added

- Schemagen is now a library that can be consumed by other packages.
([https://github.com/pulumi/pulumi-kubernetes/pull/3187](https://github.com/pulumi/pulumi-kubernetes/pull/3187))

##### Changed

- Updated beta Kubernetes client libraries to stable v1.31 release.
([https://github.com/pulumi/pulumi-kubernetes/pull/3196](https://github.com/pulumi/pulumi-kubernetes/pull/3196))

###
[`v4.18.0`](https://github.com/pulumi/pulumi-kubernetes/blob/HEAD/CHANGELOG.md#4180-September-3-2024)

[Compare
Source](https://github.com/pulumi/pulumi-kubernetes/compare/v4.17.1...v4.18.0)

##### Added

- The new `enableSecretMutable` provider configuration option treats
changes to
    `Secrets` as updates instead of replacements (similar to the
    `enableConfigMapMutable` option).

The default replacement behavior can be preserved for a particular
`Secret`
    by setting its `immutable` field to `true`.

[https://github.com/pulumi/pulumi-kubernetes/issues/2291](https://github.com/pulumi/pulumi-kubernetes/issues/2291)2291)

**Note:** These options (`enableSecretMutable` and
`enableConfigMapMutable`)
may become the default behavior in a future v5 release of the provider.
Programs that depend on the replacement of `Secrets` and `ConfigMaps`
(e.g.
to trigger updates for downstream dependencies like `Deployments`) are
    recommended to explicitly specify `immutable: true`.

- A warning is now emitted if an object has finalizers which might be
blocking

deletio[https://github.com/pulumi/pulumi-kubernetes/issues/1418](https://github.com/pulumi/pulumi-kubernetes/issues/1418)1418)

- **EXPERIMENTAL**: Generic await logic is now available as an opt-in
feature.
Running a program with `PULUMI_K8S_AWAIT_ALL=true` will now cause Pulumi
to
    await readiness for *all* resources, including custom resources.

Generic readiness is determined according to some well-known conventions
(like
the "Ready" condition) as determined by
[cli-utils](https://github.com/kubernetes-sigs/cli-utils/tree/master/pkg/kstatus).

Pulumi's current behavior, without this feature enabled, is to assume
some
resources are immediately available, which can cause downstream
resources to
    fail.

    Existing readiness logic is unaffected by this setting.

[https://github.com/pulumi/pulumi-kubernetes/issues/2996](https://github.com/pulumi/pulumi-kubernetes/issues/2996)2996)

- **EXPERIMENTAL**: The `pulumi.com/waitFor` annotation was introduced
to allow
for custom readiness checks. This override Pulumi's own await logic for
the
    resource (however the `pulumi.com/skipAwait` annotation still takes
    precedence).

    The value of this annotation can take 3 forms:

    1.  A string prefixed with `jsonpath=` followed by a
[JSONPath](https://kubernetes.io/docs/reference/kubectl/jsonpath/)
        expression and an optional value.

        The JSONPath expression accepts the same syntax as
        `kubectl get -o jsonpath={...}`.

If a value is provided, the resource is considered ready when the
JSONPath expression evaluates to the same value. For example this
        resource expects its "phase" field to have a value of "Running":

            `pulumi.com/waitFor: "jsonpath={.status.phase}=Running"`

If a value is not provided, the resource will be considered ready when
any value exists at the given path, similar to `kubectl wait --for
jsonpath=...`. This resource will wait until it has a webhook configured
        with a CA bundle:

`pulumi.com/waitFor: "jsonpath={.webhooks[*].clientConfig.caBundle}"`

    2.  A string prefixed with `condition=` followed by the type of the
        condition and an optional status. This matches the behavior of
`kubectl wait --for=condition=...` and will wait until the resource has
a
matching condition. The expected status defaults to "True" if not
        specified.

            `pulumi.com/waitFor: "condition=Synced"`

            `pulumi.com/waitFor: "condition=Reconciling=False"`

    3.  A string containing a JSON array of multiple `jsonpath=` and
        `condition=` expressions.

`pulumi.com/waitFor: '["jsonpath={.foo}", "condition=Bar"]'`

- Pulumi will now emit logs for any Kubernetes "Warning" Events
associated with
resources being created, updated or
delete[https://github.com/pulumi/pulumi-kubernetes/pull/3135](https://github.com/pulumi/pulumi-kubernetes/pull/3135)ull/3135/files)

##### Fixed

- The `immutable` field is now respected for `ConfigMaps` when the
provider is configured with `enableConfigMapMutable`.

[https://github.com/pulumi/pulumi-kubernetes/issues/3181](https://github.com/pulumi/pulumi-kubernetes/issues/3181)3181)

- Fixed a panic that could occur during deletion.
([https://github.com/pulumi/pulumi-kubernetes/issues/3157](https://github.com/pulumi/pulumi-kubernetes/issues/3157))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://github.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC43Ny4wIiwidXBkYXRlZEluVmVyIjoiMzguNzcuNiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsidHlwZS9taW5vciJdfQ==-->

Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
lumiere-bot bot referenced this pull request in coolguy1771/home-ops Sep 16, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@pulumi/kubernetes](https://pulumi.com)
([source](https://github.com/pulumi/pulumi-kubernetes)) |
dependencies | minor | [`4.17.1` ->
`4.18.1`](https://renovatebot.com/diffs/npm/@pulumi%2fkubernetes/4.17.1/4.18.1)
|

---

### Release Notes

<details>
<summary>pulumi/pulumi-kubernetes (@&#8203;pulumi/kubernetes)</summary>

###
[`v4.18.1`](https://github.com/pulumi/pulumi-kubernetes/blob/HEAD/CHANGELOG.md#4181-September-13-2024)

[Compare
Source](https://github.com/pulumi/pulumi-kubernetes/compare/v4.18.0...v4.18.1)

##### Added

- Schemagen is now a library that can be consumed by other packages.
([https://github.com/pulumi/pulumi-kubernetes/pull/3187](https://github.com/pulumi/pulumi-kubernetes/pull/3187))

##### Changed

- Updated beta Kubernetes client libraries to stable v1.31 release.
([https://github.com/pulumi/pulumi-kubernetes/pull/3196](https://github.com/pulumi/pulumi-kubernetes/pull/3196))

###
[`v4.18.0`](https://github.com/pulumi/pulumi-kubernetes/blob/HEAD/CHANGELOG.md#4180-September-3-2024)

[Compare
Source](https://github.com/pulumi/pulumi-kubernetes/compare/v4.17.1...v4.18.0)

##### Added

- The new `enableSecretMutable` provider configuration option treats
changes to
    `Secrets` as updates instead of replacements (similar to the
    `enableConfigMapMutable` option).

The default replacement behavior can be preserved for a particular
`Secret`
    by setting its `immutable` field to `true`.

[https://github.com/pulumi/pulumi-kubernetes/issues/2291](https://github.com/pulumi/pulumi-kubernetes/issues/2291)2291)

**Note:** These options (`enableSecretMutable` and
`enableConfigMapMutable`)
may become the default behavior in a future v5 release of the provider.
Programs that depend on the replacement of `Secrets` and `ConfigMaps`
(e.g.
to trigger updates for downstream dependencies like `Deployments`) are
    recommended to explicitly specify `immutable: true`.

- A warning is now emitted if an object has finalizers which might be
blocking

deletio[https://github.com/pulumi/pulumi-kubernetes/issues/1418](https://github.com/pulumi/pulumi-kubernetes/issues/1418)1418)

- **EXPERIMENTAL**: Generic await logic is now available as an opt-in
feature.
Running a program with `PULUMI_K8S_AWAIT_ALL=true` will now cause Pulumi
to
    await readiness for *all* resources, including custom resources.

Generic readiness is determined according to some well-known conventions
(like
the "Ready" condition) as determined by
[cli-utils](https://github.com/kubernetes-sigs/cli-utils/tree/master/pkg/kstatus).

Pulumi's current behavior, without this feature enabled, is to assume
some
resources are immediately available, which can cause downstream
resources to
    fail.

    Existing readiness logic is unaffected by this setting.

[https://github.com/pulumi/pulumi-kubernetes/issues/2996](https://github.com/pulumi/pulumi-kubernetes/issues/2996)2996)

- **EXPERIMENTAL**: The `pulumi.com/waitFor` annotation was introduced
to allow
for custom readiness checks. This override Pulumi's own await logic for
the
    resource (however the `pulumi.com/skipAwait` annotation still takes
    precedence).

    The value of this annotation can take 3 forms:

    1.  A string prefixed with `jsonpath=` followed by a
[JSONPath](https://kubernetes.io/docs/reference/kubectl/jsonpath/)
        expression and an optional value.

        The JSONPath expression accepts the same syntax as
        `kubectl get -o jsonpath={...}`.

If a value is provided, the resource is considered ready when the
JSONPath expression evaluates to the same value. For example this
        resource expects its "phase" field to have a value of "Running":

            `pulumi.com/waitFor: "jsonpath={.status.phase}=Running"`

If a value is not provided, the resource will be considered ready when
any value exists at the given path, similar to `kubectl wait --for
jsonpath=...`. This resource will wait until it has a webhook configured
        with a CA bundle:

`pulumi.com/waitFor: "jsonpath={.webhooks[*].clientConfig.caBundle}"`

    2.  A string prefixed with `condition=` followed by the type of the
        condition and an optional status. This matches the behavior of
`kubectl wait --for=condition=...` and will wait until the resource has
a
matching condition. The expected status defaults to "True" if not
        specified.

            `pulumi.com/waitFor: "condition=Synced"`

            `pulumi.com/waitFor: "condition=Reconciling=False"`

    3.  A string containing a JSON array of multiple `jsonpath=` and
        `condition=` expressions.

`pulumi.com/waitFor: '["jsonpath={.foo}", "condition=Bar"]'`

- Pulumi will now emit logs for any Kubernetes "Warning" Events
associated with
resources being created, updated or
delete[https://github.com/pulumi/pulumi-kubernetes/pull/3135](https://github.com/pulumi/pulumi-kubernetes/pull/3135)ull/3135/files)

##### Fixed

- The `immutable` field is now respected for `ConfigMaps` when the
provider is configured with `enableConfigMapMutable`.

[https://github.com/pulumi/pulumi-kubernetes/issues/3181](https://github.com/pulumi/pulumi-kubernetes/issues/3181)3181)

- Fixed a panic that could occur during deletion.
([https://github.com/pulumi/pulumi-kubernetes/issues/3157](https://github.com/pulumi/pulumi-kubernetes/issues/3157))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://github.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC43Ny4wIiwidXBkYXRlZEluVmVyIjoiMzguNzcuNiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsidHlwZS9taW5vciJdfQ==-->

Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v4.18.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
3 participants