Refactor ID based partitioning, add unit tests (#309)

* 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
2025-09-16 15:29:04 +00:00 · 2024-10-29 09:27:12 -04:00
parent b2f2bab3c4
commit 2175e090fe
4 changed files with 1394 additions and 33 deletions
--- a/pkg/stores/proxy/rbac_store_test.go
+++ b/pkg/stores/proxy/rbac_store_test.go
@@ -11,7 +11,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 )

-func TestAll(t *testing.T) {
+func TestVerbList(t *testing.T) {
 	tests := []struct {
 		name           string
 		apiOp          *types.APIRequest
@@ -223,7 +223,7 @@ func TestAll(t *testing.T) {
 			},
 		},
 		{
-			name:  "by id",
+			name:  "by id fully unauthorized",
 			apiOp: &types.APIRequest{},
 			id:    "n1/r1",
 			schema: &types.APISchema{
@@ -231,6 +231,72 @@ func TestAll(t *testing.T) {
 					ID: "foo",
 				},
 			},
+			wantPartitions: nil,
+		},
+		{
+			name:  "by id missing namespace",
+			apiOp: &types.APIRequest{},
+			id:    "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"list": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n2",
+									ResourceName: "r2",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: nil,
+		},
+		{
+			name:  "by id missing resource",
+			apiOp: &types.APIRequest{},
+			id:    "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"list": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "r2",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: nil,
+		},
+		{
+			name:  "by id authorized by name",
+			apiOp: &types.APIRequest{},
+			id:    "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"list": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "r1",
+								},
+							},
+						},
+					},
+				},
+			},
 			wantPartitions: []partition.Partition{
 				Partition{
 					Namespace: "n1",
@@ -238,8 +304,227 @@ func TestAll(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "by id authorized by namespace",
+			apiOp: &types.APIRequest{
+				Namespace: "n1",
+			},
+			id: "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"list": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "*",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "n1",
+					Names:     sets.NewString("r1"),
+				},
+			},
+		},
+		{
+			name: "by namespaced id authorized by name",
+			apiOp: &types.APIRequest{
+				Namespace: "n1",
+			},
+			id: "r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"list": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "r1",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "n1",
+					Names:     sets.NewString("r1"),
+				},
+			},
+		},
+		{
+			name:  "by id ignores unrequested resources",
+			apiOp: &types.APIRequest{},
+			id:    "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"list": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "r1",
+								},
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "r2",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "n1",
+					Names:     sets.NewString("r1"),
+				},
+			},
+		},
+		// Note: this is deprecated fallback behavior. When we remove the behavior,
+		// rewrite this test to expect an error instead.
+		{
+			name: "by id prefers id embedded namespace",
+			apiOp: &types.APIRequest{
+				Namespace: "n2",
+			},
+			id: "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"list": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "r1",
+								},
+								accesscontrol.Access{
+									Namespace:    "n2",
+									ResourceName: "r2",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "n1",
+					Names:     sets.NewString("r1"),
+				},
+			},
+		},
+		{
+			name:  "cluster scoped id unauthorized",
+			apiOp: &types.APIRequest{},
+			id:    "c1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+				},
+			},
+			wantPartitions: nil,
+		},
+		{
+			name:  "cluster scoped id authorized by name",
+			apiOp: &types.APIRequest{},
+			id:    "c1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": false,
+						"access": accesscontrol.AccessListByVerb{
+							"list": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "*",
+									ResourceName: "c1",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "",
+					Names:     sets.NewString("c1"),
+				},
+			},
+		},
+		{
+			name:  "cluster scoped id authorized globally",
+			apiOp: &types.APIRequest{},
+			id:    "c1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": false,
+						"access": accesscontrol.AccessListByVerb{
+							"list": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "*",
+									ResourceName: "*",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "",
+					Names:     sets.NewString("c1"),
+				},
+			},
+		},
+		{
+			name:  "cluster scoped id ignores unrequested resources",
+			apiOp: &types.APIRequest{},
+			id:    "c1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": false,
+						"access": accesscontrol.AccessListByVerb{
+							"list": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "*",
+									ResourceName: "c1",
+								},
+								accesscontrol.Access{
+									Namespace:    "*",
+									ResourceName: "c2",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "",
+					Names:     sets.NewString("c1"),
+				},
+			},
+		},
 	}
