Repository: containous/maesh
Pull request #716 (2020-08-27 09:44:03)

Remove unnecessary RBAC rules for controller and proxy
## What does this PR do?

This PR:

- Removes unnecessary RBAC rules for controller and proxy
- Removes dead code

### How to test it

* make test-integration


Patch:
diff --git a/helm/chart/maesh/templates/controller/controller-rbac.yaml b/helm/chart/maesh/templates/controller/controller-rbac.yaml
index 293e1d0..7feac8c 100644
--- a/helm/chart/maesh/templates/controller/controller-rbac.yaml
+++ b/helm/chart/maesh/templates/controller/controller-rbac.yaml
@@ -19,13 +19,6 @@ rules:
     verbs:
       - list
       - watch
-  - apiGroups:
-      - ""
-    resources:
-      - namespaces
-    verbs:
-      - get
-      - create
   - apiGroups:
       - ""
     resources:
@@ -34,8 +27,6 @@ rules:
       - get
       - create
       - update
-      - list
-      - watch
   - apiGroups:
       - ""
     resources:
@@ -54,7 +45,6 @@ rules:
     verbs:
       - get
       - update
-      - create
   - apiGroups:
       - access.smi-spec.io
       - specs.smi-spec.io
diff --git a/helm/chart/maesh/templates/dns/coredns/coredns-configmap.yaml b/helm/chart/maesh/templates/coredns/coredns-configmap.yaml
similarity index 100%
rename from helm/chart/maesh/templates/dns/coredns/coredns-configmap.yaml
rename to helm/chart/maesh/templates/coredns/coredns-configmap.yaml
diff --git a/helm/chart/maesh/templates/dns/coredns/coredns-deployment.yaml b/helm/chart/maesh/templates/coredns/coredns-deployment.yaml
similarity index 100%
rename from helm/chart/maesh/templates/dns/coredns/coredns-deployment.yaml
rename to helm/chart/maesh/templates/coredns/coredns-deployment.yaml
diff --git a/helm/chart/maesh/templates/dns/coredns/coredns-rbac.yaml b/helm/chart/maesh/templates/coredns/coredns-rbac.yaml
similarity index 100%
rename from helm/chart/maesh/templates/dns/coredns/coredns-rbac.yaml
rename to helm/chart/maesh/templates/coredns/coredns-rbac.yaml
diff --git a/helm/chart/maesh/templates/dns/coredns/coredns-sa.yaml b/helm/chart/maesh/templates/coredns/coredns-sa.yaml
similarity index 100%
rename from helm/chart/maesh/templates/dns/coredns/coredns-sa.yaml
rename to helm/chart/maesh/templates/coredns/coredns-sa.yaml
diff --git a/helm/chart/maesh/templates/dns/coredns/coredns-service.yaml b/helm/chart/maesh/templates/coredns/coredns-service.yaml
similarity index 100%
rename from helm/chart/maesh/templates/dns/coredns/coredns-service.yaml
rename to helm/chart/maesh/templates/coredns/coredns-service.yaml
diff --git a/helm/chart/maesh/templates/mesh/mesh-daemonset.yaml b/helm/chart/maesh/templates/mesh/mesh-daemonset.yaml
index 8bd9c0f..b4099a8 100644
--- a/helm/chart/maesh/templates/mesh/mesh-daemonset.yaml
+++ b/helm/chart/maesh/templates/mesh/mesh-daemonset.yaml
@@ -34,7 +34,6 @@ spec:
         {{- end }}
     spec:
       serviceAccountName: maesh-mesh
-      automountServiceAccountToken: true
       securityContext:
         runAsNonRoot: true
         runAsUser: 999
