Skip to content

Commit

Permalink
fix(crds) disable KongPlugin programmed condition
Browse files Browse the repository at this point in the history 
Disable the Kong(Cluster)Plugin programmed condition. This caused a bug
in multi-class environments.

Add a TODO explaining the issue.
  • Loading branch information
@rainest
rainest committed Aug 29, 2023
1 parent db0ad91 commit 8cc578c
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 169 deletions.
8 changes: 4 additions & 4 deletions hack/generators/controllers/networking/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ var inputControllersNeeded = &typesNeeded{
Plural: "kongplugins",
CacheType: "Plugin",
NeedsStatusPermissions: true,
ConfigStatusNotificationsEnabled: true,
ProgrammedConditionUpdatesEnabled: true,
ConfigStatusNotificationsEnabled: false, // TODO true after https://github.com/Kong/kubernetes-ingress-controller/issues/4578
ProgrammedConditionUpdatesEnabled: false, // TODO true after https://github.com/Kong/kubernetes-ingress-controller/issues/4578
AcceptsIngressClassNameAnnotation: false,
AcceptsIngressClassNameSpec: false,
NeedsUpdateReferences: true,
Expand All @@ -131,8 +131,8 @@ var inputControllersNeeded = &typesNeeded{
Plural: "kongclusterplugins",
CacheType: "ClusterPlugin",
NeedsStatusPermissions: true,
ConfigStatusNotificationsEnabled: true,
ProgrammedConditionUpdatesEnabled: true,
ConfigStatusNotificationsEnabled: false, // TODO true after https://github.com/Kong/kubernetes-ingress-controller/issues/4578
ProgrammedConditionUpdatesEnabled: false, // TODO true after https://github.com/Kong/kubernetes-ingress-controller/issues/4578
AcceptsIngressClassNameAnnotation: true,
AcceptsIngressClassNameSpec: false,
NeedsUpdateReferences: true,
Expand Down
6 changes: 4 additions & 2 deletions internal/manager/controllerdef.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,8 @@ func setupControllers(
DataplaneClient: dataplaneClient,
CacheSyncTimeout: c.CacheSyncTimeout,
ReferenceIndexers: referenceIndexers,
StatusQueue: kubernetesStatusQueue,
// TODO https://github.com/Kong/kubernetes-ingress-controller/issues/4578
// StatusQueue: kubernetesStatusQueue,
},
},
{
Expand Down Expand Up @@ -313,7 +314,8 @@ func setupControllers(
DisableIngressClassLookups: !c.IngressClassNetV1Enabled,
CacheSyncTimeout: c.CacheSyncTimeout,
ReferenceIndexers: referenceIndexers,
StatusQueue: kubernetesStatusQueue,
// TODO https://github.com/Kong/kubernetes-ingress-controller/issues/4578
// StatusQueue: kubernetesStatusQueue,
},
},
// ---------------------------------------------------------------------------
Expand Down
329 changes: 166 additions & 163 deletions test/envtest/programmed_condition_envtest_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ 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"
Expand Down Expand Up @@ -125,168 +124,172 @@ func TestKongCRDs_ProgrammedCondition(t *testing.T) {
expectedProgrammedStatus: metav1.ConditionTrue,
expectedProgrammedReason: kongv1.ReasonProgrammed,
},
{
name: "valid KongPlugin",
objects: []client.Object{
&kongv1.KongPlugin{
ObjectMeta: metav1.ObjectMeta{
Name: "kong-plugin",
Namespace: ns.Name,
Annotations: map[string]string{annotations.IngressClassKey: annotations.DefaultIngressClass},
},
PluginName: "plugin",
},
&kongv1.KongConsumer{
ObjectMeta: metav1.ObjectMeta{
Name: "consumer-for-plugin",
Namespace: ns.Name,
Annotations: map[string]string{
annotations.IngressClassKey: annotations.DefaultIngressClass,
annotations.AnnotationPrefix + annotations.PluginsKey: "kong-plugin",
},
},
Username: "foo",
},
},
getExpectedObjectConditions: func(ctrlClient client.Client) ([]metav1.Condition, error) {
var plugin kongv1.KongPlugin
err := ctrlClient.Get(ctx, k8stypes.NamespacedName{
Name: "kong-plugin",
Namespace: ns.Name,
}, &plugin)
if err != nil {
return nil, err
}
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",
objects: []client.Object{
&kongv1.KongClusterPlugin{
ObjectMeta: metav1.ObjectMeta{
Name: "kong-cluster-plugin",
Annotations: map[string]string{annotations.IngressClassKey: annotations.DefaultIngressClass},
},
PluginName: "plugin",
},
&kongv1.KongConsumer{
ObjectMeta: metav1.ObjectMeta{
Name: "consumer-for-cluster-plugin",
Namespace: ns.Name,
Annotations: map[string]string{
annotations.IngressClassKey: annotations.DefaultIngressClass,
annotations.AnnotationPrefix + annotations.PluginsKey: "kong-cluster-plugin",
},
},
Username: "foo",
},
},
getExpectedObjectConditions: func(ctrlClient client.Client) ([]metav1.Condition, error) {
var clusterPlugin kongv1.KongClusterPlugin
err := ctrlClient.Get(ctx, k8stypes.NamespacedName{
Name: "kong-cluster-plugin",
}, &clusterPlugin)
if err != nil {
return nil, err
}
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,
},
// TODO https://github.com/Kong/kubernetes-ingress-controller/issues/4578
// if there are multiple KIC instances within a cluster, they will fight over setting this condition because the
// controllers do not filter on ingress class. we need to limit them to only resources referenced from others,
// similar to Secrets, to use this
//{
// name: "valid KongPlugin",
// objects: []client.Object{
// &kongv1.KongPlugin{
// ObjectMeta: metav1.ObjectMeta{
// Name: "kong-plugin",
// Namespace: ns.Name,
// Annotations: map[string]string{annotations.IngressClassKey: annotations.DefaultIngressClass},
// },
// PluginName: "plugin",
// },
// &kongv1.KongConsumer{
// ObjectMeta: metav1.ObjectMeta{
// Name: "consumer-for-plugin",
// Namespace: ns.Name,
// Annotations: map[string]string{
// annotations.IngressClassKey: annotations.DefaultIngressClass,
// annotations.AnnotationPrefix + annotations.PluginsKey: "kong-plugin",
// },
// },
// Username: "foo",
// },
// },
// getExpectedObjectConditions: func(ctrlClient client.Client) ([]metav1.Condition, error) {
// var plugin kongv1.KongPlugin
// err := ctrlClient.Get(ctx, k8stypes.NamespacedName{
// Name: "kong-plugin",
// Namespace: ns.Name,
// }, &plugin)
// if err != nil {
// return nil, err
// }
// 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",
// objects: []client.Object{
// &kongv1.KongClusterPlugin{
// ObjectMeta: metav1.ObjectMeta{
// Name: "kong-cluster-plugin",
// Annotations: map[string]string{annotations.IngressClassKey: annotations.DefaultIngressClass},
// },
// PluginName: "plugin",
// },
// &kongv1.KongConsumer{
// ObjectMeta: metav1.ObjectMeta{
// Name: "consumer-for-cluster-plugin",
// Namespace: ns.Name,
// Annotations: map[string]string{
// annotations.IngressClassKey: annotations.DefaultIngressClass,
// annotations.AnnotationPrefix + annotations.PluginsKey: "kong-cluster-plugin",
// },
// },
// Username: "foo",
// },
// },
// getExpectedObjectConditions: func(ctrlClient client.Client) ([]metav1.Condition, error) {
// var clusterPlugin kongv1.KongClusterPlugin
// err := ctrlClient.Get(ctx, k8stypes.NamespacedName{
// Name: "kong-cluster-plugin",
// }, &clusterPlugin)
// if err != nil {
// return nil, err
// }
// 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,
//},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 8cc578c

Please sign in to comment.