Skip to content
This repository has been archived by the owner on Jun 26, 2023. It is now read-only.

Give HNC permission to all verbs on all resources #1311

Closed
0xVox opened this issue Dec 9, 2020 · 6 comments · Fixed by #1489
Closed

Give HNC permission to all verbs on all resources #1311

0xVox opened this issue Dec 9, 2020 · 6 comments · Fixed by #1489
Assignees
Milestone

Comments

@0xVox
Copy link

0xVox commented Dec 9, 2020

user "system:serviceaccount:hnc-system:default" (groups=["system:serviceaccounts" "system:serviceaccounts:hnc-system" "system:authenticated"]) is attempting to grant RBAC permissions not currently held

When a sub-namespace is created under a parent namespace containing RoleBindings that are attached to a Role that have permissions such as:

- apiGroups:
  - '*'
  resources:
  - 'persistentvolumeclaims'
  verbs:
  - '*'

The hnc-controller will not have enough permissions to propogate the rolebinding. Note the key problem here is that the hnc-manager-role does not have verbs: '*' permissions itself.

I don't suspect adding this permission is desirable, and this error has given me some roles to review, but I do think it would be desirable to note somewhere in the documentation or installation guide that this might be among the first issues a new user might come across if they've got some quite permissive roles hanging around, created by themselves or through third-party controller installs.

Thank you for your work on this project!

@adrianludwin adrianludwin added this to the hnc-v0.8 milestone Dec 14, 2020
@adrianludwin adrianludwin changed the title hnc-controller-manager ClusterRole cannot propogate when "verbs: '*'" is present on target role. Better document the permissions included by a default HNC installation Dec 15, 2020
@adrianludwin
Copy link
Contributor

Thanks @0xVox ! We've tried to make the default HNC permissions equivalent to the builtin admin ClusterRole. Do you know if this missing permission would have been part of that role? If so, perhaps we should simply start using the admin role instead of trying to simulate it.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 15, 2021
@yiqigao217
Copy link
Contributor

/remove-lifecycle stale
@adrianludwin move this to backlog?

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 16, 2021
@adrianludwin
Copy link
Contributor

adrianludwin commented Mar 16, 2021 via email

@adrianludwin
Copy link
Contributor

@0xVox - it turns out we already do document the permissions (here), but I think it makes sense to just grant HNC permission to all verbs. There's no particular reason to exclude anything.

I verified that once I did this, HNC was able to propagate cluster-admin rolebindings in namespaces, which would probably have solved your problem too. PR forthcoming.

@adrianludwin
Copy link
Contributor

/assign

@adrianludwin adrianludwin changed the title Better document the permissions included by a default HNC installation Give HNC permission to all verbs on all resources Apr 27, 2021
adrianludwin added a commit to adrianludwin/multi-tenancy that referenced this issue Apr 27, 2021
Rather than continuing to play whack-a-mole with the list of verbs HNC
can propagate, this change allows it to perform all verbs on all
resources. This is equivalent to `cluster-admin` so I've updated the
docs accordingly (see also issue kubernetes-retired#1311).

I also noticed that the docs referred to K8s v1.15, which is no longer
supported, so I updated them to v1.16.

Finally, this change adds the `HNC_FOCUS` makefile var, allowing you to
say something like:

```
HNC_FOCUS=772 make test
```

which only runs the e2e tests with "772" in the title.

Tested: All quickstart e2e tests pass. Updated the test for issue kubernetes-retired#772
and verified that it failed without the other changes in this commit,
and passed with them.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants