Skip to content

Commit

Permalink
feat: use rond logger instead of logrus (#220)
Browse files Browse the repository at this point in the history 
* feat: use rond logger instead of logrus

* feat: upgrade to use glogger v4

* remove logrus from sdk and core

* rename logger in logging

* now logger is no more required in sdk

* remove old TODO

* refactor: change import sorting

* Update logging/logrus/logger_test.go

Co-authored-by: Federico Maggi <[email protected]>

* Update logging/test/logger_test.go

Co-authored-by: Federico Maggi <[email protected]>

* Update sdk/sdk_test.go

Co-authored-by: Federico Maggi <[email protected]>

* update

---------

Co-authored-by: Federico Maggi <[email protected]>
  • Loading branch information
@davidebianchi @fredmaggiowski
davidebianchi and fredmaggiowski authored Jul 18, 2023
1 parent 8ce2333 commit daa8ad1
Show file tree
Hide file tree
Showing 41 changed files with 1,171 additions and 248 deletions.
11 changes: 5 additions & 6 deletions core/input.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ import (
"net/url"
"time"

"github.com/rond-authz/rond/logging"
"github.com/rond-authz/rond/types"

"github.com/sirupsen/logrus"
)

type Input struct {
Expand Down Expand Up @@ -54,7 +53,7 @@ type InputUser struct {
ResourcePermissionsMap PermissionsOnResourceMap `json:"resourcePermissionsMap,omitempty"`
}

func (input *Input) buildOptimizedResourcePermissionsMap(logger *logrus.Entry, enableResourcePermissionsMapOptimization bool) {
func (input *Input) buildOptimizedResourcePermissionsMap(logger logging.Logger, enableResourcePermissionsMapOptimization bool) {
if !enableResourcePermissionsMapOptimization {
return
}
Expand Down Expand Up @@ -85,15 +84,15 @@ func (input *Input) buildOptimizedResourcePermissionsMap(logger *logrus.Entry, e
}
}
input.User.ResourcePermissionsMap = permissionsOnResourceMap
logger.WithField("resourcePermissionMapCreationTime", fmt.Sprintf("%+v", time.Since(opaPermissionsMapTime))).Tracef("resource permission map creation")
logger.WithField("resourcePermissionMapCreationTime", fmt.Sprintf("%+v", time.Since(opaPermissionsMapTime))).Trace("resource permission map creation")
}

type RegoInputOptions struct {
EnableResourcePermissionsMapOptimization bool
}

func CreateRegoQueryInput(
logger *logrus.Entry,
logger logging.Logger,
input Input,
options RegoInputOptions,
) ([]byte, error) {
Expand All @@ -107,7 +106,7 @@ func CreateRegoQueryInput(
}
logger.
WithField("inputCreationTimeMicroseconds", time.Since(opaInputCreationTime).Microseconds()).
Tracef("input creation time")
Trace("input creation time")
return inputBytes, nil
}

Expand Down
19 changes: 8 additions & 11 deletions core/input_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,17 @@ import (
"fmt"
"testing"

"github.com/rond-authz/rond/logging"
"github.com/rond-authz/rond/types"

"github.com/sirupsen/logrus"
"github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/require"
)

func TestCreateRegoInput(t *testing.T) {
logrusLogger, _ := test.NewNullLogger()
logger := logrus.NewEntry(logrusLogger)
log := logging.NewNoOpLogger()

t.Run("returns correctly", func(t *testing.T) {
actual, err := CreateRegoQueryInput(logger, Input{}, RegoInputOptions{})
actual, err := CreateRegoQueryInput(log, Input{}, RegoInputOptions{})
require.NoError(t, err)
require.Equal(t, "{\"request\":{\"method\":\"\",\"path\":\"\"},\"response\":{},\"user\":{}}", string(actual))
})
Expand Down Expand Up @@ -79,7 +77,7 @@ func TestCreateRegoInput(t *testing.T) {
User: user,
}

input.buildOptimizedResourcePermissionsMap(logger, true)
input.buildOptimizedResourcePermissionsMap(log, true)
expected := PermissionsOnResourceMap{
"permission1:type1:resource1": true,
"permission2:type1:resource1": true,
Expand All @@ -101,7 +99,7 @@ func TestCreateRegoInput(t *testing.T) {
User: user,
}

input.buildOptimizedResourcePermissionsMap(logger, false)
input.buildOptimizedResourcePermissionsMap(log, false)
require.Nil(t, input.User.ResourcePermissionsMap)
})

Expand Down Expand Up @@ -136,7 +134,7 @@ func TestCreateRegoInput(t *testing.T) {
},
}

input.buildOptimizedResourcePermissionsMap(logger, true)
input.buildOptimizedResourcePermissionsMap(log, true)
expected := PermissionsOnResourceMap{
"permission1:type1:resource1": true,
"permission2:type1:resource1": true,
Expand Down Expand Up @@ -185,7 +183,7 @@ func TestCreateRegoInput(t *testing.T) {
},
}

input.buildOptimizedResourcePermissionsMap(logger, true)
input.buildOptimizedResourcePermissionsMap(log, true)
expected := PermissionsOnResourceMap{
"permission3:type2:resource2": true,
"permission3:type3:resource3": true,
Expand Down Expand Up @@ -228,8 +226,7 @@ func BenchmarkBuildOptimizedResourcePermissionsMap(b *testing.B) {
Bindings: bindings,
}

logrusLogger, _ := test.NewNullLogger()
logger := logrus.NewEntry(logrusLogger)
logger := logging.NewNoOpLogger()
input := Input{
User: user,
}
Expand Down
33 changes: 19 additions & 14 deletions core/opaevaluator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ import (
"github.com/rond-authz/rond/internal/mongoclient"
"github.com/rond-authz/rond/internal/opatranslator"
"github.com/rond-authz/rond/internal/utils"
"github.com/rond-authz/rond/logging"
"github.com/rond-authz/rond/types"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/rego"
"github.com/prometheus/client_golang/prometheus"
"github.com/sirupsen/logrus"
"go.mongodb.org/mongo-driver/bson/primitive"
)

Expand Down Expand Up @@ -75,11 +75,13 @@ type OPAEvaluator struct {
context context.Context
mongoClient types.IMongoClient
generateQuery bool
logger logging.Logger
}

type OPAEvaluatorOptions struct {
EnablePrintStatements bool
MongoClient types.IMongoClient
Logger logging.Logger
}

func newQueryOPAEvaluator(ctx context.Context, policy string, opaModuleConfig *OPAModuleConfig, input []byte, options *OPAEvaluatorOptions) (*OPAEvaluator, error) {
Expand Down Expand Up @@ -113,19 +115,19 @@ func newQueryOPAEvaluator(ctx context.Context, policy string, opaModuleConfig *O
context: ctx,
mongoClient: options.MongoClient,
generateQuery: true,
logger: options.Logger,
}, nil
}

func (config *OPAModuleConfig) CreateQueryEvaluator(ctx context.Context, logger *logrus.Entry, policy string, input []byte, options *OPAEvaluatorOptions) (*OPAEvaluator, error) {
// TODO: remove logger and set in sdk
logger.WithFields(logrus.Fields{
func (config *OPAModuleConfig) CreateQueryEvaluator(ctx context.Context, logger logging.Logger, policy string, input []byte, options *OPAEvaluatorOptions) (*OPAEvaluator, error) {
logger.WithFields(map[string]any{
"policyName": policy,
}).Info("Policy to be evaluated")

opaEvaluatorInstanceTime := time.Now()
evaluator, err := newQueryOPAEvaluator(ctx, policy, config, input, options)
if err != nil {
logger.WithError(err).Error(ErrEvaluatorCreationFailed)
logger.WithField("error", err).Error(ErrEvaluatorCreationFailed)
return nil, err
}
logger.
Expand All @@ -134,7 +136,7 @@ func (config *OPAModuleConfig) CreateQueryEvaluator(ctx context.Context, logger
return evaluator, nil
}

func (evaluator *OPAEvaluator) partiallyEvaluate(logger *logrus.Entry, options *PolicyEvaluationOptions) (primitive.M, error) {
func (evaluator *OPAEvaluator) partiallyEvaluate(logger logging.Logger, options *PolicyEvaluationOptions) (primitive.M, error) {
if options == nil {
options = &PolicyEvaluationOptions{}
}
Expand All @@ -150,7 +152,7 @@ func (evaluator *OPAEvaluator) partiallyEvaluate(logger *logrus.Entry, options *
"policy_name": evaluator.PolicyName,
}).Observe(float64(opaEvaluationTime.Milliseconds()))

fields := logrus.Fields{
fields := map[string]any{
"evaluationTimeMicroseconds": opaEvaluationTime.Microseconds(),
"policyName": evaluator.PolicyName,
"partialEval": true,
Expand All @@ -166,15 +168,15 @@ func (evaluator *OPAEvaluator) partiallyEvaluate(logger *logrus.Entry, options *
return nil, err
}

logger.WithFields(logrus.Fields{
logger.WithFields(map[string]any{
"allowed": true,
"query": q,
}).Tracef("policy results and query")
}).Trace("policy results and query")

return q, nil
}

func (evaluator *OPAEvaluator) Evaluate(logger *logrus.Entry, options *PolicyEvaluationOptions) (interface{}, error) {
func (evaluator *OPAEvaluator) Evaluate(logger logging.Logger, options *PolicyEvaluationOptions) (interface{}, error) {
if options == nil {
options = &PolicyEvaluationOptions{}
}
Expand All @@ -192,7 +194,7 @@ func (evaluator *OPAEvaluator) Evaluate(logger *logrus.Entry, options *PolicyEva
}).Observe(float64(opaEvaluationTime.Milliseconds()))

allowed, responseBodyOverwriter := processResults(results)
fields := logrus.Fields{
fields := map[string]any{
"evaluationTimeMicroseconds": opaEvaluationTime.Microseconds(),
"policyName": evaluator.PolicyName,
"partialEval": false,
Expand All @@ -203,7 +205,7 @@ func (evaluator *OPAEvaluator) Evaluate(logger *logrus.Entry, options *PolicyEva

logger.WithFields(fields).Debug("policy evaluation completed")

logger.WithFields(logrus.Fields{
logger.WithFields(map[string]any{
"policyName": evaluator.PolicyName,
"allowed": allowed,
}).Info("policy result")
Expand All @@ -220,7 +222,10 @@ func (evaluator *OPAEvaluator) getContext() context.Context {
ctx = context.Background()
}
if evaluator.mongoClient != nil {
return mongoclient.WithMongoClient(ctx, evaluator.mongoClient)
ctx = mongoclient.WithMongoClient(ctx, evaluator.mongoClient)
}
if evaluator.logger != nil {
ctx = logging.WithContext(ctx, evaluator.logger)
}
return ctx
}
Expand All @@ -237,7 +242,7 @@ func (evaluator *PolicyEvaluationOptions) metrics() metrics.Metrics {
return metrics.SetupMetrics("rond")
}

func (evaluator *OPAEvaluator) PolicyEvaluation(logger *logrus.Entry, options *PolicyEvaluationOptions) (interface{}, primitive.M, error) {
func (evaluator *OPAEvaluator) PolicyEvaluation(logger logging.Logger, options *PolicyEvaluationOptions) (interface{}, primitive.M, error) {
if evaluator.generateQuery {
query, err := evaluator.partiallyEvaluate(logger, options)
return nil, query, err
Expand Down
24 changes: 19 additions & 5 deletions core/opaevaluator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ import (

"github.com/rond-authz/rond/internal/mocks"
"github.com/rond-authz/rond/internal/mongoclient"
"github.com/rond-authz/rond/logging"
"github.com/rond-authz/rond/types"

"github.com/sirupsen/logrus"
"github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/require"
)

Expand All @@ -47,14 +46,17 @@ func TestNewOPAEvaluator(t *testing.T) {

func TestOPAEvaluator(t *testing.T) {
t.Run("get context", func(t *testing.T) {
t.Run("no context and no mongo client", func(t *testing.T) {
t.Run("no context", func(t *testing.T) {
opaEval := OPAEvaluator{}
ctx := opaEval.getContext()

require.NotNil(t, ctx)
client, err := mongoclient.GetMongoClientFromContext(ctx)
require.NoError(t, err)
require.Nil(t, client)

logger := logging.FromContext(ctx)
require.NotNil(t, logger)
})

t.Run("passed context with mongo client", func(t *testing.T) {
Expand Down Expand Up @@ -84,6 +86,19 @@ func TestOPAEvaluator(t *testing.T) {
require.NoError(t, err)
require.Equal(t, mongoClient, client)
})

t.Run("passed logger", func(t *testing.T) {
log := logging.NewNoOpLogger()
opaEval := OPAEvaluator{
context: context.Background(),
logger: log,
}
ctx := opaEval.getContext()

require.NotNil(t, ctx)
actualLog := logging.FromContext(ctx)
require.Equal(t, log, actualLog)
})
})
}

Expand Down Expand Up @@ -126,8 +141,7 @@ column_policy{

opaModuleConfig := &OPAModuleConfig{Name: "mypolicy.rego", Content: policy}

log, _ := test.NewNullLogger()
logger := logrus.NewEntry(log)
logger := logging.NewNoOpLogger()

input := Input{Request: InputRequest{}, Response: InputResponse{}}
inputBytes, _ := json.Marshal(input)
Expand Down
13 changes: 8 additions & 5 deletions core/partialevaluators.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import (

"github.com/rond-authz/rond/custom_builtins"
"github.com/rond-authz/rond/internal/mongoclient"
"github.com/rond-authz/rond/logging"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/rego"
"github.com/sirupsen/logrus"
)

type PartialResultsEvaluators map[string]PartialEvaluator
Expand All @@ -35,7 +35,7 @@ type PartialEvaluator struct {
PartialEvaluator *rego.PartialResult
}

func createPartialEvaluator(ctx context.Context, logger *logrus.Entry, policy string, opaModuleConfig *OPAModuleConfig, options *OPAEvaluatorOptions) (*PartialEvaluator, error) {
func createPartialEvaluator(ctx context.Context, logger logging.Logger, policy string, opaModuleConfig *OPAModuleConfig, options *OPAEvaluatorOptions) (*PartialEvaluator, error) {
logger.WithField("policyName", policy).Info("precomputing rego policy")

policyEvaluatorTime := time.Now()
Expand All @@ -45,7 +45,7 @@ func createPartialEvaluator(ctx context.Context, logger *logrus.Entry, policy st
}

logger.
WithFields(logrus.Fields{
WithFields(map[string]any{
"policyName": policy,
"computationTimeMicroserconds": time.Since(policyEvaluatorTime).Microseconds,
}).
Expand All @@ -54,12 +54,12 @@ func createPartialEvaluator(ctx context.Context, logger *logrus.Entry, policy st
return &PartialEvaluator{PartialEvaluator: partialResultEvaluator}, nil
}

func (policyEvaluators PartialResultsEvaluators) AddFromConfig(ctx context.Context, logger *logrus.Entry, opaModuleConfig *OPAModuleConfig, rondConfig *RondConfig, options *OPAEvaluatorOptions) error {
func (policyEvaluators PartialResultsEvaluators) AddFromConfig(ctx context.Context, logger logging.Logger, opaModuleConfig *OPAModuleConfig, rondConfig *RondConfig, options *OPAEvaluatorOptions) error {
allowPolicy := rondConfig.RequestFlow.PolicyName
responsePolicy := rondConfig.ResponseFlow.PolicyName

logger.
WithFields(logrus.Fields{
WithFields(map[string]any{
"policyName": allowPolicy,
"responsePolicyName": responsePolicy,
}).
Expand Down Expand Up @@ -144,6 +144,9 @@ func newPartialResultEvaluator(ctx context.Context, policy string, opaModuleConf
ctx = mongoclient.WithMongoClient(ctx, evaluatorOptions.MongoClient)
options = append(options, custom_builtins.MongoFindOne, custom_builtins.MongoFindMany)
}
if evaluatorOptions.Logger != nil {
ctx = logging.WithContext(ctx, evaluatorOptions.Logger)
}
regoInstance := rego.New(options...)

results, err := regoInstance.PartialResult(ctx)
Expand Down
Loading

0 comments on commit daa8ad1

Please sign in to comment.