Skip to content

Commit

Permalink
[v2.9] Refactor ID based partitioning, add unit tests (#310)
Browse files Browse the repository at this point in the history
* Refactor ID based partitioning, add unit tests

This resolves an issue where the requested namespace filter was not
always honored.

* Correct naming issues to appease the linter
  • Loading branch information
nflynt authored Oct 29, 2024
1 parent 7dafe0c commit 6e30359
Show file tree
Hide file tree
Showing 4 changed files with 1,392 additions and 31 deletions.
97 changes: 84 additions & 13 deletions pkg/stores/proxy/rbac_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/rancher/steve/pkg/attributes"
"github.com/rancher/steve/pkg/stores/partition"
"github.com/rancher/wrangler/v3/pkg/kv"
"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/watch"
Expand Down Expand Up @@ -64,17 +65,10 @@ func (p *rbacPartitioner) All(apiOp *types.APIRequest, schema *types.APISchema,
fallthrough
case "watch":
if id != "" {
ns, name := kv.RSplit(id, "/")
return []partition.Partition{
Partition{
Namespace: ns,
All: false,
Passthrough: false,
Names: sets.NewString(name),
},
}, nil
partitions := generatePartitionsByID(apiOp, schema, verb, id)
return partitions, nil
}
partitions, passthrough := isPassthrough(apiOp, schema, verb)
partitions, passthrough := generateAggregatePartitions(apiOp, schema, verb)
if passthrough {
return passthroughPartitions, nil
}
Expand Down Expand Up @@ -126,15 +120,92 @@ func (b *byNameOrNamespaceStore) Watch(apiOp *types.APIRequest, schema *types.AP
return b.Store.WatchNames(apiOp, schema, wr, b.partition.Names)
}

// isPassthrough determines whether a request can be passed through directly to the underlying store
// generatePartitionsById determines whether a requester can access a particular resource
// and if so, returns the corresponding partitions
func generatePartitionsByID(apiOp *types.APIRequest, schema *types.APISchema, verb string, id string) []partition.Partition {
accessListByVerb, _ := attributes.Access(schema).(accesscontrol.AccessListByVerb)
resources := accessListByVerb.Granted(verb)

idNamespace, name := kv.RSplit(id, "/")
apiNamespace := apiOp.Namespace
effectiveNamespace := idNamespace

// If a non-empty namespace was provided, be sure to select that for filtering and permissions checks
if idNamespace == "" && apiNamespace != "" {
effectiveNamespace = apiNamespace
}

// The external API is flexible, and permits specifying a namespace as a separate key or embedded
// within the ID of the object. Both of these cases should be valid:
// {"namespace": "n1", "id": "r1"}
// {"id": "n1/r1"}
// however, the following conflicting request is not valid, but was previously accepted:
// {"namespace": "n1", "id": "n2/r1"}
// To avoid breaking UI plugins that may inadvertently rely on the feature, we issue a deprecation
// warning for now. We still need to pick one of the namespaces for permission verification purposes.
if idNamespace != "" && apiNamespace != "" && idNamespace != apiNamespace {
logrus.Warningf("DEPRECATION: Conflicting namespaces '%v' and '%v' requested. "+
"Selecting '%v' as the effective namespace. Future steve versions will reject this request.",
idNamespace, apiNamespace, effectiveNamespace)
}

if accessListByVerb.All(verb) {
return []partition.Partition{
Partition{
Namespace: effectiveNamespace,
All: false,
Passthrough: false,
Names: sets.NewString(name),
},
}
}

if effectiveNamespace != "" {
if resources[effectiveNamespace].All {
return []partition.Partition{
Partition{
Namespace: effectiveNamespace,
All: false,
Passthrough: false,
Names: sets.NewString(name),
},
}
}
}

// For cluster-scoped resources, we will have parsed a "" out
// of the ID field from RSplit, but accessListByVerb specifies "*" for
// the namespace, so correct that here
resourceNamespace := effectiveNamespace
if resourceNamespace == "" {
resourceNamespace = accesscontrol.All
}

nameset, ok := resources[resourceNamespace]
if ok && nameset.Names.Has(name) {
return []partition.Partition{
Partition{
Namespace: effectiveNamespace,
All: false,
Passthrough: false,
Names: sets.NewString(name),
},
}
}

return nil
}

// generateAggregatePartitions determines whether a request can be passed through directly to the underlying store
// or if the results need to be partitioned by namespace and name based on the requester's access.
func isPassthrough(apiOp *types.APIRequest, schema *types.APISchema, verb string) ([]partition.Partition, bool) {
func generateAggregatePartitions(apiOp *types.APIRequest, schema *types.APISchema, verb string) ([]partition.Partition, bool) {
accessListByVerb, _ := attributes.Access(schema).(accesscontrol.AccessListByVerb)
resources := accessListByVerb.Granted(verb)

if accessListByVerb.All(verb) {
return nil, true
}

resources := accessListByVerb.Granted(verb)
if apiOp.Namespace != "" {
if resources[apiOp.Namespace].All {
return nil, true
Expand Down
Loading

0 comments on commit 6e30359

Please sign in to comment.