Convert all singleton lists in the MR APIs to embedded objects #508

ulucinar · 2024-04-20T09:46:58Z

Description of your changes

Depends on: crossplane/upjet#387, crossplane/upjet#400

Terraform configuration blocks, even if they have a MaxItems constraint of 1, are (almost) always generated as lists. We
now generate the lists with a MaxItems constraint of 1 as embedded objects in our MR APIs.

This also helps when updating or patching via SSA the (previously list) objects. The merging strategy implemented by SSA requires configuration for associative lists and converting the singleton lists into embedded objects removes the configuration need.

The provider generates the converted embedded objects in the new v1beta2 versions of the CRD APIs and registers upjet's identity converter and the singleton list converters to be invoked, in chain, by the CRD API conversion webhooks. This implies that the v1beta1 versions stay intact, and old clients not aware of the new APIs should continue functioning as before (backwards-compatibility). Any clients willing to use the embedded objects-based APIs should be updated to use the v1beta2 versions of the CRD APIs.

If a resource contains no singleton lists, then the v1beta2 version is not generated for it.

We had to configure field name overrides for HTTP{S}HealthCheck.compute and Version.dialogflowcx so that the correspnding type names for the spec.forProvider, spec.initParameter and state.atProvider fields are not duplicated with the introduction of v1beta2 packages.

We have also removed the SSA merge strategy configurations from the v1beta2 versions of Cluster.container & NodePool.container resources because the related fields are no longer lists and have been converted to embedded objects in their v1beta2 versions.

I have:

Read and followed Crossplane's contribution process.
Run make reviewable to ensure this PR is ready for review.
Added backport release-x.y labels to auto-backport this PR if necessary.
New versions for the converted CRD APIs
Replace config.Resource.References[*].Type with config.Resource.References[*].TerraformName
Adjust the manually authored example manifests: With Add example manifests for v1beta2 version #528
API Conversion functions (invoked by the Conversion webhooks) to convert between the new & old API versions

How has this code been tested

A successful uptest run for the 'NodePool.container` resource with the API converters is registered here.
Another run with GKE back plans is here.

Also via the following uptest runs:

ulucinar · 2024-04-20T10:29:23Z

/test-examples="examples/container/v1beta2/nodepool.yaml"

ulucinar · 2024-04-20T20:12:43Z

/test-examples="examples/container/v1beta2/nodepool.yaml"

ulucinar · 2024-04-20T20:35:27Z

/test-examples="examples/container/v1beta2/nodepool.yaml"

ulucinar · 2024-04-21T00:25:25Z

/test-examples="examples/container/v1beta2/nodepool.yaml"

ulucinar · 2024-04-21T18:31:27Z

/test-examples="examples/container/v1beta2/nodepool.yaml"

ulucinar · 2024-04-21T19:07:39Z

/test-examples="examples/container/v1beta2/nodepool.yaml"

ulucinar · 2024-04-21T19:50:48Z

/test-examples="examples/container/v1beta2/nodepool.yaml"

ulucinar · 2024-04-21T20:53:24Z

/test-examples="examples/gke/v1beta2/backupbackupplan.yaml"

ulucinar · 2024-04-21T21:08:44Z

/test-examples="examples/gke/v1beta2/backupbackupplan.yaml"

ulucinar · 2024-04-22T07:31:31Z

/test-examples="examples/gke/v1beta2/backupbackupplan.yaml"

ulucinar · 2024-04-22T08:03:54Z

/test-examples="examples/gke/v1beta2/backupbackupplan.yaml"

ulucinar · 2024-04-22T08:10:29Z

/test-examples="examples/container/v1beta2/nodepool.yaml"

ulucinar · 2024-04-22T10:15:38Z

Hi @JonathanO,
This PR converts the singleton lists in the provider API to embedded objects together with the converter functions invoked by the conversion webhooks to convert between the (old) v1beta1 APIs (where the APIs are singleton lists) and the new v1beta2 APIs (where the APIs are embedded objects).

We've done some preliminary tests using the {Cluster,NodePool}.eks, ServiceAccount.cloudplatform and BackupBackupPlan.gke resources using uptest. Also built resource providers using the v1.1.0-db914f4f8174cbf845f9df9895b9826ed26d2e6c tag.

It would be great if you could provide us some feedback using these providers:

Family config provider: xpkg.upbound.io/upbound/provider-family-gcp:v1.1.0-db914f4f8174cbf845f9df9895b9826ed26d2e6c
Monolith: xpkg.upbound.io/upbound/provider-gcp:v1.1.0-db914f4f8174cbf845f9df9895b9826ed26d2e6c

