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

ulucinar · 2024-04-17T14:30:34Z

Description of your changes

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 version for the converted CRD APIs
Adjust the manually authored example manifests
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 with the converted APIs is observed here: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8723779942

Examples from successful uptest runs with conversion Webhooks enabled:

Uptest runs for available uptestable resources:

v1beta1 resources

examples/administrativeunits/v1beta1/unit.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783425311
examples/app/v1beta1/roleassignment.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783436140
examples/applications/v1beta1/application.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783428288
examples/applications/v1beta1/certificate.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783429397
examples/applications/v1beta1/federatedidentitycredential.yaml https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783592783
examples/applications/v1beta1/password.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783850825
examples/applications/v1beta1/preauthorized.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783430121
examples/conditionalaccess/v1beta1/accesspolicy.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783431498
examples/conditionalaccess/v1beta1/location.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783432163
examples/directoryroles/v1beta1/customdirectoryrole.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783432941
examples/directoryroles/v1beta1/roleassignment.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8782162727
examples/groups/v1beta1/group.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783599499
examples/groups/v1beta1/member.yaml: Uptest failed in the delete step (testhooks/delete-member.sh: permission denied), manual test successful.
examples/policies/v1beta1/claimsmappingpolicy.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783602801
examples/serviceprincipaldelegated/v1beta1/permissiongrant.yaml: Uptest failed in the delete step (testhooks/delete-permissiongrant.sh: permission denied), manual test successful.
examples/serviceprincipals/v1beta1/certificate.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8787919126
examples/serviceprincipals/v1beta1/claimsmappingpolicyassignment.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8784604890
examples/serviceprincipals/v1beta1/password.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8785038521
examples/serviceprincipals/v1beta1/principal.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8784021673
examples/synchronization/v1beta1/job.yaml: Uptest failed in the delete step (testhooks/delete-secret.sh: permission denied), manual test successful.
examples/users/v1beta1/user.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783620282

v1beta2 ressources

examples/applications/v1beta2/application.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783430807
examples/conditionalaccess/v1beta2/accesspolicy.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783431498
examples/conditionalaccess/v1beta2/location.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783929325
examples/groups/v1beta2/group.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8783439239
examples/serviceprincipals/v1beta2/principal.yaml: https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8784864220

ulucinar · 2024-04-17T14:32:03Z

/test-examples="examples/conditionalaccess/accesspolicy.yaml"

ulucinar · 2024-04-17T16:33:31Z

/test-examples="examples/conditionalaccess/accesspolicy.yaml"

ulucinar · 2024-04-17T16:37:25Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

ulucinar · 2024-04-17T22:05:50Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

ulucinar · 2024-04-17T23:17:48Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

ulucinar · 2024-04-18T00:18:01Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

ulucinar · 2024-04-19T05:58:57Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

ulucinar · 2024-04-19T06:06:51Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

ulucinar · 2024-04-19T06:09:56Z

/test-examples="examples/conditionalaccess/v1beta2/location.yaml"

ulucinar · 2024-04-19T06:15:27Z

Hi @sergenyalcin,
Looks like the status subresource we are doing as part of the import tests (after scaling the provider deployment down) is now failing with the conversion webhooks:
https://github.com/crossplane-contrib/provider-upjet-azuread/actions/runs/8749171407/job/24010390302

I'll just disable the import tests for v1beta2 (non-storage version) for now. They should be running fine for v1beta1, which does not require a conversion when updated and I think it should be fine to update the status (of v1beta1) when the provider pod is down.

ulucinar · 2024-04-19T06:19:30Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

ulucinar · 2024-04-19T06:19:38Z

/test-examples="examples/conditionalaccess/v1beta2/location.yaml"

ulucinar · 2024-04-19T06:49:48Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

ulucinar · 2024-04-19T06:49:55Z

/test-examples="examples/conditionalaccess/v1beta2/location.yaml"

ulucinar · 2024-04-19T06:53:53Z

/test-examples="examples/conditionalaccess/v1beta1/accesspolicy.yaml"

ulucinar · 2024-04-19T08:15:37Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

ulucinar · 2024-04-19T08:15:44Z

/test-examples="examples/conditionalaccess/v1beta2/location.yaml"

ulucinar · 2024-04-19T08:59:43Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

ulucinar · 2024-04-19T09:04:19Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

ulucinar · 2024-04-19T09:07:53Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

turkenf · 2024-04-24T18:50:08Z

/test-examples="examples/applications/v1beta1/application.yaml"

turkenf · 2024-05-08T08:10:51Z

/test-examples="examples/groups/v1beta1/member.yaml"

turkenf · 2024-05-08T08:11:19Z

/test-examples="examples/serviceprincipaldelegated/v1beta1/permissiongrant.yaml"

turkenf · 2024-05-08T08:11:55Z

/test-examples="examples/synchronization/v1beta1/job.yaml"

