Sync the MaxItems constraints between the Go & JSON schemas #537
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Generate the secret references for the sensitive fields under the spec.initProvider API tree. Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
- We are introducing a schema traverser in upjet that's capable of synching the MaxItems constraints from the Go resource schema to the JSON schema. So we switch from manual configuration to automatic constraint syncing (for now, only MaxItems constraints are synced). - We would like to get rid of the JSON schema completely with a major version bump. - Automatic syncing will also help with the new resources as we will not need to consider syncing the MaxItems constraints on their fields. Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
from the Go resource schema to the JSON schema. Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
|
/test-examples="examples/compute/v1beta2/instancefromtemplate.yaml" |
|
/test-examples="examples/container/v1beta2/nodepool.yaml" |
sergenyalcin
approved these changes
Jun 6, 2024
There was a problem hiding this comment.
Thanks @ulucinar LGTM. I left a nit comment that is proposing the removal of the function that is no longer functional after the change. Also, checked the diffs in the apis and package to ensure that the configuration changes do not affect the APIs. It seems that all changes are related-to generation of SecretRefs in InitProvider blocks.
3 tasks
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
|
/test-examples="examples/container/v1beta2/nodepool.yaml" |
|
/test-examples="examples/compute/v1beta2/instancefromtemplate.yaml" |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of your changes
Depends on: crossplane/upjet#411
When replacing the singleton lists with embedded objects in the MR APIs, we had figured out that the MaxItems constraints in the JSON resource schema were not in-sync with the constraints declared in the Go schema. For more context, please see the description of #527.
We've previously addressed this issue by manually configuring the
MaxItemsconstraints for the affected resources in #527. This also implies that if the constraints are not in sync for any future resources, then we will also need to manually configure them.With crossplane/upjet#411, we introduce a new schema traverser,
traverser.maxItemsSync, that syncs theMaxItemsconstraints from a source schema to a target schema.This PR removes the manual
MaxItemsconfigurations and usestraverser.maxItemsSyncto sync theMaxItemsconstraints from the Go resource schema to the JSON resource schema. This will also help us with any future resources for which theMaxItemsconstraints are not in sync. We could in theory sync the other constraints thanMaxItemsbut currently it's the only known constraint with inconsistencies causing us trouble.With a major version bump of the provider, we would also like to get rid of the JSON resource schema that we only use during the code generation, as a long term strategy.
Note:
As we bump the upjet version with this PR to be able to use the
traverser.maxItemsSync, we also generate the secret references for sensitive fields under thespec.initProviderAPI tree. This has been implemented in a separate initial commit before the manualMaxItemsconstraints are removed, so that it becomes easy for the reviewers to observe how the schema traverser behaves and whether it can effective replace the current manual configuration.I have:
make reviewableto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR if necessary.How has this code been tested
Via the uptest runs of the resources tested in #527:
Also, please observe that commit
46b1b34e5e9cdd65edb737e1978dcf519fae6bfe(which invokes thesyncMaxItemsschema traverser) on top of the commita2af467da6eeab7d4460f7cb95443676cd7e465f(which removes theMaxItemsconfiguration) produces no diff in the generated code.