From 87728f177171c21cb11da786000eab3816a90038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Wed, 23 Aug 2023 14:29:56 +0200 Subject: [PATCH] refactor: background controller permissions (#8083) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: reduce background controller permissions Signed-off-by: Charles-Edouard Brétéché * debug Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * codegen Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché --------- Signed-off-by: Charles-Edouard Brétéché --- charts/kyverno/README.md | 1 + .../admission-controller/clusterrole.yaml | 2 +- .../background-controller/clusterrole.yaml | 58 ++++--------- charts/kyverno/values.yaml | 47 +++++++++++ config/install-latest-testing.yaml | 82 +++++++++++-------- pkg/policy/generate/validate.go | 10 ++- 6 files changed, 120 insertions(+), 80 deletions(-) diff --git a/charts/kyverno/README.md b/charts/kyverno/README.md index 0f9712dcf4..aef5880ee9 100644 --- a/charts/kyverno/README.md +++ b/charts/kyverno/README.md @@ -408,6 +408,7 @@ The chart values are organised per component. | backgroundController.rbac.create | bool | `true` | Create RBAC resources | | backgroundController.rbac.serviceAccount.name | string | `nil` | Service account name | | backgroundController.rbac.serviceAccount.annotations | object | `{}` | Annotations for the ServiceAccount | +| backgroundController.rbac.coreClusterRole.extraResources | list | See [values.yaml](values.yaml) | Extra resource permissions to add in the core cluster role. This was introduced to avoid breaking change in the chart but should ideally be moved in `clusterRole.extraResources`. | | backgroundController.rbac.clusterRole.extraResources | list | `[]` | Extra resource permissions to add in the cluster role | | backgroundController.image.registry | string | `"ghcr.io"` | Image registry | | backgroundController.image.repository | string | `"kyverno/background-controller"` | Image repository | diff --git a/charts/kyverno/templates/admission-controller/clusterrole.yaml b/charts/kyverno/templates/admission-controller/clusterrole.yaml index 937eb23039..69cc6e5b56 100644 --- a/charts/kyverno/templates/admission-controller/clusterrole.yaml +++ b/charts/kyverno/templates/admission-controller/clusterrole.yaml @@ -39,8 +39,8 @@ rules: - rolebindings - clusterrolebindings verbs: - - watch - list + - watch - apiGroups: - kyverno.io resources: diff --git a/charts/kyverno/templates/background-controller/clusterrole.yaml b/charts/kyverno/templates/background-controller/clusterrole.yaml index a872653675..f830664b01 100644 --- a/charts/kyverno/templates/background-controller/clusterrole.yaml +++ b/charts/kyverno/templates/background-controller/clusterrole.yaml @@ -18,17 +18,11 @@ metadata: labels: {{- include "kyverno.background-controller.labels" . | nindent 4 }} rules: - - apiGroups: - - '*' - resources: - - '*' - verbs: - - get - - list - - watch - apiGroups: - kyverno.io resources: + - policies + - clusterpolicies - updaterequests - updaterequests/status verbs: @@ -40,6 +34,15 @@ rules: - update - watch - deletecollection + - apiGroups: + - '' + resources: + - namespaces + - configmaps + verbs: + - get + - list + - watch - apiGroups: - '' - events.k8s.io @@ -47,41 +50,14 @@ rules: - events verbs: - create - - update + - get + - list - patch - - apiGroups: - - networking.k8s.io - resources: - - ingresses - - ingressclasses - - networkpolicies - verbs: - - create - update - - patch - - delete - - apiGroups: - - "" - resources: - - configmaps - - secrets - - resourcequotas - - limitranges - verbs: - - create - - update - - patch - - delete - - apiGroups: - - rbac.authorization.k8s.io - resources: - - rolebindings - - roles - verbs: - - create - - update - - patch - - delete + - watch +{{- with .Values.backgroundController.rbac.coreClusterRole.extraResources }} + {{- toYaml . | nindent 2 }} +{{- end }} {{- with .Values.backgroundController.rbac.clusterRole.extraResources }} --- apiVersion: rbac.authorization.k8s.io/v1 diff --git a/charts/kyverno/values.yaml b/charts/kyverno/values.yaml index f7cc607a40..3b39cd5165 100644 --- a/charts/kyverno/values.yaml +++ b/charts/kyverno/values.yaml @@ -960,6 +960,53 @@ backgroundController: annotations: {} # example.com/annotation: value + coreClusterRole: + # -- Extra resource permissions to add in the core cluster role. + # This was introduced to avoid breaking change in the chart but should ideally be moved in `clusterRole.extraResources`. + # @default -- See [values.yaml](values.yaml) + extraResources: + - apiGroups: + - '*' + resources: + - '*' + verbs: + - get + - list + - watch + - apiGroups: + - networking.k8s.io + resources: + - ingresses + - ingressclasses + - networkpolicies + verbs: + - create + - update + - patch + - delete + - apiGroups: + - rbac.authorization.k8s.io + resources: + - rolebindings + - roles + verbs: + - create + - update + - patch + - delete + - apiGroups: + - '' + resources: + - configmaps + - secrets + - resourcequotas + - limitranges + verbs: + - create + - update + - patch + - delete + clusterRole: # -- Extra resource permissions to add in the cluster role extraResources: [] diff --git a/config/install-latest-testing.yaml b/config/install-latest-testing.yaml index 89e1009584..bfcec41163 100644 --- a/config/install-latest-testing.yaml +++ b/config/install-latest-testing.yaml @@ -39931,8 +39931,8 @@ rules: - rolebindings - clusterrolebindings verbs: - - watch - list + - watch - apiGroups: - kyverno.io resources: @@ -40021,17 +40021,11 @@ metadata: app.kubernetes.io/part-of: kyverno app.kubernetes.io/version: latest rules: - - apiGroups: - - '*' - resources: - - '*' - verbs: - - get - - list - - watch - apiGroups: - kyverno.io resources: + - policies + - clusterpolicies - updaterequests - updaterequests/status verbs: @@ -40043,6 +40037,15 @@ rules: - update - watch - deletecollection + - apiGroups: + - '' + resources: + - namespaces + - configmaps + verbs: + - get + - list + - watch - apiGroups: - '' - events.k8s.io @@ -40050,41 +40053,52 @@ rules: - events verbs: - create - - update + - get + - list - patch + - update + - watch - apiGroups: - - networking.k8s.io + - '*' resources: - - ingresses - - ingressclasses - - networkpolicies + - '*' verbs: - - create - - update - - patch - - delete + - get + - list + - watch - apiGroups: - - "" + - networking.k8s.io resources: - - configmaps - - secrets - - resourcequotas - - limitranges + - ingresses + - ingressclasses + - networkpolicies verbs: - - create - - update - - patch - - delete + - create + - update + - patch + - delete - apiGroups: - - rbac.authorization.k8s.io + - rbac.authorization.k8s.io resources: - - rolebindings - - roles + - rolebindings + - roles verbs: - - create - - update - - patch - - delete + - create + - update + - patch + - delete + - apiGroups: + - "" + resources: + - configmaps + - secrets + - resourcequotas + - limitranges + verbs: + - create + - update + - patch + - delete --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/pkg/policy/generate/validate.go b/pkg/policy/generate/validate.go index d4faeedaa1..e4eaa8dbc4 100644 --- a/pkg/policy/generate/validate.go +++ b/pkg/policy/generate/validate.go @@ -16,6 +16,7 @@ import ( // Generate provides implementation to validate 'generate' rule type Generate struct { + user string // rule to hold 'generate' rule specifications rule kyvernov1.Generation // authCheck to check access for operations @@ -27,6 +28,7 @@ type Generate struct { // NewGenerateFactory returns a new instance of Generate validation checker func NewGenerateFactory(client dclient.Interface, rule kyvernov1.Generation, user string, log logr.Logger) *Generate { g := Generate{ + user: user, rule: rule, authCheck: NewAuth(client, user, log), log: log, @@ -112,7 +114,7 @@ func (g *Generate) canIGenerate(ctx context.Context, gvk, namespace, subresource return err } if !ok { - return fmt.Errorf("kyverno does not have permissions to 'create' resource %s/%s/%s. Grant proper permissions to the background controller", gvk, subresource, namespace) + return fmt.Errorf("%s does not have permissions to 'create' resource %s/%s/%s. Grant proper permissions to the background controller", g.user, gvk, subresource, namespace) } ok, err = authCheck.CanIUpdate(ctx, gvk, namespace, subresource) @@ -120,7 +122,7 @@ func (g *Generate) canIGenerate(ctx context.Context, gvk, namespace, subresource return err } if !ok { - return fmt.Errorf("kyverno does not have permissions to 'update' resource %s/%s/%s. Grant proper permissions to the background controller", gvk, subresource, namespace) + return fmt.Errorf("%s does not have permissions to 'update' resource %s/%s/%s. Grant proper permissions to the background controller", g.user, gvk, subresource, namespace) } ok, err = authCheck.CanIGet(ctx, gvk, namespace, subresource) @@ -128,7 +130,7 @@ func (g *Generate) canIGenerate(ctx context.Context, gvk, namespace, subresource return err } if !ok { - return fmt.Errorf("kyverno does not have permissions to 'get' resource %s/%s/%s. Grant proper permissions to the background controller", gvk, subresource, namespace) + return fmt.Errorf("%s does not have permissions to 'get' resource %s/%s/%s. Grant proper permissions to the background controller", g.user, gvk, subresource, namespace) } ok, err = authCheck.CanIDelete(ctx, gvk, namespace, subresource) @@ -136,7 +138,7 @@ func (g *Generate) canIGenerate(ctx context.Context, gvk, namespace, subresource return err } if !ok { - return fmt.Errorf("kyverno does not have permissions to 'delete' resource %s/%s/%s. Grant proper permissions to the background controller", gvk, subresource, namespace) + return fmt.Errorf("%s does not have permissions to 'delete' resource %s/%s/%s. Grant proper permissions to the background controller", g.user, gvk, subresource, namespace) } } else { g.log.V(2).Info("resource Kind uses variables, so cannot be resolved. Skipping Auth Checks.")