Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rbac-manager doesn't handle the case where a namespace does not exist and is later created #53

Closed
sudermanjr opened this issue May 23, 2019 · 12 comments · Fixed by #128
Closed
Assignees
Labels
pinned Prevents stalebot from removing question Further information is requested

Comments

@sudermanjr
Copy link
Member

I would expect the namespace reconciler to handle the case where an rbac definition is applied that creates a service account in a namespace that doesn't exist yet. However, if I do this and then create the namespace, the serviceaccount never gets created.

Rbacdefinition:

apiVersion: rbacmanager.reactiveops.io/v1beta1
kind: RBACDefinition
metadata:
name: ci-access
rbacBindings:

  • name: ci
    subjects:
    • kind: ServiceAccount
      name: ci
      namespace: infra
      roleBindings:
    • clusterRole: cluster-admin
      namespaceSelector:
      matchLabels:
      team: ci-access

Log output from rbac-manager:

rbac-manager-5b796664cf-9fphp rbac-manager time="2019-05-22T14:38:44Z" level=info msg="Watching RBAC Definitions"
rbac-manager-5b796664cf-9fphp rbac-manager time="2019-05-22T14:38:57Z" level=info msg="Reconciling RBACDefinition ci-access"
rbac-manager-5b796664cf-9fphp rbac-manager time="2019-05-22T14:38:57Z" level=info msg="Creating Service Account: ci"
rbac-manager-5b796664cf-9fphp rbac-manager time="2019-05-22T14:38:57Z" level=error msg="Error creating Service Account: namespaces "infra" not found"
rbac-manager-5b796664cf-9fphp rbac-manager time="2019-05-22T14:39:02Z" level=info msg="Reconciling infra namespace for ci-access"

Commands that I issued through this process:

k apply -f rbacdefinition.yaml
k create ns infra

This was done with the latest dev version from this PR - image dev-b00465642bcb377030cad73386bcf6cf8cf565e3 and the all.yaml from this PR as well.

@tklovett
Copy link

tklovett commented Aug 5, 2019

:plus-1: on this. Here's our scenario.

In a development cluster, we have automation that dynamically creates and deletes Namepaces for Merge Request "review apps". These namespaces have a label like review-app-name=my-app.

Our process is essentially:

  1. Developer opens a merge request, and the pipeline begins execution
  2. A GitLab runner on k8s creates a Namespace with an "arbitrary" name (based on the branch name) and label is-review-app=true
  3. Based on a pre-existing RBACDefinition, rbac-manager is expected to create a RoleBinding granting the runner admin within the namespace – based on namespaceSelector.matchLabels.is-review-app=true
  4. The same GitLab runner then does a helm apply to create the application Deployment, etc.

However, step 3 does not happen. rbac-manager never logs any errors, and its quits the reconciliation process without completing. If it fails in the middle of reconciliation, it will not create the remaining resources. It will also not log any failure or early termination.

@tklovett
Copy link

tklovett commented Aug 5, 2019

One possible solution:

RBAC Manager could run reconciliation on a periodic schedule. My impression is that it only runs when an RBACDefintion is modified.

@robscott
Copy link
Contributor

robscott commented Aug 5, 2019

I added code as part of the 0.7.0 release that was supposed to help with this. We run a namespace controller that should trigger new reconcile loops for any RBAC Definitions with namespace selectors. If that's still not working in 0.7.0 or beyond, hopefully it's an easy fix.

@tklovett
Copy link

tklovett commented Aug 6, 2019

Ah ok – I was glomming on to this existing issue, but the problem I saw is sort of a subset of the original poster's issues. My issue was failure / early termination of the first reconciliation run(s) before any namespace exists matching the namespaceSelector.matchLabels. I hadn't actually experienced whether 0.7.0 would match on a namespace when it did come into existence.

@sudermanjr
Copy link
Member Author

@tklovett I tested this with the latest rbac-manager and I can't seem to reproduce your issue. I have this rbac-defintion:

apiVersion: rbacmanager.reactiveops.io/v1beta1
kind: RBACDefinition
metadata:
  name: rbac-definition
rbacBindings:
  - name: circleci
    subjects:
      - kind: ServiceAccount
        name: circleci
        namespace: helm-system
    roleBindings:
      - clusterRole: tiller-user
        namespace: helm-system
      - clusterRole: cluster-admin
        namespaceSelector:
          matchLabels:
            circleci-admin: "true"
    clusterRoleBindings:
      - clusterRole: circleci

And I ran through this process. I checked the bindings using rbac-lookup, created a namespace, labelled it, and then I checked the bindings again. After the namespace is labelled, rbac-manager has created the desired binding.

└─ rbac-lookup circleci
SUBJECT     SCOPE          ROLE
circleci    demo           ClusterRole/cluster-admin
circleci    helm-system    ClusterRole/tiller-user
circleci    cluster-wide   ClusterRole/circleci

└─ k create ns testrbac
namespace/testrbac created

└─ k label ns testrbac circleci-admin=true
namespace/testrbac labeled

└─ rbac-lookup circleci
SUBJECT     SCOPE          ROLE
circleci    cluster-wide   ClusterRole/circleci
circleci    demo           ClusterRole/cluster-admin
circleci    helm-system    ClusterRole/tiller-user
circleci    testrbac       ClusterRole/cluster-admin

@sudermanjr
Copy link
Member Author

If you're still seeing the issue, could you please provide your rbac-definition, your rbac-manager version, and an example of the namespace yaml that you are expecting to see the issue with?

@sudermanjr
Copy link
Member Author

In addition, I tested the original issue posted here, and in fact, rbac-manager does create the service account, but creates it in the default namespace.

To reproduce:

  1. Install an rbac-definition with a service account in a namespace that does not exist.
  2. Create the namespace.
  3. Check the cluster for the mentioned service account.
testrbacsa              default                                     1         36s

@sudermanjr sudermanjr self-assigned this Nov 12, 2019
@sudermanjr sudermanjr added the question Further information is requested label Nov 12, 2019
@sudermanjr
Copy link
Member Author

Still need to verify/resolve the SA getting created in the wrong NS, but the issue with the namespace not existing would be solved by #103

@stale
Copy link

stale bot commented Jan 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Marked as stale by stalebot label Jan 11, 2020
@lucasreed lucasreed added the pinned Prevents stalebot from removing label Jan 13, 2020
@stale stale bot removed the stale Marked as stale by stalebot label Jan 13, 2020
lucasreed added a commit that referenced this issue Mar 19, 2020
lucasreed pushed a commit that referenced this issue Mar 20, 2020
@lucasreed
Copy link
Contributor

I'm reopening this because I hit this again today using 0.9.3

@lucasreed lucasreed reopened this May 7, 2020
@lucasreed lucasreed assigned lucasreed and unassigned sudermanjr May 7, 2020
@sudermanjr
Copy link
Member Author

Cannot reproduce

@justinas-b
Copy link

Same is present on v1.0.0. When we create new kubernetes namespace, roles and bindings are created not immediately. It seems that this is happening on a schedule every 5 hours. Will try to upgrade to latest version to see if same still applies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned Prevents stalebot from removing question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants