From 36c44815e8cc297198e4aa0440647b7d340f16f6 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Wed, 5 Jun 2019 10:53:29 +0200 Subject: [PATCH] add code comments per review comments Signed-off-by: Stephan Renatus --- components/authz-service/engine/opa/opa.go | 13 ++++--------- components/authz-service/server/v2/policy.go | 1 + components/authz-service/server/v2/policy_test.go | 6 +++++- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/components/authz-service/engine/opa/opa.go b/components/authz-service/engine/opa/opa.go index 15cdbbad8a2b..91def0759e61 100644 --- a/components/authz-service/engine/opa/opa.go +++ b/components/authz-service/engine/opa/opa.go @@ -148,9 +148,6 @@ func (s *State) initModules() error { // IsAuthorized(), we want to do as little work per call as possible. func (s *State) initPartialResult(ctx context.Context) error { // Reset compiler to avoid state issues - // Note: PartialResult will _compile_ the passed module; so when the engine is - // initialized, everything will also be ready to serve the other, non-partial - // queries compiler, err := s.newCompiler() if err != nil { return err @@ -175,8 +172,6 @@ func (s *State) initPartialResultV2(ctx context.Context) error { if err != nil { return err } - - // Partial eval for authzV2Query. r := rego.New( rego.ParsedQuery(s.queries[authzV2Query]), rego.Compiler(compiler), @@ -621,8 +616,8 @@ func (s *State) SetPolicies(ctx context.Context, policies map[string]interface{} // OR does the entire OPA store have to be re-evaluated at once. IF that's true, // should we have the same OPA instance in general for rules? // -// V2SetPolicies replaces OPA's data with a new set of policies and roles, -// and resets the partial evaluation cache for v2 +// V2SetPolicies replaces OPA's data with a new set of policies, roles, and +// rules, 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 { @@ -636,8 +631,8 @@ func (s *State) V2SetPolicies( return s.initPartialResultV2(ctx) } -// V2p1SetPolicies replaces OPA's data with a new set of policies and roles, -// and resets the partial evaluation cache for v2 +// V2p1SetPolicies replaces OPA's data with a new set of policies, roles, and +// rules, and resets the partial evaluation cache for v2.l func (s *State) V2p1SetPolicies( ctx context.Context, policyMap map[string]interface{}, roleMap map[string]interface{}, ruleMap map[string][]interface{}) error { diff --git a/components/authz-service/server/v2/policy.go b/components/authz-service/server/v2/policy.go index 90af451870e0..2e75d7eac6dc 100644 --- a/components/authz-service/server/v2/policy.go +++ b/components/authz-service/server/v2/policy.go @@ -122,6 +122,7 @@ func NewPoliciesServer( } } + // now that the data is all set, attempt to feed it into OPA: if err := srv.updateEngineStore(ctx); err != nil { return nil, errors.Wrapf(err, "initialize engine storage (%v)", v) } diff --git a/components/authz-service/server/v2/policy_test.go b/components/authz-service/server/v2/policy_test.go index 9912645e36a6..c8eadca64062 100644 --- a/components/authz-service/server/v2/policy_test.go +++ b/components/authz-service/server/v2/policy_test.go @@ -2938,7 +2938,9 @@ type testSetup struct { func setupV2WithWriter(t *testing.T, writer engine.V2pXWriter) testSetup { return setupV2WithMigrationState(t, nil, writer, nil, make(chan api_v2.Version, 1), - func(s storage.MigrationStatusProvider) error { return s.Success(context.Background()) }) // IAM v2 + // if the MigrationStatus is set to "Success", that means we've migrated + // successfully to IAM v2 ("SuccessBeta1" is v2.1). + func(s storage.MigrationStatusProvider) error { return s.Success(context.Background()) }) } func setupV2(t *testing.T, @@ -3229,6 +3231,8 @@ type testEngine struct { func (te *testEngine) V2SetPolicies( ctx context.Context, policies map[string]interface{}, roles map[string]interface{}, rules map[string][]interface{}) error { + // these only set the structs {policy,rule,role}Map, nothing specific to v2 and + // v2.1 yet return te.V2p1SetPolicies(ctx, policies, roles, rules) }