diff --git a/helm/chart/maesh/templates/mesh/mesh-rbac.yaml b/helm/chart/maesh/templates/mesh/mesh-rbac.yaml
deleted file mode 100644
index 9977b85..0000000
--- a/helm/chart/maesh/templates/mesh/mesh-rbac.yaml
+++ /dev/null
@@ -1,40 +0,0 @@
----
-kind: ClusterRole
-apiVersion: rbac.authorization.k8s.io/v1beta1
-metadata:
-  name: maesh-mesh-role
-  namespace: {{ .Release.Namespace }}
-  labels:
-    app: maesh
-    component: maesh-mesh
-    chart: {{ include "maesh.chartLabel" . | quote}}
-    release: {{ .Release.Name | quote }}
-    heritage: {{ .Release.Service | quote }}
-rules:
-  - apiGroups:
-      - ""
-    resources:
-      - pods
-      - services
-    verbs:
-      - list
----
-kind: ClusterRoleBinding
-apiVersion: rbac.authorization.k8s.io/v1beta1
-metadata:
-  name: maesh-mesh
-  namespace: {{ .Release.Namespace }}
-  labels:
-    app: maesh
-    component: maesh-mesh
-    chart: {{ include "maesh.chartLabel" . | quote}}
-    release: {{ .Release.Name | quote }}
-    heritage: {{ .Release.Service | quote }}
-roleRef:
-  apiGroup: rbac.authorization.k8s.io
-  kind: ClusterRole
-  name: maesh-mesh-role
-subjects:
-  - kind: ServiceAccount
-    name: maesh-mesh
-    namespace: {{ .Release.Namespace }}
diff --git a/helm/chart/maesh/templates/mesh/mesh-sa.yaml b/helm/chart/maesh/templates/mesh/mesh-sa.yaml
index 764468e..8d33544 100644
--- a/helm/chart/maesh/templates/mesh/mesh-sa.yaml
+++ b/helm/chart/maesh/templates/mesh/mesh-sa.yaml
@@ -10,3 +10,4 @@ metadata:
     chart: {{ include "maesh.chartLabel" . | quote}}
     release: {{ .Release.Name | quote }}
     heritage: {{ .Release.Service | quote }}
+automountServiceAccountToken: false
diff --git a/pkg/k8s/client_mock.go b/pkg/k8s/client_mock.go
index da7702f..7678ef8 100644
--- a/pkg/k8s/client_mock.go
+++ b/pkg/k8s/client_mock.go
@@ -13,23 +13,14 @@ import (
 	split "github.com/servicemeshinterface/smi-sdk-go/pkg/apis/split/v1alpha3"
 	accessclient "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/access/clientset/versioned"
 	fakeaccessclient "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/access/clientset/versioned/fake"
-	accessinformer "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/access/informers/externalversions"
-	accesslister "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/access/listers/access/v1alpha2"
 	specsclient "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/specs/clientset/versioned"
 	fakespecsclient "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/specs/clientset/versioned/fake"
-	specsinformer "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/specs/informers/externalversions"
-	specslister "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/specs/listers/specs/v1alpha3"
 	splitclient "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/split/clientset/versioned"
 	fakesplitclient "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/split/clientset/versioned/fake"
-	splitinformer "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/split/informers/externalversions"
-	splitlister "github.com/servicemeshinterface/smi-sdk-go/pkg/gen/client/split/listers/split/v1alpha3"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/client-go/informers"
 	kubeclient "k8s.io/client-go/kubernetes"
 	fakekubeclient "k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/kubernetes/scheme"
-	listers "k8s.io/client-go/listers/core/v1"
-	"k8s.io/client-go/tools/cache"
 )
 
 // Ensure the client mock fits the Client interface.
@@ -61,24 +52,10 @@ type ClientMock struct {
 	accessClient *fakeaccessclient.Clientset
 	specsClient  *fakespecsclient.Clientset
 	splitClient  *fakesplitclient.Clientset
-
-	informerFactory       informers.SharedInformerFactory
-	accessInformerFactory accessinformer.SharedInformerFactory
-	specsInformerFactory  specsinformer.SharedInformerFactory
-	splitInformerFactory  splitinformer.SharedInformerFactory
-
-	PodLister            listers.PodLister
-	ServiceLister        listers.ServiceLister
-	EndpointsLister      listers.EndpointsLister
-	NamespaceLister      listers.NamespaceLister
-	TrafficTargetLister  accesslister.TrafficTargetLister
-	HTTPRouteGroupLister specslister.HTTPRouteGroupLister
-	TCPRouteLister       specslister.TCPRouteLister
-	TrafficSplitLister   splitlister.TrafficSplitLister
 }
 
 // NewClientMock create a new client mock.
