Make MaxItems values consistent between JSON schema and provider schema #745
Conversation
sergenyalcin
force-pushed
the
configure-max-items
branch
from
2714b56 to
a53483b
Compare
|
/test-examples="examples/network/v1beta1/routefilter.yaml" |
|
/test-examples="examples/network/v1beta2/subnet.yaml" |
|
/test-examples="examples/cdn/v1beta2/endpoint.yaml" |
|
/test-examples="examples/compute/v1beta2/sharedimage.yaml" |
|
/test-examples="examples/network/v1beta2/profile.yaml" |
There was a problem hiding this comment.
Thanks @sergenyalcin for fixing these configurations. We will need to fix the snake_case second arguments to Resource.AddSingletonListConversion throughout the PR. We expect a CRD path as the second argument.
Let's merge #746 and rebase this PR before merging to get a green check-examples job.
| r.Version = "v1beta2" | ||
| r.PreviousVersions = []string{"v1beta1"} | ||
| // we would like to set the storage version to v1beta1 to facilitate | ||
| // downgrades. | ||
| r.SetCRDStorageVersion("v1beta1") | ||
| r.ControllerReconcileVersion = "v1beta1" | ||
| r.Conversions = []conversion.Conversion{ | ||
| conversion.NewIdentityConversionExpandPaths(conversion.AllVersions, conversion.AllVersions, conversion.DefaultPathPrefixes(), r.CRDListConversionPaths()...), | ||
| conversion.NewSingletonListConversion("v1beta1", "v1beta2", conversion.DefaultPathPrefixes(), r.CRDListConversionPaths(), conversion.ToEmbeddedObject), | ||
| conversion.NewSingletonListConversion("v1beta2", "v1beta1", conversion.DefaultPathPrefixes(), r.CRDListConversionPaths(), conversion.ToSingletonList)} |
There was a problem hiding this comment.
We need this explicit singleton list conversion configuration because there are no other singleton list fields in this resource than the rule field, right? And the rule field's MaxItems constraint is not 1 in the JSON schema, whereas it's set to 1 in the Go schema?
There was a problem hiding this comment.
Yes this is the case here. We only rule field for conversion here. So, I specifically configure this resource version and conversion functions because they can't be configured via central function. Also the case is that we make consistent the JSON schema value with Go schema.
config/containerregistry/config.go
Outdated
| r.TerraformResource.Schema["encryption"].MaxItems = 1 | ||
| r.TerraformResource.Schema["network_rule_set"].MaxItems = 1 | ||
|
|
||
| r.AddSingletonListConversion("retention_policy", "retention_policy") |
There was a problem hiding this comment.
Looks like the CRD paths are misconfigured, i.e.:
| r.AddSingletonListConversion("retention_policy", "retention_policy") | |
| r.AddSingletonListConversion("retention_policy", "retentionPolicy") |
We also need to fix the other snake_case second arguments to Resource.AddSingletonListConversion.
Signed-off-by: Sergen Yalçın <[email protected]>
Signed-off-by: Sergen Yalçın <[email protected]>
Signed-off-by: Sergen Yalçın <[email protected]>
Signed-off-by: Sergen Yalçın <[email protected]>
sergenyalcin
force-pushed
the
configure-max-items
branch
from
a004992 to
0dc06cf
Compare
Description of your changes
During tests of the `Convert all singleton lists in the MR APIs to embedded objects' PRs, we observed an issue regarding the inconsistency between the JSON schema and provider schema.
The GetProvider function returns provider configuration in the providers. This function has a parameter generationProvider. If it is set to true, the JSON schema is used to obtain provider configuration. If it is false, the Go schema is used.
In theory, the two schemas are equivalent. However, during tests, we observed inconsistencies in some fields MaxItems values. This means that we use different schemas during generation and reconciliation.
This causes issues with the conversion of singleton lists to embedded objects. This type of change may also cause bigger problems.
This PR makes MaxItems values consistent between JSON schema and provider schema by manipulating the resource schemas.
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