DFP proposal #10596
DFP proposal #10596
Conversation
| if !p.needFilter[fc.FilterChainName] { | ||
| return nil, nil | ||
| var ret []plugins.StagedHttpFilter | ||
| if p.needAwsFilter[fc.FilterChainName] { |
There was a problem hiding this comment.
does it matter in which order we add the aws / dfp filters?
There was a problem hiding this comment.
i dont think so; DFP filter only takes affect on DFP cluster. aws filter won't be active for those.. so they should never work at the same time.
| needAwsFilter map[string]bool | ||
| neededDfpFilter map[string]map[string]*envoydfp.FilterConfig |
There was a problem hiding this comment.
can you add a comment on what the keys/values are?
| } | ||
| var anyCluster anypb.Any | ||
| err := anypb.MarshalFrom(&anyCluster, c, proto.MarshalOptions{Deterministic: true}) | ||
| // error should never happen here. panic? |
There was a problem hiding this comment.
can we have processDFPRoute/processDFPCluster return an error instead?
| @@ -62,7 +64,8 @@ func (d *upstreamDestination) Equals(in any) bool { | |||
| } | |||
|
|
|||
| type UpstreamIr struct { | |||
| AwsSecret *ir.Secret | |||
| AwsSecret *ir.Secret | |||
| dfpFilterConfig *envoydfp.FilterConfig | |||
There was a problem hiding this comment.
should this field be exported?
There was a problem hiding this comment.
i don't think so, this struct is the IR (internal representation) that is only used inside this plug-in, and is meant to remain opaque to the rest of the system.
There was a problem hiding this comment.
in that case should awsSecret also be private?
very rough draft - mainly to get early feedback.
Adding dynamic forward proxy support.
Notes:
Unlike the previous impl that represents DFP as a route action, this one represents it as an upstream type. This makes more sense for the k8s-GW-API, as we cannot add custom route actions.
For every DFP upstream we add a DFP cluster and a filter on the relevant filter chain.