Skip to content

Commit

Permalink
Refactor and cleanup max-depth implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
doodlesbykumbi authored Dec 9, 2021
1 parent afb88c4 commit 6afb2b4
Show file tree
Hide file tree
Showing 22 changed files with 68 additions and 42 deletions.
2 changes: 1 addition & 1 deletion cmd/check/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func newCheckCmd() *cobra.Command {

client.RegisterRemoteURLFlags(cmd.Flags())
cmdx.RegisterFormatFlags(cmd.Flags())
cmd.Flags().Int32P(FlagMaxDepth, "d", 0, "maximum depth of the search tree")
cmd.Flags().Int32P(FlagMaxDepth, "d", 0, "Maximum depth of the search tree. If the value is less than 1 or greater than the global max-depth then the global max-depth will be used instead.")

return cmd
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/expand/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func NewExpandCmd() *cobra.Command {
client.RegisterRemoteURLFlags(cmd.Flags())
cmdx.RegisterJSONFormatFlags(cmd.Flags())
cmdx.RegisterNoiseFlags(cmd.Flags())
cmd.Flags().Int32P(FlagMaxDepth, "d", 100, "maximum depth of the tree")
cmd.Flags().Int32P(FlagMaxDepth, "d", 0, "Maximum depth of the tree to be returned. If the value is less than 1 or greater than the global max-depth then the global max-depth will be used instead.")

return cmd
}
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/cli/keto-check.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ keto check <subject> <relation> <namespace> <object> [fl
```
-f, --format string Set the output format. One of table, json, and json-pretty. (default "default")
-h, --help help for check
-d, --max-depth int32 maximum depth of the search tree (default 0)
-d, --max-depth int32 Maximum depth of the search tree. If the value is less than 1 or greater than the global max-depth then the global max-depth will be used instead. (default 0)
-q, --quiet Be quiet with output printing.
--read-remote string Remote URL of the read API endpoint. (default "127.0.0.1:4466")
--write-remote string Remote URL of the write API endpoint. (default "127.0.0.1:4467")
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/cli/keto-expand.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ keto expand <relation> <namespace> <object> [flags]
```
-f, --format string Set the output format. One of default, json, and json-pretty. (default "default")
-h, --help help for expand
-d, --max-depth int32 maximum depth of the tree (default 100)
-d, --max-depth int32 Maximum depth of the tree to be returned. If the value is less than 1 or greater than the global max-depth then the global max-depth will be used instead. (default 0)
-q, --quiet Be quiet with output printing.
--read-remote string Remote URL of the read API endpoint. (default "127.0.0.1:4466")
--write-remote string Remote URL of the write API endpoint. (default "127.0.0.1:4467")
Expand Down
10 changes: 7 additions & 3 deletions docs/docs/concepts/api-overview.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ This API resolves [subject sets](./subjects.mdx#subject-sets) and
This API is primarily used to
[check permissions to restrict actions](../guides/simple-access-check-guide.mdx).

A check-request has to include the maximum depth of the search tree.
This is required to ensure low latency and limit the resource usage per request.
A check-request can include the maximum depth of the search tree. If the value is
less than 1 or greater than the global max-depth then the global max-depth will
be used instead.
This is to ensure low latency and limit the resource usage per request.
To find out more about Ory Keto's performance, head over to the
[performance considerations](../performance.mdx).

Expand All @@ -61,7 +63,9 @@ tuples including the operands as defined in the
- determine why someone has access to an object
- audit permissions in the system

An expand-request has to include the maximum depth of the tree to be returned.
An expand-request can include the maximum depth of the tree to be returned. If the value is
less than 1 or greater than the global max-depth then the global max-depth will
be used instead.
This is required to ensure low latency and limit the resource usage per request.
To find out more about Ory Keto's performance, head over to the
[performance considerations](../performance.mdx).
Expand Down
4 changes: 2 additions & 2 deletions docs/docs/reference/proto-api.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ Checks whether a specific subject is related to an object.
| subject | [Subject](#ory.keto.acl.v1alpha1.Subject) | | The related subject in this check. |
| latest | [bool](#bool) | | This field is not implemented yet and has no effect. <!-- Set this field to `true` in case your application needs to authorize depending on up to date ACLs, also called a "content-change check".<br/>If set to `true` the `snaptoken` field is ignored, the check is evaluated at the latest snapshot (globally consistent) and the response includes a snaptoken for clients to store along with object contents that can be used for subsequent checks of the same content version.<br/>Example use case: - You need to authorize a user to modify/delete some resource and it is unacceptable that if the permission to do that had just been revoked some seconds ago so that the change had not yet been fully replicated to all availability zones. --> |
| snaptoken | [string](#string) | | This field is not implemented yet and has no effect. <!-- Optional. Like reads, a check is always evaluated at a consistent snapshot no earlier than the given snaptoken.<br/>Leave this field blank if you want to evaluate the check based on eventually consistent ACLs, benefiting from very low latency, but possibly slightly stale results.<br/>If the specified token is too old and no longer known, the server falls back as if no snaptoken had been specified.<br/>If not specified the server tries to evaluate the check on the best snapshot version where it is very likely that ACLs had already been replicated to all availability zones. --> |
| max_depth | [int32](#int32) | | The maximum depth to search for a relation. |
| max_depth | [int32](#int32) | | The maximum depth to search for a relation.<br/>If the value is less than 1 or greater than the global max-depth then the global max-depth will be used instead. |



Expand Down Expand Up @@ -163,7 +163,7 @@ Expands the given subject set.
| Field | Type | Label | Description |
| ----- | ---- | ----- | ----------- |
| subject | [Subject](#ory.keto.acl.v1alpha1.Subject) | | The subject to expand. |
| max_depth | [int32](#int32) | | The maximum depth of tree to build. It is important to set this parameter to a meaningful value. Ponder how deep you really want to display this. |
| max_depth | [int32](#int32) | | The maximum depth of tree to build.<br/>If the value is less than 1 or greater than the global max-depth then the global max-depth will be used instead.<br/>It is important to set this parameter to a meaningful value. Ponder how deep you really want to display this. |
| snaptoken | [string](#string) | | This field is not implemented yet and has no effect. <!-- Optional. Like reads, a expand is always evaluated at a consistent snapshot no earlier than the given snaptoken.<br/>Leave this field blank if you want to expand based on eventually consistent ACLs, benefiting from very low latency, but possibly slightly stale results.<br/>If the specified token is too old and no longer known, the server falls back as if no snaptoken had been specified.<br/>If not specified the server tries to build the tree on the best snapshot version where it is very likely that ACLs had already been replicated to all availability zones. --> |


Expand Down
2 changes: 2 additions & 0 deletions internal/check/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type (
EngineDependencies interface {
relationtuple.ManagerProvider
config.Provider
x.LoggerProvider
}
)

Expand Down Expand Up @@ -85,6 +86,7 @@ func (e *Engine) checkOneIndirectionFurther(
restDepth int,
) (bool, error) {
if restDepth <= 0 {
e.d.Logger().WithFields(requested.ToLoggerFields()).Debug("reached max-depth, therefore this query will not be further expanded")
return false, nil
}

Expand Down
19 changes: 17 additions & 2 deletions internal/check/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,25 @@ import (
"github.com/ory/keto/internal/driver"
)

func newDepsProvider(t *testing.T, namespaces []*namespace.Namespace, pageOpts ...x.PaginationOptionSetter) *relationtuple.ManagerWrapper {
type configProvider = config.Provider
type loggerProvider = x.LoggerProvider
// deps is defined to capture engine dependencies in a single struct
type deps struct {
*relationtuple.ManagerWrapper // managerProvider
configProvider
loggerProvider
}

func newDepsProvider(t *testing.T, namespaces []*namespace.Namespace, pageOpts ...x.PaginationOptionSetter) *deps {
reg := driver.NewSqliteTestRegistry(t, false)
require.NoError(t, reg.Config().Set(config.KeyNamespaces, namespaces))
return relationtuple.NewManagerWrapper(t, reg, pageOpts...)
mr := relationtuple.NewManagerWrapper(t, reg, pageOpts...)

return &deps{
ManagerWrapper: mr,
configProvider: reg,
loggerProvider: reg,
}
}

func TestEngine(t *testing.T) {
Expand Down
7 changes: 3 additions & 4 deletions internal/check/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,10 @@ type RESTResponse struct {
Allowed bool `json:"allowed"`
}

// swagger:parameters getCheckRequest
// swagger:parameters getCheck postCheck
// nolint:deadcode,unused
type getCheckRequest struct {
// in:query
// required: true
MaxDepth int `json:"max-depth"`
}

Expand All @@ -91,7 +90,7 @@ type getCheckRequest struct {
// 403: getCheckResponse
// 500: genericError
func (h *Handler) getCheck(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
maxDepth, err := x.GetMaxDepthFromQuery(r.URL.Query(), false)
maxDepth, err := x.GetMaxDepthFromQuery(r.URL.Query())
if err != nil {
h.d.Writer().WriteError(w, r, herodot.ErrBadRequest.WithError(err.Error()))
return
Expand Down Expand Up @@ -140,7 +139,7 @@ func (h *Handler) getCheck(w http.ResponseWriter, r *http.Request, _ httprouter.
// 403: getCheckResponse
// 500: genericError
func (h *Handler) postCheck(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
maxDepth, err := x.GetMaxDepthFromQuery(r.URL.Query(), false)
maxDepth, err := x.GetMaxDepthFromQuery(r.URL.Query())
if err != nil {
h.d.Writer().WriteError(w, r, herodot.ErrBadRequest.WithError(err.Error()))
return
Expand Down
3 changes: 3 additions & 0 deletions internal/expand/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package expand
import (
"context"

"github.com/ory/keto/internal/driver/config"
"github.com/ory/keto/internal/x"
"github.com/ory/keto/internal/x/graph"

Expand All @@ -12,6 +13,8 @@ import (
type (
EngineDependencies interface {
relationtuple.ManagerProvider
config.Provider
x.LoggerProvider
}
Engine struct {
d EngineDependencies
Expand Down
15 changes: 14 additions & 1 deletion internal/expand/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,24 @@ import (
"github.com/ory/keto/internal/driver"
)

type configProvider = config.Provider
type loggerProvider = x.LoggerProvider
// deps is defined to capture engine dependencies in a single struct
type deps struct {
*relationtuple.ManagerWrapper // managerProvider
configProvider
loggerProvider
}

func newTestEngine(t *testing.T, namespaces []*namespace.Namespace, paginationOpts ...x.PaginationOptionSetter) (*relationtuple.ManagerWrapper, *expand.Engine) {
innerReg := driver.NewSqliteTestRegistry(t, false)
require.NoError(t, innerReg.Config().Set(config.KeyNamespaces, namespaces))
reg := relationtuple.NewManagerWrapper(t, innerReg, paginationOpts...)
e := expand.NewEngine(reg)
e := expand.NewEngine(&deps{
ManagerWrapper: reg,
configProvider: innerReg,
loggerProvider: innerReg,
})
return reg, e
}

Expand Down
3 changes: 1 addition & 2 deletions internal/expand/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ func (h *handler) RegisterWriteGRPC(s *grpc.Server) {}
// nolint:deadcode,unused
type getExpandRequest struct {
// in:query
// required: true
MaxDepth int `json:"max-depth"`
}

Expand All @@ -76,7 +75,7 @@ type getExpandRequest struct {
// 404: genericError
// 500: genericError
func (h *handler) getExpand(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
maxDepth, err := x.GetMaxDepthFromQuery(r.URL.Query(), true)
maxDepth, err := x.GetMaxDepthFromQuery(r.URL.Query())
if err != nil {
h.d.Writer().WriteError(w, r, herodot.ErrBadRequest.WithError(err.Error()))
return
Expand Down
10 changes: 0 additions & 10 deletions internal/expand/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,6 @@ func TestRESTHandler(t *testing.T) {
ts := httptest.NewServer(r)
defer ts.Close()

t.Run("case=returns required query parameter max-depth is missing", func(t *testing.T) {
resp, err := ts.Client().Get(ts.URL + expand.RouteBase)
require.NoError(t, err)

assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
assert.Contains(t, string(body), "required query parameter 'max-depth'")
})

t.Run("case=returns bad request on malformed int", func(t *testing.T) {
resp, err := ts.Client().Get(ts.URL + expand.RouteBase + "?max-depth=foo")
require.NoError(t, err)
Expand Down
Empty file.
Empty file.
Empty file.
6 changes: 0 additions & 6 deletions internal/relationtuple/definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ import (

"github.com/pkg/errors"

"github.com/ory/keto/internal/driver/config"
"github.com/ory/keto/internal/x"
)

type (
ManagerProvider interface {
RelationTupleManager() Manager
Config() *config.Config
}
Manager interface {
GetRelationTuples(ctx context.Context, query *RelationQuery, options ...x.PaginationOptionSetter) ([]*InternalRelationTuple, string, error)
Expand Down Expand Up @@ -662,10 +660,6 @@ func NewManagerWrapper(_ *testing.T, reg ManagerProvider, options ...x.Paginatio
}
}

func (t *ManagerWrapper) Config() *config.Config {
return t.Reg.Config()
}

func (t *ManagerWrapper) GetRelationTuples(ctx context.Context, query *RelationQuery, options ...x.PaginationOptionSetter) ([]*InternalRelationTuple, string, error) {
opts := x.GetPaginationOptions(options...)
t.RequestedPages = append(t.RequestedPages, opts.Token)
Expand Down
9 changes: 1 addition & 8 deletions internal/x/max_depth.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,7 @@ import (
"strconv"
)

func GetMaxDepthFromQuery(q url.Values, required bool) (int, error) {
if !q.Has("max-depth") {
if required {
return 0, fmt.Errorf("required query parameter 'max-depth' is missing")
}
return 0, nil
}

func GetMaxDepthFromQuery(q url.Values) (int, error) {
maxDepth, err := strconv.ParseInt(q.Get("max-depth"), 0, 0)
if err != nil {
return 0, fmt.Errorf("unable to parse 'max-depth' query parameter to int: %s", err)
Expand Down
3 changes: 3 additions & 0 deletions proto/ory/keto/acl/v1alpha1/check_service.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions proto/ory/keto/acl/v1alpha1/check_service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ message CheckRequest {
// -->
string snaptoken = 6;
// The maximum depth to search for a relation.
//
// If the value is less than 1 or greater than the global
// max-depth then the global max-depth will be used instead.
int32 max_depth = 7;
}

Expand Down
4 changes: 4 additions & 0 deletions proto/ory/keto/acl/v1alpha1/expand_service.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions proto/ory/keto/acl/v1alpha1/expand_service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ message ExpandRequest {
// The subject to expand.
Subject subject = 1;
// The maximum depth of tree to build.
//
// If the value is less than 1 or greater than the global
// max-depth then the global max-depth will be used instead.
//
// It is important to set this parameter to a meaningful
// value. Ponder how deep you really want to display this.
int32 max_depth = 2;
Expand Down

0 comments on commit 6afb2b4

Please sign in to comment.