From 3216c32efa4b5c7ddedd8e7bfeb2b4d840d36670 Mon Sep 17 00:00:00 2001 From: Rahul Anand Date: Wed, 8 Jun 2022 20:06:44 +0530 Subject: [PATCH] add back envoyfilter to add limitador cluster patch (#163) * add back envoyfilter to add limitador cluster patch * update the docs to reflect changes * skip envoyfilter deletion if not present already * fix manifests --- README.md | 7 +- config/deploy/manifests.yaml | 12 ++ config/rbac/role.yaml | 12 ++ .../apim/ratelimitpolicy_controller.go | 55 +++++--- .../apim/ratelimitpolicy_finalizers.go | 36 ++++-- pkg/common/common.go | 3 +- pkg/istio/envoy_filters.go | 118 ++++++++++++++++++ .../local-deployment/deploy-kuadrant-deps.sh | 1 - utils/local-deployment/limitador-cluster.yaml | 27 ---- 9 files changed, 215 insertions(+), 56 deletions(-) create mode 100644 pkg/istio/envoy_filters.go delete mode 100644 utils/local-deployment/limitador-cluster.yaml diff --git a/README.md b/README.md index 34883d1..b425742 100644 --- a/README.md +++ b/README.md @@ -167,8 +167,13 @@ spec: EOF ``` -To verify wasm wasmplugin has been created: +To verify envoyfilter and wasmplugin has been created: +``` +kubectl get envoyfilter -n kuadrant-system +NAME AGE +limitador-cluster-patch 19s +``` ``` kubectl get wasmplugin -A NAMESPACE NAME AGE diff --git a/config/deploy/manifests.yaml b/config/deploy/manifests.yaml index db6102c..35ac5d5 100644 --- a/config/deploy/manifests.yaml +++ b/config/deploy/manifests.yaml @@ -700,6 +700,18 @@ rules: - patch - update - watch +- apiGroups: + - networking.istio.io + resources: + - envoyfilters + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - security.istio.io resources: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 30ce701..a28cfc0 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -91,6 +91,18 @@ rules: - patch - update - watch +- apiGroups: + - networking.istio.io + resources: + - envoyfilters + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - security.istio.io resources: diff --git a/controllers/apim/ratelimitpolicy_controller.go b/controllers/apim/ratelimitpolicy_controller.go index ed7a659..a8c207f 100644 --- a/controllers/apim/ratelimitpolicy_controller.go +++ b/controllers/apim/ratelimitpolicy_controller.go @@ -24,7 +24,8 @@ import ( "github.com/go-logr/logr" "github.com/kuadrant/limitador-operator/api/v1alpha1" - istioextensionv1alpha3 "istio.io/client-go/pkg/apis/extensions/v1alpha1" + istioextensionv1alpha1 "istio.io/client-go/pkg/apis/extensions/v1alpha1" + istionetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3" apierrors "k8s.io/apimachinery/pkg/api/errors" meta "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" @@ -50,6 +51,7 @@ type RateLimitPolicyReconciler struct { //+kubebuilder:rbac:groups=apim.kuadrant.io,resources=ratelimitpolicies,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=apim.kuadrant.io,resources=ratelimitpolicies/status,verbs=get;update;patch //+kubebuilder:rbac:groups=apim.kuadrant.io,resources=ratelimitpolicies/finalizers,verbs=update +//+kubebuilder:rbac:groups=networking.istio.io,resources=envoyfilters,verbs=get;list;watch;create;delete;update;patch //+kubebuilder:rbac:groups=extensions.istio.io,resources=wasmplugins,verbs=get;list;watch;create;delete;update;patch //+kubebuilder:rbac:groups=limitador.kuadrant.io,resources=ratelimits,verbs=get;list;watch;create;update;delete;patch @@ -163,7 +165,6 @@ func (r *RateLimitPolicyReconciler) reconcileSpec(ctx context.Context, rlp *apim if err := r.cleanUpOrphanWASMPlugins(ctx, rlp); err != nil { return ctrl.Result{}, err } - return ctrl.Result{}, nil } @@ -231,15 +232,19 @@ func (r *RateLimitPolicyReconciler) reconcileNetworkResourceBackReference(ctx co return nil } -// Finds gateways with WASMPLugins with rate limit configuration from the current RLP +// Finds gateways with WASMPlugins with rate limit configuration from the current RLP // Delete RL conf from the current RLP from gateways not referenced by the current RLP // Cleans up RL conf when: // - HTTPRoute updates parentRefs (gateways) // - RLP updates targetRef to another HTTPRoute func (r *RateLimitPolicyReconciler) cleanUpOrphanWASMPlugins(ctx context.Context, rlp *apimv1alpha1.RateLimitPolicy) error { logger, _ := logr.FromContext(ctx) + httpRoute, err := r.fetchHTTPRoute(ctx, rlp) + if err != nil { + return err + } - currentGatewayRefs, err := r.gatewayRefList(ctx, rlp) + currentGatewayRefs := r.gatewayRefList(httpRoute) if err != nil { return err } @@ -260,10 +265,11 @@ func (r *RateLimitPolicyReconciler) cleanUpOrphanWASMPlugins(ctx context.Context RateLimitStages := []apimv1alpha1.RateLimitStage{apimv1alpha1.RateLimitStagePREAUTH, apimv1alpha1.RateLimitStagePOSTAUTH} for _, gwKey := range notReferencedGatewayKeys { + wasmPluginDeleted := false for _, stage := range RateLimitStages { wasmKey := kuadrantistioutils.WASMPluginKey(gwKey, stage) - wasmplugin := &istioextensionv1alpha3.WasmPlugin{} + wasmplugin := &istioextensionv1alpha1.WasmPlugin{} err = r.Client().Get(ctx, wasmKey, wasmplugin) logger.V(1).Info("cleanUpOrphanWASMPlugins: get WasmPlugin", "wasmplugin", wasmKey, "err", err) if apierrors.IsNotFound(err) { @@ -273,7 +279,23 @@ func (r *RateLimitPolicyReconciler) cleanUpOrphanWASMPlugins(ctx context.Context if err != nil { return err } - err = r.finalizeSingleWASMPlugins(ctx, rlp, wasmplugin) + wasmPluginDeleted, err = r.finalizeSingleWASMPlugins(ctx, rlp, wasmplugin) + if err != nil { + return err + } + } + + // finalize pre-requisite of WasmPlugin i.e. EnvoyFilter adding the limitador cluster entry + if wasmPluginDeleted { + ef := &istionetworkingv1alpha3.EnvoyFilter{} + efKey := client.ObjectKey{Namespace: gwKey.Namespace, Name: kuadrantistioutils.LimitadorClusterEnvoyFilterName} + err := r.Client().Get(ctx, efKey, ef) + logger.V(1).Info("cleanUpOrphanWASMPlugins: get EnvoyFilter", "envoyfilter", efKey, "err", err) + if apierrors.IsNotFound(err) { + logger.Info("cleanUpOrphanWASMPlugins: envoyfilter not found", "envoyFilter", efKey) + continue + } + err = r.DeleteResource(ctx, ef) if err != nil { return err } @@ -292,7 +314,7 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPlugins(ctx context.Context, rl return err } - currentGatewayRefs, err := r.gatewayRefList(ctx, rlp) + currentGatewayRefs := r.gatewayRefList(httpRoute) if err != nil { return err } @@ -307,6 +329,13 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPlugins(ctx context.Context, rl return err } + // Pre-requisite before WASMPlugins can be installed. + ef := kuadrantistioutils.LimitadorClusterEnvoyFilter(gwKey, gateway.GetLabels()) + err = r.ReconcileResource(ctx, &istionetworkingv1alpha3.EnvoyFilter{}, ef, kuadrantistioutils.AlwaysUpdateEnvoyFilter) + if err != nil { + return err + } + // Reconcile two WasmPlugins per gateway // Gateway API Gateway resource labels will be copied to the deployment in the automated deployment // For the manual deployment, the Gateway resource labels must match deployment/pod labels or WASMPlugins selector will not match @@ -317,7 +346,7 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPlugins(ctx context.Context, rl } for _, wp := range wps { - err = r.ReconcileResource(ctx, &istioextensionv1alpha3.WasmPlugin{}, wp, kuadrantistioutils.WASMPluginMutator) + err = r.ReconcileResource(ctx, &istioextensionv1alpha1.WasmPlugin{}, wp, kuadrantistioutils.WASMPluginMutator) if err != nil { return err } @@ -349,13 +378,7 @@ func (r *RateLimitPolicyReconciler) fetchHTTPRoute(ctx context.Context, rlp *api return httpRoute, nil } -func (r *RateLimitPolicyReconciler) gatewayRefList(ctx context.Context, rlp *apimv1alpha1.RateLimitPolicy) ([]client.ObjectKey, error) { - httpRoute, err := r.fetchHTTPRoute(ctx, rlp) - if err != nil { - // The object should also exist - return nil, err - } - +func (r *RateLimitPolicyReconciler) gatewayRefList(httpRoute *gatewayapiv1alpha2.HTTPRoute) []client.ObjectKey { gwKeys := make([]client.ObjectKey, 0) for _, parentRef := range httpRoute.Spec.CommonRouteSpec.ParentRefs { gwKey := client.ObjectKey{Name: string(parentRef.Name), Namespace: httpRoute.Namespace} @@ -365,7 +388,7 @@ func (r *RateLimitPolicyReconciler) gatewayRefList(ctx context.Context, rlp *api gwKeys = append(gwKeys, gwKey) } - return gwKeys, nil + return gwKeys } func (r *RateLimitPolicyReconciler) validateHTTPRoute(ctx context.Context, rlp *apimv1alpha1.RateLimitPolicy) error { diff --git a/controllers/apim/ratelimitpolicy_finalizers.go b/controllers/apim/ratelimitpolicy_finalizers.go index a402aed..a6d9d22 100644 --- a/controllers/apim/ratelimitpolicy_finalizers.go +++ b/controllers/apim/ratelimitpolicy_finalizers.go @@ -6,6 +6,7 @@ import ( "github.com/go-logr/logr" istioextensionv1alpha3 "istio.io/client-go/pkg/apis/extensions/v1alpha1" + istionetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3" apierrors "k8s.io/apimachinery/pkg/api/errors" "sigs.k8s.io/controller-runtime/pkg/client" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -54,6 +55,7 @@ func (r *RateLimitPolicyReconciler) finalizeWASMPlugins(ctx context.Context, rlp return err } + wasmPluginDeleted := false for _, desiredWP := range desiredWPs { wasmPlugin := &istioextensionv1alpha3.WasmPlugin{} err = r.Client().Get(ctx, client.ObjectKeyFromObject(desiredWP), wasmPlugin) @@ -65,7 +67,23 @@ func (r *RateLimitPolicyReconciler) finalizeWASMPlugins(ctx context.Context, rlp if err != nil { return err } - err = r.finalizeSingleWASMPlugins(ctx, rlp, wasmPlugin) + wasmPluginDeleted, err = r.finalizeSingleWASMPlugins(ctx, rlp, wasmPlugin) + if err != nil { + return err + } + } + + // finalize pre-requisite of WasmPlugin i.e. EnvoyFilter adding the limitador cluster entry + if wasmPluginDeleted { + ef := &istionetworkingv1alpha3.EnvoyFilter{} + efKey := client.ObjectKey{Namespace: gwKey.Namespace, Name: kuadrantistioutils.LimitadorClusterEnvoyFilterName} + err := r.Client().Get(ctx, efKey, ef) + logger.V(1).Info("finalizeWASMPlugins: get EnvoyFilter", "envoyfilter", efKey, "err", err) + if apierrors.IsNotFound(err) { + logger.Info("finalizeWASMPlugins: envoyfilter not found", "envoyFilter", efKey) + continue + } + err = r.DeleteResource(ctx, ef) if err != nil { return err } @@ -76,20 +94,20 @@ func (r *RateLimitPolicyReconciler) finalizeWASMPlugins(ctx context.Context, rlp } // finalizeSingleWASMPlugins removes the configuration of this RLP -// If the WASMPlugin ends up with empty conf, the resource will be removed. +// If the WASMPlugin ends up with empty conf, the resource and its pre-requisite will be removed. func (r *RateLimitPolicyReconciler) finalizeSingleWASMPlugins( ctx context.Context, rlp *apimv1alpha1.RateLimitPolicy, wasmPlugin *istioextensionv1alpha3.WasmPlugin, -) error { +) (bool, error) { updateObject := false configEmpty := false // Deserialize config into PluginConfig struct configJSON, err := wasmPlugin.Spec.PluginConfig.MarshalJSON() if err != nil { - return err + return false, err } pluginConfig := &kuadrantistioutils.PluginConfig{} if err := json.Unmarshal(configJSON, pluginConfig); err != nil { - return err + return false, err } pluginKey := client.ObjectKeyFromObject(rlp).String() @@ -98,7 +116,7 @@ func (r *RateLimitPolicyReconciler) finalizeSingleWASMPlugins( updateObject = true finalPluginConfig, err := kuadrantistioutils.PluginConfigToWasmPluginStruct(pluginConfig) if err != nil { - return err + return false, err } wasmPlugin.Spec.PluginConfig = finalPluginConfig @@ -106,12 +124,12 @@ func (r *RateLimitPolicyReconciler) finalizeSingleWASMPlugins( } if configEmpty { - return r.DeleteResource(ctx, wasmPlugin) + return true, r.DeleteResource(ctx, wasmPlugin) } else if updateObject { - return r.UpdateResource(ctx, wasmPlugin) + return false, r.UpdateResource(ctx, wasmPlugin) } - return nil + return false, nil } func (r *RateLimitPolicyReconciler) deleteRateLimits(ctx context.Context, rlp *apimv1alpha1.RateLimitPolicy) error { diff --git a/pkg/common/common.go b/pkg/common/common.go index 81de193..db017ed 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -30,8 +30,7 @@ const ( KuadrantAuthorizationProvider = "kuadrant-authorization" LimitadorServiceGrpcPort = 8081 - HTTPRouteKind = "HTTPRoute" - VirtualServiceKind = "VirtualService" + HTTPRouteKind = "HTTPRoute" KuadrantManagedLabel = "kuadrant.io/managed" KuadrantAuthProviderAnnotation = "kuadrant.io/auth-provider" diff --git a/pkg/istio/envoy_filters.go b/pkg/istio/envoy_filters.go new file mode 100644 index 0000000..265532a --- /dev/null +++ b/pkg/istio/envoy_filters.go @@ -0,0 +1,118 @@ +package istio + +import ( + "encoding/json" + "fmt" + + istioapiv1alpha3 "istio.io/api/networking/v1alpha3" + istionetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/kuadrant/kuadrant-controller/pkg/common" +) + +const LimitadorClusterEnvoyFilterName = "limitador-cluster-patch" + +type EnvoyFilterFactory struct { + ObjectName string + Namespace string + Patches []*istioapiv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch + Labels map[string]string +} + +func (v *EnvoyFilterFactory) EnvoyFilter() *istionetworkingv1alpha3.EnvoyFilter { + return &istionetworkingv1alpha3.EnvoyFilter{ + TypeMeta: metav1.TypeMeta{ + Kind: "EnvoyFilter", + APIVersion: "networking.istio.io/v1alpha3", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: v.ObjectName, + Namespace: v.Namespace, + }, + Spec: istioapiv1alpha3.EnvoyFilter{ + WorkloadSelector: &istioapiv1alpha3.WorkloadSelector{ + Labels: v.Labels, + }, + ConfigPatches: v.Patches, + }, + } +} + +// LimitadorClusterEnvoyFilter returns an EnvoyFilter that adds a custom cluster entry to compensate for kuadrant/limitador#53. +// Note: This should be removed once the mentioned issue is fixed but that will take some time. +func LimitadorClusterEnvoyFilter(gwKey client.ObjectKey, labels map[string]string) *istionetworkingv1alpha3.EnvoyFilter { + // The patch defines the rate_limit_cluster, which provides the endpoint location of the external rate limit service. + patchUnstructured := map[string]interface{}{ + "operation": "ADD", + "value": map[string]interface{}{ + "name": PatchedLimitadorClusterName, + "type": "STRICT_DNS", + "connect_timeout": "1s", + "lb_policy": "ROUND_ROBIN", + "http2_protocol_options": map[string]interface{}{}, + "load_assignment": map[string]interface{}{ + "cluster_name": PatchedLimitadorClusterName, + "endpoints": []map[string]interface{}{ + { + "lb_endpoints": []map[string]interface{}{ + { + "endpoint": map[string]interface{}{ + "address": map[string]interface{}{ + "socket_address": map[string]interface{}{ + "address": common.LimitadorServiceClusterHost, + "port_value": common.LimitadorServiceGrpcPort, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + patchRaw, _ := json.Marshal(patchUnstructured) + + patch := &istioapiv1alpha3.EnvoyFilter_Patch{} + err := patch.UnmarshalJSON(patchRaw) + if err != nil { + panic(err) + } + + envoyFilterFactory := EnvoyFilterFactory{ + ObjectName: LimitadorClusterEnvoyFilterName, + Namespace: gwKey.Namespace, + Patches: []*istioapiv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch{{ + ApplyTo: istioapiv1alpha3.EnvoyFilter_CLUSTER, + Match: &istioapiv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch{ + ObjectTypes: &istioapiv1alpha3.EnvoyFilter_EnvoyConfigObjectMatch_Cluster{ + Cluster: &istioapiv1alpha3.EnvoyFilter_ClusterMatch{ + Service: common.LimitadorServiceClusterHost, + }, + }, + }, + Patch: patch, + }}, + } + return envoyFilterFactory.EnvoyFilter() +} + +func AlwaysUpdateEnvoyFilter(existingObj, desiredObj client.Object) (bool, error) { + existing, ok := existingObj.(*istionetworkingv1alpha3.EnvoyFilter) + if !ok { + return false, fmt.Errorf("%T is not a *istionetworkingv1alpha3.EnvoyFilter", existingObj) + } + desired, ok := desiredObj.(*istionetworkingv1alpha3.EnvoyFilter) + if !ok { + return false, fmt.Errorf("%T is not a *istionetworkingv1alpha3.EnvoyFilter", desiredObj) + } + existing.Spec = istioapiv1alpha3.EnvoyFilter{ + WorkloadSelector: desired.Spec.WorkloadSelector, + ConfigPatches: desired.Spec.ConfigPatches, + Priority: desired.Spec.Priority, + } + return true, nil +} diff --git a/utils/local-deployment/deploy-kuadrant-deps.sh b/utils/local-deployment/deploy-kuadrant-deps.sh index 6db6bbb..6406683 100755 --- a/utils/local-deployment/deploy-kuadrant-deps.sh +++ b/utils/local-deployment/deploy-kuadrant-deps.sh @@ -28,7 +28,6 @@ kubectl apply -f utils/local-deployment/istio-manifests/autogenerated/Base/Base. kubectl apply -f utils/local-deployment/istio-manifests/autogenerated/Base/Pilot/Pilot.yaml kubectl apply -f utils/local-deployment/istio-manifests/autogenerated/Base/Pilot/IngressGateways/IngressGateways.yaml kubectl apply -n "${KUADRANT_NAMESPACE}" -f utils/local-deployment/istio-manifests/kuadrant-gateway.yaml -kubectl apply -n "${KUADRANT_NAMESPACE}" -f utils/local-deployment/limitador-cluster.yaml echo "Installing Gateway API CRDs and Applying the Gateway resource" kubectl apply -f utils/local-deployment/gatewayapi-manifests/autogenerated/Base.yaml diff --git a/utils/local-deployment/limitador-cluster.yaml b/utils/local-deployment/limitador-cluster.yaml deleted file mode 100644 index c6885e1..0000000 --- a/utils/local-deployment/limitador-cluster.yaml +++ /dev/null @@ -1,27 +0,0 @@ -apiVersion: networking.istio.io/v1alpha3 -kind: EnvoyFilter -metadata: - name: kuadrant-limitador-cluster-entry -spec: - configPatches: - - applyTo: CLUSTER - match: - cluster: - service: limitador.kuadrant-system.svc.cluster.local - patch: - operation: ADD - value: - connect_timeout: 1s - http2_protocol_options: {} - lb_policy: ROUND_ROBIN - load_assignment: - cluster_name: rate-limit-cluster - endpoints: - - lb_endpoints: - - endpoint: - address: - socket_address: - address: limitador.kuadrant-system.svc.cluster.local - port_value: 8081 - name: rate-limit-cluster - type: STRICT_DNS \ No newline at end of file