- 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. Signed-off-by: Alper Rifat Ulucinar <[email protected]>

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

to facilitate downgrades Signed-off-by: Alper Rifat Ulucinar <[email protected]>

…bjects - Fix the example manifest examples/directoryroles/v1beta1/customdirectoryrole.yaml Signed-off-by: Alper Rifat Ulucinar <[email protected]>

…dded object API versions Signed-off-by: Alper Rifat Ulucinar <[email protected]>

…eta1 & v1beta2 versions of the CRDs that have converted singleton lists in their APIs and enable the conversion webhooks. - Use kustomize to inject the spec.conversion stanzas to the generated CRDs - Dependency version Bumps: kind -> v0.21.0, up -> v0.28.0, uptest -> v0.11.1 - Pin uxp version to 1.14.6-up.1 Signed-off-by: Alper Rifat Ulucinar <[email protected]>

…ion to set the webhook TLS certificates directory. - Duplicate GVKs in zz_register.go are removed. - Hubs and spokes are now generated from scratch at each "make generate". Signed-off-by: Alper Rifat Ulucinar <[email protected]>

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

ulucinar · 2024-05-08T10:50:35Z

/test-examples="examples/groups/v1beta1/member.yaml"

ulucinar · 2024-05-08T10:50:59Z

/test-examples="examples/serviceprincipaldelegated/v1beta1/permissiongrant.yaml"

ulucinar · 2024-05-08T10:51:12Z

/test-examples="examples/synchronization/v1beta1/job.yaml"

ulucinar · 2024-05-08T14:04:40Z

/test-examples="examples/groups/v1beta1/member.yaml"

ulucinar · 2024-05-08T14:06:21Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

sergenyalcin

Thanks @ulucinar LGTM!

- Bump upjet to commit 03a207b6416a - Bump uptest to version v0.12.0-9.gac371c9 Signed-off-by: Alper Rifat Ulucinar <[email protected]>

ulucinar · 2024-05-08T14:19:41Z

/test-examples="examples/conditionalaccess/v1beta2/accesspolicy.yaml"

ulucinar · 2024-05-08T14:19:57Z

/test-examples="examples/groups/v1beta1/member.yaml"

turkenf · 2024-05-16T16:26:55Z

Upgrade Test

Installed the package: xpkg.upbound.io/upbound/provider-azuread:v1.1.0
Applied the examples/conditionalaccess/v1beta1/accesspolicy.yaml
We can see the singleton lists in get -o yaml output:

    conditions:
    - applications:
      - includedApplications:
        - All

Edited the package with package: xpkg.upbound.io/upbound/provider-azuread:v1.2.0-rc.0.16.g14395d1

NAME               INSTALLED   HEALTHY   PACKAGE                                                            AGE
provider-azuread   True        True      xpkg.upbound.io/upbound/provider-azuread:v1.2.0-rc.0.16.g14395d1   11m

and we can see the following outputs from get -o yaml

apiVersion:

apiVersion: conditionalaccess.azuread.upbound.io/v1beta2

embedded objects:

   conditions:
     applications:
       includedApplications:
       - All

conditions:

 - lastTransitionTime: "2024-05-16T15:54:56Z"
   reason: Available
   status: "True"
   type: Ready
 - lastTransitionTime: "2024-05-16T15:54:49Z"
   reason: ReconcileSuccess
   status: "True"
   type: Synced
 - lastTransitionTime: "2024-05-16T15:54:53Z"
   reason: Success
   status: "True"
   type: LastAsyncOperation
 - lastTransitionTime: "2024-05-16T16:03:17Z"
   reason: UpToDate
   status: "True"
   type: Test

Upgrade test successful

Downgrade Test

Installed the package: xpkg.upbound.io/upbound/provider-azuread:v1.2.0-rc.0.16.g14395d1
Applied the examples/conditionalaccess/v1beta2/accesspolicy.yaml
We can see the embedded objects in get -o yaml output:

    conditions:
      applications:
        includedApplications:
        - All

Edited the package with package: xpkg.upbound.io/upbound/provider-azuread:v1.1.0

NAME               INSTALLED   HEALTHY   PACKAGE                                           AGE
provider-azuread   True        True      xpkg.upbound.io/upbound/provider-azuread:v1.1.0   8m32s

and we can see the following outputs from get -o yaml

apiVersion:

apiVersion: conditionalaccess.azuread.upbound.io/v1beta1

singleton lists:

 conditions:
 - applications:
   - includedApplications:
     - All

conditions:

 - lastTransitionTime: "2024-05-16T16:18:52Z"
   reason: Available
   status: "True"
   type: Ready
 - lastTransitionTime: "2024-05-16T16:18:46Z"
   reason: ReconcileSuccess
   status: "True"
   type: Synced
 - lastTransitionTime: "2024-05-16T16:18:48Z"
   reason: Success
   status: "True"
   type: LastAsyncOperation
 - lastTransitionTime: "2024-05-16T16:24:42Z"
   reason: UpToDate
   status: "True"
   type: Test

