Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Why doesn't garbage collection work when restricting Flux's RBAC permissions? #2648

Closed
tl-alexandre-kaskasoli opened this issue Nov 30, 2019 · 13 comments
Assignees

Comments

@tl-alexandre-kaskasoli
Copy link

tl-alexandre-kaskasoli commented Nov 30, 2019

Describe the bug
fluxd doesn't delete resources managed by flux when they are removed from the git repository, when fluxd is running with a role with limited privileges and garbage collection enabled.

we want to restrict the resources that developers can create/alter in a given namespace, so we're limiting fluxd's privileges using RBAC.

when we don't limit privileges (ie: when flux is running with cluster-admin privileges), garbage collection works as expected.

To Reproduce
Steps to reproduce the behaviour:

  1. use the following service account and role for flux:
---
# The service account, cluster roles, and cluster role binding are
# only needed for Kubernetes with role-based access control (RBAC).
apiVersion: v1
kind: ServiceAccount
metadata:
  labels:
    name: flux-team-a
  name: flux-team-a
  namespace: team-a
---
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: Role
metadata:
  labels:
    name: flux-team-a
  name: flux-team-a
  namespace: team-a
rules:
  - apiGroups: ["*"]
    resources:
      [
        "configmaps",
        "cronjobs",
        "deployments",
        "horizontalpodautoscalers",
        "jobs",
        "networkpolicies",
        "persistentvolumeclaims",
        "persistentvolumes",
        "pods",
        "replicasets",
        "rolebindings",
        "roles",
        "serviceaccounts",
        "services",
        "secrets",
        "statefulsets",
      ]
    verbs: ["*"]
---
apiVersion: rbac.authorization.k8s.io/v1beta1
kind: RoleBinding
metadata:
  labels:
    name: flux-team-a
  name: flux-team-a
  namespace: team-a
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: Role
  name: flux-team-a
subjects:
  - kind: ServiceAccount
    name: flux-team-a
    namespace: team-a
  1. create a resource in the flux git repository, for example a configmap, witness the resource being created
  2. delete the resource in the flux git repository, notice the resource does not get deleted

Expected behavior
expected that flux would delete resources create by flux when it has sufficient permissions to do so.

Logs
N/A - no output in logs regarding the above.

Additional context

  • Flux version: 1.15
  • Helm Operator version: N/A
  • Kubernetes version: 1.14
  • Git provider: GitHub
  • Container registry provider: N/A
@tl-alexandre-kaskasoli tl-alexandre-kaskasoli added blocked-needs-validation Issue is waiting to be validated before we can proceed bug labels Nov 30, 2019
@tl-alexandre-kaskasoli
Copy link
Author

tl-alexandre-kaskasoli commented Dec 1, 2019

I noticed that adding the following ClusterRole is sufficient to make garbage collection work.

apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRole
metadata:
  labels:
    name: flux-team-a
  name: flux-team-a
rules:
  - apiGroups: ["*"]
    resources: ["*"]
    verbs: ["get", "list", "watch"]

The above is still too permissive. It would be useful to document the minimum permissions needed for garbage collection, so users can apply the principle of least privileges.

Should fluxd support using Garbage Collection without a ClusterRole?

@2opremio
Copy link
Contributor

2opremio commented Dec 2, 2019

In order to garbage-collect resources of certain apiVersion and Kind (e.g. v1 ConfigMap), Flux needs to be able to:

  1. Discover that apiVersion/Kind through the discovery API, which is allowed by default for every user. I doubt your configuration needs changing anything in this regard, since everything works when being permissive about get/watch/list
  2. list resources of apiVersion/Kind, in order to figure out whether they are not referenced by the git repository anymore.
  3. delete resources of apiVersion/Kind in order to garbage-collect unreferenced resources.

