Skip to content

Commit 9df75e6

Browse files
mflendrichmflendrich
committed
Merge branch 'feat/ingressv1-informers-store' into next
Ingress V1: apiversion negotiation and flags, support in store, parser-k8s decoupling, ingress parsing refactor closes Kong#788 This PR does the following (each high-level bullet point roughly matching a group of commits): - refactors `parseIngressRules` into separate functions (without side effects) and places them in `translate.go`: `fromIngressV1beta1` and `fromTCPIngressV1beta1`. Also moves `parseKnativeIngressRules` to follow suit. All of these get merged in the end using `mergeIngressRules` recently implemented in Kong#822. - this comes with a tiny, potentially behavior changing, logic modification: after this PR, default backends get added to the overall set of `ingressRules` at a different stage of _"parsing"_ than they were before. - this change is here as a preparation for introduction of `fromIngressV1` which will be a copy-paste of `fromIngressV1beta1` with changes as described in the ingress v1 KEP. - removes concrete k8s types (`<your-favorite-apiversion>.Ingress`) and `TCPIngress` from `package kongstate` by introducing an intermediate `utils.K8sObjectInfo`: - I considered using k8s `ObjectMeta` instead of implementing `K8sObjectInfo` but that would violate separation between Kong and Kubernetes data formats that I wanted to achieve in `package kongstate`. - this change is here to accommodate Ingress V1. - reimplements Ingress API version negotiation: - to look not only at supported API groups, but also at the specific resource kind available in that API group (because `networking.k8s.io/v1` existed in k8s before the `Ingress` kind was added thereto) - to support `networking.k8s.io/v1` alongside the already supported versions - to support enabling and disabling support for specific APIs in Kong. This is for several reasons: - this PR does not implement Ingress V1 fully, so we can merge this PR in with ingress v1 code paths disabled - if Ingress V1 support is enabled in KIC, and the apiserver is >=1.19, then KIC will use a completely new code path in parser **for all preexisting Ingress resources**, posing a significant regression risk for existing users. There are two possible strategies for Ingress v1 rollout: - defensive: choose to leave ingress V1 APIs disabled by default - less defensive but with a workaround by design: make ingress V1 APIs enabled by default, but have the disabling flag readily at hand as a workaround for users whom we break - drive-by side effect: makes it possible to "soft-disable" extensions/v1beta1 ingress by setting its flag default to false - implements Ingress v1 alongside v1beta1 in store. - Note that the distinction between v1 and both v1beta1 types is implemented differently than between `extensions` and `networking` v1beta1. - This is because `extensions` and `networking` v1beta1 Ingresses are fully compatible, and they reuse the same parsing logic. Ingress V1, however, will be a separate implementation of the parsing logic. I wanted to ensure type safety (between v1beta1 and v1 ingresses), which would be impossible without differentiated storage. - additionally: adds `*.orig` to `.gitignore`. `*.orig` files originate from manually resolved Git conflicts and are easy to accidentally commit into the repo. Notes to the reviewer: - Consider reviewing commit by commit. - Commits are (intended to be) self-contained. Consider not squashing when merging. From Kong#826
2 parents b3e9849 + bdf07d1 commit 9df75e6

25 files changed

+1755
-1293
lines changed

‎.gitignore

+3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
# Git merge residuals
2+
*.orig
3+
14
# Compiled Object files, Static and Dynamic libs (Shared Objects)
25
*.o
36
*.a

‎cli/ingress-controller/flag_test.go

