Aa-subscription-peerings #93
Conversation
|
|
greg-oc
left a comment
greg-oc
left a comment
There was a problem hiding this comment.
LGTM, just one comment I'd like clarified
| for i, region := range peering.Regions { | ||
| for j, vpcPeering := range region.VPCPeerings { | ||
| if vpcPeering.VPCCidr != nil { | ||
| peering.Regions[i].VPCPeerings[j].VPCCidrs = append(peering.Regions[i].VPCPeerings[j].VPCCidrs, vpcPeering.VPCCidr) |
There was a problem hiding this comment.
Just checking, is there a chance that the api will have the same cidr return in both the VPCCidr and VPCCidrs fields? e.g.
VPCCidr = "0.0.0.0/0" and VPCCidrs - ["0.0.0.0/0"] ?
In this case this function would return
.VPCCidrs = ["0.0.0.0/0", "0.0.0.0/0"]
However I do think the API returns
VPCCidr = null if there are multiple VPCCidrs
OR
VPCCidr = and VPCCidrs = null if there is only one cidr
If the above is correct, then I think the function is perfect as is, otherwise it may need a check to avoid two duplicate CIDRs in the VPCCidrs field. Does that make sense?
There was a problem hiding this comment.
@greg-oc I believe that's the case, as it wouldn't make sense to have both fields set 👍🏼
No description provided.