Depending on your configuration, e.g. if you use namespace filtering (--k8s-allow-namespace), you may also need to allow listing namespaces. In versions < 1.16 (like your case), you had to be able to list namespaces regardless of your --k8s-allow-namespace flag (because #2520 wasn't yet included).

So, following my description above, it should be enough to use your initial RBAC configuration with minor adjustments. You could either:

  • Add permissions for listing namespaces
  • If you don't use --k8s-allow-namespace, upgrading to flux 1.16 is probably enough.

I haven't tested this explicitly (I am just using my knowledge not Flux). Could you give it a try and report back?

@2opremio 2opremio added question and removed blocked-needs-validation Issue is waiting to be validated before we can proceed bug labels Dec 2, 2019
@2opremio 2opremio changed the title Garbage collection does not work when using restricting fluxd role Why doesn't garbage collection work when restricting Flux's RBAC permissions? Dec 2, 2019
@tl-alexandre-kaskasoli
Copy link
Author

I tried

apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRole
metadata:
  labels:
    name: flux-cluster-read
  name: flux-cluster-read
rules:
  - apiGroups: ["*"]
    resources: ["namespaces", "customresourcedefinitions"]
    verbs: ["get", "list", "watch"]

That wasn't sufficient. Currently using:

apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRole
metadata:
  labels:
    name: flux-cluster-read
  name: flux-cluster-read
rules:
  - apiGroups: ["*"]
    resources: ["*"]
    verbs: ["get", "list", "watch"]

Which gets the job done.

The logs don't show any output related to a lack of permissions on the resources that need to be garbage-collected.

@2opremio
Copy link
Contributor

2opremio commented Dec 2, 2019

I tried

apiVersion: rbac.authorization.k8s.io/v1beta1
kind: ClusterRole
metadata:
  labels:
    name: flux-cluster-read
  name: flux-cluster-read
rules:
  - apiGroups: ["*"]
    resources: ["namespaces", "customresourcedefinitions"]
    verbs: ["get", "list", "watch"]

That wasn't sufficient.

I meant adding that on top of what you already have. Is that what you did?

@tl-alexandre-kaskasoli
Copy link
Author

Apologies for not being clear. Yes that was added on top of what I outlined in the first post.

@goober
Copy link

goober commented Jan 2, 2020

We are facing the same issues where flux does not delete objects that have been deleted from the repository. We are using the the 1.17.0 version. And if I remember correctly it worked with the feature-flag --sync-garbage-collection in 1.16.x.

We have currently the following RBAC on each namespace that flux should handle:

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: flux:application-deployer
rules:
  - apiGroups:
      - ""
    resources:
      - services
      - configmaps
      - serviceaccounts
    verbs:
      - create
      - get
      - patch
      - delete
      - list
  - apiGroups:
      - apps
    resources:
      - deployments
    verbs:
      - create
      - get
      - patch
      - delete
      - list
  - apiGroups:
      - route.openshift.io
    resources:
      - routes
    verbs:
      - create
      - get
      - patch
      - delete
      - list
  - apiGroups:
      - route.openshift.io
    resources:
      - 'routes/custom-host'
    verbs:
      - create
  - apiGroups:
      - apps.openshift.io
    resources:
      - deploymentconfigs
    verbs:
      - create
      - get
      - patch
      - delete
      - list
  - apiGroups:
      - "extensions"
    resources:
      - ingresses
    verbs:
      - create
      - get
      - patch
      - delete
      - list
  - apiGroups:
      - "batch"
    resources:
      - cronjobs
      - jobs
    verbs:
      - create
      - get
      - patch
      - delete
      - list
  - apiGroups:
      - "bitnami.com"
    resources:
      - sealedsecrets
    verbs:
      - create
      - get
      - patch
      - delete
      - list

And a clusterrolebinding with the following RBAC:

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: namespace-viewer
rules:
  - apiGroups:
      - ""
    resources:
      - namespaces
    verbs:
      - get
      - list

@darryl-sw
Copy link

I'm facing something similar at the moment. Has anybody figured out anything regarding this issues?

@goober
Copy link

goober commented Jul 22, 2020

We have still this issue, haven't had time to investigate it further we are running 1.20.0

@kingdonb kingdonb self-assigned this Feb 23, 2021
@kingdonb
Copy link
Member

Thank you for your patience and for the reports. Flux v2 is a complete rewrite which has multi-cluster and multi-tenancy support, and as of the 0.8 release we are declaring the Feature Parity milestone is complete.

https://toolkit.fluxcd.io/#where-do-i-start

If you haven't already upgraded (many users are on Flux v2 in production already), it's definitely time to start thinking about it, and making plans to upgrade if you can't start right away. While it would be great to solve this issue for Flux v1 users, we are in maintenance mode and that means the priority is higher on security, maintenance/bug fixes, and migration issues, (above new features or enhancements.)

Respecting you may have moved on already, I will go ahead and close out this issue for now. Welcome to follow up with more questions. If you've been following our development efforts then of course we hope you are able to upgrade, here's more info on how to find support with that: https://fluxcd.io/support/

@goober
Copy link

goober commented Sep 15, 2021

For those who are still running Flux v1 and are having this issue.
The problem was introduced in #2520 and specifically in the following change:

https://github.com/fluxcd/flux/pull/2520/files#diff-27d973758746714f895410e458a181608dce9fd1a98ea96984260acad746c496L319-L324

The logic here assumes that if you do not specify the --k8s-allow-namespace argument Flux will assume that you have access to all namespaces.

So you have basically two choices with the current version of Flux v1 when your flux user does not have access to list resources in all namespaces. Either you give flux a cluster-reader-role as mention in #2648 (comment) or you explicitly specify all namespaces that flux has access to in the --k8s-allow-namespace argument.

@kingdonb I guess that this is not enough to make it worth fixing in the v1 version since there is a workaround for it?

@kingdonb
Copy link
Member

I'm going to reopen this, since someone knows something about it. Please excuse the delay in replying, I've been tied up with KubeCon things and haven't been keeping up with e-mails very well... I have just come upon this issue in my backlog.

If there is a fix possible, at this point I would guess it's going to be a docs improvement, since I'm leery of making any changes that would break things for people who made the workaround already. But for visibility, since we have someone confirming this is a real issue and seems to understand it well enough to explain the details more deeply than at least I've understood, it can be reopened, and I'll be happy to review any PRs to address this!

Thanks for your interest in Flux 💖👍

@kingdonb kingdonb reopened this Oct 25, 2021
@kingdonb
Copy link
Member

(If the original submitter is no longer interested in this, it may still be better to re-open as an entirely separate issue, and just link back to this thread if there was important context here.)

@pjbgf pjbgf added the wontfix label Jul 26, 2022
@pjbgf
Copy link
Member

pjbgf commented Jul 26, 2022

This project is in Migration and security support only, so unfortunately this issue won't be fixed. We recommend users migrate to Flux 2 at their earliest convenience.

More information about the Flux 2 transition timetable can be found at: https://fluxcd.io/docs/migration/timetable/.

@pjbgf pjbgf closed this as completed Jul 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants