Skip to content

Commit

Permalink
chore(k8s) Use a single scheme for all of k8s (#2796)
Browse files Browse the repository at this point in the history
* chore(k8s) Use a single scheme for all of k8s

We were modifying the same scheme in many places when configuring the
controllers. This is unecessary and is likely to introduce bugs.
We now use the same scheme for everything and modify it at initialization

Signed-off-by: Charly Molter <charly.molter@konghq.com>
  • Loading branch information
lahabana authored Sep 21, 2021
1 parent 868c71c commit fdc5f1a
Show file tree
Hide file tree
Showing 20 changed files with 75 additions and 134 deletions.
2 changes: 2 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ linters-settings:
alias: mesh_proto
- pkg: github.com/kumahq/kuma/pkg/util/proto
alias: util_proto
- pkg: github.com/kumahq/kuma/pkg/plugins/bootstrap/k8s
alias: bootstrap_k8s
gomodguard:
blocked:
modules:
Expand Down
9 changes: 4 additions & 5 deletions pkg/plugins/bootstrap/k8s/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ func init() {
}

func (p *plugin) BeforeBootstrap(b *core_runtime.Builder, _ core_plugins.PluginConfig) error {
scheme := kube_runtime.NewScheme()
scheme, err := NewScheme()
if err != nil {
return err
}
config := kube_ctrl.GetConfigOrDie()
mgr, err := kube_ctrl.NewManager(
config,
Expand Down Expand Up @@ -91,10 +94,6 @@ func secretClient(systemNamespace string, config *rest.Config, scheme *kube_runt
if err != nil {
return nil, err
}
// Add kube core scheme first, otherwise cache won't start
if err := kube_core.AddToScheme(scheme); err != nil {
return nil, errors.Wrapf(err, "could not add %q to scheme", kube_core.SchemeGroupVersion)
}

// We are listing secrets by our custom "type", therefore we need to add index by this field into cache
err = kubeCache.IndexField(context.Background(), &kube_core.Secret{}, "type", func(object kube_runtime.Object) []string {
Expand Down
33 changes: 33 additions & 0 deletions pkg/plugins/bootstrap/k8s/scheme.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package k8s

import (
"github.com/pkg/errors"
appsv1 "k8s.io/api/apps/v1"
kube_core "k8s.io/api/core/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
kube_runtime "k8s.io/apimachinery/pkg/runtime"

mesh_k8s "github.com/kumahq/kuma/pkg/plugins/resources/k8s/native/api/v1alpha1"
k8scnicncfio "github.com/kumahq/kuma/pkg/plugins/runtime/k8s/apis/k8s.cni.cncf.io"
)

// NewScheme creates a new scheme with all the necessary schemas added already (kuma CRD, builtin resources, cni CRDs...).
func NewScheme() (*kube_runtime.Scheme, error) {
s := kube_runtime.NewScheme()
if err := kube_core.AddToScheme(s); err != nil {
return nil, errors.Wrapf(err, "could not add %q to scheme", kube_core.SchemeGroupVersion)
}
if err := mesh_k8s.AddToScheme(s); err != nil {
return nil, errors.Wrapf(err, "could not add %q to scheme", mesh_k8s.GroupVersion)
}
if err := k8scnicncfio.AddToScheme(s); err != nil {
return nil, errors.Wrapf(err, "could not add %q to scheme", k8scnicncfio.GroupVersion)
}
if err := apiextensionsv1.AddToScheme(s); err != nil {
return nil, errors.Wrapf(err, "could not add %q to scheme", apiextensionsv1.SchemeGroupVersion)
}
if err := appsv1.AddToScheme(s); err != nil {
return nil, errors.Wrapf(err, "could not add %q to scheme", apiextensionsv1.SchemeGroupVersion)
}
return s, nil
}
6 changes: 3 additions & 3 deletions pkg/plugins/config/k8s/k8s_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
kube_core "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"

"github.com/kumahq/kuma/pkg/plugins/bootstrap/k8s"
"github.com/kumahq/kuma/pkg/test"
)

var k8sClient client.Client
var testEnv *envtest.Environment
var k8sClientScheme = runtime.NewScheme()
var k8sClientScheme *runtime.Scheme

func TestKubernetes(t *testing.T) {
test.RunSpecs(t, "Kubernetes Config Suite")
Expand All @@ -30,7 +30,7 @@ var _ = BeforeSuite(test.Within(time.Minute, func() {
Expect(err).ToNot(HaveOccurred())
Expect(cfg).ToNot(BeNil())

err = kube_core.AddToScheme(k8sClientScheme)
k8sClientScheme, err = k8s.NewScheme()
Expect(err).NotTo(HaveOccurred())

// +kubebuilder:scaffold:scheme
Expand Down
4 changes: 0 additions & 4 deletions pkg/plugins/config/k8s/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package k8s

import (
"github.com/pkg/errors"
kube_core "k8s.io/api/core/v1"

core_plugins "github.com/kumahq/kuma/pkg/core/plugins"
core_store "github.com/kumahq/kuma/pkg/core/resources/store"
Expand All @@ -22,9 +21,6 @@ func (p *plugin) NewConfigStore(pc core_plugins.PluginContext, _ core_plugins.Pl
if !ok {
return nil, errors.Errorf("k8s controller runtime Manager hasn't been configured")
}
if err := kube_core.AddToScheme(mgr.GetScheme()); err != nil {
return nil, errors.Wrapf(err, "could not add %q to scheme", kube_core.SchemeGroupVersion)
}
converter, ok := k8s_extensions.FromResourceConverterContext(pc.Extensions())
if !ok {
return nil, errors.Errorf("k8s resource converter hasn't been configured")
Expand Down
14 changes: 5 additions & 9 deletions pkg/plugins/resources/k8s/k8s_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"

mesh_k8s "github.com/kumahq/kuma/pkg/plugins/resources/k8s/native/api/v1alpha1"
"github.com/kumahq/kuma/pkg/plugins/bootstrap/k8s"
k8s_registry "github.com/kumahq/kuma/pkg/plugins/resources/k8s/native/pkg/registry"

// +kubebuilder:scaffold:imports
Expand All @@ -41,7 +41,7 @@ import (

var k8sClient client.Client
var testEnv *envtest.Environment
var k8sClientScheme = runtime.NewScheme()
var k8sClientScheme *runtime.Scheme

func TestKubernetes(t *testing.T) {
test.RunSpecs(t, "Kubernetes Resources Suite")
Expand All @@ -60,14 +60,10 @@ var _ = BeforeSuite(test.Within(time.Minute, func() {
Expect(err).ToNot(HaveOccurred())
Expect(cfg).ToNot(BeNil())

err = sample_v1alpha1.AddToScheme(k8sClientScheme)
Expect(err).NotTo(HaveOccurred())

err = mesh_k8s.AddToScheme(k8sClientScheme)
Expect(err).NotTo(HaveOccurred())
k8sClientScheme, err = k8s.NewScheme()
Expect(err).ToNot(HaveOccurred())

err = kube_core.AddToScheme(k8sClientScheme)
Expect(err).NotTo(HaveOccurred())
Expect(sample_v1alpha1.AddToScheme(k8sClientScheme)).To(Succeed())

// +kubebuilder:scaffold:scheme

Expand Down
5 changes: 0 additions & 5 deletions pkg/plugins/runtime/k8s/controllers/configmap_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,6 @@ func (r *ConfigMapReconciler) Reconcile(req kube_ctrl.Request) (kube_ctrl.Result
}

func (r *ConfigMapReconciler) SetupWithManager(mgr kube_ctrl.Manager) error {
for _, addToScheme := range []func(*kube_runtime.Scheme) error{kube_core.AddToScheme, mesh_k8s.AddToScheme} {
if err := addToScheme(mgr.GetScheme()); err != nil {
return err
}
}
return kube_ctrl.NewControllerManagedBy(mgr).
For(&kube_core.ConfigMap{}).
Watches(&kube_source.Kind{Type: &kube_core.Service{}}, &kube_handler.EnqueueRequestsFromMapFunc{
Expand Down
8 changes: 0 additions & 8 deletions pkg/plugins/runtime/k8s/controllers/mesh_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"context"

"github.com/go-logr/logr"
"github.com/pkg/errors"
kube_core "k8s.io/api/core/v1"
kube_apierrs "k8s.io/apimachinery/pkg/api/errors"
kube_runtime "k8s.io/apimachinery/pkg/runtime"
kube_ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -69,12 +67,6 @@ func (r *MeshReconciler) Reconcile(req kube_ctrl.Request) (kube_ctrl.Result, err
}

func (r *MeshReconciler) SetupWithManager(mgr kube_ctrl.Manager) error {
if err := kube_core.AddToScheme(mgr.GetScheme()); err != nil {
return errors.Wrapf(err, "could not add %q to scheme", kube_core.SchemeGroupVersion)
}
if err := mesh_k8s.AddToScheme(mgr.GetScheme()); err != nil {
return errors.Wrapf(err, "could not add %q to scheme", mesh_k8s.GroupVersion)
}
return kube_ctrl.NewControllerManagedBy(mgr).
For(&mesh_k8s.Mesh{}).
Complete(r)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"

"github.com/pkg/errors"
kube_core "k8s.io/api/core/v1"
kube_apierrs "k8s.io/apimachinery/pkg/api/errors"
kube_ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
Expand Down Expand Up @@ -58,12 +57,6 @@ func (r *MeshDefaultsReconciler) Reconcile(req kube_ctrl.Request) (kube_ctrl.Res
}

func (r *MeshDefaultsReconciler) SetupWithManager(mgr kube_ctrl.Manager) error {
if err := kube_core.AddToScheme(mgr.GetScheme()); err != nil {
return errors.Wrapf(err, "could not add %q to scheme", kube_core.SchemeGroupVersion)
}
if err := mesh_k8s.AddToScheme(mgr.GetScheme()); err != nil {
return errors.Wrapf(err, "could not add %q to scheme", mesh_k8s.GroupVersion)
}
return kube_ctrl.NewControllerManagedBy(mgr).
For(&mesh_k8s.Mesh{}, builder.WithPredicates(onlyCreate)).
Complete(r)
Expand Down
10 changes: 0 additions & 10 deletions pkg/plugins/runtime/k8s/controllers/namespace_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/predicate"

k8scnicncfio "github.com/kumahq/kuma/pkg/plugins/runtime/k8s/apis/k8s.cni.cncf.io"
network_v1 "github.com/kumahq/kuma/pkg/plugins/runtime/k8s/apis/k8s.cni.cncf.io/v1"
"github.com/kumahq/kuma/pkg/plugins/runtime/k8s/metadata"
)
Expand Down Expand Up @@ -123,15 +122,6 @@ func (r *NamespaceReconciler) deleteNetworkAttachmentDefinition(log logr.Logger,
}

func (r *NamespaceReconciler) SetupWithManager(mgr kube_ctrl.Manager) error {
if err := kube_core.AddToScheme(mgr.GetScheme()); err != nil {
return errors.Wrapf(err, "could not add %q to scheme", kube_core.SchemeGroupVersion)
}
if err := k8scnicncfio.AddToScheme(mgr.GetScheme()); err != nil {
return errors.Wrapf(err, "could not add %q to scheme", k8scnicncfio.GroupVersion)
}
if err := apiextensionsv1.AddToScheme(mgr.GetScheme()); err != nil {
return errors.Wrapf(err, "could not add %q to scheme", apiextensionsv1.SchemeGroupVersion)
}
return kube_ctrl.NewControllerManagedBy(mgr).
For(&kube_core.Namespace{}, builder.WithPredicates(namespaceEvents)).
Complete(r)
Expand Down
5 changes: 0 additions & 5 deletions pkg/plugins/runtime/k8s/controllers/pod_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,6 @@ func (r *PodReconciler) createOrUpdateIngress(pod *kube_core.Pod, services []*ku
}

func (r *PodReconciler) SetupWithManager(mgr kube_ctrl.Manager) error {
for _, addToScheme := range []func(*kube_runtime.Scheme) error{kube_core.AddToScheme, mesh_k8s.AddToScheme} {
if err := addToScheme(mgr.GetScheme()); err != nil {
return err
}
}
return kube_ctrl.NewControllerManagedBy(mgr).
For(&kube_core.Pod{}).
// on Service update reconcile affected Pods (all Pods in the same namespace)
Expand Down
6 changes: 0 additions & 6 deletions pkg/plugins/runtime/k8s/controllers/pod_status_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,6 @@ func (r *PodStatusReconciler) Reconcile(req kube_ctrl.Request) (kube_ctrl.Result
}

func (r *PodStatusReconciler) SetupWithManager(mgr kube_ctrl.Manager) error {
for _, addToScheme := range []func(*kube_runtime.Scheme) error{kube_core.AddToScheme, mesh_k8s.AddToScheme} {
if err := addToScheme(mgr.GetScheme()); err != nil {
return err
}
}

return kube_ctrl.NewControllerManagedBy(mgr).
For(&kube_core.Pod{}, builder.WithPredicates(podStatusEvents)).
Complete(r)
Expand Down
11 changes: 0 additions & 11 deletions pkg/plugins/runtime/k8s/controllers/service_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"github.com/go-logr/logr"
"github.com/pkg/errors"
kube_core "k8s.io/api/core/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
kube_apierrs "k8s.io/apimachinery/pkg/api/errors"
kube_types "k8s.io/apimachinery/pkg/types"
kube_ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -16,7 +15,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/predicate"

k8scnicncfio "github.com/kumahq/kuma/pkg/plugins/runtime/k8s/apis/k8s.cni.cncf.io"
"github.com/kumahq/kuma/pkg/plugins/runtime/k8s/metadata"
)

Expand Down Expand Up @@ -71,15 +69,6 @@ func (r *ServiceReconciler) Reconcile(req kube_ctrl.Request) (kube_ctrl.Result,
}

func (r *ServiceReconciler) SetupWithManager(mgr kube_ctrl.Manager) error {
if err := kube_core.AddToScheme(mgr.GetScheme()); err != nil {
return errors.Wrapf(err, "could not add %q to scheme", kube_core.SchemeGroupVersion)
}
if err := k8scnicncfio.AddToScheme(mgr.GetScheme()); err != nil {
return errors.Wrapf(err, "could not add %q to scheme", k8scnicncfio.GroupVersion)
}
if err := apiextensionsv1.AddToScheme(mgr.GetScheme()); err != nil {
return errors.Wrapf(err, "could not add %q to scheme", apiextensionsv1.SchemeGroupVersion)
}
return kube_ctrl.NewControllerManagedBy(mgr).
For(&kube_core.Service{}, builder.WithPredicates(serviceEvents)).
Complete(r)
Expand Down
15 changes: 3 additions & 12 deletions pkg/plugins/runtime/k8s/controllers/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,17 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
kube_core "k8s.io/api/core/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"

meshv1alpha1 "github.com/kumahq/kuma/pkg/plugins/resources/k8s/native/api/v1alpha1"
k8scnicncfio "github.com/kumahq/kuma/pkg/plugins/runtime/k8s/apis/k8s.cni.cncf.io"
"github.com/kumahq/kuma/pkg/plugins/bootstrap/k8s"
"github.com/kumahq/kuma/pkg/test"
)

var k8sClient client.Client
var testEnv *envtest.Environment
var k8sClientScheme = runtime.NewScheme()
var k8sClientScheme *runtime.Scheme

func TestAPIs(t *testing.T) {
test.RunSpecs(t, "Namespace Controller Suite")
Expand All @@ -36,13 +33,7 @@ var _ = BeforeSuite(test.Within(time.Minute, func() {
Expect(err).ToNot(HaveOccurred())
Expect(cfg).ToNot(BeNil())

err = meshv1alpha1.AddToScheme(k8sClientScheme)
Expect(err).NotTo(HaveOccurred())
err = kube_core.AddToScheme(k8sClientScheme)
Expect(err).NotTo(HaveOccurred())
err = k8scnicncfio.AddToScheme(k8sClientScheme)
Expect(err).NotTo(HaveOccurred())
err = apiextensionsv1.AddToScheme(k8sClientScheme)
k8sClientScheme, err = k8s.NewScheme()
Expect(err).NotTo(HaveOccurred())

// +kubebuilder:scaffold:scheme
Expand Down
10 changes: 3 additions & 7 deletions pkg/plugins/runtime/k8s/webhooks/injector/injector_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,17 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
kube_core "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"

mesh_k8s "github.com/kumahq/kuma/pkg/plugins/resources/k8s/native/api/v1alpha1"
"github.com/kumahq/kuma/pkg/plugins/bootstrap/k8s"
"github.com/kumahq/kuma/pkg/test"
)

var k8sClient client.Client
var testEnv *envtest.Environment
var k8sClientScheme = runtime.NewScheme()
var k8sClientScheme *runtime.Scheme

func TestInjector(t *testing.T) {
test.RunSpecs(t, "Kubernetes Resources Suite")
Expand All @@ -52,10 +51,7 @@ var _ = BeforeSuite(test.Within(time.Minute, func() {
Expect(err).ToNot(HaveOccurred())
Expect(cfg).ToNot(BeNil())

err = mesh_k8s.AddToScheme(k8sClientScheme)
Expect(err).NotTo(HaveOccurred())

err = kube_core.AddToScheme(k8sClientScheme)
k8sClientScheme, err = k8s.NewScheme()
Expect(err).NotTo(HaveOccurred())

// +kubebuilder:scaffold:scheme
Expand Down
8 changes: 3 additions & 5 deletions pkg/plugins/runtime/k8s/webhooks/service_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ import (
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
admissionv1beta1 "k8s.io/api/admission/v1beta1"
kube_core "k8s.io/api/core/v1"
kube_runtime "k8s.io/apimachinery/pkg/runtime"
kube_admission "sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"github.com/kumahq/kuma/pkg/plugins/bootstrap/k8s"
. "github.com/kumahq/kuma/pkg/plugins/runtime/k8s/webhooks"
)

Expand All @@ -20,11 +19,10 @@ var _ = Describe("ServiceValidator", func() {
var decoder *kube_admission.Decoder

BeforeEach(func() {
scheme := kube_runtime.NewScheme()
scheme, err := k8s.NewScheme()
// expect
Expect(kube_core.AddToScheme(scheme)).To(Succeed())
Expect(err).ToNot(HaveOccurred())

var err error
// when
decoder, err = kube_admission.NewDecoder(scheme)
// then
Expand Down
Loading

0 comments on commit fdc5f1a

Please sign in to comment.