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

Commit

Permalink
Merge pull request #1489 from adrianludwin/rbac
Browse files Browse the repository at this point in the history
Support cluster-admin propagation and update docs
  • Loading branch information
k8s-ci-robot authored Apr 27, 2021
2 parents 28c5bfe + 9fde938 commit ba35057
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 30 deletions.
9 changes: 9 additions & 0 deletions incubator/hnc/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,19 @@ kind-deploy:
# Note the `-timeout 0` that's passed to the `go test` command - by default, a
# Go test suite has a 10m timeout, and the flag disables that timeout (as of
# July 2020, these tests take ~15m and that number is expected to grow).
#
# To focus on specific tests, use the HNC_FOCUS var as follows:
#
# HNC_FOCUS=772 make test-e2e # only runs the test for issue 772
# HNC_FOCUS=Quickstart make test-e2e # Runs all tests in the "Quickstart" Describe block
.PHONY: test-e2e
test-e2e: exclude-system-namespaces
go clean -testcache
ifndef HNC_FOCUS
go test -v -timeout 0 ./test/e2e/...
else
go test -v -timeout 0 ./test/e2e/... -args --ginkgo.focus ${HNC_FOCUS}
endif

# This batch test will run e2e tests N times on the current cluster the user
# deployed (either kind or a kubernetes cluster), e.g. "make test-e2e-batch N=10"
Expand Down
10 changes: 1 addition & 9 deletions incubator/hnc/config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,7 @@ rules:
resources:
- '*'
verbs:
- create
- delete
- deletecollection
- get
- impersonate
- list
- patch
- update
- watch
- '*'
- apiGroups:
- ""
resources:
Expand Down
24 changes: 12 additions & 12 deletions incubator/hnc/docs/user-guide/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,22 @@ we're not constantly on Slack.

## What are HNC's minimum requirements?

HNC technically requires Kubernetes 1.15 or higher, although we don't test on
HNC technically requires Kubernetes 1.16 or higher, although we don't test on
every version of Kubernetes. See the release notes for the version you're
downloading for a full list of the K8s distributions on which that release has
been tested.

By default, HNC's service account is given the equivalent of the [admin cluster
role](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles),
and therefore is able to propagate RoleBindings to that role, since (under the
normal rules of RBAC) an account is not allowed to grant rolebindings to
permission it does not have. For example, HNC is not able to propagate
`cluster-admin` rolebindings.

You may modify HNC's own role bindings in the `hnc-system` namespace to grant it
addition (or fewer) permissions if you wish. At a minimum, HNC must be able to
access (create, read, list, watch, update and delete) all of its own CRs as well
as namespaces, roles, and role bindings.
By default, HNC's service account is given the ability to perform any verb on
any resource. HNC does not need these permissions itself, but it does require
them to be able to propagate RoleBindings with arbitrary permissions. This
includes namespace RoleBindings to the [`cluster-admin` cluster
role](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#user-facing-roles).

You may modify HNC's own role bindings in the `hnc-system` namespace to restrict
its permissions if you wish, but then it will be unable to propagate
RoleBindings that include the missing permissions. At a minimum, HNC must be
able to access (create, read, list, watch, update and delete) all of its own CRs
as well as namespaces, roles, and role bindings.

## Is there a limit to how many levels of child namespaces you can have?

Expand Down
6 changes: 2 additions & 4 deletions incubator/hnc/docs/user-guide/how-to.md
Original file line number Diff line number Diff line change
Expand Up @@ -408,10 +408,8 @@ EOF

### Install or upgrade HNC on a cluster

We recommend installing HNC onto clusters running Kubernetes v1.15 or later.
Earlier versions of Kubernetes are missing some admission controller features
that leave us unable to validate certain dangerous operations such as deleting
namespaces (see [#680](https://github.com/kubernetes-sigs/multi-tenancy/issues/680)).
HNC requires Kubernetes v1.16 or later, since it relies on APIs (such as CRDs
and webhooks) that were only introduced in v1.16.

There is no need to uninstall HNC before upgrading it unless specified in the
release notes for that version.
Expand Down
6 changes: 3 additions & 3 deletions incubator/hnc/internal/reconcilers/object.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ type ObjectReconciler struct {
}

// HNC doesn't actually need all these permissions, but we *do* need to have them to be able to
// propagate RoleBindings for them. These match the permissions required by the builtin `admin`
// role, as seen in hack/test-issue-772.sh.
// propagate RoleBindings for them. These match the permissions required by the builtin
// `cluster-admin` role, as seen in issues #772 and #1311.
//
// +kubebuilder:rbac:groups=*,resources=*,verbs=get;list;watch;create;update;patch;delete;deletecollection;impersonate
// +kubebuilder:rbac:groups=*,resources=*,verbs=*

// SyncNamespace can be called manually by the HierarchyConfigReconciler when the hierarchy changes.
// It enqueues all the current objects in the namespace and local copies of the original objects
Expand Down
4 changes: 2 additions & 2 deletions incubator/hnc/test/e2e/issues_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,13 +227,13 @@ var _ = Describe("Issues", func() {
RunShouldContain("Could not write from source namespace \""+nsParent+"\"", defTimeout, "kubectl get events -n", nsChild, "--field-selector reason=CannotUpdateObject")
})

It("Should propogate admin rolebindings - issue #772", func() {
It("Should propagate cluster-admin rolebindings - issue #772, #1311", func() {
// set up
CreateNamespace(nsParent)
CreateNamespace(nsChild)
MustRun("kubectl hns set", nsChild, "--parent", nsParent)
// Creating admin rolebinding object
MustRun("kubectl create rolebinding --clusterrole=admin --serviceaccount=default:default -n", nsParent, "foo")
MustRun("kubectl create rolebinding --clusterrole=cluster-admin --serviceaccount=default:default -n", nsParent, "foo")
// Object should exist in the child, and there should be no conditions
MustRun("kubectl get rolebinding foo -n", nsChild, "-oyaml")
})
Expand Down

0 comments on commit ba35057

Please sign in to comment.