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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 11 commits into from
May 21, 2024
Merged

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

merged 11 commits into from
May 21, 2024

Conversation

ulucinar
Copy link
Collaborator

@ulucinar ulucinar commented May 7, 2024 •
edited
Loading

Description of your changes

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

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 need #745 to fix the configurations to address an inconsistency between the JSON & Go resource schemas.

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: Add example manifests for v1beta2 version #746 adds the v1beta2 manifests.
  • API Conversion functions (invoked by the Conversion webhooks) to convert between the new & old API versions
  • Switch controller watch target API versions to v1beta1

How has this code been tested

Via various uptest runs below.

@ulucinar ulucinar marked this pull request as draft May 7, 2024 08:47
@ulucinar ulucinar force-pushed the embed-singleton branch from a026f06 to 595423c Compare May 7, 2024 08:48
@ulucinar ulucinar mentioned this pull request May 10, 2024
Merged
3 tasks
@ulucinar ulucinar force-pushed the embed-singleton branch 2 times, most recently from 2bc9ecb to 41c9bbf Compare May 13, 2024 15:02
@ulucinar ulucinar mentioned this pull request May 14, 2024
Merged
3 tasks
@ulucinar ulucinar force-pushed the embed-singleton branch 2 times, most recently from ceab332 to 7f8c647 Compare May 14, 2024 11:50
@ulucinar ulucinar marked this pull request as ready for review May 14, 2024 11:54
@sergenyalcin
Copy link
Collaborator

/test-examples="examples/cdn/v1beta2/endpoint.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/cdn/v1beta2/frontdoorrule.yaml"

@sergenyalcin
Copy link
Collaborator

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

@sergenyalcin
Copy link
Collaborator

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

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/cdn/v1beta2/endpoint.yaml"

1 similar comment
@sergenyalcin
Copy link
Collaborator

/test-examples="examples/cdn/v1beta2/endpoint.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/cdn/v1beta2/frontdoorrule.yaml"

@sergenyalcin
Copy link
Collaborator

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

@sergenyalcin
Copy link
Collaborator

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

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/containerservice/v1beta2/kubernetescluster.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/cosmosdb/v1beta2/account.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/dbforpostgresql/v1beta2/server.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/network/v1beta2/profile.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/network/v1beta2/routefilter.yaml"

@sergenyalcin
Copy link
Collaborator

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

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/web/v1beta2/appserviceplan.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/network/v1beta1/routefilter.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/network/v1beta2/subnet.yaml"

@sergenyalcin
Copy link
Collaborator

/test-examples="examples/cosmosdb/v1beta2/mongocollection.yaml"

@sergenyalcin sergenyalcin mentioned this pull request May 20, 2024
Merged
3 tasks
sergenyalcin
sergenyalcin approved these changes May 20, 2024
View reviewed changes
Copy link
Collaborator

@sergenyalcin sergenyalcin left a comment

Choose a reason for hiding this comment

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

Thanks @ulucinar LGTM!

@ulucinar
Copy link
Collaborator Author

/test-examples="examples/network/v1beta2/subnet.yaml"

ulucinar and others added 10 commits May 20, 2024 17:47
@ulucinar
Convert all singleton lists in the MR APIs to embedded objects
c10ea6e 
- 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.
- Bump up to v0.28.0
- Temporarily add the $(UPTEST_LOCAL) make target to consume uptest
  from the main channel.

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@ulucinar
Do "OverrideFieldNames" configurations to prevent duplicated type dec…
8f8c510 
…larations

- Run "make generate"

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@ulucinar
Regenerate the cross-resource reference resolvers after replacing con…
a7d0dc1 
…fig.Resource.Type's with

config.Resource.TerraformName's.

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@ulucinar
Bump upjet to commit 94634891cc5a
223a5a0 
Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@ulucinar
Replace config.Resource.OverrideFieldNames usages with config.Resourc…
3887491 
…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 b5622a10e14d.

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@ulucinar
Move example manifests under to v1beta1 version folders
40d6564 
Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@sergenyalcin @ulucinar
Add v1beta2 versioned example manifests for some resources
2b0ec73 
Signed-off-by: Sergen Yalçın <[email protected]>
@ulucinar
Add the singleton list Terraform conversion as the controller's recon…
6948378 
…cile version is v1beta2

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@sergenyalcin @ulucinar
Bump UPTEST_LOCAL version
33a7cba 
Signed-off-by: Sergen Yalçın <[email protected]>
@sergenyalcin @ulucinar
Remove RouteFilter v1beta2 example because the v1beta2 version is not…
ab444ec 
… generated

Signed-off-by: Sergen Yalçın <[email protected]>
@ulucinar
Copy link
Collaborator Author

/test-examples="examples/network/v1beta2/subnet.yaml"

@ulucinar
Use v1beta1 as the controller reconcile version for the converted APIs
fc58f12 
with singleton lists

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
@ulucinar
Copy link
Collaborator Author

/test-examples="examples/network/v1beta2/subnet.yaml"

@ulucinar
Copy link
Collaborator Author

Example manifests for the converted APIs are being introduced with: #746

We also need #745 to fix a configuration issue.

@ulucinar ulucinar merged commit 34e9a3d into crossplane-contrib:main May 21, 2024
11 of 12 checks passed
@ulucinar ulucinar deleted the embed-singleton branch May 21, 2024 07:17
@ulucinar
Copy link
Collaborator Author

We need #745 to fix the configurations to address an inconsistency between the JSON & Go resource schemas. #746 introduces the missing v1beta2 manifests.

@turkenf turkenf mentioned this pull request May 28, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sergenyalcin sergenyalcin sergenyalcin approved these changes

@turkenf turkenf Awaiting requested review from turkenf turkenf is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ulucinar @sergenyalcin