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

Replace NoFork Terminology with TerraformPluginSDK #338

Merged
merged 5 commits into from
Feb 1, 2024

Conversation

ulucinar
Copy link
Collaborator

@ulucinar ulucinar commented Feb 1, 2024

Description of your changes

The nofork architecture now has the native implementations for both the Terraform plugin SDK v2 resources and the Terraform plugin framework resources. The repo uses NoFork in the file & type names related to the plugin SDK v2 implementation. This PR aims to replace that terminology with the TerraformPluginSDK naming.

This PR also adds a more explanatory error message when immutable fields of a managed resource have changed. We currently do not generate CRD validation rules for immutable fields and thus, if they are updated, we block it at the Terraform layer. The improve the error message for such cases as follows:
refuse to update the external resource because the following update requires replacing it

This PR also fixes a race condition on a global state in the Terraform plugin SDK v2 & Terraform plugin framework based external client tests. The global state is a fake.Terraformed object, which is shared between multiple test cases. The theory is that changing the names of the test modules caused a reordering of the test executions, which revealed the race. Resolved by not sharing this state between test cases.

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 via crossplane-contrib/provider-upjet-aws#1121.

- pkg/controller/nofork_store.go is common to both the TF plugin SDK & framework
  so its name is preserved.

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
…raformPluginSDK"

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
@ulucinar ulucinar marked this pull request as draft February 1, 2024 10:08
…ed resource have changed

- We currentl do not generate CRD validation rules for immutable fields and thus, if they
  are updated, we block it at the Terraform layer. The improve the error message for
  such cases as follows:
  "refuse to update the external resource because the following update requires replacing it"

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
…n SDK v2 & framework tests

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
ulucinar added a commit to ulucinar/upbound-provider-aws that referenced this pull request Feb 1, 2024
- Please see: crossplane/upjet#338
  for further details.

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
@ulucinar ulucinar marked this pull request as ready for review February 1, 2024 11:39
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!

…ugin SDK &

framework based managed resources.

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
ulucinar added a commit to ulucinar/upbound-provider-aws that referenced this pull request Feb 1, 2024
- Please see: crossplane/upjet#338
  for further details.

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
@ulucinar ulucinar merged commit 602c7d4 into crossplane:main Feb 1, 2024
7 checks passed
@ulucinar ulucinar deleted the rename-to-pluginsdk branch February 1, 2024 14:17
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