Do not prefix JSON fieldpaths starting with status.atProvider in resource.GetSensitiveParameters #417
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
ulucinar
requested review from
sergenyalcin,
erhancagirici and
mergenci
as code owners
…urce.GetSensitiveParameters - If the MR API has a spec.forProvider.status field and there are sensitive attributes, then fieldpath.Paved.ExpandWildcards complains instead of expanding as an empty slice, which breaks the reconciliation. Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
3 tasks
|
Successfully created backport PR #418 for |
3 tasks
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
Fixes #416
If the MR API has a
spec.forProvider.statusfield and there are sensitive attributes, thenfieldpath.Paved.ExpandWildcardscomplains instead of expanding as an empty slice, which breaks the reconciliation.This bug has been observed with the
AccessKey.iamresource ofcrossplane-contrib/provider-upjet-aws@v1.5.0, where we have introduced the sensitive parameters under thespec.initProviderAPI tree by consuming the upjet changes from #406, and reported in #416.This bug only affects those resources with a
spec.forProvider.statusfield and with sensitive attributes, which are removed fromstatus.atProviderin the corresponding CRD schema but still represented in the connection details mapping so that the corresponding sensitive information can be published in the connections details secret.Before #406, such mappings were effectively being ignored by the
fieldpath.Paved.ExpandWildcardsbecause such sensitive attributes are removed, during code generation, fromstatus.atProvider. However, #406 now prefixes such JSON fieldpath expressions withspec.forProvider(andspec.initProvider) and if there already exists aspec.forProvider.statusfield, then thefieldpathlibrary now errors instead of expanding the expressions as an empty slice (as it does in other cases).Arguably we could address this in the
fieldpathlibrary but this PR proposes to address this issue by reverting back to the old conditions: If the JSON fieldpath expression starts with astatus.atProvider, then we do not prefix it withspec.forProviderorspec.initProvider, which restores the old behavior as there should not exist the correspondingstatus.atProviderfields and thefieldpathlibrary now correctly handles the wildcard expansion by returning an empty expanded set instead of erroring.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
Tested with crossplane-contrib/provider-upjet-aws#1344.