+16
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,10 @@ func TestOverrideViaCLIFlags(t *testing.T) {
182182
"--apiserver-host", "kube-apiserver.internal",
183183
"--kubeconfig", "/path/to/kubeconfig",
184184

185+
"--disable-ingress-extensionsv1beta1",
186+
"--disable-ingress-networkingv1beta1",
187+
"--disable-ingress-networkingv1",
188+
185189
"--log-format", "json",
186190

187191
"--profiling=false",
@@ -221,6 +225,10 @@ func TestOverrideViaCLIFlags(t *testing.T) {
221225
APIServerHost: "kube-apiserver.internal",
222226
KubeConfigFilePath: "/path/to/kubeconfig",
223227

228+
DisableIngressNetworkingV1: true,
229+
DisableIngressNetworkingV1beta1: true,
230+
DisableIngressExtensionsV1beta1: true,
231+
224232
LogLevel: "info",
225233
LogFormat: "json",
226234

@@ -251,6 +259,10 @@ func TestOverrideViaEnvVars(t *testing.T) {
251259
"CONTROLLER_LOG_LEVEL": "panic",
252260

253261
"CONTROLLER_KONG_CUSTOM_ENTITIES_SECRET": "foons/barsecretname",
262+
263+
"CONTROLLER_DISABLE_INGRESS_EXTENSIONSV1BETA1": "true",
264+
"CONTROLLER_DISABLE_INGRESS_NETWORKINGV1BETA1": "true",
265+
"CONTROLLER_DISABLE_INGRESS_NETWORKINGV1": "true",
254266
}
255267
for k, v := range envs {
256268
os.Setenv(k, v)
@@ -290,6 +302,10 @@ func TestOverrideViaEnvVars(t *testing.T) {
290302
APIServerHost: "",
291303
KubeConfigFilePath: "",
292304

305+
DisableIngressNetworkingV1: true,
306+
DisableIngressNetworkingV1beta1: true,
307+
DisableIngressExtensionsV1beta1: true,
308+
293309
LogLevel: "panic",
294310
LogFormat: "text",
295311

‎cli/ingress-controller/flags.go

+21
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ type cliConfig struct {
8383
APIServerHost string
8484
KubeConfigFilePath string
8585

86+
// Allowed Ingress resource versions
87+
DisableIngressExtensionsV1beta1 bool
88+
DisableIngressNetworkingV1beta1 bool
89+
DisableIngressNetworkingV1 bool
90+
8691
// Performance
8792
EnableProfiling bool
8893

@@ -226,6 +231,17 @@ Kubernetes cluster and local discovery is attempted.`)
226231
flags.String("kubeconfig", "", "Path to kubeconfig file with "+
227232
"authorization and master location information.")
228233

234+
// Allowed Ingress resource versions
235+
flags.Bool("disable-ingress-extensionsv1beta1", false,
236+
`If set, the ingress controller won't try extensions/v1beta1 when negotiating the newest supported
237+
Ingress API with Kubernetes.`)
238+
flags.Bool("disable-ingress-networkingv1beta1", false,
239+
`If set, the ingress controller won't try networking.k8s.io/v1beta1 when negotiating the newest supported
240+
Ingress API with Kubernetes.`)
241+
flags.Bool("disable-ingress-networkingv1", false,
242+
`If set, the ingress controller won't try networking/v1 when negotiating the newest supported
243+
Ingress API with Kubernetes.`)
244+
229245
// Misc
230246
flags.Bool("profiling", true, `Enable profiling via web interface host:port/debug/pprof/`)
231247
flags.Bool("version", false,
@@ -350,6 +366,11 @@ func parseFlags() (cliConfig, error) {
350366
config.APIServerHost = viper.GetString("apiserver-host")
351367
config.KubeConfigFilePath = viper.GetString("kubeconfig")
352368

369+
// Disabled Ingress resource versions
370+
config.DisableIngressExtensionsV1beta1 = viper.GetBool("disable-ingress-extensionsv1beta1")
371+
config.DisableIngressNetworkingV1beta1 = viper.GetBool("disable-ingress-networkingv1beta1")
372+
config.DisableIngressNetworkingV1 = viper.GetBool("disable-ingress-networkingv1")
373+
353374
// Misc
354375
config.EnableProfiling = viper.GetBool("profiling")
355376
config.ShowVersion = viper.GetBool("version")

‎cli/ingress-controller/main.go

+27-11
Original file line numberDiff line numberDiff line change
@@ -307,13 +307,6 @@ func main() {
307307
}
308308
controllerConfig.Kong.Client = kongClient
309309

310-
err = discovery.ServerSupportsVersion(kubeClient.Discovery(), schema.GroupVersion{
311-
Group: "networking.k8s.io",
312-
Version: "v1beta1",
313-
})
314-
if err == nil {
315-
controllerConfig.UseNetworkingV1beta1 = true
316-
}
317310
coreInformerFactory := informers.NewSharedInformerFactoryWithOptions(
318311
kubeClient,
319312
cliConfig.SyncPeriod,
@@ -351,18 +344,41 @@ func main() {
351344
UpdateCh: updateChannel,
352345
}
353346

347+
var preferredIngressAPIs []utils.IngressAPI
348+
if !cliConfig.DisableIngressNetworkingV1 {
349+
preferredIngressAPIs = append(preferredIngressAPIs, utils.NetworkingV1)
350+
}
351+
if !cliConfig.DisableIngressNetworkingV1beta1 {
352+
preferredIngressAPIs = append(preferredIngressAPIs, utils.NetworkingV1beta1)
353+
}
354+
if !cliConfig.DisableIngressExtensionsV1beta1 {
355+
preferredIngressAPIs = append(preferredIngressAPIs, utils.ExtensionsV1beta1)
356+
}
357+
controllerConfig.IngressAPI, err = utils.NegotiateResourceAPI(kubeClient, "Ingress", preferredIngressAPIs)
358+
if err != nil {
359+
log.Fatalf("NegotiateIngressAPI failed: %v, tried: %+v", err, preferredIngressAPIs)
360+
}
361+
log.Infof("chosen Ingress API version: %v", controllerConfig.IngressAPI)
362+
354363
var informers []cache.SharedIndexInformer
355364
var cacheStores store.CacheStores
356365

357366
var ingInformer cache.SharedIndexInformer
358-
if controllerConfig.UseNetworkingV1beta1 {
367+
switch controllerConfig.IngressAPI {
368+
case utils.NetworkingV1:
369+
ingInformer = coreInformerFactory.Networking().V1().Ingresses().Informer()
370+
cacheStores.IngressV1 = ingInformer.GetStore()
371+
cacheStores.IngressV1beta1 = newEmptyStore()
372+
case utils.NetworkingV1beta1:
359373
ingInformer = coreInformerFactory.Networking().V1beta1().Ingresses().Informer()
360-
} else {
374+
cacheStores.IngressV1 = newEmptyStore()
375+
cacheStores.IngressV1beta1 = ingInformer.GetStore()
376+
case utils.ExtensionsV1beta1:
361377
ingInformer = coreInformerFactory.Extensions().V1beta1().Ingresses().Informer()
378+
cacheStores.IngressV1 = newEmptyStore()
379+
cacheStores.IngressV1beta1 = ingInformer.GetStore()
362380
}
363-
364381
ingInformer.AddEventHandler(reh)
365-
cacheStores.Ingress = ingInformer.GetStore()
366382
informers = append(informers, ingInformer)
367383

368384
endpointsInformer := coreInformerFactory.Core().V1().Endpoints().Informer()

‎cli/ingress-controller/util.go

+6
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@ package main
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"regexp"
78
"strings"
89

910
"github.com/blang/semver"
1011
"github.com/kong/go-kong/kong"
12+
"k8s.io/client-go/tools/cache"
1113
)
1214

1315
func getSemVerVer(v string) (semver.Version, error) {
@@ -57,3 +59,7 @@ func createWorkspace(ctx context.Context, client *kong.Client, workspace string)
5759
_, err = client.Do(ctx, req, nil)
5860
return err
5961
}
62+
63+
func newEmptyStore() cache.Store {
64+
return cache.NewStore(func(interface{}) (string, error) { return "", errors.New("this store cannot add elements") })
65+
}

‎cli/ingress-controller/util_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package main
22

3-
import "testing"
3+
import (
4+
"testing"
5+
)
46

57
func TestFixVersion(t *testing.T) {
68
validVersions := map[string]string{

‎internal/ingress/controller/controller.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"github.com/kong/kubernetes-ingress-controller/internal/ingress/status"
3232
"github.com/kong/kubernetes-ingress-controller/internal/ingress/store"
3333
"github.com/kong/kubernetes-ingress-controller/internal/ingress/task"
34+
"github.com/kong/kubernetes-ingress-controller/internal/ingress/utils"
3435
configurationv1 "github.com/kong/kubernetes-ingress-controller/pkg/apis/configuration/v1"
3536
configClientSet "github.com/kong/kubernetes-ingress-controller/pkg/client/configuration/clientset/versioned"
3637
"github.com/mitchellh/mapstructure"
@@ -87,7 +88,8 @@ type Configuration struct {
8788
UpdateStatusOnShutdown bool
8889
ElectionID string
8990

90-
UseNetworkingV1beta1 bool
91+
IngressAPI utils.IngressAPI
92+
9193
EnableKnativeIngressSupport bool
9294

9395
Logger logrus.FieldLogger
@@ -180,7 +182,7 @@ func NewKongController(ctx context.Context,
180182
PublishStatusAddress: config.PublishStatusAddress,
181183
IngressLister: n.store,
182184
UpdateStatusOnShutdown: config.UpdateStatusOnShutdown,
183-
UseNetworkingV1beta1: config.UseNetworkingV1beta1,
185+
IngressAPI: config.IngressAPI,
184186
OnStartedLeading: func() {
185187
// force a sync
186188
n.syncQueue.Enqueue(&networking.Ingress{})

‎internal/ingress/controller/parser/kongstate/kongstate.go

+7-19
Original file line numberDiff line numberDiff line change
@@ -147,24 +147,12 @@ func (ks *KongState) FillOverrides(log logrus.FieldLogger, s store.Storer) {
147147
for j := 0; j < len(ks.Services[i].Routes); j++ {
148148
var kongIngress *configurationv1.KongIngress
149149
var err error
150-
if ks.Services[i].Routes[j].IsTCP {
151-
kongIngress, err = getKongIngressFromTCPIngress(s,
152-
&ks.Services[i].Routes[j].TCPIngress)
153-
if err != nil {
154-
log.WithFields(logrus.Fields{
155-
"tcpingress_name": ks.Services[i].Routes[j].TCPIngress.Name,
156-
"tcpingress_namespace": ks.Services[i].Routes[j].TCPIngress.Namespace,
157-
}).Errorf("failed to fetch KongIngress resource for Ingress: %v", err)
158-
}
159-
} else {
160-
kongIngress, err = getKongIngressFromIngress(s,
161-
&ks.Services[i].Routes[j].Ingress)
162-
if err != nil {
163-
log.WithFields(logrus.Fields{
164-
"ingress_name": ks.Services[i].Routes[j].Ingress.Name,
165-
"ingress_namespace": ks.Services[i].Routes[j].Ingress.Namespace,
166-
}).Errorf("failed to fetch KongIngress resource for Ingress: %v", err)
167-
}
150+
kongIngress, err = getKongIngressFromObjectMeta(s, &ks.Services[i].Routes[j].Ingress)
151+
if err != nil {
152+
log.WithFields(logrus.Fields{
153+
"resource_name": ks.Services[i].Routes[j].Ingress.Name,
154+
"resource_namespace": ks.Services[i].Routes[j].Ingress.Namespace,
155+
}).Errorf("failed to fetch KongIngress resource: %v", err)
168156
}
169157

170158
ks.Services[i].Routes[j].override(log, kongIngress)
@@ -230,7 +218,7 @@ func (ks *KongState) getPluginRelations() map[string]util.ForeignRelations {
230218
// route
231219
for j := range ks.Services[i].Routes {
232220
ingress := ks.Services[i].Routes[j].Ingress
233-
pluginList := annotations.ExtractKongPluginsFromAnnotations(ingress.GetAnnotations())
221+
pluginList := annotations.ExtractKongPluginsFromAnnotations(ingress.Annotations)
234222
for _, pluginName := range pluginList {
235223
addRouteRelation(ingress.Namespace, pluginName, *ks.Services[i].Routes[j].Name)
236224
}

‎internal/ingress/controller/parser/kongstate/kongstate_test.go

+25-36
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"github.com/kong/kubernetes-ingress-controller/internal/ingress/controller/parser/util"
1010
configurationv1 "github.com/kong/kubernetes-ingress-controller/pkg/apis/configuration/v1"
1111
corev1 "k8s.io/api/core/v1"
12-
networking "k8s.io/api/networking/v1beta1"
1312
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1413
)
1514

@@ -92,13 +91,11 @@ func Test_getPluginRelations(t *testing.T) {
9291
Route: kong.Route{
9392
Name: kong.String("foo-route"),
9493
},
95-
Ingress: networking.Ingress{
96-
ObjectMeta: metav1.ObjectMeta{
97-
Name: "some-ingress",
98-
Namespace: "ns2",
99-
Annotations: map[string]string{
100-
annotations.DeprecatedPluginsKey: "foo,bar",
101-
},
94+
Ingress: util.K8sObjectInfo{
95+
Name: "some-ingress",
96+
Namespace: "ns2",
97+
Annotations: map[string]string{
98+
annotations.DeprecatedPluginsKey: "foo,bar",
10299
},
103100
},
104101
},
@@ -126,27 +123,23 @@ func Test_getPluginRelations(t *testing.T) {
126123
Route: kong.Route{
127124
Name: kong.String("foo-route"),
128125
},
129-
Ingress: networking.Ingress{
130-
ObjectMeta: metav1.ObjectMeta{
131-
Name: "some-ingress",
132-
Namespace: "ns2",
133-
Annotations: map[string]string{
134-
annotations.DeprecatedPluginsKey: "foo,bar",
135-
},
126+
Ingress: util.K8sObjectInfo{
127+
Name: "some-ingress",
128+
Namespace: "ns2",
129+
Annotations: map[string]string{
130+
annotations.DeprecatedPluginsKey: "foo,bar",
136131
},
137132
},
138133
},
139134
{
140135
Route: kong.Route{
141136
Name: kong.String("bar-route"),
142137
},
143-
Ingress: networking.Ingress{
144-
ObjectMeta: metav1.ObjectMeta{
145-
Name: "some-ingress",
146-
Namespace: "ns2",
147-
Annotations: map[string]string{
148-
annotations.DeprecatedPluginsKey: "bar,baz",
149-
},
138+
Ingress: util.K8sObjectInfo{
139+
Name: "some-ingress",
140+
Namespace: "ns2",
141+
Annotations: map[string]string{
142+
annotations.DeprecatedPluginsKey: "bar,baz",
150143
},
151144
},
152145
},
@@ -224,27 +217,23 @@ func Test_getPluginRelations(t *testing.T) {
224217
Route: kong.Route{
225218
Name: kong.String("foo-route"),
226219
},
227-
Ingress: networking.Ingress{
228-
ObjectMeta: metav1.ObjectMeta{
229-
Name: "some-ingress",
230-
Namespace: "ns2",
231-
Annotations: map[string]string{
232-
annotations.DeprecatedPluginsKey: "foo,bar",
233-
},
220+
Ingress: util.K8sObjectInfo{
221+
Name: "some-ingress",
222+
Namespace: "ns2",
223+
Annotations: map[string]string{
224+
annotations.DeprecatedPluginsKey: "foo,bar",
234225
},
235226
},
236227
},
237228
{
238229
Route: kong.Route{
239230
Name: kong.String("bar-route"),
240231
},
241-
Ingress: networking.Ingress{
242-
ObjectMeta: metav1.ObjectMeta{
243-
Name: "some-ingress",
244-
Namespace: "ns2",
245-
Annotations: map[string]string{
246-
annotations.DeprecatedPluginsKey: "bar,baz",
247-
},
232+
Ingress: util.K8sObjectInfo{
233+
Name: "some-ingress",
234+
Namespace: "ns2",
235+
Annotations: map[string]string{
236+
annotations.DeprecatedPluginsKey: "bar,baz",
248237
},
249238
},
250239
},

0 commit comments

Comments
 (0)