engine/opa: cleanup V2SetPolicies vs V2p1SetPolicies

Plain v2 knows nothing about rules, and I think that's how it should
stay.

Signed-off-by: Stephan Renatus <[email protected]>

components/authz-service/engine/conformance/conformance_test.go

@@ -21,7 +21,7 @@ import (
 )
 
 func TestIsAuthorized(t *testing.T) {
-	ctx, engines := setup(t)
+	ctx, engines := setupV1(t)
 	sub, act, res := "user:local:admin", "read", "auth:users:admin"
 
 	// We're always passing the same arguments to IsAuthorized(). This allows for
@@ -210,7 +210,7 @@ func TestIsAuthorized(t *testing.T) {
 }
 
 func TestHierarchicalResourcePolicies(t *testing.T) {
-	ctx, engines := setup(t)
+	ctx, engines := setupV1(t)
 	sub, act, res := "user:local:someid", "read", "compliance:scans:123"
 	args := func() (context.Context, engine.Subjects, engine.Action, engine.Resource) {
 		return ctx, engine.Subject(sub), engine.Action(act), engine.Resource(res)
@@ -308,7 +308,7 @@ func TestHierarchicalResourcePolicies(t *testing.T) {
 }
 
 func TestWildcardActionPolicies(t *testing.T) {
-	ctx, engines := setup(t)
+	ctx, engines := setupV1(t)
 	sub, act, res := "user:local:someid", "read", "nodes:property"
 	args := func() (context.Context, engine.Subjects, engine.Action, engine.Resource) {
 		return ctx, engine.Subject(sub), engine.Action(act), engine.Resource(res)
@@ -397,7 +397,7 @@ func TestWildcardActionPolicies(t *testing.T) {
 }
 
 func TestWildcardSubjectsPolicies(t *testing.T) {
-	ctx, engines := setup(t)
+	ctx, engines := setupV1(t)
 	sub, act, res := "user:local:someid", "read", "nodes:property"
 	args := func() (context.Context, engine.Subjects, engine.Action, engine.Resource) {
 		return ctx, engine.Subject(sub), engine.Action(act), engine.Resource(res)
@@ -489,7 +489,7 @@ func TestWildcardSubjectsPolicies(t *testing.T) {
 }
 
 func TestFilterAuthorizedPairs(t *testing.T) {
-	ctx, engines := setup(t)
+	ctx, engines := setupV1(t)
 	sub, act0, res0, act1, res1 := "user:local:someid", "read", "nodes:someid", "delete", "compliance:profiles"
 	pair0 := engine.Pair{Resource: engine.Resource(res0), Action: engine.Action(act0)}
 	pair1 := engine.Pair{Resource: engine.Resource(res1), Action: engine.Action(act1)}
@@ -663,6 +663,30 @@ func BenchmarkFilterAuthorizedPairs(b *testing.B) {
 	}
 }
 
+func setupV1(t testing.TB) (context.Context, map[string]engine.Engine) {
+	ctx, engines := setup(t)
+	if o, ok := engines["opa"]; ok {
+		o.SetPolicies(ctx, map[string]interface{}{})
+	}
+	return ctx, engines
+}
+
+func setupV2(t testing.TB) (context.Context, map[string]engine.Engine) {
+	ctx, engines := setup(t)
+	if o, ok := engines["opa"]; ok {
+		o.V2SetPolicies(ctx, map[string]interface{}{}, map[string]interface{}{})
+	}
+	return ctx, engines
+}
+
+func setupV2p1(t testing.TB) (context.Context, map[string]engine.Engine) {
+	ctx, engines := setup(t)
+	if o, ok := engines["opa"]; ok {
+		o.V2p1SetPolicies(ctx, map[string]interface{}{}, map[string]interface{}{}, map[string][]interface{}{})
+	}
+	return ctx, engines
+}
+
 func setup(t testing.TB) (context.Context, map[string]engine.Engine) {
 	t.Helper()
 	ctx := context.Background()

components/authz-service/engine/conformance/conformance_v2_test.go

@@ -22,7 +22,7 @@ import (
  ************ ************ ************ ************ ************ ************/
 
 func TestV2IsAuthorized(t *testing.T) {
-	ctx, engines := setup(t)
+	ctx, engines := setupV2(t)
 	sub, act, res := "user:local:admin", "iam:users:create", "iam:users"
 
 	// We're always passing the same arguments to IsAuthorized(). This allows for
@@ -81,7 +81,7 @@ func TestV2IsAuthorized(t *testing.T) {
 }
 
 func TestV2p1ProjectsAuthorized(t *testing.T) {
-	ctx, engines := setup(t)
+	ctx, engines := setupV2p1(t)
 	sub, act, res := "user:local:admin", "iam:users:create", "iam:users"
 	proj1, proj2, proj3, proj4, unassigned := "proj-1", "proj-2", "proj-3", "proj-4", constants.UnassignedProjectID
 	allProjects := []string{proj1, proj2, proj3, proj4, unassigned}
@@ -370,7 +370,7 @@ func TestV2p1ProjectsAuthorized(t *testing.T) {
 						},
 					},
 				}
-				setPoliciesV2(t, e, pol)
+				setPoliciesV2p1(t, e, pol)
 				// in the server, we fetch the list of all projects when the projects filter is empty
 				actual, err := e.V2ProjectsAuthorized(args(allProjects))
 				require.NoError(t, err)
@@ -381,7 +381,7 @@ func TestV2p1ProjectsAuthorized(t *testing.T) {
 }
 
 func TestV2FilterAuthorizedPairs(t *testing.T) {
-	ctx, engines := setup(t)
+	ctx, engines := setupV2(t)
 	sub, act0, res0, act1, res1 := "user:local:someid", "iam:users:create",
 		"nodes:someid", "compliance:profiles:delete", "compliance:profiles"
 	pair0 := engine.Pair{Resource: engine.Resource(res0), Action: engine.Action(act0)}
@@ -493,7 +493,7 @@ func TestV2FilterAuthorizedPairs(t *testing.T) {
 }
 
 func TestV2FilterAuthorizedProjects(t *testing.T) {
-	ctx, engines := setup(t)
+	ctx, engines := setupV2p1(t)
 	sub := "user:local:someid"
 	act0, res0 := "iam:users:create", "nodes:someid"
 	act1, res1 := "compliance:profiles:delete", "compliance:profiles"
@@ -712,11 +712,11 @@ func setPoliciesV2pX(t testing.TB, twoPointOne bool, e engine.Engine, policiesAn
 	for _, role := range roles {
 		roleMap[role["id"].(string)] = role
 	}
-	var err error
 	if twoPointOne {
-		err = e.V2p1SetPolicies(ctx, policyMap, roleMap, make(map[string][]interface{}))
+		err := e.V2p1SetPolicies(ctx, policyMap, roleMap, make(map[string][]interface{}))
+		require.NoError(t, err, "set policies(v2.1)")
 	} else {
-		err = e.V2SetPolicies(ctx, policyMap, roleMap, make(map[string][]interface{}))
+		err := e.V2SetPolicies(ctx, policyMap, roleMap)
+		require.NoError(t, err, "set policies(v2)")
 	}
-	require.NoError(t, err, "set policies(v2)")
 }

components/authz-service/engine/engine.go

@@ -72,7 +72,7 @@ type Writer interface {
 }
 
 type V2Writer interface {
-	V2SetPolicies(context.Context, map[string]interface{}, map[string]interface{}, map[string][]interface{}) error
+	V2SetPolicies(context.Context, map[string]interface{}, map[string]interface{}) error
 }
 
 type V2p1Writer interface {

components/authz-service/engine/opa/opa.go

@@ -631,12 +631,10 @@ func (s *State) SetPolicies(ctx context.Context, policies map[string]interface{}
 // and resets the partial evaluation cache for v2
 func (s *State) V2SetPolicies(
 	ctx context.Context, policyMap map[string]interface{},
-	roleMap map[string]interface{}, ruleMap map[string][]interface{}) error {
-	// TODO: v2 doesn't care about rules
+	roleMap map[string]interface{}) error {
 	s.v2Store = inmem.NewFromObject(map[string]interface{}{
 		"policies": policyMap,
 		"roles":    roleMap,
-		"rules":    ruleMap,
 	})
 
 	return s.initPartialResultV2(ctx)

components/authz-service/server/v2/policy_refresher.go

@@ -184,7 +184,7 @@ func (refresher *policyRefresher) updateEngineStore(ctx context.Context, vsn api
 	case vsn.Major == api.Version_V2 && vsn.Minor == api.Version_V1: // v2.1
 		return refresher.engine.V2p1SetPolicies(ctx, policyMap, roleMap, ruleMap)
 	default: // v2.0
-		return refresher.engine.V2SetPolicies(ctx, policyMap, roleMap, ruleMap)
+		return refresher.engine.V2SetPolicies(ctx, policyMap, roleMap)
 	}
 }
 

components/authz-service/server/v2/policy_test.go

@@ -3209,7 +3209,13 @@ type testEngine struct {
 }
 
 func (te *testEngine) V2SetPolicies(
-	_ context.Context, policies map[string]interface{},
+	ctx context.Context, policies map[string]interface{},
+	roles map[string]interface{}) error {
+	return te.V2p1SetPolicies(ctx, policies, roles, map[string][]interface{}{})
+}
+
+func (te *testEngine) V2p1SetPolicies(
+	ctx context.Context, policies map[string]interface{},
 	roles map[string]interface{}, rules map[string][]interface{}) error {
 	te.policyMap = policies
 	// TODO: use these
@@ -3218,12 +3224,6 @@ func (te *testEngine) V2SetPolicies(
 	return nil
 }
 
-func (te *testEngine) V2p1SetPolicies(
-	ctx context.Context, policies map[string]interface{},
-	roles map[string]interface{}, rules map[string][]interface{}) error {
-	return te.V2SetPolicies(ctx, policies, roles, rules)
-}
-
 func id(t *testing.T, p *prng.Prng) string {
 	t.Helper()
 	faker := faker.NewWithSeed(p)

components/authz-service/server/v2/system_integration_test.go

@@ -19,7 +19,7 @@ import (
 
 func TestSystemPolicies(t *testing.T) {
 	ctx := context.Background()
-	ts := setupWithOPA(t)
+	ts := setupWithOPAV2(t)
 	cl := ts.authz
 
 	isAuthorized := func(subject, action, resource string) func(*testing.T) {
@@ -55,7 +55,7 @@ func TestSystemPolicies(t *testing.T) {
 
 func TestFilterAuthorizedProjectsWithSystemPolicies(t *testing.T) {
 	ctx := context.Background()
-	ts := setupWithOPA(t)
+	ts := setupWithOPAV2p1(t)
 
 	t.Run("user should only get projects they have non-system level access to", func(t *testing.T) {
 		_, err := ts.projects.CreateProject(ctx, &api_v2.CreateProjectReq{
@@ -89,7 +89,15 @@ func TestFilterAuthorizedProjectsWithSystemPolicies(t *testing.T) {
 	})
 }
 
-func setupWithOPA(t *testing.T) testSetup {
+func setupWithOPAV2(t *testing.T) testSetup {
+	return setupWithOPAV2pX(t, false)
+}
+
+func setupWithOPAV2p1(t *testing.T) testSetup {
+	return setupWithOPAV2pX(t, true)
+}
+
+func setupWithOPAV2pX(t *testing.T, twoPointOne bool) testSetup {
 	t.Helper()
 	ctx := context.Background()
 
@@ -102,7 +110,13 @@ func setupWithOPA(t *testing.T) testSetup {
 	vChan := make(chan api_v2.Version, 1)
 	emptyV1List := v1Lister{}
 	ts := setupV2(t, o, o, &emptyV1List, vChan)
-	_, err = ts.policy.MigrateToV2(ctx, &api_v2.MigrateToV2Req{Flag: api_v2.Flag_VERSION_2_1})
+	var flag api_v2.Flag
+	if twoPointOne {
+		flag = api_v2.Flag_VERSION_2_1
+	} else {
+		flag = api_v2.Flag_VERSION_2_0
+	}
+	_, err = ts.policy.MigrateToV2(ctx, &api_v2.MigrateToV2Req{Flag: flag})
 	require.NoError(t, err)
 	return ts
 }