Kong / kubernetes-configuration

MIT License
3 stars 0 forks source link

How to configure `KongPluginBindings` to be global #7

Open mlavacca opened 1 month ago

mlavacca commented 1 month ago

Problem Statement

https://github.com/Kong/kubernetes-configuration/pull/1 introduced the new KongPluginBinding CRD to allow users to bind plugins to specific kong resources. The field PluginRef has been added to the .spec field, and such a field contains the Kind along with the plugin Name. Such a kind field is meant to be used to either refer to KongPlugin or KongClusterPlugin.

Since https://docs.konghq.com/gateway/latest/key-concepts/plugins/#precedence defines the precedence for each entity that can have plugins attached, we could think of just using the KongPluginBinding resource to determine the scope of a plugin. In konnect, no entity attached to a plugin means "global" and so we could do.

A separate effort is about using the pluginBinding to configure regular Kubernetes resources in a Gateway, but that's a separate effort described in https://github.com/Kong/kubernetes-configuration/issues/8.

Proposed Solution

Additional Information

Acceptance Criteria

pmalek commented 1 month ago

I believe that making the binding explicitly global would be beneficial. So something along these lines should work:

diff --git a/api/configuration/v1alpha1/kongpluginbinding_types.go b/api/configuration/v1alpha1/kongpluginbinding_types.go
index 2c9b3dd..33c02ec 100644
--- a/api/configuration/v1alpha1/kongpluginbinding_types.go
+++ b/api/configuration/v1alpha1/kongpluginbinding_types.go
@@ -60,6 +60,14 @@ func (c *KongPluginBinding) SetConditions(conditions []metav1.Condition) {

 // KongPluginBindingSpec defines specification of a KongPluginBinding.
 type KongPluginBindingSpec struct {
+   // Global indicates if the plugin is global or not.
+   // If set to true, the plugin will be applied to all entities in the Kong
+   // cluster. If set to false, the plugin will be applied only to the entities
+   // specified in the references.
+   //
+   // +kubebuilder:default:=false
+   Global bool `json:"global"`
+
    // PluginReference is a reference to the KongPlugin or KongClusterPlugin resource. It is required
    PluginReference PluginRef `json:"pluginRef"`

WDYT?

The only thing that I believe we can't (easily) do now is to block more than 1 global plugin on admission. Neither CEL on a CRD nor ValidatingAdmissionPolicy allow inspecting other objects during admission.

Given this I think the only option to enforce this would be to use the admission webhook.

mlavacca commented 1 month ago

Re-opening as the addition of a global field in the KongPluginBinding breaks the namespace isolation of a cluster: a namespaces resource shouldn't be able to affect resources belonging to different namespaces.

We may need to introduce a new cluster-wide CRD named KongClusterPluginBinding with no targets. Such a CRD will allow to set a KongClusterPlugin as global, in the context of both KIC and Konnect.

pmalek commented 1 month ago

Meeting with @mlavacca today we've decided to not implement this ATM as this would have more implications on crossing the namespace boundaries.

Let's postpone this until we have product demand for this.