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

Only add singleton list API converters for the existing APIs #1367

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

ulucinar
Copy link
Collaborator

@ulucinar ulucinar commented Jun 14, 2024

Description of your changes

Only apply the CRD API conversions for converting between singleton list & embedded object APIs for the already existing resources with the generated singleton list CRD APIs.

When we try to add a new Terraform resource with a singleton list in its schema, currently, we need to do a manual configuration such as the following:

p.AddResourceConfigurator("azurerm_pim_active_role_assignment", func(r *config.Resource) {
		r.PreviousVersions = nil
		r.Version = "v1beta1"
		r.Conversions = nil
		r.TerraformConversions = []config.TerraformConversion{
			config.NewTFSingletonConversion(),
		}
	})

, so that the API converters registered are removed and versioning starts with v1beta1 instead of v1beta2 (for such new resources, the v1beta1 version does not already exist).

This PR records the names of resources with previous CRD APIs containing (now converted) singleton lists so that the API converters are only registered for them.
This makes adding new resources with singleton lists in their Terraform schemas easier by removing the need to do a manual configuration like shown above.

The newly introduced list should mostly be immutable (it records a set of existing resources with a specific condition at a point in time). Bugs may result in exceptions where we may need to update this list. In other words, new resources should normally be never added to this list, doing so will result in an invalid configuration.

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

Via the following uptest runs:

@ulucinar
Copy link
Collaborator Author

/test-examples="examples/firehose/v1beta2/deliverystream.yaml"

@ulucinar
Copy link
Collaborator Author

/test-examples="examples/opensearchserverless/v1beta1/securityconfig.yaml"

@ulucinar
Copy link
Collaborator Author

/test-examples="examples/eks/v1beta2/nodegroup.yaml"

Copy link
Collaborator

@mergenci mergenci 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 🙏 This PR should make new resource configurators' job significantly easier by eliminating the need to understand the shenanigans of singleton list conversions. Please see my suggestion regarding documentation below.

config/registry.go Show resolved Hide resolved
…& embedded object APIs

for the already existing resources with generated singleton list CRD APIs.

- This makes adding new resources with singleton lists in their Terraform schemas
  easier by removing the need to do a manual configuration.

Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
@ulucinar ulucinar force-pushed the selective-sl-config branch from 8cc608f to caf2fe9 Compare June 14, 2024 16:22
@ulucinar
Copy link
Collaborator Author

/test-examples="examples/firehose/v1beta2/deliverystream.yaml"

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