feat: Updating chart to support CRD struct, Fixes #419 #420

rshade · 2025-04-19T18:42:51Z

This pull request introduces significant changes to the Pulumi Kubernetes Cert-Manager provider, focusing on replacing the deprecated installCRDs field with a new crds configuration object. The changes improve customization and default behavior for CRD installation while maintaining backward compatibility. Additionally, test coverage and dependencies have been updated to support these changes.

Key Changes:

CRD Configuration Enhancements:

Replaced the installCRDs field with a new crds object, which includes enabled and keep properties for more granular control over CRD installation and retention during chart uninstall. (examples/simple-cert-manager-ts/index.ts, provider/pkg/provider/chart.go, provider/cmd/pulumi-resource-kubernetes-cert-manager/schema.json, sdk/dotnet/CertManager.cs, sdk/go/kubernetes-cert-manager/certManager.go, sdk/go/kubernetes-cert-manager/pulumiTypes.go) [1] [2] [3] [4] [5] [6] [7]
Added logic to handle default values for the crds object when installCRDs is still used, ensuring backward compatibility. (provider/pkg/provider/chart.go, provider/pkg/provider/provider.go) [1] [2]

Testing Improvements:

Introduced new unit tests to verify default behavior and custom configurations for the crds object. (provider/pkg/provider/provider_test.go)

Dependency Updates:

Updated go.mod to include new dependencies for testing (stretchr/testify, davecgh/go-spew, pmezard/go-difflib) and removed an unused dependency (stretchr/objx). (provider/go.mod) [1] [2] [3] [4]

SDK Updates:

Updated the .NET and Go SDKs to include the new crds configuration object, with appropriate type definitions and defaults. (sdk/dotnet/Inputs/CertManagerCrdsArgs.cs, sdk/go/kubernetes-cert-manager/pulumiTypes.go) [1] [2] [3] [4]

These changes enhance the flexibility and maintainability of the Cert-Manager provider while ensuring compatibility with existing configurations.

github-actions · 2025-04-19T18:43:45Z

Does the PR have any schema changes?

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

Copilot

Pull Request Overview

This pull request adds support for the new CRDs configuration in the cert-manager chart while maintaining backward compatibility with the deprecated installCRDs option. Key changes include updating the input structures and APIs in Python, Node.js, Java, Go, and .NET, modifying the schema to include the new deleteOnDestroy field, and implementing CRD deletion functionality in the provider code.

Reviewed Changes

Copilot reviewed 15 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
sdk/python/pulumi_kubernetes_cert_manager/cert_manager.py	Added the new crds parameter and corresponding getter/setter, plus updated the deprecation message for install_crds.
sdk/python/pulumi_kubernetes_cert_manager/_inputs.py	Introduced CertManagerCrdsArgs and CertManagerCrdsArgsDict for CRD configuration.
sdk/nodejs/types/input.ts, sdk/nodejs/certManager.ts	Added the new crds field in the types and component class with updated deprecation handling for installCRDs.
sdk/java/...	Updated CertManagerCrdsArgs and CertManagerArgs to incorporate the new CRD configuration.
sdk/go/...	Added CertManagerCrds types and updated certManagerArgs to include the new crds property.
sdk/dotnet/...	Added new input type CertManagerCrdsArgs and updated CertManager to handle the new crds property.
provider/pkg/provider/provider.go, chart.go	Modified provider code to handle backward compatibility and implement CRD deletion using the deleteOnDestroy flag.
examples/	Updated the TypeScript example to showcase the usage of the new CRDs configuration.

Files not reviewed (3)

examples/simple-cert-manager-with-crds-ts/package.json: Language not supported
examples/simple-cert-manager-with-crds-ts/tsconfig.json: Language not supported
provider/cmd/pulumi-resource-kubernetes-cert-manager/schema.json: Language not supported

provider/pkg/provider/provider.go

Copilot

Pull Request Overview

This PR updates the CertManager component to support a new CRDs structure while ensuring backward compatibility with the deprecated installCRDs option. Key changes include:

Introducing the new crds parameter and its getter/setter implementations across Python, Node.js, Java, Go, and .NET SDKs.
Updating types, input definitions, and examples (in README) to reflect the new CRDs struct.
Maintaining backward compatibility by keeping the deprecated installCRDs option in the APIs.

