Skip to content

Commit

Permalink
add code comments per review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Stephan Renatus <[email protected]>
  • Loading branch information
srenatus committed Jun 5, 2019
1 parent 29e9509 commit 069a19d
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 20 deletions.
34 changes: 15 additions & 19 deletions components/authz-service/engine/opa/opa.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),
Expand Down Expand Up @@ -220,11 +215,20 @@ func (s *State) newCompiler() (*ast.Compiler, error) {
return compiler, nil
}

// DumpData is a bit fast-and-loose when it comes to error checking; it's not meant
// to be used in production
// DumpData is a bit fast-and-loose when it comes to error checking; it's not
// meant to be used in production. Anywhere you have an OPA engine struct (i.e.
// `State`), you can use either one of these on it and it'll log the store
// contents.
func (s *State) DumpData(ctx context.Context) error {
return dumpData(ctx, s.store, s.log)
}
func (s *State) DumpDataV2(ctx context.Context) error {
return dumpData(ctx, s.v2Store, s.log)
}

func (s *State) DumpDataV2p1(ctx context.Context) error {
return dumpData(ctx, s.v2p1Store, s.log)
}

func dumpData(ctx context.Context, store storage.Store, l logger.Logger) error {
txn, err := store.NewTransaction(ctx)
Expand All @@ -239,14 +243,6 @@ func dumpData(ctx context.Context, store storage.Store, l logger.Logger) error {
return store.Commit(ctx, txn)
}

func (s *State) DumpDataV2(ctx context.Context) error {
return dumpData(ctx, s.v2Store, s.log)
}

func (s *State) DumpDataV2p1(ctx context.Context) error {
return dumpData(ctx, s.v2p1Store, s.log)
}

// IsAuthorized evaluates whether a given [subject, resource, action] tuple
// is authorized given the service's state
func (s *State) IsAuthorized(
Expand Down Expand Up @@ -621,8 +617,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 {
Expand All @@ -636,8 +632,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 {
Expand Down
1 change: 1 addition & 0 deletions components/authz-service/server/v2/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
6 changes: 5 additions & 1 deletion components/authz-service/server/v2/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}

Expand Down

0 comments on commit 069a19d

Please sign in to comment.