-
Notifications
You must be signed in to change notification settings - Fork 91
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
types: slices and maps of secret key selector should not have pointer #135
Conversation
… types since they are already pointers Signed-off-by: Muvaffak Onus <[email protected]>
Not top of my mind. You can print effected resources by running generate against a custom change. |
Looks like unit tests are failing with a relevant issue, can you check that? |
@muvaf are there anything else you did to get the issue fixed with this PR? Because I cannot get it fixed with this PR as well:
Generates the following
|
Hi, I am attempting to create a Linode provider using upjet and hitting this issue as well. Specifically for the "linode_instance" resource via https://registry.terraform.io/providers/linode/linode/latest/docs/resources/instance The errors are similar to here:
In this case, with this PR/patch, the StackscriptDataSecretRef becomes 202 StackscriptDataSecretRefMap map[string]_v1.SecretKeySelector But it is able to generate successfully with this patch. |
Closing this since the underlying problem fixed by #138 |
Description of your changes
*map[string]v1.SecretKeySelector
and*[]v1.SecretKeySelector
are not compatible with marshaling and slices & maps are already pointers, so, no need to make them pointer of pointers to have them optional in OpenAPI spec. The only functional change in this PR is to not make those two cases pointer but onlystring
one.Also opened #134 since map with struct value type is actually anti-pattern. So we should decide whether we should switch to something else now or later. @turkenh Do you know of CRDs that have sensitive map values like this one in other providers? I wonder how much breaking change we'd inflict if we were to switch to either of the options proposed in the issue.
Part of #126
I have:
make reviewable
to ensure this PR is ready for review.backport release-x.y
labels to auto-backport this PR if necessary.How has this code been tested
Confirmed that #126 is fixed with this PR. It also results in rename & retype of
PasswordSecretRef *[]v1.SecretKeySelector
inUser
ofelasticache
toPasswordSecretRefs []v1.SecretKeySelector
, which is a breaking change in the schema due to rename. I need to check if it was working in the first place.