-
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			partitioner := rbacPartitioner{}
@@ -250,3 +535,325 @@ func TestAll(t *testing.T) {
 		})
 	}
 }
+
+func TestVerbWatch(t *testing.T) {
+	tests := []struct {
+		name           string
+		apiOp          *types.APIRequest
+		id             string
+		schema         *types.APISchema
+		wantPartitions []partition.Partition
+	}{
+		{
+			name:  "by id fully unauthorized",
+			apiOp: &types.APIRequest{},
+			id:    "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+				},
+			},
+			wantPartitions: nil,
+		},
+		{
+			name:  "by id missing namespace",
+			apiOp: &types.APIRequest{},
+			id:    "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"watch": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n2",
+									ResourceName: "r2",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: nil,
+		},
+		{
+			name:  "by id missing resource",
+			apiOp: &types.APIRequest{},
+			id:    "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"watch": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "r2",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: nil,
+		},
+		{
+			name:  "by id authorized by name",
+			apiOp: &types.APIRequest{},
+			id:    "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"watch": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "r1",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "n1",
+					Names:     sets.NewString("r1"),
+				},
+			},
+		},
+		{
+			name: "by namespaced id authorized by name",
+			apiOp: &types.APIRequest{
+				Namespace: "n1",
+			},
+			id: "r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"watch": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "r1",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "n1",
+					Names:     sets.NewString("r1"),
+				},
+			},
+		},
+		{
+			name: "by id authorized by namespace",
+			apiOp: &types.APIRequest{
+				Namespace: "n1",
+			},
+			id: "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"watch": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "*",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "n1",
+					Names:     sets.NewString("r1"),
+				},
+			},
+		},
+		{
+			name:  "by id ignores unrequested resources",
+			apiOp: &types.APIRequest{},
+			id:    "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"watch": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "r1",
+								},
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "r2",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "n1",
+					Names:     sets.NewString("r1"),
+				},
+			},
+		},
+		// Note: this is deprecated fallback behavior. When we remove the behavior,
+		// rewrite this test to expect an error instead.
+		{
+			name: "by id prefers id embedded namespace",
+			apiOp: &types.APIRequest{
+				Namespace: "n2",
+			},
+			id: "n1/r1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": true,
+						"access": accesscontrol.AccessListByVerb{
+							"watch": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "n1",
+									ResourceName: "r1",
+								},
+								accesscontrol.Access{
+									Namespace:    "n2",
+									ResourceName: "r2",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "n1",
+					Names:     sets.NewString("r1"),
+				},
+			},
+		},
+		{
+			name:  "cluster scoped id unauthorized",
+			apiOp: &types.APIRequest{},
+			id:    "c1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+				},
+			},
+			wantPartitions: nil,
+		},
+		{
+			name:  "cluster scoped id authorized by name",
+			apiOp: &types.APIRequest{},
+			id:    "c1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": false,
+						"access": accesscontrol.AccessListByVerb{
+							"watch": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "*",
+									ResourceName: "c1",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "",
+					Names:     sets.NewString("c1"),
+				},
+			},
+		},
+		{
+			name:  "cluster scoped id authorized globally",
+			apiOp: &types.APIRequest{},
+			id:    "c1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": false,
+						"access": accesscontrol.AccessListByVerb{
+							"watch": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "*",
+									ResourceName: "*",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "",
+					Names:     sets.NewString("c1"),
+				},
+			},
+		},
+		{
+			name:  "cluster scoped id ignores unrequested resources",
+			apiOp: &types.APIRequest{},
+			id:    "c1",
+			schema: &types.APISchema{
+				Schema: &schemas.Schema{
+					ID: "foo",
+					Attributes: map[string]interface{}{
+						"namespaced": false,
+						"access": accesscontrol.AccessListByVerb{
+							"watch": accesscontrol.AccessList{
+								accesscontrol.Access{
+									Namespace:    "*",
+									ResourceName: "c1",
+								},
+								accesscontrol.Access{
+									Namespace:    "*",
+									ResourceName: "c2",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantPartitions: []partition.Partition{
+				Partition{
+					Namespace: "",
+					Names:     sets.NewString("c1"),
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			partitioner := rbacPartitioner{}
+			verb := "watch"
+			gotPartitions, gotErr := partitioner.All(test.apiOp, test.schema, verb, test.id)
+			assert.Nil(t, gotErr)
+			assert.Equal(t, test.wantPartitions, gotPartitions)
+		})
+	}
+}