feat: propagate translation failures for KongPlugin and KongClusterPlugin

CHANGELOG.md

@@ -138,6 +138,11 @@ Adding a new version? You'll need three changes:
  `Programmed` condition of an object when `kubectl get` is used.
  [#4425](https:/Kong/kubernetes-ingress-controller/pull/4425)
  [#4423](https:/Kong/kubernetes-ingress-controller/pull/4423)
+- Parser instead of logging errors for invalid `KongPlugin` or `KongClusterPlugin`
+ configuration, will now propagate a translation failure that will result
+ in the `Programmed` condition of the object being set to `False` and an
+ event being emitted.
+ [#4428](https:/Kong/kubernetes-ingress-controller/pull/4428)
 
 ### Changed
 

internal/dataplane/kongstate/kongstate.go

@@ -277,21 +277,42 @@ func (ks *KongState) getPluginRelations() map[string]util.ForeignRelations {
  return pluginRels
 }
 
-func buildPlugins(log logrus.FieldLogger, s store.Storer, pluginRels map[string]util.ForeignRelations) []Plugin {
+func buildPlugins(
+ log logrus.FieldLogger,
+ s store.Storer,
+ failuresCollector *failures.ResourceFailuresCollector,
+ pluginRels map[string]util.ForeignRelations,
+) []Plugin {
  var plugins []Plugin
 
  for pluginIdentifier, relations := range pluginRels {
  identifier := strings.Split(pluginIdentifier, ":")
  namespace, kongPluginName := identifier[0], identifier[1]
- plugin, err := getPlugin(s, namespace, kongPluginName)
+ k8sPlugin, k8sClusterPlugin, err := getKongPluginOrKongClusterPlugin(s, namespace, kongPluginName)
  if err != nil {
  log.WithFields(logrus.Fields{
  "kongplugin_name": kongPluginName,
  "kongplugin_namespace": namespace,
- }).WithError(err).Errorf("failed to fetch KongPlugin")
+ }).WithError(err).Errorf("failed to fetch KongPlugin resource")
  continue
  }
 
+ var plugin Plugin
+ if k8sPlugin != nil {
+ plugin, err = kongPluginFromK8SPlugin(s, *k8sPlugin)
+ if err != nil {
+ failuresCollector.PushResourceFailure(err.Error(), k8sPlugin)
+ continue
+ }
+ }
+ if k8sClusterPlugin != nil {
+ plugin, err = kongPluginFromK8SClusterPlugin(s, *k8sClusterPlugin)
+ if err != nil {
+ failuresCollector.PushResourceFailure(err.Error(), k8sClusterPlugin)
+ continue
+ }
+ }
+
  for _, rel := range relations.GetCombinations() {
  plugin := plugin.DeepCopy()
  // ID is populated because that is read by decK and in_memory
@@ -377,8 +398,12 @@ func globalPlugins(log logrus.FieldLogger, s store.Storer) ([]Plugin, error) {
  return plugins, nil
 }
 
-func (ks *KongState) FillPlugins(log logrus.FieldLogger, s store.Storer) {
- ks.Plugins = buildPlugins(log, s, ks.getPluginRelations())
+func (ks *KongState) FillPlugins(
+ log logrus.FieldLogger,
+ s store.Storer,
+ failuresCollector *failures.ResourceFailuresCollector,
+) {
+ ks.Plugins = buildPlugins(log, s, failuresCollector, ks.getPluginRelations())
 }
 
 // FillIDs iterates over the KongState and fills in the ID field for each entity

internal/dataplane/kongstate/util.go

@@ -78,37 +78,34 @@ func getKongIngressFromObjAnnotations(
  return nil, nil
 }
 
-// getPlugin constructs a plugins from a KongPlugin resource.
-func getPlugin(s store.Storer, namespace, name string) (Plugin, error) {
- var plugin Plugin
- k8sPlugin, err := s.GetKongPlugin(namespace, name)
- if err != nil {
- // if no namespaced plugin definition, then
- // search for cluster level-plugin definition
- if errors.As(err, &store.ErrNotFound{}) {
- clusterPlugin, err := s.GetKongClusterPlugin(name)
- // not found
- if errors.As(err, &store.ErrNotFound{}) {
- return plugin, errors.New(
- "no KongPlugin or KongClusterPlugin was found")
- }
- if err != nil {
- return plugin, err
- }
- if clusterPlugin.PluginName == "" {
- return plugin, fmt.Errorf("invalid empty 'plugin' property")
+// getKongPluginOrKongClusterPlugin fetches a KongPlugin or KongClusterPlugin (as fallback) from the store.
+// If both are not found, an error is returned.
+func getKongPluginOrKongClusterPlugin(s store.Storer, namespace, name string) (
+ *configurationv1.KongPlugin,
+ *configurationv1.KongClusterPlugin,
+ error,
+) {
+ plugin, pluginErr := s.GetKongPlugin(namespace, name)
+ if pluginErr != nil {
+ if !errors.As(pluginErr, &store.ErrNotFound{}) {
+ return nil, nil, fmt.Errorf("failed fetching KongPlugin: %w", pluginErr)
+ }
+
+ // If KongPlugin is not found, try to fetch KongClusterPlugin.
+ clusterPlugin, err := s.GetKongClusterPlugin(name)
+ if err != nil {
+ if !errors.As(err, &store.ErrNotFound{}) {
+ return nil, nil, fmt.Errorf("failed fetching KongClusterPlugin: %w", err)
  }
- plugin, err = kongPluginFromK8SClusterPlugin(s, *clusterPlugin)
- return plugin, err
+
+ // Both KongPlugin and KongClusterPlugin are not found.
+ return nil, nil, fmt.Errorf("no KongPlugin or KongClusterPlugin was found for %s/%s", namespace, name)
  }
- }
- // ignore plugins with no name
- if k8sPlugin.PluginName == "" {
- return plugin, fmt.Errorf("invalid empty 'plugin' property")
+
+ return nil, clusterPlugin, nil
  }
 
- plugin, err = kongPluginFromK8SPlugin(s, *k8sPlugin)
- return plugin, err
+ return plugin, nil, nil
 }
 
 func kongPluginFromK8SClusterPlugin(

internal/dataplane/parser/parser.go

@@ -233,7 +233,7 @@ func (p *Parser) BuildKongConfig() KongConfigBuildingResult {
  }
 
  // process annotation plugins
- result.FillPlugins(p.logger, p.storer)
+ result.FillPlugins(p.logger, p.storer, p.failuresCollector)
  for i := range result.Plugins {
  p.registerSuccessfullyParsedObject(result.Plugins[i].K8sParent)
  }

test/envtest/programmed_condition_envtest_test.go

@@ -6,6 +6,7 @@ import (
  "time"
 
  "github.com/stretchr/testify/require"
+ apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
  k8stypes "k8s.io/apimachinery/pkg/types"
  "sigs.k8s.io/controller-runtime/pkg/client"
@@ -38,6 +39,7 @@ func TestKongCRDs_ProgrammedCondition(t *testing.T) {
  objects []client.Object
  getExpectedObjectConditions func(ctrlClient client.Client) ([]metav1.Condition, error)
  expectedProgrammedStatus metav1.ConditionStatus
+ expectedProgrammedReason kongv1.ConditionReason
  }{
  {
  name: "valid KongConsumer",
@@ -65,6 +67,7 @@ func TestKongCRDs_ProgrammedCondition(t *testing.T) {
  return consumer.Status.Conditions, nil
  },
  expectedProgrammedStatus: metav1.ConditionTrue,
+ expectedProgrammedReason: kongv1.ReasonProgrammed,
  },
  {
  name: "KongConsumer referencing non-existent secret",
@@ -93,6 +96,7 @@ func TestKongCRDs_ProgrammedCondition(t *testing.T) {
  return consumer.Status.Conditions, nil
  },
  expectedProgrammedStatus: metav1.ConditionFalse,
+ expectedProgrammedReason: kongv1.ReasonInvalid,
  },
  {
  name: "valid KongConsumerGroup",
@@ -119,6 +123,7 @@ func TestKongCRDs_ProgrammedCondition(t *testing.T) {
  return consumerGroup.Status.Conditions, nil
  },
  expectedProgrammedStatus: metav1.ConditionTrue,
+ expectedProgrammedReason: kongv1.ReasonProgrammed,
  },
  {
  name: "valid KongPlugin",
@@ -155,6 +160,52 @@ func TestKongCRDs_ProgrammedCondition(t *testing.T) {
  return plugin.Status.Conditions, nil
  },
  expectedProgrammedStatus: metav1.ConditionTrue,
+ expectedProgrammedReason: kongv1.ReasonProgrammed,
+ },
+ {
+ name: "invalid KongPlugin",
+ objects: []client.Object{
+ &kongv1.KongPlugin{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "invalid-kong-plugin",
+ Namespace: ns.Name,
+ Annotations: map[string]string{annotations.IngressClassKey: annotations.DefaultIngressClass},
+ },
+ PluginName: "plugin",
+ // Specifying both Config and ConfigFrom is invalid.
+ Config: apiextensionsv1.JSON{Raw: []byte(`{"key": "value"}`)},
+ ConfigFrom: &kongv1.ConfigSource{
+ SecretValue: kongv1.SecretValueFromSource{
+ Secret: "secret",
+ Key: "key",
+ },
+ },
+ },
+ &kongv1.KongConsumer{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "consumer-for-invalid-plugin",
+ Namespace: ns.Name,
+ Annotations: map[string]string{
+ annotations.IngressClassKey: annotations.DefaultIngressClass,
+ annotations.AnnotationPrefix + annotations.PluginsKey: "invalid-kong-plugin",
+ },
+ },
+ Username: "foo",
+ },
+ },
+ getExpectedObjectConditions: func(ctrlClient client.Client) ([]metav1.Condition, error) {
+ var plugin kongv1.KongPlugin
+ err := ctrlClient.Get(ctx, k8stypes.NamespacedName{
+ Name: "invalid-kong-plugin",
+ Namespace: ns.Name,
+ }, &plugin)
+ if err != nil {
+ return nil, err
+ }
+ return plugin.Status.Conditions, nil
+ },
+ expectedProgrammedStatus: metav1.ConditionFalse,
+ expectedProgrammedReason: kongv1.ReasonInvalid,
  },
  {
  name: "valid KongClusterPlugin",
@@ -189,6 +240,52 @@ func TestKongCRDs_ProgrammedCondition(t *testing.T) {
  return clusterPlugin.Status.Conditions, nil
  },
  expectedProgrammedStatus: metav1.ConditionTrue,
+ expectedProgrammedReason: kongv1.ReasonProgrammed,
+ },
+ {
+ name: "invalid KongClusterPlugin",
+ objects: []client.Object{
+ &kongv1.KongPlugin{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "invalid-kong-cluster-plugin",
+ Namespace: ns.Name,
+ Annotations: map[string]string{annotations.IngressClassKey: annotations.DefaultIngressClass},
+ },
+ PluginName: "plugin",
+ // Specifying both Config and ConfigFrom is invalid.
+ Config: apiextensionsv1.JSON{Raw: []byte(`{"key": "value"}`)},
+ ConfigFrom: &kongv1.ConfigSource{
+ SecretValue: kongv1.SecretValueFromSource{
+ Secret: "secret",
+ Key: "key",
+ },
+ },
+ },
+ &kongv1.KongConsumer{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "consumer-for-invalid-cluster-plugin",
+ Namespace: ns.Name,
+ Annotations: map[string]string{
+ annotations.IngressClassKey: annotations.DefaultIngressClass,
+ annotations.AnnotationPrefix + annotations.PluginsKey: "invalid-kong-cluster-plugin",
+ },
+ },
+ Username: "foo",
+ },
+ },
+ getExpectedObjectConditions: func(ctrlClient client.Client) ([]metav1.Condition, error) {
+ var plugin kongv1.KongPlugin
+ err := ctrlClient.Get(ctx, k8stypes.NamespacedName{
+ Name: "invalid-kong-cluster-plugin",
+ Namespace: ns.Name,
+ }, &plugin)
+ if err != nil {
+ return nil, err
+ }
+ return plugin.Status.Conditions, nil
+ },
+ expectedProgrammedStatus: metav1.ConditionFalse,
+ expectedProgrammedReason: kongv1.ReasonInvalid,
  },
  }
 
@@ -208,6 +305,7 @@ func TestKongCRDs_ProgrammedCondition(t *testing.T) {
  if !conditions.Contain(
  cs,
  conditions.WithType(string(kongv1.ConditionProgrammed)),
+ conditions.WithReason(string(tc.expectedProgrammedReason)),
  conditions.WithStatus(tc.expectedProgrammedStatus),
  ) {
  t.Logf("Programmed condition not found, actual: %v", cs)