-
Notifications
You must be signed in to change notification settings - Fork 89
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
Add reference fields to the InitProvider #315
Add reference fields to the InitProvider #315
Conversation
Signed-off-by: Sergen Yalçın <[email protected]>
if isInit { | ||
r.initTags = append(r.initTags, refTags...) | ||
r.initFields = append(r.initFields, refFields...) | ||
} else { | ||
r.paramTags = append(r.paramTags, refTags...) | ||
r.paramFields = append(r.paramFields, refFields...) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the reference fields will now always be added to the spec.forProvider
and spec.initProvider
, is this correct? If we don't have the branching here, what would cause an issue is the fact that this function is called once for the forProvider
and once for initProvider
, is this correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the both are correct. Now we always generate the reference fields for both. And if we do not have a branch here, this will cause an issue.
Signed-off-by: Sergen Yalçın <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sergenyalcin, lgtm.
Description of your changes
Fixes #307
This PR supports generation of reference fields and reference resolver functions to the InitProvider.
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
This PR tested against to the upjet-based provider-aws