Missing permission (#20)

* Add missing permission to role #19 * cleanup code
bakito · Jan 15, 2020 · 0f1a4b7 · 0f1a4b7
1 parent ac8166d
commit 0f1a4b7
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 34 deletions.
diff --git a/helm/templates/_helpers.tpl b/helm/templates/_helpers.tpl
@@ -36,6 +36,7 @@ Common labels
 */}}
 {{- define "k8s-event-logger-operator.labels" -}}
 helm.sh/chart: {{ include "k8s-event-logger-operator.chart" . }}
+helm.sh/namespace: {{ .Release.Namespace }}
 {{ include "k8s-event-logger-operator.selectorLabels" . }}
 {{- if .Chart.AppVersion }}
 app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}

diff --git a/pkg/controller/event/event_controller.go b/pkg/controller/event/event_controller.go
@@ -50,15 +50,14 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 
 	// Watch for changes to primary resource EventLogger
 	err = c.Watch(&source.Kind{Type: &eventloggerv1.EventLogger{}}, &handler.EnqueueRequestForObject{})
+	if err != nil {
+		return err
+	}
 
 	// Watch for changes to primary resource Event
 	p := &loggingPredicate{}
 	p.lastVersion, err = getLatestRevision(mgr)
 
-	if err != nil {
-		return err
-	}
-
 	return c.Watch(&source.Kind{Type: &corev1.Event{}}, &handler.Funcs{}, p)
 }
 

diff --git a/pkg/controller/event/event_controller_test.go b/pkg/controller/event/event_controller_test.go
@@ -69,72 +69,72 @@ var shouldLogData = []struct {
 		false,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod"}}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod"}}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}},
 		true,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "ConfigMap"}}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "ConfigMap"}}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}},
 		false,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod", EventTypes: []string{}}}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod", EventTypes: []string{}}}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}},
 		true,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod"}}, EventTypes: []string{"Normal"}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod"}}, EventTypes: []string{"Normal"}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Type: "Normal"},
 		true,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod"}}, EventTypes: []string{"Warning"}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod"}}, EventTypes: []string{"Warning"}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Type: "Normal"},
 		false,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod", EventTypes: []string{"Normal"}}}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod", EventTypes: []string{"Normal"}}}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Type: "Normal"},
 		true,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod", EventTypes: []string{"Warning"}}}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod", EventTypes: []string{"Warning"}}}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Type: "Normal"},
 		false,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod", EventTypes: []string{"Normal"}}}, EventTypes: []string{"Warning"}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod", EventTypes: []string{"Normal"}}}, EventTypes: []string{"Warning"}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Type: "Normal"},
 		true,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod", MatchingPatterns: []string{".*message.*"}}}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod", MatchingPatterns: []string{".*message.*"}}}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Message: "This is a test message"},
 		true,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod", MatchingPatterns: []string{".*Message.*"}}}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod", MatchingPatterns: []string{".*Message.*"}}}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Message: "This is a test message"},
 		false,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod", MatchingPatterns: []string{".*message.*"}, SkipOnMatch: &varFalse}}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod", MatchingPatterns: []string{".*message.*"}, SkipOnMatch: &varFalse}}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Message: "This is a test message"},
 		true,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod", MatchingPatterns: []string{".*Message.*"}, SkipOnMatch: &varFalse}}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod", MatchingPatterns: []string{".*Message.*"}, SkipOnMatch: &varFalse}}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Message: "This is a test message"},
 		false,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod", MatchingPatterns: []string{".*message.*"}, SkipOnMatch: &varTrue}}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod", MatchingPatterns: []string{".*message.*"}, SkipOnMatch: &varTrue}}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Message: "This is a test message"},
 		false,
 	},
 	{
-		v1.EventLoggerSpec{Kinds: []v1.Kind{v1.Kind{Name: "Pod", MatchingPatterns: []string{".*Message.*"}, SkipOnMatch: &varTrue}}},
+		v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod", MatchingPatterns: []string{".*Message.*"}, SkipOnMatch: &varTrue}}},
 		corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Message: "This is a test message"},
 		true,
 	},