Reviewed Changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
sdk/python/pulumi_kubernetes_cert_manager/cert_manager.py	Added new crds parameter and getter/setter for CRDs support.
sdk/python/pulumi_kubernetes_cert_manager/_inputs.py	Introduced CertManagerCrdsArgs and Dict types for CRDs.
sdk/nodejs/types/input.ts	Added CertManagerCrdsArgs interface to support CRDs configuration.
sdk/nodejs/certManager.ts	Integrated crds field and updated deprecated installCRDs message.
sdk/java/src/main/java/com/pulumi/kubernetescertmanager/inputs/CertManagerCrdsArgs.java & CertManagerArgs.java	Created new Java types and updated builder methods for CRDs.
sdk/go/kubernetes-cert-manager/pulumiTypes.go & certManager.go	Extended Go types and structs to include CRDs support.
sdk/dotnet/Inputs/CertManagerCrdsArgs.cs & CertManager.cs	Added .NET inputs and updated resource Args for CRDs.
provider/pkg/provider/chart.go	Updated chart arguments to include the new crds field.
README.md	Updated examples and installation instructions to showcase new CRDs usage.

Files not reviewed (1)

provider/cmd/pulumi-resource-kubernetes-cert-manager/schema.json: Language not supported

Comments suppressed due to low confidence (3)

sdk/go/kubernetes-cert-manager/certManager.go:65

Consider adding logic or documentation on how conflicts between the new 'Crds' field and the deprecated 'InstallCRDs' field are resolved to ensure predictable behavior.

InstallCRDs  *bool                `pulumi:"installCRDs"`

sdk/java/src/main/java/com/pulumi/kubernetescertmanager/CertManagerArgs.java:218

[nitpick] Verify that the deprecation messaging and naming for 'installCRDs' remain consistent with other SDK implementations to avoid confusion.

@Import(name="installCRDs")

README.md:18

Consider adding tests to verify the new CRDs functionality and its backward compatibility with the deprecated 'installCRDs' option.

