fix(Cloudfront): LateInit missing fields #1298
Conversation
ezgidemirel
force-pushed
the
fix-cf-rhp-ref
branch
from
8666088 to
5c7a243
Compare
ezgidemirel
force-pushed
the
fix-cf-rhp-ref
branch
from
5c7a243 to
7d91fad
Compare
| continue | ||
| } | ||
| // FunctionARN must be unique for each FunctionAssociation | ||
| existing[awsclients.StringValue(o.FunctionARN)] = o |
There was a problem hiding this comment.
We have checked the API reference and the console UI for cache behavior with @ezgidemirel. It looks like the key for this resource should be the event type.
| if o.FunctionARN == nil { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Please see the comment below regarding the chosen key. We would not expect the key type to be a pointer but although in the console UI, it's not a pointer type, the SDK expects a pointer type. We did not check what happens if you call the SDK function with a nil EventType but from what we observed from the console UI, this should not be allowed.
| if o.FunctionARN == nil { | |
| continue | |
| } |
There was a problem hiding this comment.
As expected, AWS doesn't allow EventType to be nil. Removing this nil check caused following error:
message: "update failed: cannot update Distribution in AWS: PreconditionFailed:
The request failed because it didn't meet the preconditions in one or more request-header
fields.\n\tstatus code: 412, request id: "
I added back with the EventType.
| continue | ||
| } | ||
| // FunctionARN must be unique for each FunctionAssociation | ||
| existing[awsclients.StringValue(o.FunctionARN)] = o |
There was a problem hiding this comment.
| existing[awsclients.StringValue(o.FunctionARN)] = o | |
| existing[awsclients.StringValue(o.EventType)] = o |
| } | ||
|
|
||
| for _, il := range in.Items { | ||
| if il.FunctionARN == nil { |
There was a problem hiding this comment.
| if il.FunctionARN == nil { | |
| if il.EventType != nil { |
| continue | ||
| } | ||
|
|
||
| fl := existing[awsclients.StringValue(il.FunctionARN)] |
There was a problem hiding this comment.
| fl := existing[awsclients.StringValue(il.FunctionARN)] | |
| fl := existing[awsclients.StringValue(il.EventType)] |
ezgidemirel
force-pushed
the
fix-cf-rhp-ref
branch
from
7d91fad to
0e27ceb
Compare
Signed-off-by: ezgidemirel <[email protected]>
ezgidemirel
force-pushed
the
fix-cf-rhp-ref
branch
from
0e27ceb to
22f7401
Compare
There was a problem hiding this comment.
Thank you @ezgidemirel. We may consider opening a separate bug to track the issue spotted for the LambdaFunctionAssociations.
| // If we have some lambda function associations, we need to late init each one of them | ||
| existing := make(map[string]*svcsdk.LambdaFunctionAssociation) | ||
| for i := range from.Items { | ||
| o := from.Items[i] | ||
| if o.LambdaFunctionARN == nil { | ||
| continue | ||
| } | ||
| // TODO(ezgidemirel): Instead of using FunctionARNs, we should use EventTypes as keys |
There was a problem hiding this comment.
We may consider opening a bug in the repo. We may also consider switching back to dynamic late initialization.
Signed-off-by: ezgidemirel [email protected]
Description of your changes
This PR late initializes ResponseHeadersPolicyID and FunctionAssociation fields.
Fixes #1295
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
Created a distribution with ResponseHeadersPolicyID.