fix(rbac): narrow permissions

Signed-off-by: Pedro Tôrres <[email protected]>
kedacore · Feb 25, 2023 · 569dbc0 · 569dbc0
1 parent ab7ce09
commit 569dbc0
Show file tree

Hide file tree

Showing 19 changed files with 200 additions and 57 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -25,7 +25,7 @@ This changelog keeps track of work items that have been completed and are ready
 
 ### Fixes
 
-- **General**: TODO ([#TODO](https://github.com/kedacore/http-add-on/issues/TODO))
+- **RBAC**: Introduce fine-grained permissions per component and reduce required permissions ([#612](https://github.com/kedacore/http-add-on/issues/612))
 
 ### Deprecations
 

diff --git a/config/interceptor/interceptor.yaml b/config/interceptor/interceptor.yaml
@@ -26,7 +26,7 @@ spec:
     spec:
       securityContext:
         runAsNonRoot: true
-      serviceAccountName: keda-http-add-on
+      serviceAccountName: keda-http-add-on-interceptor
       containers:
         - name: interceptor
           image: ghcr.io/kedacore/http-add-on-interceptor:latest

diff --git a/config/interceptor/kustomization.yaml b/config/interceptor/kustomization.yaml
@@ -2,6 +2,9 @@ resources:
 - interceptor.yaml
 - service-admin.yaml
 - service-proxy.yaml
+- role.yaml
+- role_binding.yaml
+- service_account.yaml
 
 apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization

diff --git a/config/interceptor/role.yaml b/config/interceptor/role.yaml
@@ -0,0 +1,31 @@
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  creationTimestamp: null
+  name: keda-http-add-on-interceptor
+rules:
+- apiGroups:
+  - apps
+  resources:
+  - deployments
+  verbs:
+  - get
+  - list
+  - watch
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  creationTimestamp: null
+  name: keda-http-add-on-interceptor
+  namespace: keda
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - configmaps
+  verbs:
+  - get
+  - list
+  - watch
diff --git a/config/interceptor/role_binding.yaml b/config/interceptor/role_binding.yaml
@@ -0,0 +1,26 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: keda-http-add-on-interceptor
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: keda-http-add-on-interceptor
+subjects:
+- kind: ServiceAccount
+  name: keda-http-add-on-interceptor
+  namespace: keda
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: keda-http-add-on-interceptor
+  namespace: keda
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: keda-http-add-on-interceptor
+subjects:
+- kind: ServiceAccount
+  name: keda-http-add-on-interceptor
+  namespace: keda
diff --git a/config/interceptor/service_account.yaml b/config/interceptor/service_account.yaml
@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  labels:
+    app.kubernetes.io/name: keda-http-add-on
+    app.kubernetes.io/version: latest
+    app.kubernetes.io/part-of: keda-http-add-on
+  name: keda-http-add-on-interceptor
+  namespace: keda
diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml
@@ -6,53 +6,23 @@ metadata:
   name: keda-http-add-on
 rules:
 - apiGroups:
-  - ""
-  resources:
-  - configmaps
-  - configmaps/status
-  - endpoint
-  - endpoints
-  - events
-  - pods
-  - services
-  verbs:
-  - create
-  - delete
-  - get
-  - list
-  - watch
-- apiGroups:
-  - apps
-  resources:
-  - deployments
-  verbs:
-  - create
-  - delete
-  - get
-  - list
-  - watch
-- apiGroups:
-  - coordination.k8s.io
+  - http.keda.sh
   resources:
-  - leases
+  - httpscaledobjects
   verbs:
   - create
   - delete
   - get
   - list
+  - patch
   - update
+  - watch
 - apiGroups:
   - http.keda.sh
   resources:
-  - httpscaledobjects
+  - httpscaledobjects/finalizers
   verbs:
-  - create
-  - delete
-  - get
-  - list
-  - patch
   - update
-  - watch
 - apiGroups:
   - http.keda.sh
   resources:
@@ -73,16 +43,6 @@ rules:
   - patch
   - update
   - watch
-- apiGroups:
-  - networking
-  resources:
-  - ingresses
-  verbs:
-  - create
-  - delete
-  - get
-  - list
-  - watch
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
@@ -96,4 +56,29 @@ rules:
   resources:
   - configmaps
   verbs:
-  - '*'
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
+- apiGroups:
+  - ""
+  resources:
+  - events
+  verbs:
+  - create
+  - patch
+- apiGroups:
+  - coordination.k8s.io
+  resources:
+  - leases
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
diff --git a/config/scaler/kustomization.yaml b/config/scaler/kustomization.yaml
@@ -1,6 +1,9 @@
 resources:
 - scaler.yaml
 - service.yaml
+- role.yaml
+- role_binding.yaml
+- service_account.yaml
 
 apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization

diff --git a/config/scaler/role.yaml b/config/scaler/role.yaml
@@ -0,0 +1,39 @@
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  creationTimestamp: null
+  name: keda-http-add-on-scaler
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - endpoints
+  verbs:
+  - get
+  - list
+  - watch
+- apiGroups:
+  - apps
+  resources:
+  - deployments
+  verbs:
+  - get
+  - list
+  - watch
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  creationTimestamp: null
+  name: keda-http-add-on-scaler
+  namespace: keda
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - configmaps
+  verbs:
+  - get
+  - list
+  - watch
diff --git a/config/scaler/role_binding.yaml b/config/scaler/role_binding.yaml
@@ -0,0 +1,26 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: keda-http-add-on-scaler
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: keda-http-add-on-scaler
+subjects:
+- kind: ServiceAccount
+  name: keda-http-add-on-scaler
+  namespace: keda
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: keda-http-add-on-scaler
+  namespace: keda
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: keda-http-add-on-scaler
+subjects:
+- kind: ServiceAccount
+  name: keda-http-add-on-scaler
+  namespace: keda
diff --git a/config/scaler/scaler.yaml b/config/scaler/scaler.yaml
@@ -26,7 +26,7 @@ spec:
     spec:
       securityContext:
         runAsNonRoot: true
-      serviceAccountName: keda-http-add-on
+      serviceAccountName: keda-http-add-on-scaler
       containers:
         - name: external-scaler
           image: ghcr.io/kedacore/http-add-on-scaler:latest

diff --git a/config/scaler/service_account.yaml b/config/scaler/service_account.yaml
@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  labels:
+    app.kubernetes.io/name: keda-http-add-on
+    app.kubernetes.io/version: latest
+    app.kubernetes.io/part-of: keda-http-add-on
+  name: keda-http-add-on-scaler
+  namespace: keda
diff --git a/interceptor/main.go b/interceptor/main.go
@@ -90,6 +90,7 @@ func main() {
 		lggr,
 		cl,
 		servingCfg.ConfigMapCacheRsyncPeriod,
+		servingCfg.CurrentNamespace,
 	)
 
 	lggr.Info(

diff --git a/operator/controllers/app.go b/operator/controllers/app.go
@@ -15,8 +15,6 @@ import (
 	"github.com/kedacore/http-add-on/pkg/routing"
 )
 
-// +kubebuilder:rbac:groups="",namespace=keda,resources=configmaps,verbs="*"
-
 func removeApplicationResources(
 	ctx context.Context,
 	logger logr.Logger,

diff --git a/operator/controllers/httpscaledobject_controller.go b/operator/controllers/httpscaledobject_controller.go
@@ -45,13 +45,10 @@ type HTTPScaledObjectReconciler struct {
 	RoutingTable         *routing.Table
 }
 
-// +kubebuilder:rbac:groups=keda.sh,resources=scaledobjects,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=http.keda.sh,resources=httpscaledobjects,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=http.keda.sh,resources=httpscaledobjects/status,verbs=get;update;patch
-// +kubebuilder:rbac:groups="",resources=pods;services;configmaps;configmaps/status;events;endpoints;endpoint,verbs=get;list;watch;create;delete
-// +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;delete
-// +kubebuilder:rbac:groups=networking,resources=ingresses,verbs=get;list;watch;create;delete
-// +kubebuilder:rbac:groups=coordination.k8s.io,resources=leases,verbs=get;list;create;update;delete
+// +kubebuilder:rbac:groups=http.keda.sh,resources=httpscaledobjects/finalizers,verbs=update
+// +kubebuilder:rbac:groups=keda.sh,resources=scaledobjects,verbs=get;list;watch;create;update;patch;delete
 
 // Reconcile reconciles a newly created, deleted, or otherwise changed
 // HTTPScaledObject

diff --git a/operator/main.go b/operator/main.go
@@ -25,11 +25,13 @@ import (
 
 	"github.com/go-logr/logr"
 	"golang.org/x/sync/errgroup"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
 	httpv1alpha1 "github.com/kedacore/http-add-on/operator/api/v1alpha1"
@@ -54,6 +56,10 @@ func init() {
 	// +kubebuilder:scaffold:scheme
 }
 
+// +kubebuilder:rbac:groups="",namespace=keda,resources=events,verbs=create;patch
+// +kubebuilder:rbac:groups="",namespace=keda,resources=configmaps,verbs=get;list;watch;create;update;patch;delete
+// +kubebuilder:rbac:groups=coordination.k8s.io,namespace=keda,resources=leases,verbs=get;list;watch;create;update;patch;delete
+
 func main() {
 	ctx := ctrl.SetupSignalHandler()
 	ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
@@ -115,6 +121,12 @@ func main() {
 		LeaderElectionID:   "f8508ff1.keda.sh",
 		// will be empty to indicate all namespaces
 		Namespace: baseConfig.WatchNamespace,
+		// TODO(pedrotorres): remove this when we stop relying on ConfigMaps for the routing table
+		// workaround for using the same K8s client for both the routing table and the HTTPScaledObject
+		// this was already broken if the operator was running only for a single namespace
+		ClientDisableCacheFor: []client.Object{
+			&corev1.ConfigMap{},
+		},
 	})
 	if err != nil {
 		setupLog.Error(err, "unable to start manager")

diff --git a/pkg/k8s/config_map_cache_informer.go b/pkg/k8s/config_map_cache_informer.go
@@ -125,10 +125,12 @@ func NewInformerConfigMapUpdater(
 	lggr logr.Logger,
 	cl kubernetes.Interface,
 	defaultResync time.Duration,
+	namespace string,
 ) *InformerConfigMapUpdater {
-	factory := informers.NewSharedInformerFactory(
+	factory := informers.NewSharedInformerFactoryWithOptions(
 		cl,
 		defaultResync,
+		informers.WithNamespace(namespace),
 	)
 	cmInformer := factory.Core().V1().ConfigMaps()
 	ret := &InformerConfigMapUpdater{

diff --git a/pkg/routing/config_map_updater_test.go b/pkg/routing/config_map_updater_test.go
@@ -88,6 +88,7 @@ func TestStartUpdateLoop(t *testing.T) {
 		lggr,
 		fakeGetter,
 		time.Second*1,
+		ns,
 	)
 
 	grp, ctx := errgroup.WithContext(ctx)
-Original file line number
+Diff line change
@@ Expand Up / @@ -90,6 +90,7 @@ func main() { @@
     		lggr,
     		cl,
     		servingCfg.ConfigMapCacheRsyncPeriod,
+    		servingCfg.CurrentNamespace,
     	)
     	lggr.Info(
@@ Expand Down @@