-func NewClientMock(testingT *testing.T, stopCh <-chan struct{}, path string, acl bool) *ClientMock {
+func NewClientMock(testingT *testing.T, path string) *ClientMock {
 	yamlContent, err := ioutil.ReadFile(filepath.FromSlash("./testdata/" + path))
 	if err != nil {
 		panic(err)
@@ -89,94 +66,11 @@ func NewClientMock(testingT *testing.T, stopCh <-chan struct{}, path string, acl
 
 	c.kubeClient = fakekubeclient.NewSimpleClientset(filterObjectsByKind(k8sObjects, CoreObjectKinds)...)
 	c.splitClient = fakesplitclient.NewSimpleClientset(filterObjectsByKind(k8sObjects, SplitObjectKinds)...)
-
-	c.informerFactory = informers.NewSharedInformerFactory(c.kubeClient, 0)
-	c.splitInformerFactory = splitinformer.NewSharedInformerFactory(c.splitClient, 0)
-
-	podInformer := c.informerFactory.Core().V1().Pods().Informer()
-	serviceInformer := c.informerFactory.Core().V1().Services().Informer()
-	endpointsInformer := c.informerFactory.Core().V1().Endpoints().Informer()
-	namespaceInformer := c.informerFactory.Core().V1().Namespaces().Informer()
-	trafficSplitInformer := c.splitInformerFactory.Split().V1alpha3().TrafficSplits().Informer()
-
-	podInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{})
-	serviceInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{})
-	endpointsInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{})
-	namespaceInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{})
-	trafficSplitInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{})
-
-	c.PodLister = c.informerFactory.Core().V1().Pods().Lister()
-	c.ServiceLister = c.informerFactory.Core().V1().Services().Lister()
-	c.EndpointsLister = c.informerFactory.Core().V1().Endpoints().Lister()
-	c.NamespaceLister = c.informerFactory.Core().V1().Namespaces().Lister()
-	c.TrafficSplitLister = c.splitInformerFactory.Split().V1alpha3().TrafficSplits().Lister()
-
-	// Start the informers.
-	c.startInformers(stopCh)
-
-	if acl {
-		c.accessClient = fakeaccessclient.NewSimpleClientset(filterObjectsByKind(k8sObjects, AccessObjectKinds)...)
-		c.specsClient = fakespecsclient.NewSimpleClientset(filterObjectsByKind(k8sObjects, SpecsObjectKinds)...)
-
-		c.accessInformerFactory = accessinformer.NewSharedInformerFactory(c.accessClient, 0)
-		c.specsInformerFactory = specsinformer.NewSharedInformerFactory(c.specsClient, 0)
-
-		trafficTargetInformer := c.accessInformerFactory.Access().V1alpha2().TrafficTargets().Informer()
-		httpRouteGroupInformer := c.specsInformerFactory.Specs().V1alpha3().HTTPRouteGroups().Informer()
-		tcpRouteInformer := c.specsInformerFactory.Specs().V1alpha3().TCPRoutes().Informer()
-
-		trafficTargetInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{})
-		httpRouteGroupInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{})
-		tcpRouteInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{})
-
-		c.TrafficTargetLister = c.accessInformerFactory.Access().V1alpha2().TrafficTargets().Lister()
-		c.HTTPRouteGroupLister = c.specsInformerFactory.Specs().V1alpha3().HTTPRouteGroups().Lister()
-		c.TCPRouteLister = c.specsInformerFactory.Specs().V1alpha3().TCPRoutes().Lister()
-
-		// Start the informers.
-		c.startACLInformers(stopCh)
-	}
+	c.specsClient = fakespecsclient.NewSimpleClientset(filterObjectsByKind(k8sObjects, SpecsObjectKinds)...)
 
 	return c
 }
 
