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.

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

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

Merged
merged 2 commits into from
May 15, 2024

Conversation

ulucinar
Copy link
Collaborator

@ulucinar ulucinar commented May 14, 2024

Description of your changes

When generating new versions of the CRD APIs associated with a managed resource, upjet's code generation pipeline can potentially run into the following issue:

  • We are generating a (new) API version v1beta2 for resource B
  • There exists another resource A in the same group as B, whose API is generated at v1beta1
  • These resources share some common parameters (configuration arguments) with identical canonical paths
  • upjet's code generation pipeline can normally resolve such conflicts while generating the Go struct names however it relies on a static pipeline that assumes all the types available are being generated. In our case, v1beta1 version of the B resource API is not generated and hence upjet is not aware of it. This results in name conflicts for the generated types.

We have a roadmap item to modularize the code generation pipeline to make it more dynamic and scalable but it's a much larger effort. As a workaround, we had introduced the config.Resource.OverrideFieldNames API through which one can override the name upjet is generating for a CRD type. However, this requires manual configuration.

For most cases, this has been feasible as we rarely do bulk breaking changes in the MR APIs. However recently, we are rolling out API changes across the official providers that replace the singleton lists in the MR APIs with embedded objects (please see #387 and crossplane-contrib/provider-upjet-gcp#508 for more context). Such systemic changes affecting multiple resources can result in a need to override the generated type names for many resources. What's worse, @sergenyalcin identified in crossplane-contrib/provider-upjet-azure#733 that the current config.Resource.OverrideFieldNames API is unable to handle certain types of overrides where we need to override the same leaf at multiple paths. We need to be able to specify the canonical paths of the overrides being defined.

So, we've decided to address this issue by making upjet's code generation pipelines aware of the existing previous versions of the CRDs (MR APIs). One can configure an older version of an API as follows:

p.AddResourceConfigurator("aws_msk_cluster", func(r *config.Resource) {
  r.Version = "v1beta2"
  r.PreviousVersions = []string{"v1beta1"}
  ...

While generating the CRDs in a package, upjet will then load the type definitions from these previous versions specified and use them as input to its already existing name conflict resolution function.

We also deprecate the config.Resource.OverrideFieldNames API whose only known use case is preventing the type name conflicts in the generated CRDs. So, config.Resource.OverrideFieldNames is deprecated in favor of the ``config.Resource.PreviousVersions`-based mechanism described above.

I have:

  • Read and followed Upjet'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.

How has this code been tested

Tested in the context of crossplane-contrib/provider-upjet-azure#733 & crossplane-contrib/provider-upjet-gcp#508. As an example, please observe the changes in here, where the API is replaced and we observe no diff in the generated APIs.

ulucinar added 2 commits May 14, 2024 14:12
…previous

versions of an MR API.

- upjet code generation pipelines can then utilize this information to load
  the already existing type names for these previous versions and prevent
  collisions for the generated CRD types.

Signed-off-by: Alper Rifat Ulucinar <[email protected]>
config.Resource.PreviousVersions.

- The only known use case for config.Resource.OverrideFieldNames is
  to resolve type confilicts between the older versions of the CRDs
  and the ones being generated. The "PreviousVersions" API allows
  loading of the existing types from the filesystem so that upjet
  code generation pipeline's type conflict resolution mechanisms
  can prevent such name collisions.

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

@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 ulucinar merged commit 92d1af8 into crossplane:main May 15, 2024
6 checks passed
@ulucinar ulucinar deleted the available-versions branch May 15, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants