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

Use config.Reference.TerraformName instead of config.Reference.Type #739

Merged
merged 1 commit into from
May 13, 2024

Conversation

ulucinar
Copy link
Collaborator

@ulucinar ulucinar commented May 10, 2024

Description of your changes

This PR replaces usages of upjet's config.Reference.Type API with config.Reference.TerraformName. In crossplane/upjet#400, we are deprecating the config.Reference.Type API in favor of config.Reference.TerraformName. We had introduced config.Reference.TerraformName in crossplane/upjet#12 and it's a more stable and less error prone API compared to config.Reference.Type because it automatically accounts for the configuration changes affecting the cross-resource reference target's kind name, group or version. We've already been discouraging the config.Reference.Type in favor of config.Reference.TerraformName since it's been introduced.

This PR is a precursor for #733, in which we will start generating the v1beta2 APIs for certain resources (those with singleton lists in their Terraform schemas). When the config.Reference.Type is used to define the reference target, the API version can be omitted for the targets in the same group and version. In those cases, if the referencing resource has been generated at v1beta2 whereas the reference target lacks that version, the generated resolver is not valid. Using config.Reference.TerraformName instead will help us in future maintenance because it automatically accounts for generating the cross-resource reference resolvers in the generated version. This is also one of the reasons we are deprecating config.Reference.Type.

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.

How has this code been tested

A successful uptest run is registered here for a resource affected by this change: https://github.com/crossplane-contrib/provider-upjet-azure/actions/runs/9037709362/

Also checked for any unexpected changes in the generated resolvers (apis/../zz_generated.resolvers.go files).

@ulucinar
Copy link
Collaborator Author

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

- config.Reference.Type will be deprecated in favor of config.Reference.TerraformName

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
@ulucinar
Copy link
Collaborator Author

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

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 ulucinar merged commit 4b8fbca into crossplane-contrib:main May 13, 2024
12 checks passed
@ulucinar ulucinar deleted the ref-via-tfname branch May 13, 2024 14:06
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.

None yet

2 participants