```bash

Copilot

Pull Request Overview

This pull request introduces CRD customization support for the Pulumi Kubernetes Cert-Manager provider by adding a new crds property to the schema and implementing default logic for its Keep field. Key changes include:

Adding a new crds property and the CertManagerCrds type in the chart.
Implementing default value logic for crds in the Construct function.
Enhancing tests, including unit tests for default behavior and custom value preservation, as well as updating dependencies.

Reviewed Changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated no comments.

File	Description
provider/pkg/provider/provider_test.go	Added unit tests for CRDs defaults and custom values; placeholder test for dynamic CRD adoption
provider/pkg/provider/provider.go	Updated default handling for crds in the Construct function
provider/pkg/provider/chart.go	Added new crds field in CertManagerArgs and defined CertManagerCrds type

Files not reviewed (2)

provider/cmd/pulumi-resource-kubernetes-cert-manager/schema.json: Language not supported
provider/go.mod: Language not supported

Comments suppressed due to low confidence (1)

provider/pkg/provider/provider_test.go:24

TestFindAndAdoptCertManagerCRDs is currently a placeholder with no assertions. Consider adding tests to validate the dynamic CRD finding and adoption functionality.

func TestFindAndAdoptCertManagerCRDs(t *testing.T) {

Copilot

Pull Request Overview

This PR updates the Cert-Manager provider to replace the deprecated installCRDs field with a more flexible crds configuration object. Key changes include updating the schema across multiple SDKs (Python, Node.js, Java, Go, .NET), enhancing defaulting logic for CRD installation, and adding new unit tests to verify default and custom CRDs behavior.

Reviewed Changes

Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
sdk/python/*	Added crds parameter and its getter/setter to support the new configuration in Python SDK
sdk/nodejs/*	Updated input and output types and added defaulting helper for crds in the Node.js SDK
sdk/java/*	Introduced new CertManagerCrdsArgs class for Java with proper defaults
sdk/go/*	Implemented CertManagerCrds types and defaulting logic along with updates to constructor logic
sdk/dotnet/*	Added the new crds property and its defaulting in the .NET SDK
provider/pkg/provider/*	Updated provider construction and chart logic to handle crds defaults and backward compatibility
examples/simple-cert-manager-ts/index.ts	Modified example to use new crds field

Files not reviewed (2)

provider/cmd/pulumi-resource-kubernetes-cert-manager/schema.json: Language not supported
provider/go.mod: Language not supported

Comments suppressed due to low confidence (1)

provider/pkg/provider/chart.go:96

The function 'R()' currently mixes defaulting logic for the 'crds' field with the return of helmOptions. Consider moving the defaulting logic for 'crds' to a dedicated initialization routine or helper method to improve code clarity and maintainability.

func (args *CertManagerArgs) R() **helmbase.ReleaseType {

provider/pkg/provider/provider.go

aureq · 2025-05-29T02:13:29Z

@rshade is there any update regarding this PR please? One of our customer is in need for this.

Copilot

Pull Request Overview

This PR replaces the deprecated installCRDs boolean with a structured crds object across the provider and all SDKs, adds backward‐compatibility logic, updates tests, and refreshes documentation.

Introduces crds object (enabled, keep) and defaulting logic in provider code
Updates NodeJS, Go, Java, and .NET SDKs to support crds
Adds and improves tests; updates examples and README

Reviewed Changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
sdk/nodejs/types/output.ts	Added import for utilities
sdk/nodejs/types/input.ts	Added `CertManagerCrdsArgs` and default helper
sdk/nodejs/certManager.ts	Wired `crds` into resourceInputs
sdk/java/.../inputs/CertManagerCrdsArgs.java	Generated Java args type for `crds`
sdk/java/.../CertManagerArgs.java	Plumbed `crds` into Java `CertManagerArgs`
sdk/java/README.md	Documented `crds` usage
sdk/go/.../pulumiTypes.go	Added Go input/output types and defaults
sdk/go/.../certManager.go	Applied `Defaults()` in Go SDK constructor
sdk/go.mod	Bumped k8s SDK version
sdk/dotnet/Inputs/CertManagerCrdsArgs.cs	Generated .NET `CertManagerCrdsArgs`
sdk/dotnet/CertManager.cs	Added `.Crds` property to .NET args
provider/pkg/provider/provider_test.go	New unit tests for CRDs defaults and legacy
provider/pkg/provider/provider.go	Added CRDs defaulting & legacy handling
provider/pkg/provider/chart.go	Enhanced `R()` to configure `crds`
provider/go.mod	Added test dependencies
provider/cmd/.../schema.json	Extended schema with `crds` type
examples/simple-cert-manager-ts/index.ts	Migrated example to `crds` field
examples/examples_ts_test.go	Added TypeScript CRDs uninstall test
README.md	Documented `crds` configuration
.pulumi.version	Updated Pulumi CLI version

Comments suppressed due to low confidence (2)

provider/cmd/pulumi-resource-kubernetes-cert-manager/schema.json:891

The schema for crds.enabled lacks an explicit default; adding "default": false will align it with the keep field and SDK defaults.

"type": "boolean"

provider/pkg/provider/provider_test.go:24

This test is a placeholder without any assertions or logic; either implement the intended behavior checks or remove the empty test.

func TestFindAndAdoptCertManagerCRDs(t *testing.T) {

sdk/nodejs/types/output.ts

sdk/nodejs/types/input.ts

sdk/go/kubernetes-cert-manager/pulumiTypes.go

Here's a summary of the changes made to implement the solution for GitHub issue 419: 1. Added the crds structure to the provider schema: - Introduced a new CertManagerCrds struct in chart.go with two fields: - Enabled (bool): Controls installation of CRDs - Keep (bool): Controls if CRDs should remain after chart uninstall 2. Set crds.keep=false as the default: - Modified the Construct function in provider.go to set a default value of false for Crds.Keep - This ensures that by default CRDs will be removed when the chart is uninstalled 3. Updated the schema.json file: - Added the new CertManagerCrds type definition to schema.json - Added the crds field to the CertManager input properties - Set the default value for keep to false in the schema 4. Added tests: - Created unit tests for the new default value behavior - Verified that the crds.keep property defaults to false - Verified that user-provided custom values are respected These changes address GitHub issue 419 by: 1. Replacing the deprecated installCRDs parameter with the newer crds.enabled and crds.keep parameters 2. Setting crds.keep=false by default to provide better cleanup behavior when removing Cert Manager 3. Ensuring backward compatibility by maintaining support for the installCRDs parameter The implementation aligns with the Helm chart's evolution and gives users more flexibility in managing the Cert Manager CRDs This will fix #419

…tioning from using installCRDs: boolean to using a structured object crds: { enabled: boolean, keep: boolean }. 2. We implemented fixes in the provider code to handle both formats compatibly: - In chart.go, we added logic to set crds.enabled when installCRDs is true - In provider.go, we added similar logic to ensure compatibility 3. The TypeScript test passed with our changes, indicating that the issue has been fixed. Let's create a more comprehensive solution by updating the other language SDKs to handle both the old and new approaches. Since we already saw the Go, Python, and .NET examples are still using the old installCRDs approach, there's no urgent need to update them right now. In summary: 1. We fixed the provider code to handle both installCRDs and the new crds object properly 2. The preview test now passes with our changes 3. The full deployment test may need more time, but the code changes we made should resolve the compatibility issue

rshade added the blocked The issue cannot be resolved without 3rd party action. label Apr 19, 2025

rshade requested a review from Copilot April 19, 2025 18:43

Copilot AI reviewed Apr 19, 2025

View reviewed changes

provider/pkg/provider/provider.go Outdated Show resolved Hide resolved

rshade force-pushed the crds.enabled branch from fe04913 to 5a20c25 Compare April 19, 2025 19:03

rshade requested a review from Copilot April 19, 2025 19:03

Copilot AI reviewed Apr 19, 2025

View reviewed changes

rshade force-pushed the crds.enabled branch 6 times, most recently from 4a6c32a to fe11488 Compare April 22, 2025 11:12

rshade force-pushed the crds.enabled branch from fe11488 to 7be4033 Compare April 28, 2025 16:31

EronWright linked an issue Apr 28, 2025 that may be closed by this pull request

Use more flexible options to manage Cert Manager's CRDs #419

Open

rshade force-pushed the crds.enabled branch 2 times, most recently from b6581a6 to 1a5267f Compare April 29, 2025 15:11

rshade requested a review from Copilot April 29, 2025 15:12

Copilot AI reviewed Apr 29, 2025

View reviewed changes

rshade requested a review from Copilot April 29, 2025 16:34

Copilot AI reviewed Apr 29, 2025

View reviewed changes

provider/pkg/provider/provider.go Outdated Show resolved Hide resolved

rshade force-pushed the crds.enabled branch from 28e35f8 to c252cb3 Compare April 29, 2025 16:45

rshade removed the blocked The issue cannot be resolved without 3rd party action. label Apr 29, 2025

rshade requested a review from Copilot April 29, 2025 16:56

This comment was marked as outdated.

Sign in to view

rshade requested a review from EronWright April 29, 2025 16:56

rshade force-pushed the crds.enabled branch 2 times, most recently from e7dc4fc to f50da14 Compare May 8, 2025 18:45

rshade force-pushed the crds.enabled branch 2 times, most recently from 6484f42 to 9886595 Compare May 13, 2025 21:23

rshade force-pushed the crds.enabled branch 2 times, most recently from a2cbb2c to 91249ea Compare May 20, 2025 13:51

rshade force-pushed the crds.enabled branch 5 times, most recently from 4bf9b40 to 3ad98a4 Compare June 5, 2025 16:15

rshade requested a review from Copilot June 5, 2025 16:55

Copilot AI reviewed Jun 5, 2025

View reviewed changes

sdk/nodejs/types/output.ts Show resolved Hide resolved

sdk/nodejs/types/input.ts Show resolved Hide resolved

sdk/nodejs/types/input.ts Show resolved Hide resolved

sdk/go/kubernetes-cert-manager/pulumiTypes.go Show resolved Hide resolved

rshade force-pushed the crds.enabled branch 2 times, most recently from f7ef25d to 9b5653e Compare June 9, 2025 16:28

rshade force-pushed the crds.enabled branch 2 times, most recently from 97304f9 to f9499fb Compare June 20, 2025 16:47

rshade force-pushed the crds.enabled branch from f9499fb to e1176ad Compare June 27, 2025 18:37

rshade force-pushed the crds.enabled branch from e1176ad to fdbcc89 Compare September 4, 2025 21:36

rshade added 2 commits September 4, 2025 16:46

rshade force-pushed the crds.enabled branch from fdbcc89 to 4f3654c Compare September 4, 2025 21:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Updating chart to support CRD struct, Fixes #419 #420

feat: Updating chart to support CRD struct, Fixes #419 #420

Uh oh!

rshade commented Apr 19, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Apr 19, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

aureq commented May 29, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat: Updating chart to support CRD struct, Fixes #419 #420

Are you sure you want to change the base?

feat: Updating chart to support CRD struct, Fixes #419 #420

Uh oh!

Conversation

rshade commented Apr 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Changes:

CRD Configuration Enhancements:

Testing Improvements:

Dependency Updates:

SDK Updates:

Uh oh!

github-actions bot commented Apr 19, 2025

Does the PR have any schema changes?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

aureq commented May 29, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rshade commented Apr 19, 2025 •

edited

Loading