As discussed above, the resource providers (except for the new containerattached provider) have also the same tag v1.1.0-db914f4f8174cbf845f9df9895b9826ed26d2e6c, i.e., xpkg.upbound.io/upbound/provider-gcp-container:v1.1.0-db914f4f8174cbf845f9df9895b9826ed26d2e6c.

This is a relatively large & involved change (both the code generation pipelines and the provider runtime are affected and we also attempt to hide the breaking API changes from the v1beta1 clients). I would really appreciate if you could give these providers a try in your test environment if you can spare some time.

Not all v1beta2 example manifests have been updated to reflect the API changes. However, if you'd like to check here are some manifests updated to facilitate testing:

examples/gke/v1beta2/backupbackupplan.yaml
examples/container/v1beta2/{cluster,nodepool}.yaml
examples/gke/v1beta2/backupbackupplan.yaml

P.S. There's also a counterpart of this PR in crossplane-contrib/provider-upjet-azuread.

turkenf · 2024-04-22T14:32:10Z

/test-examples="examples/bigquery/v1beta1/routine.yaml"

turkenf · 2024-04-22T14:33:12Z

/test-examples="examples/compute/v1beta1/nodegroup.yaml"

turkenf · 2024-04-22T14:34:29Z

/test-examples="examples/dns/v1beta1/policy.yaml"

turkenf · 2024-04-22T14:35:10Z

/test-examples="examples/sql/v1beta1/user.yaml"

sergenyalcin · 2024-05-10T14:36:42Z

/test-examples="examples/compute/v1beta2/firewall.yaml"

sergenyalcin · 2024-05-10T14:36:47Z

/test-examples="examples/compute/v1beta2/instance.yaml"

sergenyalcin · 2024-05-10T14:36:55Z

/test-examples="examples/compute/v1beta2/router.yaml"

sergenyalcin · 2024-05-10T14:37:01Z

/test-examples="examples/pubsub/v1beta2/litetopic.yaml"

sergenyalcin · 2024-05-10T14:39:07Z

/test-examples="examples/cloudrun/v1beta2/v2service.yaml"

sergenyalcin · 2024-05-10T14:39:19Z

/test-examples="examples/logging/v1beta2/metric.yaml"

sergenyalcin · 2024-05-10T14:39:26Z

/test-examples="examples/monitoring/v1beta2/service.yaml"

sergenyalcin · 2024-05-10T14:39:36Z

/test-examples="examples/notebooks/v1beta2/environment.yaml"

sergenyalcin · 2024-05-10T14:39:54Z

/test-examples="examples/notebooks/v1beta2/instance.yaml"

sergenyalcin · 2024-05-10T14:40:08Z

/test-examples="examples/secretmanager/v1beta2/secret.yaml"

sergenyalcin · 2024-05-10T14:40:19Z

/test-examples="examples/sql/v1beta2/user.yaml"

sergenyalcin · 2024-05-10T14:40:25Z

/test-examples="examples/dns/v1beta2/policy.yaml"

sergenyalcin · 2024-05-10T14:40:34Z

/test-examples="examples/container/v1beta2/cluster.yaml"

sergenyalcin · 2024-05-10T14:40:45Z

/test-examples="examples/storage/v1beta2/bucket.yaml"

sergenyalcin · 2024-05-10T14:41:00Z

/test-examples="examples/spanner/v1beta2/database.yaml"

sergenyalcin · 2024-05-10T14:41:08Z

/test-examples="examples/redis/v1beta2/instance.yaml"

sergenyalcin · 2024-05-10T14:50:24Z

/test-examples="examples/compute/v1beta2/securitypolicy.yaml"

sergenyalcin · 2024-05-10T14:50:31Z

/test-examples="examples/compute/v1beta2/urlmap.yaml"

ulucinar · 2024-05-14T12:24:50Z

/test-examples="examples/container/v1beta2/nodepool.yaml"

ulucinar · 2024-05-14T12:25:35Z

/test-examples="examples/compute/v1beta2/image.yaml"

sergenyalcin

Thanks @ulucinar for your amazing efforts on this PR, LGTM! I think after removing the replace in go.mod and pointing to uptest latest, we will be ready to merge.

sergenyalcin · 2024-05-16T12:16:22Z

Makefile

+UPTEST_LOCAL_VERSION = v0.12.0-9.gac371c9
+UPTEST_LOCAL_CHANNEL = main


nit: We may consider using after releaseing the uptest with the import fix. I am not proposing to do this in this PR. I want to only note this.

Yes, agreed. Let's revisit this once upbound/build#255 is merged. Thanks for the note.

sergenyalcin · 2024-05-16T12:17:58Z

examples-generated/accesscontextmanager/v1beta2/accesslevel.yaml

