Skip to content

Commit

Permalink
remove unused sdk fn
Browse files Browse the repository at this point in the history
  • Loading branch information
davidebianchi committed Jun 26, 2023
1 parent 87e8148 commit 89bf712
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 42 deletions.
73 changes: 43 additions & 30 deletions core/sdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,13 @@ type PolicyResult struct {
// Warning: This interface is experimental, and it could change with breaking also in rond patches.
// Does not use outside this repository until it is not ready.
type SDK interface {
// Warning: this method will be removed in the near future. Do not use it outside Rond.
Metrics() metrics.Metrics

FindEvaluator(logger *logrus.Entry, method, path string) (SDKEvaluator, error)
}

// Warning: This interface is experimental, and it could change with breaking also in rond patches.
// Do not use outside this repository until it is not ready.
type SDKEvaluator interface {
Config() openapi.RondConfig
PartialResultsEvaluators() PartialResultsEvaluators

EvaluateRequestPolicy(ctx context.Context, req RondInput, userInfo types.User) (PolicyResult, error)
EvaluateResponsePolicy(ctx context.Context, rondInput RondInput, userInfo types.User, decodedBody any) ([]byte, error)
Expand All @@ -61,12 +57,16 @@ type evaluator struct {
routeInfo openapi.RouterInfo
}

func (e evaluator) Config() openapi.RondConfig {
return e.rondConfig
func (e evaluator) metrics() metrics.Metrics {
return e.rond.metrics
}

func (e evaluator) PartialResultsEvaluators() PartialResultsEvaluators {
return e.rond.evaluator
func (e evaluator) partialResultEvaluators() PartialResultsEvaluators {
return e.rond.partialResultEvaluators
}

func (e evaluator) Config() openapi.RondConfig {
return e.rondConfig
}

func (e evaluator) EvaluateRequestPolicy(ctx context.Context, req RondInput, userInfo types.User) (PolicyResult, error) {
Expand All @@ -90,7 +90,7 @@ func (e evaluator) EvaluateRequestPolicy(ctx context.Context, req RondInput, use

var evaluatorAllowPolicy *OPAEvaluator
if !rondConfig.RequestFlow.GenerateQuery {
evaluatorAllowPolicy, err = e.rond.evaluator.GetEvaluatorFromPolicy(ctx, rondConfig.RequestFlow.PolicyName, regoInput, e.rond.evaluatorOptions)
evaluatorAllowPolicy, err = e.partialResultEvaluators().GetEvaluatorFromPolicy(ctx, rondConfig.RequestFlow.PolicyName, regoInput, e.rond.evaluatorOptions)
if err != nil {
return PolicyResult{}, err
}
Expand All @@ -107,7 +107,7 @@ func (e evaluator) EvaluateRequestPolicy(ctx context.Context, req RondInput, use

policyName := rondConfig.RequestFlow.PolicyName
opaEvaluationTime := time.Since(opaEvaluationTimeStart)
e.rond.Metrics().PolicyEvaluationDurationMilliseconds.With(prometheus.Labels{
e.metrics().PolicyEvaluationDurationMilliseconds.With(prometheus.Labels{
"policy_name": policyName,
}).Observe(float64(opaEvaluationTime.Milliseconds()))

Expand Down Expand Up @@ -161,7 +161,26 @@ func (e evaluator) EvaluateResponsePolicy(ctx context.Context, rondInput RondInp
return nil, err
}

evaluator, err := e.PartialResultsEvaluators().GetEvaluatorFromPolicy(ctx, e.rondConfig.ResponseFlow.PolicyName, regoInput, e.rond.evaluatorOptions)
opaEvaluationTimeStart := time.Now()

evaluator, err := e.partialResultEvaluators().GetEvaluatorFromPolicy(ctx, e.rondConfig.ResponseFlow.PolicyName, regoInput, e.rond.evaluatorOptions)

policyName := rondConfig.ResponseFlow.PolicyName
opaEvaluationTime := time.Since(opaEvaluationTimeStart)
e.metrics().PolicyEvaluationDurationMilliseconds.With(prometheus.Labels{
"policy_name": policyName,
}).Observe(float64(opaEvaluationTime.Milliseconds()))

e.logger.WithFields(logrus.Fields{
"evaluationTimeMicroseconds": opaEvaluationTime.Microseconds(),
"policyName": policyName,
"partialEval": false,
"allowed": err == nil,
"matchedPath": e.routeInfo.MatchedPath,
"requestedPath": e.routeInfo.RequestedPath,
"method": e.routeInfo.Method,
}).Debug("policy evaluation completed")

if err != nil {
return nil, err
}
Expand All @@ -180,14 +199,13 @@ func (e evaluator) EvaluateResponsePolicy(ctx context.Context, rondInput RondInp
}

type rondImpl struct {
evaluator PartialResultsEvaluators
evaluatorOptions *EvaluatorOptions
oasRouter *bunrouter.CompatRouter
oas *openapi.OpenAPISpec
opaModuleConfig *OPAModuleConfig
partialResultEvaluators PartialResultsEvaluators
evaluatorOptions *EvaluatorOptions
oasRouter *bunrouter.CompatRouter
oas *openapi.OpenAPISpec
opaModuleConfig *OPAModuleConfig

metrics metrics.Metrics
registry *prometheus.Registry
metrics metrics.Metrics

clientTypeHeaderKey string
}
Expand All @@ -203,10 +221,6 @@ func (r rondImpl) FindEvaluator(logger *logrus.Entry, method, path string) (SDKE
}, err
}

func (r rondImpl) Metrics() metrics.Metrics {
return r.metrics
}

// The SDK is now into core because there are coupled function here which should use the SDK itself
// (which uses core, so it will result in a cyclic dependency). In the future, sdk should be in a
// specific package.
Expand Down Expand Up @@ -237,14 +251,13 @@ func NewSDK(
}

return rondImpl{
evaluator: evaluator,
oasRouter: oasRouter,
evaluatorOptions: evaluatorOptions,
oas: oas,
opaModuleConfig: opaModuleConfig,

metrics: m,
registry: registry,
partialResultEvaluators: evaluator,
oasRouter: oasRouter,
evaluatorOptions: evaluatorOptions,
oas: oas,
opaModuleConfig: opaModuleConfig,

metrics: m,

clientTypeHeaderKey: clientTypeHeaderKey,
}, nil
Expand Down
9 changes: 1 addition & 8 deletions core/sdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,6 @@ func TestSDK(t *testing.T) {
rond, ok := sdk.(rondImpl)
require.True(t, ok, "rondImpl is not sdk")

t.Run("metrics", func(t *testing.T) {
require.Equal(t, rond.metrics, sdk.Metrics())
})

t.Run("FindEvaluator", func(t *testing.T) {
t.Run("throws if path and method not found", func(t *testing.T) {
actual, err := sdk.FindEvaluator(logger, http.MethodGet, "/not-existent/path")
Expand Down Expand Up @@ -140,14 +136,11 @@ func TestSDK(t *testing.T) {
},
}, actual.Config())
})

t.Run("get partial evaluators", func(t *testing.T) {
require.Equal(t, rond.evaluator, actual.PartialResultsEvaluators())
})
})
})
}

// TODO: test metrics both in request and response
func TestEvaluateRequestPolicy(t *testing.T) {
logger := logrus.NewEntry(logrus.New())

Expand Down
4 changes: 0 additions & 4 deletions internal/fake/sdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,3 @@ func (e SDKEvaluator) EvaluateResponsePolicy(ctx context.Context, rondInput core
func (s SDKEvaluator) Config() openapi.RondConfig {
return s.permission
}

func (s SDKEvaluator) PartialResultsEvaluators() core.PartialResultsEvaluators {
return s.partialEvaluator
}

0 comments on commit 89bf712

Please sign in to comment.