diff --git a/internal/dataplane/kongstate/kongstate.go b/internal/dataplane/kongstate/kongstate.go index 46d97c5354..6facb85364 100644 --- a/internal/dataplane/kongstate/kongstate.go +++ b/internal/dataplane/kongstate/kongstate.go @@ -389,76 +389,9 @@ func buildPlugins( plugins = append(plugins, plugin) } } - - globalPlugins, err := globalPlugins(log, s) - if err != nil { - log.WithError(err).Error("failed to fetch global plugins") - } - // global plugins have no instance_name transform as they can only be applied once - plugins = append(plugins, globalPlugins...) - return plugins } -func globalPlugins(log logrus.FieldLogger, s store.Storer) ([]Plugin, error) { - // removed as of 0.10.0 - // only retrieved now to warn users - globalPlugins, err := s.ListGlobalKongPlugins() - if err != nil { - return nil, fmt.Errorf("error listing global KongPlugins: %w", err) - } - if len(globalPlugins) > 0 { - log.Warning("global KongPlugins found. These are no longer applied and", - " must be replaced with KongClusterPlugins.", - " Please run \"kubectl get kongplugin -l global=true --all-namespaces\" to list existing plugins") - } - res := make(map[string]Plugin) - var duplicates []string // keep track of duplicate - // TODO respect the oldest CRD - // Current behavior is to skip creating the plugin but in case - // of duplicate plugin definitions, we should respect the oldest one - // This is important since if a user comes in to k8s and creates a new - // CRD, the user now deleted an older plugin - - globalClusterPlugins, err := s.ListGlobalKongClusterPlugins() - if err != nil { - return nil, fmt.Errorf("error listing global KongClusterPlugins: %w", err) - } - for i := 0; i < len(globalClusterPlugins); i++ { - k8sPlugin := *globalClusterPlugins[i] - pluginName := k8sPlugin.PluginName - // empty pluginName skip it - if pluginName == "" { - log.WithFields(logrus.Fields{ - "kongclusterplugin_name": k8sPlugin.Name, - }).Errorf("invalid KongClusterPlugin: empty plugin property") - continue - } - if _, ok := res[pluginName]; ok { - log.Error("multiple KongPlugin definitions found with"+ - " 'global' label for '", pluginName, - "', the plugin will not be applied") - duplicates = append(duplicates, pluginName) - continue - } - if plugin, err := kongPluginFromK8SClusterPlugin(s, k8sPlugin); err == nil { - res[pluginName] = plugin - } else { - log.WithFields(logrus.Fields{ - "kongclusterplugin_name": k8sPlugin.Name, - }).WithError(err).Error("failed to generate configuration from KongClusterPlugin") - } - } - for _, plugin := range duplicates { - delete(res, plugin) - } - var plugins []Plugin - for _, p := range res { - plugins = append(plugins, p) - } - return plugins, nil -} - func (ks *KongState) FillPlugins( log logrus.FieldLogger, s store.Storer, diff --git a/internal/dataplane/parser/parser_test.go b/internal/dataplane/parser/parser_test.go index f8e2309fb4..cd146d9f73 100644 --- a/internal/dataplane/parser/parser_test.go +++ b/internal/dataplane/parser/parser_test.go @@ -36,47 +36,6 @@ import ( "github.com/kong/kubernetes-ingress-controller/v2/test/helpers/certificate" ) -func TestGlobalPlugin(t *testing.T) { - assert := assert.New(t) - t.Run("global plugins are processed correctly", func(t *testing.T) { - store, err := store.NewFakeStore(store.FakeObjects{ - KongClusterPlugins: []*kongv1.KongClusterPlugin{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "bar-plugin", - Labels: map[string]string{ - "global": "true", - }, - Annotations: map[string]string{ - annotations.IngressClassKey: annotations.DefaultIngressClass, - }, - }, - Protocols: kongv1.StringsToKongProtocols([]string{"http"}), - PluginName: "basic-auth", - Config: apiextensionsv1.JSON{ - Raw: []byte(`{"foo1": "bar1"}`), - }, - }, - }, - }) - require.NoError(t, err) - p := mustNewParser(t, store) - result := p.BuildKongConfig() - require.Empty(t, result.TranslationFailures) - state := result.KongState - require.NotNil(t, state) - assert.Equal(1, len(state.Plugins), - "expected one plugin to be rendered") - - sort.SliceStable(state.Plugins, func(i, j int) bool { - return strings.Compare(*state.Plugins[i].Name, *state.Plugins[j].Name) > 0 - }) - - assert.Equal("basic-auth", *state.Plugins[0].Name) - assert.Equal(kong.Configuration{"foo1": "bar1"}, state.Plugins[0].Config) - }) -} - func TestSecretConfigurationPlugin(t *testing.T) { jwtPluginConfig := `{"run_on_preflight": false}` // JSON basicAuthPluginConfig := "hide_credentials: true" // YAML @@ -182,30 +141,7 @@ func TestSecretConfigurationPlugin(t *testing.T) { objects.KongClusterPlugins = []*kongv1.KongClusterPlugin{ { ObjectMeta: metav1.ObjectMeta{ - Name: "global-bar-plugin", - Labels: map[string]string{ - "global": "true", - }, - Annotations: map[string]string{ - annotations.IngressClassKey: annotations.DefaultIngressClass, - }, - }, - Protocols: kongv1.StringsToKongProtocols([]string{"http"}), - PluginName: "basic-auth", - ConfigFrom: &kongv1.NamespacedConfigSource{ - SecretValue: kongv1.NamespacedSecretValueFromSource{ - Key: "basic-auth-config", - Secret: "conf-secret", - Namespace: "default", - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "global-broken-bar-plugin", - Labels: map[string]string{ - "global": "true", - }, + Name: "broken-bar-plugin", Annotations: map[string]string{ // explicitly none, this should not get rendered }, @@ -256,8 +192,7 @@ func TestSecretConfigurationPlugin(t *testing.T) { require.NoError(t, err) state := result.KongState require.NotNil(t, state) - assert.Equal(3, len(state.Plugins), - "expected three plugins to be rendered") + require.Len(t, state.Plugins, 2) sort.SliceStable(state.Plugins, func(i, j int) bool { return strings.Compare(*state.Plugins[i].Name, @@ -269,32 +204,13 @@ func TestSecretConfigurationPlugin(t *testing.T) { assert.Equal("basic-auth", *state.Plugins[1].Name) assert.Equal(kong.Configuration{"hide_credentials": true}, - state.Plugins[2].Config) - assert.Equal("basic-auth", *state.Plugins[2].Name) - assert.Equal(kong.Configuration{"hide_credentials": true}, - state.Plugins[2].Config) + state.Plugins[1].Config) }) t.Run("plugins with missing secrets or keys are not constructed", func(t *testing.T) { objects := stock objects.KongPlugins = []*kongv1.KongPlugin{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "global-foo-plugin", - Namespace: "default", - Labels: map[string]string{ - "global": "true", - }, - }, - PluginName: "jwt", - ConfigFrom: &kongv1.ConfigSource{ - SecretValue: kongv1.SecretValueFromSource{ - Key: "missing-key", - Secret: "conf-secret", - }, - }, - }, { ObjectMeta: metav1.ObjectMeta{ Name: "foo-plugin", @@ -310,23 +226,6 @@ func TestSecretConfigurationPlugin(t *testing.T) { }, } objects.KongClusterPlugins = []*kongv1.KongClusterPlugin{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "global-bar-plugin", - Labels: map[string]string{ - "global": "true", - }, - }, - Protocols: kongv1.StringsToKongProtocols([]string{"http"}), - PluginName: "basic-auth", - ConfigFrom: &kongv1.NamespacedConfigSource{ - SecretValue: kongv1.NamespacedSecretValueFromSource{ - Key: "basic-auth-config", - Secret: "missing-secret", - Namespace: "default", - }, - }, - }, { ObjectMeta: metav1.ObjectMeta{ Name: "bar-plugin", @@ -363,33 +262,13 @@ func TestSecretConfigurationPlugin(t *testing.T) { require.NoError(t, err) state := result.KongState require.NotNil(t, state) - assert.Equal(0, len(state.Plugins), - "expected no plugins to be rendered") + require.Len(t, state.Plugins, 0) }) t.Run("plugins with both config and configFrom are not constructed", func(t *testing.T) { objects := stock objects.KongPlugins = []*kongv1.KongPlugin{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "global-foo-plugin", - Namespace: "default", - Labels: map[string]string{ - "global": "true", - }, - }, - PluginName: "jwt", - Config: apiextensionsv1.JSON{ - Raw: []byte(`{"fake": true}`), - }, - ConfigFrom: &kongv1.ConfigSource{ - SecretValue: kongv1.SecretValueFromSource{ - Key: "jwt-config", - Secret: "conf-secret", - }, - }, - }, { ObjectMeta: metav1.ObjectMeta{ Name: "foo-plugin", @@ -408,26 +287,6 @@ func TestSecretConfigurationPlugin(t *testing.T) { }, } objects.KongClusterPlugins = []*kongv1.KongClusterPlugin{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "global-bar-plugin", - Labels: map[string]string{ - "global": "true", - }, - }, - Protocols: kongv1.StringsToKongProtocols([]string{"http"}), - PluginName: "basic-auth", - Config: apiextensionsv1.JSON{ - Raw: []byte(`{"fake": true}`), - }, - ConfigFrom: &kongv1.NamespacedConfigSource{ - SecretValue: kongv1.NamespacedSecretValueFromSource{ - Key: "basic-auth-config", - Secret: "conf-secret", - Namespace: "default", - }, - }, - }, { ObjectMeta: metav1.ObjectMeta{ Name: "bar-plugin", @@ -538,11 +397,8 @@ func TestSecretConfigurationPlugin(t *testing.T) { objects.KongPlugins = []*kongv1.KongPlugin{ { ObjectMeta: metav1.ObjectMeta{ - Name: "global-foo-plugin", + Name: "foo-plugin", Namespace: "default", - Labels: map[string]string{ - "global": "true", - }, }, PluginName: "jwt", ConfigFrom: &kongv1.ConfigSource{ @@ -554,7 +410,7 @@ func TestSecretConfigurationPlugin(t *testing.T) { }, { ObjectMeta: metav1.ObjectMeta{ - Name: "foo-plugin", + Name: "bar-plugin", Namespace: "default", }, PluginName: "jwt", @@ -569,10 +425,7 @@ func TestSecretConfigurationPlugin(t *testing.T) { objects.KongClusterPlugins = []*kongv1.KongClusterPlugin{ { ObjectMeta: metav1.ObjectMeta{ - Name: "global-bar-plugin", - Labels: map[string]string{ - "global": "true", - }, + Name: "foo-plugin", }, Protocols: kongv1.StringsToKongProtocols([]string{"http"}), PluginName: "basic-auth", @@ -619,8 +472,7 @@ func TestSecretConfigurationPlugin(t *testing.T) { require.Empty(t, result.TranslationFailures) state := result.KongState require.NotNil(t, state) - assert.Equal(0, len(state.Plugins), - "expected no plugins to be rendered") + require.Empty(t, state.Plugins) }) } diff --git a/internal/store/fake_store_test.go b/internal/store/fake_store_test.go index c6b079c464..e7c5d44ed5 100644 --- a/internal/store/fake_store_test.go +++ b/internal/store/fake_store_test.go @@ -515,7 +515,6 @@ func TestFakeStoreConsumerGroup(t *testing.T) { } func TestFakeStorePlugins(t *testing.T) { - assert := assert.New(t) require := require.New(t) plugins := []*kongv1.KongPlugin{ @@ -527,32 +526,18 @@ func TestFakeStorePlugins(t *testing.T) { }, } store, err := NewFakeStore(FakeObjects{KongPlugins: plugins}) - assert.Nil(err) - assert.NotNil(store) - - plugins = []*kongv1.KongPlugin{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "baz", - Namespace: "default", - }, - }, - } - store, err = NewFakeStore(FakeObjects{KongPlugins: plugins}) - require.Nil(err) + require.NoError(err) require.NotNil(store) - plugins, err = store.ListGlobalKongPlugins() - assert.NoError(err) - assert.Len(plugins, 0) + + plugins = store.ListKongPlugins() + require.Len(plugins, 1) plugin, err := store.GetKongPlugin("default", "does-not-exist") - assert.NotNil(err) - assert.True(errors.As(err, &ErrNotFound{})) - assert.Nil(plugin) + require.ErrorAs(err, &ErrNotFound{}) + require.Nil(plugin) } func TestFakeStoreClusterPlugins(t *testing.T) { - assert := assert.New(t) require := require.New(t) plugins := []*kongv1.KongClusterPlugin{ @@ -563,54 +548,37 @@ func TestFakeStoreClusterPlugins(t *testing.T) { }, } store, err := NewFakeStore(FakeObjects{KongClusterPlugins: plugins}) - require.Nil(err) + require.NoError(err) require.NotNil(store) - plugins, err = store.ListGlobalKongClusterPlugins() - assert.NoError(err) - assert.Len(plugins, 0) plugins = []*kongv1.KongClusterPlugin{ { ObjectMeta: metav1.ObjectMeta{ Name: "foo", - Labels: map[string]string{ - "global": "true", - }, Annotations: map[string]string{ annotations.IngressClassKey: annotations.DefaultIngressClass, }, }, }, { - // invalid due to lack of class, not loaded + // Invalid due to lack of class, not loaded. ObjectMeta: metav1.ObjectMeta{ - Name: "bar", - Labels: map[string]string{ - "global": "true", - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "baz", + Name: "broken-bar", }, }, } store, err = NewFakeStore(FakeObjects{KongClusterPlugins: plugins}) - require.Nil(err) + require.NoError(err) require.NotNil(store) - plugins, err = store.ListGlobalKongClusterPlugins() - assert.NoError(err) - assert.Len(plugins, 1) + require.Len(store.ListKongClusterPlugins(), 1) plugin, err := store.GetKongClusterPlugin("foo") - assert.NotNil(plugin) - assert.Nil(err) + require.NoError(err) + require.NotNil(plugin) plugin, err = store.GetKongClusterPlugin("does-not-exist") - assert.NotNil(err) - assert.True(errors.As(err, &ErrNotFound{})) - assert.Nil(plugin) + require.ErrorAs(err, &ErrNotFound{}) + require.Nil(plugin) } func TestFakeStoreSecret(t *testing.T) { diff --git a/internal/store/store.go b/internal/store/store.go index dfa90d625d..08791f7dd8 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -95,8 +95,6 @@ type Storer interface { ListTCPIngresses() ([]*kongv1beta1.TCPIngress, error) ListUDPIngresses() ([]*kongv1beta1.UDPIngress, error) ListKnativeIngresses() ([]*knative.Ingress, error) - ListGlobalKongPlugins() ([]*kongv1.KongPlugin, error) - ListGlobalKongClusterPlugins() ([]*kongv1.KongClusterPlugin, error) ListKongPlugins() []*kongv1.KongPlugin ListKongClusterPlugins() []*kongv1.KongClusterPlugin ListKongConsumers() []*kongv1.KongConsumer @@ -927,55 +925,6 @@ func (s Store) ListKongConsumerGroups() []*kongv1beta1.KongConsumerGroup { return consumerGroups } -// ListGlobalKongPlugins returns all KongPlugin resources -// filtered by the ingress.class annotation and with the -// label global:"true". -// Support for these global namespaced KongPlugins was removed in 0.10.0 -// This function remains only to provide warnings to users with old configuration. -func (s Store) ListGlobalKongPlugins() ([]*kongv1.KongPlugin, error) { - var plugins []*kongv1.KongPlugin - req, err := labels.NewRequirement("global", selection.Equals, []string{"true"}) - if err != nil { - return nil, err - } - err = cache.ListAll(s.stores.Plugin, - labels.NewSelector().Add(*req), - func(ob interface{}) { - p, ok := ob.(*kongv1.KongPlugin) - if ok && s.isValidIngressClass(&p.ObjectMeta, annotations.IngressClassKey, s.getIngressClassHandling()) { - plugins = append(plugins, p) - } - }) - if err != nil { - return nil, err - } - return plugins, nil -} - -// ListGlobalKongClusterPlugins returns all KongClusterPlugin resources -// filtered by the ingress.class annotation and with the -// label global:"true". -func (s Store) ListGlobalKongClusterPlugins() ([]*kongv1.KongClusterPlugin, error) { - var plugins []*kongv1.KongClusterPlugin - - req, err := labels.NewRequirement("global", selection.Equals, []string{"true"}) - if err != nil { - return nil, err - } - err = cache.ListAll(s.stores.ClusterPlugin, - labels.NewSelector().Add(*req), - func(ob interface{}) { - p, ok := ob.(*kongv1.KongClusterPlugin) - if ok && s.isValidIngressClass(&p.ObjectMeta, annotations.IngressClassKey, s.getIngressClassHandling()) { - plugins = append(plugins, p) - } - }) - if err != nil { - return nil, err - } - return plugins, nil -} - // ListKongClusterPlugins lists all KongClusterPlugins that match expected ingress.class annotation. func (s Store) ListKongClusterPlugins() []*kongv1.KongClusterPlugin { var plugins []*kongv1.KongClusterPlugin diff --git a/test/e2e/features_test.go b/test/e2e/features_test.go index cfc32511ce..bf615fa4d7 100644 --- a/test/e2e/features_test.go +++ b/test/e2e/features_test.go @@ -493,13 +493,10 @@ func TestDefaultIngressClass(t *testing.T) { t.Log("getting kong proxy IP after LB provisioning") proxyURLForDefaultIngress := "http://" + getKongProxyIP(ctx, t, env) - t.Log("creating classless global KongClusterPlugin") - kongclusterplugin := &kongv1.KongClusterPlugin{ + t.Log("creating classless KongClusterPlugin") + kongClusterPlugin := &kongv1.KongClusterPlugin{ ObjectMeta: metav1.ObjectMeta{ Name: "test", - Labels: map[string]string{ - "global": "true", - }, }, PluginName: "cors", Config: apiextensionsv1.JSON{ @@ -508,7 +505,7 @@ func TestDefaultIngressClass(t *testing.T) { } c, err := clientset.NewForConfig(env.Cluster().Config()) require.NoError(t, err) - _, err = c.ConfigurationV1().KongClusterPlugins().Create(ctx, kongclusterplugin, metav1.CreateOptions{}) + _, err = c.ConfigurationV1().KongClusterPlugins().Create(ctx, kongClusterPlugin, metav1.CreateOptions{}) require.NoError(t, err) t.Log("waiting for routes from Ingress to be operational")