Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

add back envoyfilter to add limitador cluster patch #163

Merged
merged 4 commits into from
Jun 8, 2022

Conversation

rahulanand16nov
Copy link
Contributor

#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.

@rahulanand16nov rahulanand16nov requested a review from eguzki as a code owner June 7, 2022 09:00
for _, stage := range RateLimitStages {
wasmKey := kuadrantistioutils.WASMPluginKey(gwKey, stage)

wasmplugin := &istioextensionv1alpha3.WasmPlugin{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

downgrade?

Copy link
Contributor Author

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.

Copy link
Contributor

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?

Copy link
Contributor Author

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)
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find :)

Copy link
Contributor

@eguzki eguzki left a 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

@rahulanand16nov rahulanand16nov requested a review from eguzki June 7, 2022 14:40
@rahulanand16nov rahulanand16nov force-pushed the add-back-envoy-filter branch from 1243913 to 0260e9d Compare June 8, 2022 12:20
@rahulanand16nov rahulanand16nov merged commit 3216c32 into main Jun 8, 2022
@rahulanand16nov rahulanand16nov deleted the add-back-envoy-filter branch June 8, 2022 14:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants