-
Notifications
You must be signed in to change notification settings - Fork 337
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
feat(kuma-cp) virtual host modifications #909
Conversation
7a63101
to
e830db9
Compare
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.
Looks good.
default: | ||
return errors.Errorf("invalid modification type %q", modification.Type) | ||
return errors.Errorf("invalid modification type %T", modification.Type) |
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.
👍
Probably even makes sense to skip VirtualHost as well and give an ability to modify only Routes of |
But you can remove default virtual hosts and add new ones. |
Yes, but in this case, you have to manually write all stuff that Kuma does automatically here: func (c RouteConfigurer) routeAction() *envoy_route.RouteAction {
routeAction := envoy_route.RouteAction{}
if len(c.subsets) == 1 {
routeAction.ClusterSpecifier = &envoy_route.RouteAction_Cluster{
Cluster: c.subsets[0].ClusterName,
}
routeAction.MetadataMatch = envoy_common.LbMetadata(c.subsets[0].Tags)
} else {
var weightedClusters []*envoy_route.WeightedCluster_ClusterWeight
var totalWeight uint32
for _, subset := range c.subsets {
weightedClusters = append(weightedClusters, &envoy_route.WeightedCluster_ClusterWeight{
Name: subset.ClusterName,
Weight: &wrappers.UInt32Value{Value: subset.Weight},
MetadataMatch: envoy_common.LbMetadata(subset.Tags),
})
totalWeight += subset.Weight
}
routeAction.ClusterSpecifier = &envoy_route.RouteAction_WeightedClusters{
WeightedClusters: &envoy_route.WeightedCluster{
Clusters: weightedClusters,
TotalWeight: &wrappers.UInt32Value{Value: totalWeight},
},
}
}
return &routeAction
} Is it correct? |
Sure, but if you are going with rewriting VirtualHosts you probably want that. And if you want to patch routes fields - like I said in the description, most of the settings are available on VirtualHost object |
…-virtualhost Signed-off-by: Jakub Dyszkiewicz <[email protected]>
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
0a5a444
to
25c130d
Compare
…-virtualhost Signed-off-by: Jakub Dyszkiewicz <[email protected]>
Signed-off-by: Jakub Dyszkiewicz <[email protected]>
Summary
Virtual Host modification
I think VirtualHost is enough for RDS modifications.
We can skip RouteConfiguration because VirtualHost offers all headers to add.
Most of the operations on Routes are possible from VirtualHost (you can remove VirtualHost and add another one with a different list of routes, most settings on routes like retries are available on VirtualHost)
Documentation
kumahq/kuma-website#238