-// startInformers waits for the kubernetes core informers to start and sync.
-func (c *ClientMock) startInformers(stopCh <-chan struct{}) {
-	c.informerFactory.Start(stopCh)
-
-	for t, ok := range c.informerFactory.WaitForCacheSync(stopCh) {
-		if !ok {
-			c.testingT.Logf("timed out waiting for controller caches to sync: %s", t)
-		}
-	}
-
-	c.splitInformerFactory.Start(stopCh)
-
-	for t, ok := range c.splitInformerFactory.WaitForCacheSync(stopCh) {
-		if !ok {
-			c.testingT.Logf("timed out waiting for controller caches to sync: %s", t)
-		}
-	}
-}
-
-// startACLInformers waits for the ACL informers to start and sync.
-func (c *ClientMock) startACLInformers(stopCh <-chan struct{}) {
-	c.accessInformerFactory.Start(stopCh)
-	c.specsInformerFactory.Start(stopCh)
-
-	for t, ok := range c.accessInformerFactory.WaitForCacheSync(stopCh) {
-		if !ok {
-			c.testingT.Logf("timed out waiting for controller caches to sync: %s", t)
-		}
-	}
-
-	for t, ok := range c.specsInformerFactory.WaitForCacheSync(stopCh) {
-		if !ok {
-			c.testingT.Logf("timed out waiting for controller caches to sync: %s", t)
-		}
-	}
-}
-
 // KubernetesClient is used to get the kubernetes clientset.
 func (c *ClientMock) KubernetesClient() kubeclient.Interface {
 	return c.kubeClient


Test patch:
diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go
index 832199d..3c8ee1b 100644
--- a/pkg/api/api_test.go
+++ b/pkg/api/api_test.go
@@ -1,7 +1,6 @@
 package api
 
 import (
-	"context"
 	"net"
 	"net/http"
 	"net/http/httptest"
@@ -147,10 +146,7 @@ func TestGetMeshNodes(t *testing.T) {
 			log.SetOutput(os.Stdout)
 			log.SetLevel(logrus.DebugLevel)
 
-			ctx, cancel := context.WithCancel(context.Background())
-			defer cancel()
-
-			clientMock := k8s.NewClientMock(t, ctx.Done(), test.mockFile, false)
+			clientMock := k8s.NewClientMock(t, test.mockFile)
 			api, err := NewAPI(log, 9000, localhost, clientMock.KubernetesClient(), "foo")
 
 			require.NoError(t, err)
@@ -203,10 +199,7 @@ func TestGetMeshNodeConfiguration(t *testing.T) {
 			log.SetOutput(os.Stdout)
 			log.SetLevel(logrus.DebugLevel)
 
-			ctx, cancel := context.WithCancel(context.Background())
-			defer cancel()
-
-			clientMock := k8s.NewClientMock(t, ctx.Done(), test.mockFile, false)
+			clientMock := k8s.NewClientMock(t, test.mockFile)
 			api, err := NewAPI(log, 9000, localhost, clientMock.KubernetesClient(), "foo")
 
 			require.NoError(t, err)
diff --git a/pkg/cleanup/cleanup_test.go b/pkg/cleanup/cleanup_test.go
index 60c305f..7740245 100644
--- a/pkg/cleanup/cleanup_test.go
+++ b/pkg/cleanup/cleanup_test.go
@@ -13,10 +13,7 @@ import (
 )
 
 func TestCleanup_New(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
-	clientMock := k8s.NewClientMock(t, ctx.Done(), "mock.yaml", false)
+	clientMock := k8s.NewClientMock(t, "mock.yaml")
 	logger := logrus.New()
 
 	logger.SetOutput(os.Stdout)
@@ -27,10 +24,7 @@ func TestCleanup_New(t *testing.T) {
 }
 
 func TestCleanup_CleanShadowServices(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
-	clientMock := k8s.NewClientMock(t, ctx.Done(), "mock.yaml", false)
+	clientMock := k8s.NewClientMock(t, "mock.yaml")
 	logger := logrus.New()
 
 	logger.SetOutput(os.Stdout)
@@ -39,16 +33,16 @@ func TestCleanup_CleanShadowServices(t *testing.T) {
 	cleanup := NewCleanup(logger, clientMock.KubernetesClient(), "maesh")
 	require.NotNil(t, cleanup)
 
-	err := cleanup.CleanShadowServices(ctx)
+	err := cleanup.CleanShadowServices(context.Background())
 	require.NoError(t, err)
 
-	serviceList, err := clientMock.KubernetesClient().CoreV1().Services(metav1.NamespaceAll).List(ctx, metav1.ListOptions{
+	serviceList, err := clientMock.KubernetesClient().CoreV1().Services(metav1.NamespaceAll).List(context.Background(), metav1.ListOptions{
 		LabelSelector: "app=maesh,type=shadow",
 	})
 	require.NoError(t, err)
 	assert.Len(t, serviceList.Items, 0)
 
-	serviceList, err = clientMock.KubernetesClient().CoreV1().Services(metav1.NamespaceAll).List(ctx, metav1.ListOptions{})
+	serviceList, err = clientMock.KubernetesClient().CoreV1().Services(metav1.NamespaceAll).List(context.Background(), metav1.ListOptions{})
 	require.NoError(t, err)
 	assert.Len(t, serviceList.Items, 2)
 }
diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go
index 7207b7d..e664c6f 100644
--- a/pkg/controller/controller_test.go
+++ b/pkg/controller/controller_test.go
@@ -1,7 +1,6 @@
 package controller
 
 import (
-	"context"
 	"os"
 	"testing"
 
@@ -29,11 +28,8 @@ func (a *storeMock) SetTopology(topo *topology.Topology)  {}
 func (a *storeMock) SetReadiness(isReady bool)            {}
 
 func TestController_NewMeshController(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
 	store := &storeMock{}
-	clientMock := k8s.NewClientMock(t, ctx.Done(), "mock.yaml", false)
+	clientMock := k8s.NewClientMock(t, "mock.yaml")
 	log := logrus.New()
 
 	log.SetOutput(os.Stdout)
@@ -57,11 +53,8 @@ func TestController_NewMeshController(t *testing.T) {
 }
 
 func TestController_NewMeshControllerWithSMI(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
 	store := &storeMock{}
-	clientMock := k8s.NewClientMock(t, ctx.Done(), "mock.yaml", true)
+	clientMock := k8s.NewClientMock(t, "mock.yaml")
 	log := logrus.New()
 
 	log.SetOutput(os.Stdout)
diff --git a/pkg/dns/dns_test.go b/pkg/dns/dns_test.go
index fa80e9b..7324e37 100644
--- a/pkg/dns/dns_test.go
+++ b/pkg/dns/dns_test.go
@@ -51,7 +51,7 @@ func TestCheckDNSProvider(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 
-			k8sClient := k8s.NewClientMock(t, ctx.Done(), test.mockFile, false)
+			k8sClient := k8s.NewClientMock(t, test.mockFile)
 
 			log := logrus.New()
 
@@ -152,7 +152,7 @@ func TestConfigureCoreDNS(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 
-			k8sClient := k8s.NewClientMock(t, ctx.Done(), test.mockFile, false)
+			k8sClient := k8s.NewClientMock(t, test.mockFile)
 
 			log := logrus.New()
 
@@ -226,7 +226,7 @@ func TestConfigureKubeDNS(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 
-			k8sClient := k8s.NewClientMock(t, ctx.Done(), test.mockFile, false)
+			k8sClient := k8s.NewClientMock(t, test.mockFile)
 
 			log := logrus.New()
 
@@ -286,7 +286,7 @@ func TestRestoreCoreDNS(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 
-			k8sClient := k8s.NewClientMock(t, ctx.Done(), test.mockFile, false)
+			k8sClient := k8s.NewClientMock(t, test.mockFile)
 
 			log := logrus.New()
 
@@ -340,7 +340,7 @@ func TestRestoreKubeDNS(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 
-			k8sClient := k8s.NewClientMock(t, ctx.Done(), test.mockFile, false)
+			k8sClient := k8s.NewClientMock(t, test.mockFile)
 
 			log := logrus.New()