diff --git a/pkg/controller/pod/pod_controller.go b/pkg/controller/pod/pod_controller.go
@@ -168,24 +168,18 @@ func (r *ReconcileEventLogger) Reconcile(request reconcile.Request) (reconcile.R
 	} else {
 		// Only delete sa if the name is different than the configured
 		if cr.Spec.ServiceAccount != sacc.GetName() {
-			err = r.client.Delete(context.TODO(), sacc)
+			err = r.saveDelete(sacc)
 			if err != nil {
-				if !errors.IsNotFound(err) {
-					return r.updateCR(cr, reqLogger, err)
-				}
+				return r.updateCR(cr, reqLogger, err)
 			}
 		}
-		err = r.client.Delete(context.TODO(), role)
+		err = r.saveDelete(role)
 		if err != nil {
-			if !errors.IsNotFound(err) {
-				return r.updateCR(cr, reqLogger, err)
-			}
+			return r.updateCR(cr, reqLogger, err)
 		}
-		err = r.client.Delete(context.TODO(), rb)
+		err = r.saveDelete(rb)
 		if err != nil {
-			if !errors.IsNotFound(err) {
-				return r.updateCR(cr, reqLogger, err)
-			}
+			return r.updateCR(cr, reqLogger, err)
 		}
 	}
 
@@ -314,6 +308,16 @@ func (r *ReconcileEventLogger) updateCR(cr *eventloggerv1.EventLogger, logger lo
 	return reconcile.Result{}, updErr
 }
 
+func (r *ReconcileEventLogger) saveDelete(obj runtime.Object) error {
+	err := r.client.Delete(context.TODO(), obj)
+	if err != nil {
+		if !errors.IsNotFound(err) {
+			return err
+		}
+	}
+	return nil
+}
+
 // podForCR returns a pod with the same name/namespace as the cr
 func podForCR(cr *eventloggerv1.EventLogger) *corev1.Pod {
 	labels := make(map[string]string)
@@ -431,9 +435,13 @@ func rbacForCR(cr *eventloggerv1.EventLogger) (*corev1.ServiceAccount, *rbacv1.R
 				Resources: []string{"events", "pods"},
 				Verbs:     []string{"watch", "get", "list"},
 			},
+			{
+				APIGroups: []string{"eventlogger.bakito.ch"},
+				Resources: []string{"eventloggers"},
+				Verbs:     []string{"get", "list", "patch", "update", "watch"},
+			},
 		},
 	}
-
 	rb := &rbacv1.RoleBinding{
 		TypeMeta: metav1.TypeMeta{
 			Kind: "RoleBinding",

diff --git a/pkg/controller/pod/pod_controller_test.go b/pkg/controller/pod/pod_controller_test.go
@@ -92,15 +92,26 @@ func TestPodController(t *testing.T) {
 	Assert(t, is.Equal(evars[c.EnvConfigName].Value, el.GetName()))
 	Assert(t, is.Equal(evars["WATCH_NAMESPACE"].Value, ns2))
 
-	// role, service account and rolebinding
+	// service account
 	saccList := &corev1.ServiceAccountList{}
 	assertEntrySize(t, cl, saccList, 1)
 	Assert(t, is.Equal(saccList.Items[0].ObjectMeta.Name, loggerName(el)))
 
+	// role
 	roleList := &rbacv1.RoleList{}
 	assertEntrySize(t, cl, roleList, 1)
-	Assert(t, is.Equal(roleList.Items[0].ObjectMeta.Name, loggerName(el)))
-
+	role := roleList.Items[0]
+	Assert(t, is.Equal(role.ObjectMeta.Name, loggerName(el)))
+	Assert(t, is.Len(role.Rules, 2))
+	Assert(t, is.DeepEqual(role.Rules[0].APIGroups, []string{""}))
+	Assert(t, is.DeepEqual(role.Rules[0].Resources, []string{"events", "pods"}))
+	Assert(t, is.DeepEqual(role.Rules[0].Verbs, []string{"watch", "get", "list"}))
+
+	Assert(t, is.DeepEqual(role.Rules[1].APIGroups, []string{"eventlogger.bakito.ch"}))
+	Assert(t, is.DeepEqual(role.Rules[1].Resources, []string{"eventloggers"}))
+	Assert(t, is.DeepEqual(role.Rules[1].Verbs, []string{"get", "list", "patch", "update", "watch"}))
+
+	// rolebinding
 	rbList := &rbacv1.RoleBindingList{}
 	assertEntrySize(t, cl, rbList, 1)
 	Assert(t, is.Equal(rbList.Items[0].ObjectMeta.Name, loggerName(el)))