Handle nested fields in SetFieldConfig #1065
Assignees
Labels
area/code-generation
Issues or PRs as related to controllers or docs code generation
kind/enhancement
Categorizes issue or PR as related to existing feature enhancements.
priority/important-soon
Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone
Comments
jaypipes
added
kind/enhancement
a-hilaly
added
the
priority/important-soon
2 tasks
ack-bot
pushed a commit
to aws-controllers-k8s/code-generator
that referenced
this issue
Jan 5, 2022
Often, the Go type of a Create Input shape field (which goes in the resource's Spec struct) is different from the Go type of the same-named field in an Output shape. This causes problems for the code generator, which doesn't know how to set the value of the Spec field due to these different source and target Go types. The `pkg/generate/config.SetConfig` struct was designed to solve this particular issue, however the original solution only worked properly when the field in the Output shape that we wanted to set the Spec field *from* was at the same "level" as the field we wanted to set the value *to*. Take for example the case of the IAM Role resource, which has a Create Input shape with a string `PermissionsBoundary` field. When creating the Role, the user specifies the PermissionsBoundary as an ARN field. In the Create and Get Output shapes, however, the `PermissionsBoundary` field is a struct containing a `PermissionsBoundaryArn` string field. We want to set the value of the Role resource's `Spec.PermissionsBoundary` field to the string value of this nested `PermissionsBoundary.PermissionsBoundaryArn` field. This patch facilitates that by modifying the `pkg/generate/code.SetResource` function to look up the field in the Output shape via a field path instead of a single field name map lookup. Issue aws-controllers-k8s/community#1065 Signed-off-by: Jay Pipes <[email protected]> By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
jaypipes
added a commit
to jaypipes/ack-code-generator
that referenced
this issue
Jan 14, 2022
A previous commit added functionality to the API/resource inference process to "unpack" a struct field's member fields into the `pkg/model.Field.MemberFields` map. However, I had failed to account for lists or maps of structs. This patch accounts for the list or map of structs fields, stepping into the list field's element struct type and adding a Field definition to the containing Field's `MemberFields` map for each member field in the element struct type. Same for containing shapes of type "map". With this patch, we're now able to "access" via field path any nested field (including its corresponding `FieldConfig` object) which means that in following patches we can finally resolve the "different Go types for nested fields" problem we've had in the code generator. Related issue: aws-controllers-k8s/community#1065 Signed-off-by: Jay Pipes <[email protected]>
jaypipes
modified the milestones:
ACK Core (2021-11-15 thru 2021-11-26),
ACK Core (2022-01-17 thru 2022-01-28
|
/assign @brycahta |
|
@a-hilaly: GitHub didn't allow me to assign the following users: brycahta. Note that only aws-controllers-k8s members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
a-hilaly
modified the milestones:
ACK Core (2022-01-17 thru 2022-01-28,
ACK Core (2022-01-31 thru 2022-02-11)
|
/assign @brycahta |
a-hilaly
modified the milestones:
ACK Core (2022-01-31 thru 2022-02-11),
ACK Core (2021-02-14 thru 2022-02-25)
|
@brycahta Is there work left to do for this? |
|
@RedbackThomson No work left. I was going to resolve once we cut a release with these changes/fixes. If y'all have a different process feel free to resolve. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
While implementing the solution for #178, Bryan pointed out that the solution would not work for nested field paths.
We should address this gap in functionality.
The text was updated successfully, but these errors were encountered: