Skip to content

Commit

Permalink
fix: hide relation tuples with deleted namespace
Browse files Browse the repository at this point in the history
  • Loading branch information
hperl authored and zepatrik committed Jul 15, 2022
1 parent e71d191 commit cb1a2dd
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 57 deletions.
34 changes: 27 additions & 7 deletions internal/e2e/cases_test.go
Expand Up @@ -18,21 +18,21 @@ import (
"github.com/ory/keto/internal/relationtuple"
)

func runCases(c client, addNamespace func(*testing.T, ...*namespace.Namespace)) func(*testing.T) {
func runCases(c client, m *namespaceTestManager) func(*testing.T) {
return func(t *testing.T) {
c.waitUntilLive(t)

t.Run("case=gets empty namespace", func(t *testing.T) {
n := &namespace.Namespace{Name: t.Name()}
addNamespace(t, n)
m.add(t, n)

resp := c.queryTuple(t, &relationtuple.RelationQuery{Namespace: n.Name})
assert.Len(t, resp.RelationTuples, 0)
})

t.Run("case=creates tuple and uses it then", func(t *testing.T) {
n := &namespace.Namespace{Name: t.Name()}
addNamespace(t, n)
m.add(t, n)

tuple := &relationtuple.InternalRelationTuple{
Namespace: n.Name,
Expand All @@ -53,7 +53,7 @@ func runCases(c client, addNamespace func(*testing.T, ...*namespace.Namespace))

t.Run("case=expand API", func(t *testing.T) {
n := &namespace.Namespace{Name: t.Name()}
addNamespace(t, n)
m.add(t, n)

obj := fmt.Sprintf("tree for client %T", c)
rel := "expand"
Expand Down Expand Up @@ -96,7 +96,7 @@ func runCases(c client, addNamespace func(*testing.T, ...*namespace.Namespace))
t.Run("case=gets result paginated", func(t *testing.T) {
const nTuples = 10
n := &namespace.Namespace{Name: t.Name()}
addNamespace(t, n)
m.add(t, n)

rel := fmt.Sprintf("some unique relation %T", c)
for i := 0; i < nTuples; i++ {
Expand Down Expand Up @@ -131,7 +131,7 @@ func runCases(c client, addNamespace func(*testing.T, ...*namespace.Namespace))

t.Run("case=deletes tuple", func(t *testing.T) {
n := &namespace.Namespace{Name: t.Name()}
addNamespace(t, n)
m.add(t, n)

for _, s := range []relationtuple.Subject{
&relationtuple.SubjectID{ID: "s"},
Expand Down Expand Up @@ -163,7 +163,7 @@ func runCases(c client, addNamespace func(*testing.T, ...*namespace.Namespace))

t.Run("case=deletes tuples based on relation query", func(t *testing.T) {
n := &namespace.Namespace{Name: t.Name()}
addNamespace(t, n)
m.add(t, n)

rts := []*relationtuple.InternalRelationTuple{
{
Expand Down Expand Up @@ -198,5 +198,25 @@ func runCases(c client, addNamespace func(*testing.T, ...*namespace.Namespace))
t.Run("case=returns error with status code on unknown namespace", func(t *testing.T) {
c.queryTupleErr(t, herodot.ErrNotFound, &relationtuple.RelationQuery{Namespace: "unknown namespace"})
})

t.Run("case=hides tuples from deleted namespace", func(t *testing.T) {
n := &namespace.Namespace{Name: t.Name()}
m.add(t, n)

c.createTuple(t, &relationtuple.InternalRelationTuple{
Namespace: n.Name,
Object: "o",
Relation: "rel",
Subject: &relationtuple.SubjectID{ID: "s"},
})

m.remove(t, n.ID)

resp := c.queryTuple(t, &relationtuple.RelationQuery{})
assert.Equal(t, len(resp.RelationTuples), 0)

// Add the namespace again here, so that we can clean up properly.
m.add(t, n)
})
}
}
6 changes: 3 additions & 3 deletions internal/e2e/full_suit_test.go
Expand Up @@ -50,7 +50,7 @@ func Test(t *testing.T) {
t.Run(fmt.Sprintf("dsn=%s", dsn.Name), func(t *testing.T) {
t.Parallel()

ctx, reg, addNamespace := newInitializedReg(t, dsn, nil)
ctx, reg, namespaceTestMgr := newInitializedReg(t, dsn, nil)

closeServer := startServer(ctx, t, reg)
t.Cleanup(closeServer)
Expand Down Expand Up @@ -79,10 +79,10 @@ func Test(t *testing.T) {
writeRemote: reg.Config(ctx).WriteAPIListenOn(),
},
} {
t.Run(fmt.Sprintf("client=%T", cl), runCases(cl, addNamespace))
t.Run(fmt.Sprintf("client=%T", cl), runCases(cl, namespaceTestMgr))

if tc, ok := cl.(transactClient); ok {
t.Run(fmt.Sprintf("transactClient=%T", cl), runTransactionCases(tc, addNamespace))
t.Run(fmt.Sprintf("transactClient=%T", cl), runTransactionCases(tc, namespaceTestMgr))
}
}

Expand Down
69 changes: 41 additions & 28 deletions internal/e2e/helpers.go
Expand Up @@ -2,7 +2,6 @@ package e2e

import (
"context"
"errors"
"testing"

"github.com/ory/keto/internal/x/dbx"
Expand All @@ -11,8 +10,6 @@ import (

"github.com/stretchr/testify/assert"

"github.com/ory/keto/internal/x"

"github.com/ory/x/configx"
"github.com/phayes/freeport"
"github.com/spf13/pflag"
Expand All @@ -26,7 +23,43 @@ import (
"github.com/ory/keto/internal/driver"
)

func newInitializedReg(t testing.TB, dsn *dbx.DsnT, cfgOverwrites map[string]interface{}) (context.Context, driver.Registry, func(*testing.T, ...*namespace.Namespace)) {
type namespaceTestManager struct {
reg driver.Registry
ctx context.Context
nspaces []*namespace.Namespace
nextID int32
}

func (m *namespaceTestManager) add(t *testing.T, nn ...*namespace.Namespace) {
for _, n := range nn {
n.ID = m.nextID
m.nextID++
m.nspaces = append(m.nspaces, n)
}

require.NoError(t, m.reg.Config(m.ctx).Set(config.KeyNamespaces, m.nspaces))

t.Cleanup(func() {
for _, n := range nn {
require.NoError(t, m.reg.RelationTupleManager().DeleteAllRelationTuples(m.ctx, &relationtuple.RelationQuery{
Namespace: n.Name,
}))
}
})
}

func (m *namespaceTestManager) remove(t *testing.T, id int32) {
newNamespaces := make([]*namespace.Namespace, 0, len(m.nspaces))
for _, n := range m.nspaces {
if n.ID != id {
newNamespaces = append(newNamespaces, n)
}
}
m.nspaces = newNamespaces
require.NoError(t, m.reg.Config(m.ctx).Set(config.KeyNamespaces, m.nspaces))
}

func newInitializedReg(t testing.TB, dsn *dbx.DsnT, cfgOverwrites map[string]interface{}) (context.Context, driver.Registry, *namespaceTestManager) {
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(func() {
cancel()
Expand Down Expand Up @@ -62,32 +95,12 @@ func newInitializedReg(t testing.TB, dsn *dbx.DsnT, cfgOverwrites map[string]int
require.NoError(t, reg.MigrateUp(ctx))
assertMigrated(ctx, t, reg)

nspaces := make([]*namespace.Namespace, 0)
addNamespaces := func(t *testing.T, nn ...*namespace.Namespace) {
for _, n := range nn {
n.ID = int32(len(nspaces))
nspaces = append(nspaces, n)
}

require.NoError(t, reg.Config(ctx).Set(config.KeyNamespaces, nspaces))

t.Cleanup(func() {
for _, n := range nn {
err := errors.New("not nil")
var pt string
var ts []*relationtuple.InternalRelationTuple
for pt != "" || err != nil {
ts, pt, err = reg.RelationTupleManager().GetRelationTuples(ctx, &relationtuple.RelationQuery{
Namespace: n.Name,
}, x.WithToken(pt))
require.NoError(t, err)
require.NoError(t, reg.RelationTupleManager().DeleteRelationTuples(ctx, ts...))
}
}
})
return ctx, reg, &namespaceTestManager{
reg: reg,
ctx: ctx,
nspaces: []*namespace.Namespace{},
}

return ctx, reg, addNamespaces
}

func assertMigrated(ctx context.Context, t testing.TB, r driver.Registry) {
Expand Down
6 changes: 3 additions & 3 deletions internal/e2e/transaction_cases_test.go
Expand Up @@ -9,12 +9,12 @@ import (
"github.com/ory/keto/internal/relationtuple"
)

func runTransactionCases(c transactClient, addNamespace func(*testing.T, ...*namespace.Namespace)) func(*testing.T) {
func runTransactionCases(c transactClient, m *namespaceTestManager) func(*testing.T) {
return func(t *testing.T) {
t.Parallel()
t.Run("case=create and delete", func(t *testing.T) {
n := &namespace.Namespace{Name: t.Name()}
addNamespace(t, n)
m.add(t, n)

tuples := []*relationtuple.InternalRelationTuple{
{
Expand Down Expand Up @@ -55,7 +55,7 @@ func runTransactionCases(c transactClient, addNamespace func(*testing.T, ...*nam
t.Run("case=expand-api-display-access docs code sample", func(t *testing.T) {
files := &namespace.Namespace{Name: t.Name() + "files"}
directories := &namespace.Namespace{Name: t.Name() + "directories"}
addNamespace(t, files, directories)
m.add(t, files, directories)

tuples := []*relationtuple.InternalRelationTuple{
{
Expand Down
7 changes: 6 additions & 1 deletion internal/persistence/sql/full_test.go
Expand Up @@ -46,6 +46,11 @@ func TestPersister(t *testing.T) {
require.NoError(t, r.Config(ctx).Set(config.KeyNamespaces, nspaces))
}
}
removeAllNamespaces := func(r driver.Registry, nspaces []*namespace.Namespace) func(context.Context, *testing.T) {
return func(ctx context.Context, t *testing.T) {
require.NoError(t, r.Config(ctx).Set(config.KeyNamespaces, []*namespace.Namespace{}))
}
}

for _, dsn := range dbx.GetDSNs(t, false) {
dsn := dsn
Expand All @@ -56,7 +61,7 @@ func TestPersister(t *testing.T) {
var nspaces []*namespace.Namespace
p, r, _ := setup(t, dsn)

relationtuple.ManagerTest(t, p, addNamespace(r, nspaces))
relationtuple.ManagerTest(t, p, addNamespace(r, nspaces), removeAllNamespaces(r, nspaces))
})

t.Run("relationtuple.IsolationTest", func(t *testing.T) {
Expand Down
21 changes: 7 additions & 14 deletions internal/persistence/sql/relationtuples.go
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/ory/x/sqlcon"
"github.com/pkg/errors"

"github.com/ory/keto/internal/namespace"
"github.com/ory/keto/internal/relationtuple"
"github.com/ory/keto/internal/x"
)
Expand Down Expand Up @@ -40,7 +39,7 @@ func (RelationTuple) TableName(_ context.Context) string {
return "keto_relation_tuples"
}

func (r *RelationTuple) toInternal(ctx context.Context, nm namespace.Manager, p *Persister) (*relationtuple.InternalRelationTuple, error) {
func (r *RelationTuple) toInternal(ctx context.Context, p *Persister) (*relationtuple.InternalRelationTuple, error) {
if r == nil {
return nil, nil
}
Expand All @@ -65,7 +64,7 @@ func (r *RelationTuple) toInternal(ctx context.Context, nm namespace.Manager, p
if err != nil {
return nil, err
}
sn, err := nm.GetNamespaceByConfigID(ctx, n.ID)
sn, err := p.GetNamespaceByID(ctx, n.ID)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -254,11 +253,6 @@ func (p *Persister) GetRelationTuples(ctx context.Context, query *relationtuple.
return nil, "", err
}

nm, err := p.d.Config(ctx).NamespaceManager()
if err != nil {
return nil, "", err
}

sqlQuery := p.QueryWithNetwork(ctx).
Order("nid, namespace_id, object, relation, subject_id, subject_set_namespace_id, subject_set_object, subject_set_relation, commit_time").
Paginate(pagination.Page, pagination.PerPage)
Expand All @@ -277,12 +271,11 @@ func (p *Persister) GetRelationTuples(ctx context.Context, query *relationtuple.
nextPageToken = ""
}

internalRes := make([]*relationtuple.InternalRelationTuple, len(res))
for i, r := range res {
var err error
internalRes[i], err = r.toInternal(ctx, nm, p)
if err != nil {
return nil, "", err
internalRes := make([]*relationtuple.InternalRelationTuple, 0, len(res))
for _, r := range res {
if rt, err := r.toInternal(ctx, p); err == nil {
// Ignore error here, which stems from a deleted namespace.
internalRes = append(internalRes, rt)
}
}

Expand Down
22 changes: 21 additions & 1 deletion internal/relationtuple/manager_requirements.go
Expand Up @@ -16,7 +16,9 @@ import (
"github.com/ory/keto/internal/x"
)

func ManagerTest(t *testing.T, m Manager, addNamespace func(context.Context, *testing.T, string)) {
func ManagerTest(t *testing.T, m Manager,
addNamespace func(context.Context, *testing.T, string),
removeAllNamespaces func(context.Context, *testing.T)) {
t.Run("method=Write", func(t *testing.T) {
t.Run("case=success", func(t *testing.T) {
nspace := t.Name()
Expand Down Expand Up @@ -258,6 +260,24 @@ func ManagerTest(t *testing.T, m Manager, addNamespace func(context.Context, *te
assert.Equal(t, []*InternalRelationTuple{}, res)
assert.Equal(t, "", nextPage)
})

t.Run("case=deleted namespace", func(t *testing.T) {
nspace := t.Name()
addNamespace(context.Background(), t, nspace)
require.NoError(t, m.WriteRelationTuples(context.Background(), &InternalRelationTuple{
Namespace: nspace,
Object: "o",
Relation: "r",
Subject: &SubjectID{ID: "s"},
}))

removeAllNamespaces(context.Background(), t)
res, nextPage, err := m.GetRelationTuples(context.Background(), &RelationQuery{})

assert.NoError(t, err)
assert.Empty(t, res)
assert.Equal(t, "", nextPage)
})
})

t.Run("method=Delete", func(t *testing.T) {
Expand Down

0 comments on commit cb1a2dd

Please sign in to comment.