> kubectl delete managed --all
accesspolicy.conditionalaccess.azuread.upbound.io "example" deleted

Downgrade test successful

ulucinar requested review from sergenyalcin and turkenf as code owners April 17, 2024 14:30

ulucinar marked this pull request as draft April 17, 2024 14:30

ulucinar force-pushed the embed-singleton branch from 87dd8fb to 834500d Compare April 17, 2024 16:37

ulucinar force-pushed the embed-singleton branch from 834500d to 0410759 Compare April 17, 2024 20:56

ulucinar force-pushed the embed-singleton branch 2 times, most recently from e53de92 to bf9bf2a Compare April 18, 2024 00:17

ulucinar force-pushed the embed-singleton branch from 8342e74 to 510d8bc Compare April 18, 2024 12:43

ulucinar force-pushed the embed-singleton branch from 94b8e78 to d3c803e Compare April 19, 2024 06:49

ulucinar force-pushed the embed-singleton branch from d3c803e to 1970fe5 Compare April 19, 2024 08:14

ulucinar marked this pull request as ready for review April 24, 2024 14:52

ulucinar force-pushed the embed-singleton branch from 50405c1 to 9e2c280 Compare April 24, 2024 17:41

ulucinar added 8 commits May 8, 2024 13:34

Generate all converted singleton list APIs as version v1beta2

5ec9d53

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

Mark the v1beta1 versions of the generated CRSs as the storage versions

ecb2c55

to facilitate downgrades Signed-off-by: Alper Rifat Ulucinar <[email protected]>

Adjust example manifests for converting singleton lists to embedded o…

73aa6fc

…bjects - Fix the example manifest examples/directoryroles/v1beta1/customdirectoryrole.yaml Signed-off-by: Alper Rifat Ulucinar <[email protected]>

CRD API conversion functions to convert between singleton list & embe…

5d78870

…dded object API versions Signed-off-by: Alper Rifat Ulucinar <[email protected]>

Add CROSSPLANE_NAMESPACE to the env. of uptest

c88df8f

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

ulucinar force-pushed the embed-singleton branch 2 times, most recently from 97f194c to 803e88e Compare May 8, 2024 10:50

ulucinar force-pushed the embed-singleton branch from 803e88e to 9be46b2 Compare May 8, 2024 14:04

sergenyalcin approved these changes May 8, 2024

View reviewed changes

Register the API types with the API resolver's runtime scheme

990d474

- Bump upjet to commit 03a207b6416a - Bump uptest to version v0.12.0-9.gac371c9 Signed-off-by: Alper Rifat Ulucinar <[email protected]>

ulucinar force-pushed the embed-singleton branch from 9be46b2 to 990d474 Compare May 8, 2024 14:19

ulucinar merged commit c33d7a8 into crossplane-contrib:main May 8, 2024
11 checks passed

ulucinar deleted the embed-singleton branch May 8, 2024 14:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ulucinar commented Apr 17, 2024 •

edited

Loading

ulucinar commented Apr 17, 2024

ulucinar commented Apr 17, 2024

ulucinar commented Apr 17, 2024

ulucinar commented Apr 17, 2024

ulucinar commented Apr 17, 2024

ulucinar commented Apr 18, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

turkenf commented Apr 24, 2024

turkenf commented May 8, 2024

turkenf commented May 8, 2024

turkenf commented May 8, 2024

ulucinar commented May 8, 2024

ulucinar commented May 8, 2024

ulucinar commented May 8, 2024

ulucinar commented May 8, 2024

ulucinar commented May 8, 2024

sergenyalcin left a comment

ulucinar commented May 8, 2024

ulucinar commented May 8, 2024

turkenf commented May 16, 2024

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

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

Conversation

ulucinar commented Apr 17, 2024 • edited Loading

Description of your changes

How has this code been tested

ulucinar commented Apr 17, 2024

ulucinar commented Apr 17, 2024

ulucinar commented Apr 17, 2024

ulucinar commented Apr 17, 2024

ulucinar commented Apr 17, 2024

ulucinar commented Apr 18, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

ulucinar commented Apr 19, 2024

turkenf commented Apr 24, 2024

turkenf commented May 8, 2024

turkenf commented May 8, 2024

turkenf commented May 8, 2024

ulucinar commented May 8, 2024

ulucinar commented May 8, 2024

ulucinar commented May 8, 2024

ulucinar commented May 8, 2024

ulucinar commented May 8, 2024

sergenyalcin left a comment

Choose a reason for hiding this comment

ulucinar commented May 8, 2024

ulucinar commented May 8, 2024

turkenf commented May 16, 2024

ulucinar commented Apr 17, 2024 •

edited

Loading