It seems that we need to fix the example generation pipeline to produce examples via using the new generated APIs.

Yes, this is another item we need to take care of. Let's merge this PR, and address the missing example manifests with a separate PR.

…e.PreviousVersions for the prevention of the type name conflicts in the generated multi-versioned CRDs. - config.Resource.OverrideFieldNames is being deprecated in favor of config.Resource.PreviousVersions. - Bump upjet to commit 92d1af84d242. Signed-off-by: Alper Rifat Ulucinar <[email protected]>

ulucinar · 2024-05-16T12:26:50Z

/test-examples="examples/container/v1beta2/nodepool.yaml"

ulucinar · 2024-05-16T12:26:58Z

/test-examples="examples/compute/v1beta2/image.yaml"

ulucinar · 2024-05-16T12:57:00Z

We've decided to merge this PR without the example manifests and @sergenyalcin is planning to open a follow-up PR to introduce the missing v1beta2 example manifests.

ulucinar requested review from sergenyalcin and turkenf as code owners April 20, 2024 09:46

ulucinar marked this pull request as draft April 20, 2024 09:47

ulucinar force-pushed the embed-singleton branch from a4b0953 to 20946ec Compare April 20, 2024 09:48

ulucinar force-pushed the embed-singleton branch from 5653360 to a26ceac Compare April 20, 2024 20:03

ulucinar force-pushed the embed-singleton branch from 9158ca4 to 0aeb502 Compare April 20, 2024 20:35

ulucinar force-pushed the embed-singleton branch 2 times, most recently from 0ec1128 to 2242799 Compare April 21, 2024 00:24

ulucinar force-pushed the embed-singleton branch from 6bfc693 to 94d21e8 Compare April 21, 2024 19:50

ulucinar force-pushed the embed-singleton branch 2 times, most recently from 535e79b to 0d7096c Compare April 21, 2024 20:52

ulucinar force-pushed the embed-singleton branch from 0d7096c to 8cfdcfa Compare April 22, 2024 07:29

ulucinar force-pushed the embed-singleton branch from 8cfdcfa to db914f4 Compare April 22, 2024 07:55

ulucinar mentioned this pull request May 14, 2024

Add config.Resource.PreviousVersions to specify the previous versions of an MR API crossplane/upjet#402

Merged

3 tasks

sergenyalcin approved these changes May 16, 2024

View reviewed changes

ulucinar force-pushed the embed-singleton branch from f425b8d to 59d870b Compare May 16, 2024 12:23

ulucinar merged commit e623a2b into crossplane-contrib:main May 16, 2024
10 of 11 checks passed

ulucinar deleted the embed-singleton branch May 16, 2024 12:57

sergenyalcin mentioned this pull request May 16, 2024

Make MaxItems values consistent between JSON schema and provider schema #527

Merged

3 tasks

		UPTEST_LOCAL_VERSION = v0.12.0-9.gac371c9
		UPTEST_LOCAL_CHANNEL = main

Convert all singleton lists in the MR APIs to embedded objects #508

Convert all singleton lists in the MR APIs to embedded objects #508

Conversation

ulucinar commented Apr 20, 2024 • edited Loading

Description of your changes

How has this code been tested

ulucinar commented Apr 20, 2024

ulucinar commented Apr 20, 2024

ulucinar commented Apr 20, 2024

ulucinar commented Apr 21, 2024

ulucinar commented Apr 21, 2024

ulucinar commented Apr 21, 2024

ulucinar commented Apr 21, 2024

ulucinar commented Apr 21, 2024

ulucinar commented Apr 21, 2024

ulucinar commented Apr 22, 2024

ulucinar commented Apr 22, 2024

ulucinar commented Apr 22, 2024

ulucinar commented Apr 22, 2024 • edited Loading

turkenf commented Apr 22, 2024

turkenf commented Apr 22, 2024

turkenf commented Apr 22, 2024

turkenf commented Apr 22, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

sergenyalcin commented May 10, 2024

ulucinar commented May 14, 2024

ulucinar commented May 14, 2024

sergenyalcin left a comment • edited Loading

Choose a reason for hiding this comment

sergenyalcin May 16, 2024

Choose a reason for hiding this comment

ulucinar May 16, 2024

Choose a reason for hiding this comment

sergenyalcin May 16, 2024

Choose a reason for hiding this comment

ulucinar May 16, 2024

Choose a reason for hiding this comment

ulucinar commented May 16, 2024

ulucinar commented May 16, 2024

ulucinar commented May 16, 2024

ulucinar commented Apr 20, 2024 •

edited

Loading

ulucinar commented Apr 22, 2024 •

edited

Loading

sergenyalcin left a comment •

edited

Loading