Kong / kubernetes-ingress-controller

:gorilla: Kong for Kubernetes: The official Ingress Controller for Kubernetes.
https://docs.konghq.com/kubernetes-ingress-controller/
Apache License 2.0
2.16k stars 592 forks source link

`isRemotePluginReferenceAllowed` judges reference permission of plugin and referrer incorrectly #6290

Open randmonkey opened 4 days ago

randmonkey commented 4 days ago

Is there an existing issue for this?

Current Behavior

In the original specification, if a service A in namespace ns refers to plugin B in ns2, there should be a ReferenceGrant in ns2 to allow reference from namespace ns1 and group/kind of A to group/kind (and maybe name) of B: https://gateway-api.sigs.k8s.io/api-types/referencegrant/. Actually, if we create a ReferenceGrant in namespace ns1 and allow reference from ns1 and group/kind of A to group/kind of B, the reference check in isRemotePluginReferenceAllowed passes. While we create it in ns2, the check fails.

Expected Behavior

The reference permission check should pass when the ReferenceGrant is created in the namespace of the plugin.

Steps To Reproduce

1. Create a service or an ingress in namespace foo:
apiVersion: v1
kind: Service
metadata:
  namespace: foo
  name: service-foo
  annotations:
    konghq.com/plugins: bar:plugin
...
2. Create a plugin in namespace bar:
apiVersion: configuration.konghq.com/v1
kind: KongPlugin
metadata:
  namespace: bar
  name: plugin-bar
...
3. Create a ReferenceGrant in namespace bar (plugin's namespace):
apiVersion: gateway.networking.k8s.io
kind: ReferenceGrant
metadata:
  name: bar
  namespace: bar
spec:
  from:
  - group: ""
    kind: Service
    namespace: foo
  to:
  - group: "configuration.konghq.com"
    kind: KongPlugin

The plugin cannot be attached to the service. If we create the ReferenceGrant in namespace foo, the plugin can be attached.

Kong Ingress Controller version

3.2.2

Kubernetes version

Not releavant

Anything else?

No response

randmonkey commented 4 days ago

Can be also reproduced by adding a unit test in internal/dataplane/kongstate/kongstate_test.go :

func TestIsRemotePluginReferenceAllowed(t *testing.T) {
    var (
        serviceTypeMeta = metav1.TypeMeta{
            Kind: "Service",
        }
    )

    testCases := []struct {
        name            string
        referrer        client.Object
        pluginNamespace string
        pluginName      string
        referenceGrants []*gatewayapi.ReferenceGrant
        shouldAllow     bool
    }{
        {
            name: "have reference grant",
            referrer: &corev1.Service{
                TypeMeta: serviceTypeMeta,
                ObjectMeta: metav1.ObjectMeta{
                    Namespace: "foo",
                    Name:      "service-foo",
                },
            },
            pluginNamespace: "bar",
            pluginName:      "plugin-bar",
            referenceGrants: []*gatewayapi.ReferenceGrant{
                {
                    ObjectMeta: metav1.ObjectMeta{
                        Namespace: "bar", // same namespace as plugin; will fail the test. If we change it to `foo` that is the same as the service, it will pass. 
                        Name:      "grant-1",
                    },
                    Spec: gatewayapi.ReferenceGrantSpec{
                        From: []gatewayapi.ReferenceGrantFrom{
                            {
                                Kind:      gatewayapi.Kind("Service"),
                                Namespace: "foo",
                            },
                        },
                        To: []gatewayapi.ReferenceGrantTo{
                            {
                                Group: gatewayapi.Group(kongv1.GroupVersion.Group),
                                Kind:  gatewayapi.Kind("KongPlugin"),
                            },
                        },
                    },
                },
            },
            shouldAllow: true,
        },
    }

    for _, tc := range testCases {
        t.Run(tc.name, func(t *testing.T) {
            s, err := store.NewFakeStore(store.FakeObjects{
                ReferenceGrants: tc.referenceGrants,
            })
            require.NoError(t, err)
            err = isRemotePluginReferenceAllowed(s, pluginReference{
                Referer:   tc.referrer,
                Namespace: tc.pluginNamespace,
                Name:      tc.pluginName,
            })
            if tc.shouldAllow {
                require.NoError(t, err)
            } else {
                require.Error(t, err)
            }
        })
    }
}
randmonkey commented 3 hours ago

More cases about consumer: If we change the To part of created ReferenceGrant in integration test case to TestPluginCrossNamespaceReference to the following:

                    // Totally irrelevant to KongPlugin CRD
                    Group: gatewayapi.Group("konghq.com"),
                    Kind:  gatewayapi.Kind("Kong"),

The test case still passes while there is no ReferenceGrant grants reference from KongConsumer to KongPlugin. While if there is no ReferneceGrant about the Kongconsumer, the access to ingress returns 401 (Is this expected? @rainest )