-
Notifications
You must be signed in to change notification settings - Fork 7
add back envoyfilter to add limitador cluster patch #163
Conversation
for _, stage := range RateLimitStages { | ||
wasmKey := kuadrantistioutils.WASMPluginKey(gwKey, stage) | ||
|
||
wasmplugin := &istioextensionv1alpha3.WasmPlugin{} |
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.
downgrade?
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.
The name was wrong. we are importing alpha1
but named it alpha3
.
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.
then the name should be fixed. You were planning to do that in another PR?
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.
By name here, I mean the name of the import i.e. istioextensionv1alpha1
. It's already in this PR.
if wasmPluginDeleted { | ||
ef := &istionetworkingv1alpha3.EnvoyFilter{} | ||
efKey := client.ObjectKey{Namespace: gwKey.Namespace, Name: kuadrantistioutils.LimitadorClusterEnvoyFilterName} | ||
err := r.Client().Get(ctx, efKey, ef) |
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.
why not delete and skip isNotFound errors?
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.
Good find :)
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.
some comments dropped. Looks ok
1243913
to
0260e9d
Compare
#152 Had removed the
EnvoyFilter
resource but due to the reason mentioned on that PR, we need to add it back so that it continues to work when installed using the operator.PR changes can be verified by trying any one of the user guides.