crossplane / crossplane/upjet

Remove immutable fields from initProvider

Open
#384 4 comments 0 reactions 0 assignees View on GitHub
enhancement v2
Dominant language
Go
Stars
481
Forks
131
Avg merge
2d 1h
Merged PRs (30d)
11

Description

### What problem are you facing?

In provider-upjet-aws, there are many resources which use the `config.IdentifierFromProvider` external name config, which doesn't set any identifier fields. However, many of these resources do have fields which serve as an identifier, in that they are part of the terraform id. It would provide a better user experience to update the external name configuration to construct the terraform id from the parameters, so that users can specify the desired `spec` and not have to worry about whether crossplane is going to create or update it, as opposed to the current behavior where often (but not always) users have to set the external-name annotation to the right format for that resource `Kind`, which is generally not documented anywhere.

When opening PRs that make these sorts of improvements to an external name configuration, the result is technically a breaking schema change, because it removes fields from `spec.initProvider`, and there's (understandably!) resistance to releasing such changes in a normal minor release.

While the terraform schema doesn't provide any specific indication that certain fields are part of the terraform id, it does indicate immutable fields with `ForceNew: true` (at least in the TF SDKv2 form of the schema), and identifier fields are almost always immutable.

### How could Upjet help solve your problem?

If we removed immutable fields from `initProvider` (based on `ForceNew: true`), then we would substantially reduce the size of our schemas, and allow external name config improvements to be non-breaking, at the cost of removing a feature it makes no sense to use.

In provider-aws, the result of applying this change locally was a decrease of about 60,000 lines of generated code in the CRDs, and another 40,000 lines in the `apis` folder.

### How can we identify an immutable field?

For schemas defined using the TF SDKv2, there's a `ForceNew` parameter.

For schemas deserialized to the TF SDKv2 format from terraform-json, as far as I can tell there is no indication of which fields are immutable.

For Terraform Plugin Framework schemas, there is a `RequiresReplace` plan modifier. See https://github.com/hashicorp/terraform-provider-aws/blob/898c9b5a1d8958366b293dad02daa44e24e360ef/internal/service/cognitoidp/user_pool_client.go#L237-L241 for an example.

### Breaking change

This would be a breaking change. I can think of two ways to release it.
1. Because it would be very strange for someone to be setting an immutable field in spec.initProvider instead of spec.forProvider, we just make the change, bump the major version of the provider (and probably also upjet), and mention it in the release notes.
2. Write some kind of generic conversion webhook (or code to generate one?) that migrates fields present in `v1beta1.initProvider` and missing from `v1beta2.initProvider` by setting the corresponding value in `v1beta2.forProvider` I'd really prefer our conversion webhooks to be a bit more mature before we add one to nearly every resource, with issues like #368 and #369 addressed.

Contributor guide

Open the contributing guide

Research direction

Start by tracing Upjet's schema generation and initProvider handling, then compare Terraform SDKv2 ForceNew, terraform-json schemas, and Plugin Framework RequiresReplace, including the referenced provider-aws example at internal/service/cognitoidp/user_pool_client.go. Review issues #368 and #369 before choosing between removing immutable fields and generating conversion webhooks; done means the chosen approach handles the supported schema formats and its breaking-change migration path.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.