Refactor Policies to reuse OSS components #2910
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This replaces the local implementation for evaluating RBAC rules with a shim for the Kubernetes implementation of the same. This involves: - ... retrieving Role and RoleBinding values from the store and using them directly, rather than flattening them into the intermediate AccessRule. The Kubernetes machinery calculates the effective rules given (Cluster) Role and RoleBinding, and it is impossible to recover those, or otherwise to do the calculation of effective rules, with AccessRules. - ... adapting between types in `models` and the Kubernetes RBAC types. In the case of API objects, this involves copying field across, since they are structs. For other types, there are wrappers or casts. In Kubernetes, the mode of use is that the user and resource are given, and you must look up the relevant rules and evaluate them. Here, the access pattern is that the user is given, then you need to evaluate access for objects which can be in several clusters and namespaces. I have kept it simple (and similar to the prior implementation) by retrieving Role and RoleBinding objects for all clusters and namespaces, then running through those when asked by the Kubernetes machinery while it figures out which rules apply. The relevant rules are calculated at most once per cluster. There's a lot of room for improvement, since that still runs through all the objects, each time. I have kept the debug API DebugAccessRules and supporting bits (notably `accesschecker#RelevantRulesForUser`), for now, since these have UI which depend on them. Last caveat: it's still necessary to translate between the kind given for an object, and the resource expected by the RBAC machinery. Previously, a map of resource to kind was constructed, and (effectively) the resource part of the rule was looked up in it to get a kind. I have switched that around: the kind of an object is looked up, to compare to the resource field of each rule -- simply because object.GetResource() is implemented here, and that's my opportunity to interpose the lookup. Signed-off-by: Michael Bridgen <[email protected]>
This test checks that the query service will keep returning results when it has to skip an object that isn't authorised. This commit rewrites it so it verifies the same thing. Signed-off-by: Michael Bridgen <[email protected]>
Kubernetes RBAC supports specifying access to individual resources
with `.resourceName`, and that field of each PolicyRule needs to be
stored.
ResourceName is usually empty, and we have to take care when
unstringifying it from the store, since
join([], ",") -> ""
but
split("", ",") -> [""]
which would represent a requirement for an empty name. Therefore: an
empty string now unstringifies to a zero-value.
This is an attempt at benchmarking the RBAC code. It tries to build a representative model -- a modest number of clusters, roles and rolebindings (most of which do not pertain to the user or objects in question), and a list of objects to check. Then it uses rbac.Authorizer in the way it's used for queries (memoising the instantiated authorisation predicate per cluster). This should give us a baseline with which to judge any improvements. It will not necessarily be a good guide to relative effectiveness of improvements, since results are sensitive to how the model is constructed. Signed-off-by: Michael Bridgen <[email protected]>
The naive implementation for RBAC ends up doing a table scan (looking at all role entries, or all rolebinding entries) for every invocation. Some possible ways of improving that will preprocess the tables, e.g., by sorting them into clusters. This commit splits the benchmark into two: one with some objects in every cluster, and one with objects in few of the clusters, to get a better picture of how preprocessing (which might do unnecessary work) affects things. Signed-off-by: Michael Bridgen <[email protected]>
From the store, we are given all Role objects and all RoleBinding objects. To adapt to the Kubernetes RBAC package, we need to provide - GetRole - ListRoleBindings and the ClusterRole and ClusterRoleBinding analogues. Since we're given a name in GetRole, the implementation can stop when it sees that namespace and name; but ListRoleBindings must scan all the RoleBindings to find all those in the namespace. With that in mind, this commit uses different ways of avoiding looking at the whole list of roles or rolebindings more than once: For GetRole, an index of name to position in the list is kept. The index is checked before starting, in case the name has been seen already. Otherwise, the list is scanned, recording the position of each Role, until it sees the named object. It keeps track of where in the list it got up to, so the next invocation can start there. For ListRoleBindings, the list is sorted into namespaces before any methods are called. Trying to do this incrementally is complicated; and besides, as noted above, any invocation will necessarily scan the whole list. These both duplicate work done for other clusters, since each implementation gets all the objects rather than just those for its cluster. Signed-off-by: Michael Bridgen <[email protected]>
Kubernetes RBAC deals with resources rather than kinds, but what we
want to check are objects with kinds. The solution to date is to map
kinds to resources (previously, resources to kinds), and to obtain
that map using a Discovery client.
It turns out that if you ask the discovery client
`ServerGroupsAndResources()`, for any kind there is usually at least
two resources in the result -- the "main" one e.g., "helmreleases",
and a subresource for the status, e.g., "helmreleases/status". This
complicates things because the latter will overwrite the first.
Previously (resource->kind), you'd get
{
"helmreleases": "HelmRelease",
"helmreleases/status": "HelmRelease"
}
but mapping the other way, you get
{
"HelmRelease": "helmreleases/status"
}
and none of the PolicyRules, which use "helmreleases" as the resource,
match.
Happily there's an easy fix: if you use `ServerPreferredResources()`
you get just the main resources.
Signed-off-by: Michael Bridgen <[email protected]>
* added metrics server and recorder for wge and explorer * refactored to metrics pkg
* disable dark mode on ee sign in page * snaps
* remove makeStyles in Breadcrumbs * fix breadcrumb font size and dark mode logo/sign in * styling and fixes for content wrapper component * snaps and reset oss version * remove size prop reference in breadcrumbs * fix pipeline detail snap * avoid state update console error * lint error
* update go.mod * update package.json * update go.sum --------- Co-authored-by: Ahmad Samir Ali <[email protected]>
* Refactor Application Violations to reuse OSS components * init reuse oss policies & violations * fix breadcrumb conflict * fix policydetails parent nav highlight * remove violation routes from EE and use OSS * remove styles page for PolicyViolationPage
serboctor
suggested changes
Jun 12, 2023
| @@ -248,3 +259,113 @@ func getPolicyConfigPolicies(ctx context.Context, s *server, clusterName string, | |||
| } | |||
| return policies, nil | |||
| } | |||
|
|
|||
| func policyToPolicyRespone(policyCRD pacv2beta2.Policy, clusterName string) (*core.PolicyObj, error) { | |||
There was a problem hiding this comment.
Should be policyCR instead of policyCRD since this is the actual resource not the definition
serboctor
approved these changes
Jun 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2857
What changed?
Why was this change made?
How was this change implemented?
How did you validate the change?
Release notes
Documentation Changes