From 61c28e48a5c3f623e5cc133e69ba368c5103f414 Mon Sep 17 00:00:00 2001 From: Patrik Date: Mon, 15 Mar 2021 15:15:17 +0100 Subject: [PATCH] test: add command tests (#487) --- .circleci/config.yml | 4 +- cmd/check/root.go | 2 +- cmd/check/root_test.go | 19 +++++++++ cmd/client/grpc_client.go | 11 ++++- cmd/client/test_helpers.go | 75 +++++++++++++++++++++++++++++++++++ cmd/expand/root_test.go | 21 ++++++++++ cmd/migrate/down.go | 8 ++++ cmd/migrate/migrate_test.go | 8 ++-- cmd/migrate/up.go | 16 ++++++-- cmd/namespace/migrate_down.go | 16 +++++++- cmd/namespace/migrate_up.go | 2 +- cmd/status/root.go | 6 ++- cmd/status/root_test.go | 70 ++++++++++++++++++++++++++++++++ internal/expand/tree.go | 8 ++++ 14 files changed, 251 insertions(+), 15 deletions(-) create mode 100644 cmd/check/root_test.go create mode 100644 cmd/client/test_helpers.go create mode 100644 cmd/expand/root_test.go create mode 100644 cmd/status/root_test.go diff --git a/.circleci/config.yml b/.circleci/config.yml index 5ddb3fa57..6906aee06 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -43,7 +43,7 @@ jobs: - go/save-cache # Tests - - run: .bin/go-acc -o coverage.txt ./... -- -v -tags sqlite + - run: .bin/go-acc -o coverage.txt ./... -- -v -tags sqlite -p 1 # Submit coverage details - run: test -z "$CIRCLE_PR_NUMBER" && .bin/goveralls -service=circle-ci -coverprofile=coverage.txt -repotoken=$COVERALLS_REPO_TOKEN || echo "forks are not allowed to push to coveralls" @@ -57,7 +57,7 @@ jobs: - go/mod-download - run: go mod tidy - go/save-cache - - run: go test -tags sqlite -race -short -v ./... + - run: go test -tags sqlite -race -short -v -p 1 ./... validate: docker: diff --git a/cmd/check/root.go b/cmd/check/root.go index 20e0b8bb5..11420a316 100644 --- a/cmd/check/root.go +++ b/cmd/check/root.go @@ -47,7 +47,7 @@ func newCheckCmd() *cobra.Command { return err } - cmdx.PrintJSONAble(cmd, &checkOutput{resp.Allowed}) + cmdx.PrintJSONAble(cmd, &checkOutput{Allowed: resp.Allowed}) return nil }, } diff --git a/cmd/check/root_test.go b/cmd/check/root_test.go new file mode 100644 index 000000000..c367fa7ab --- /dev/null +++ b/cmd/check/root_test.go @@ -0,0 +1,19 @@ +package check + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/ory/keto/cmd/client" + "github.com/ory/keto/internal/namespace" +) + +func TestCheckCommand(t *testing.T) { + nspace := &namespace.Namespace{Name: t.Name()} + ts := client.NewTestServer(t, client.ReadServer, []*namespace.Namespace{nspace}, newCheckCmd) + defer ts.Shutdown(t) + + stdOut := ts.Cmd.ExecNoErr(t, "subject", "access", nspace.Name, "object") + assert.Equal(t, "Denied\n", stdOut) +} diff --git a/cmd/client/grpc_client.go b/cmd/client/grpc_client.go index 9a844a595..cbea9869e 100644 --- a/cmd/client/grpc_client.go +++ b/cmd/client/grpc_client.go @@ -13,12 +13,16 @@ import ( "google.golang.org/grpc" ) +type contextKeys string + const ( FlagReadRemote = "read-remote" FlagWriteRemote = "write-remote" EnvReadRemote = "KETO_READ_REMOTE" EnvWriteRemote = "KETO_WRITE_REMOTE" + + ContextKeyTimeout contextKeys = "timeout" ) func getRemote(cmd *cobra.Command, flagRemote, envRemote string) string { @@ -43,7 +47,12 @@ func GetWriteConn(cmd *cobra.Command) (*grpc.ClientConn, error) { } func Conn(ctx context.Context, remote string) (*grpc.ClientConn, error) { - ctx, cancel := context.WithTimeout(ctx, 3*time.Second) + timeout := 3 * time.Second + if d, ok := ctx.Value(ContextKeyTimeout).(time.Duration); ok { + timeout = d + } + + ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() return grpc.DialContext(ctx, remote, grpc.WithInsecure(), grpc.WithBlock(), grpc.WithDisableHealthCheck()) } diff --git a/cmd/client/test_helpers.go b/cmd/client/test_helpers.go new file mode 100644 index 000000000..689ae5c0f --- /dev/null +++ b/cmd/client/test_helpers.go @@ -0,0 +1,75 @@ +package client + +import ( + "net" + "testing" + + "github.com/ory/x/cmdx" + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" + "golang.org/x/sync/errgroup" + "google.golang.org/grpc" + + "github.com/ory/keto/internal/driver" + "github.com/ory/keto/internal/namespace" +) + +type ( + TestServer struct { + Reg driver.Registry + Namespace *namespace.Namespace + Addr, FlagRemote string + Cmd *cmdx.CommandExecuter + Server *grpc.Server + NewServer func() *grpc.Server + + errG *errgroup.Group + } + ServerType string +) + +const ( + WriteServer ServerType = "write" + ReadServer ServerType = "read" +) + +func NewTestServer(t *testing.T, rw ServerType, nspaces []*namespace.Namespace, newCmd func() *cobra.Command) *TestServer { + ts := &TestServer{ + Reg: driver.NewMemoryTestRegistry(t, nspaces), + } + + switch rw { + case ReadServer: + ts.NewServer = ts.Reg.ReadGRPCServer + ts.FlagRemote = FlagReadRemote + case WriteServer: + ts.NewServer = ts.Reg.WriteGRPCServer + ts.FlagRemote = FlagWriteRemote + default: + t.Logf("Got unknown server type %s", rw) + t.FailNow() + } + + ts.Server = ts.NewServer() + + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + ts.Addr = l.Addr().String() + + ts.errG = &errgroup.Group{} + ts.errG.Go(func() error { + return ts.Server.Serve(l) + }) + + ts.Cmd = &cmdx.CommandExecuter{ + New: newCmd, + PersistentArgs: []string{"--" + ts.FlagRemote, ts.Addr}, + } + + return ts +} + +func (ts *TestServer) Shutdown(t *testing.T) { + ts.Server.GracefulStop() + require.NoError(t, ts.errG.Wait()) +} diff --git a/cmd/expand/root_test.go b/cmd/expand/root_test.go new file mode 100644 index 000000000..c7c2aca8a --- /dev/null +++ b/cmd/expand/root_test.go @@ -0,0 +1,21 @@ +package expand + +import ( + "testing" + + "github.com/ory/x/cmdx" + "github.com/stretchr/testify/assert" + + "github.com/ory/keto/cmd/client" + "github.com/ory/keto/internal/namespace" +) + +func TestExpandCommand(t *testing.T) { + nspace := &namespace.Namespace{Name: t.Name()} + ts := client.NewTestServer(t, client.ReadServer, []*namespace.Namespace{nspace}, NewExpandCmd) + defer ts.Shutdown(t) + + ts.Cmd.PersistentArgs = append(ts.Cmd.PersistentArgs, "--"+cmdx.FlagFormat, string(cmdx.FormatJSON)) + stdOut := ts.Cmd.ExecNoErr(t, "access", nspace.Name, "object") + assert.Equal(t, "null\n", stdOut) +} diff --git a/cmd/migrate/down.go b/cmd/migrate/down.go index f8292a7d2..4f53a4651 100644 --- a/cmd/migrate/down.go +++ b/cmd/migrate/down.go @@ -4,6 +4,8 @@ import ( "fmt" "strconv" + "github.com/ory/x/flagx" + "github.com/ory/x/cmdx" "github.com/spf13/cobra" @@ -35,6 +37,11 @@ func newDownCmd() *cobra.Command { } cmdx.PrintTable(cmd, s) + if !flagx.MustGetBool(cmd, FlagYes) && !cmdx.AskForConfirmation("Do you really want to migrate down? This will delete data.", cmd.InOrStdin(), cmd.OutOrStdout()) { + _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Migration aborted.") + return nil + } + if err := reg.Migrator().MigrateDown(ctx, int(steps)); err != nil { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Could apply down migrations: %+v\n", err) return cmdx.FailSilently(cmd) @@ -51,6 +58,7 @@ func newDownCmd() *cobra.Command { }, } + registerYesFlag(cmd.Flags()) cmdx.RegisterFormatFlags(cmd.Flags()) return cmd diff --git a/cmd/migrate/migrate_test.go b/cmd/migrate/migrate_test.go index 656df0654..feefe5af3 100644 --- a/cmd/migrate/migrate_test.go +++ b/cmd/migrate/migrate_test.go @@ -100,10 +100,10 @@ func TestMigrate(t *testing.T) { t.Cleanup(func() { // migrate all down - cmd.ExecNoErr(t, "down", "0") + t.Log(cmd.ExecNoErr(t, "down", "0", "--"+FlagYes)) }) - parts := strings.Split(stdOut, "Do you want to apply above planned migrations?") + parts := strings.Split(stdOut, "Are you sure that you want to apply this migration?") require.Len(t, parts, 2) assertNoneApplied(t, parts[0]) @@ -115,7 +115,7 @@ func TestMigrate(t *testing.T) { t.Cleanup(func() { // migrate all down - cmd.ExecNoErr(t, "down", "0") + t.Log(cmd.ExecNoErr(t, "down", "0", "--"+FlagYes)) }) parts := strings.Split(out, "Applying migrations...") @@ -130,7 +130,7 @@ func TestMigrate(t *testing.T) { t.Cleanup(func() { // migrate all down - cmd.ExecNoErr(t, "down", "0") + t.Log(cmd.ExecNoErr(t, "down", "0", "--"+FlagYes)) }) parts := strings.Split(out, "Applying migrations...") diff --git a/cmd/migrate/up.go b/cmd/migrate/up.go index f53b859ad..3fbad2547 100644 --- a/cmd/migrate/up.go +++ b/cmd/migrate/up.go @@ -3,6 +3,10 @@ package migrate import ( "fmt" + "github.com/ory/x/flagx" + + "github.com/spf13/pflag" + "github.com/pkg/errors" "github.com/ory/x/cmdx" @@ -17,7 +21,7 @@ const ( ) func newUpCmd() *cobra.Command { - var yes, allNamespaces bool + var allNamespaces bool cmd := &cobra.Command{ Use: "up", @@ -43,7 +47,7 @@ func newUpCmd() *cobra.Command { return nil } - if !yes && !cmdx.AskForConfirmation("Do you want to apply above planned migrations?", cmd.InOrStdin(), cmd.OutOrStdout()) { + if !flagx.MustGetBool(cmd, FlagYes) && !cmdx.AskForConfirmation("Are you sure that you want to apply this migration? Make sure to check the CHANGELOG.md for breaking changes beforehand.", cmd.InOrStdin(), cmd.OutOrStdout()) { _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Aborting") return nil } @@ -95,7 +99,7 @@ func newUpCmd() *cobra.Command { continue } - if !yes && !cmdx.AskForConfirmation(fmt.Sprintf("Do you want to apply above planned migrations for namespace %s?", nspace.Name), cmd.InOrStdin(), cmd.OutOrStdout()) { + if !flagx.MustGetBool(cmd, FlagYes) && !cmdx.AskForConfirmation(fmt.Sprintf("Do you want to apply above planned migrations for namespace %s?", nspace.Name), cmd.InOrStdin(), cmd.OutOrStdout()) { _, _ = fmt.Fprintf(cmd.OutOrStdout(), "Skipping namespace %s\n", nspace.Name) continue } @@ -112,10 +116,14 @@ func newUpCmd() *cobra.Command { }, } - cmd.Flags().BoolVarP(&yes, FlagYes, "y", false, "yes to all questions, no user input required") + registerYesFlag(cmd.Flags()) cmd.Flags().BoolVar(&allNamespaces, FlagAllNamespace, false, "migrate all pending namespaces as well") cmdx.RegisterFormatFlags(cmd.Flags()) return cmd } + +func registerYesFlag(flags *pflag.FlagSet) { + flags.BoolP(FlagYes, "y", false, "yes to all questions, no user input required") +} diff --git a/cmd/namespace/migrate_down.go b/cmd/namespace/migrate_down.go index 4db138b58..b4017140f 100644 --- a/cmd/namespace/migrate_down.go +++ b/cmd/namespace/migrate_down.go @@ -2,6 +2,9 @@ package namespace import ( "fmt" + "strconv" + + "github.com/ory/x/flagx" "github.com/ory/x/cmdx" "github.com/spf13/cobra" @@ -17,6 +20,12 @@ func NewMigrateDownCmd() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() + steps, err := strconv.ParseInt(args[1], 0, 0) + if err != nil { + // return this error so it gets printed along the usage + return fmt.Errorf("malformed argument %s for : %+v", args[0], err) + } + reg, err := driver.NewDefaultRegistry(ctx, cmd.Flags()) if err != nil { return err @@ -34,7 +43,12 @@ func NewMigrateDownCmd() *cobra.Command { return cmdx.FailSilently(cmd) } - if err := reg.NamespaceMigrator().MigrateNamespaceDown(ctx, n, 0); err != nil { + if !flagx.MustGetBool(cmd, YesFlag) && !cmdx.AskForConfirmation(fmt.Sprintf("Do you really want to delete namespace %s? This will irrecoverably delete all relation tuples within the namespace.", n.Name), cmd.InOrStdin(), cmd.OutOrStdout()) { + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "Migration of namespace \"%s\" aborted.\n", n.Name) + return nil + } + + if err := reg.NamespaceMigrator().MigrateNamespaceDown(ctx, n, int(steps)); err != nil { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Could not apply namespace migration: %+v\n", err) return cmdx.FailSilently(cmd) } diff --git a/cmd/namespace/migrate_up.go b/cmd/namespace/migrate_up.go index 8e7199b3e..6a64dfbb6 100644 --- a/cmd/namespace/migrate_up.go +++ b/cmd/namespace/migrate_up.go @@ -53,7 +53,7 @@ func NewMigrateUpCmd() *cobra.Command { } if !flagx.MustGetBool(cmd, YesFlag) { - if !cmdx.AskForConfirmation("Are you sure that you want to apply this migration? Make sure to check the CHANGELOG.md and UPGRADE.md for breaking changes beforehand.", cmd.InOrStdin(), cmd.OutOrStdout()) { + if !cmdx.AskForConfirmation("Are you sure that you want to apply this migration? Make sure to check the CHANGELOG.md for breaking changes beforehand.", cmd.InOrStdin(), cmd.OutOrStdout()) { _, _ = fmt.Fprintf(cmd.OutOrStdout(), "Migration of namespace \"%s\" aborted.\n", n.Name) return nil } diff --git a/cmd/status/root.go b/cmd/status/root.go index 51384aa7f..e5fb3f749 100644 --- a/cmd/status/root.go +++ b/cmd/status/root.go @@ -67,7 +67,10 @@ func newStatusCmd() *cobra.Command { GetStatus() grpcHealthV1.HealthCheckResponse_ServingStatus } if block { - wc, err := c.Watch(cmd.Context(), &grpcHealthV1.HealthCheckRequest{}) + ctx, cancel := context.WithCancel(cmd.Context()) + defer cancel() + + wc, err := c.Watch(ctx, &grpcHealthV1.HealthCheckRequest{}) if err != nil { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Could not start watching the status: %+v\n", err) return cmdx.FailSilently(cmd) @@ -87,6 +90,7 @@ func newStatusCmd() *cobra.Command { } if status.GetStatus() == grpcHealthV1.HealthCheckResponse_SERVING { + cancel() break } diff --git a/cmd/status/root_test.go b/cmd/status/root_test.go new file mode 100644 index 000000000..8aa420162 --- /dev/null +++ b/cmd/status/root_test.go @@ -0,0 +1,70 @@ +package status + +import ( + "context" + "net" + "strings" + "testing" + "time" + + "github.com/ory/x/cmdx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/sync/errgroup" + grpcHealthV1 "google.golang.org/grpc/health/grpc_health_v1" + + "github.com/ory/keto/cmd/client" + "github.com/ory/keto/internal/namespace" +) + +func TestStatusCmd(t *testing.T) { + for _, serverType := range []client.ServerType{client.ReadServer, client.WriteServer} { + t.Run("server_type="+string(serverType), func(t *testing.T) { + ts := client.NewTestServer(t, serverType, []*namespace.Namespace{{Name: t.Name()}}, newStatusCmd) + defer ts.Shutdown(t) + ts.Cmd.PersistentArgs = append(ts.Cmd.PersistentArgs, "--"+cmdx.FlagQuiet, "--"+FlagEndpoint, string(serverType)) + + t.Run("case=timeout,noblock", func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond) + defer cancel() + + stdOut := cmdx.ExecNoErrCtx(ctx, t, newStatusCmd(), "--"+FlagEndpoint, string(serverType), "--"+ts.FlagRemote, ts.Addr+"0") + assert.Equal(t, grpcHealthV1.HealthCheckResponse_NOT_SERVING.String()+"\n", stdOut) + }) + + t.Run("case=noblock", func(t *testing.T) { + stdOut := ts.Cmd.ExecNoErr(t) + assert.Equal(t, grpcHealthV1.HealthCheckResponse_SERVING.String()+"\n", stdOut) + }) + + t.Run("case=block", func(t *testing.T) { + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + s := ts.NewServer() + + startServe := make(chan struct{}) + + serveErr := &errgroup.Group{} + serveErr.Go(func() error { + // wait until we get the signal to start + <-startServe + return s.Serve(l) + }) + + // dispatch start after one context timeout + go func() { + <-time.After(11 * time.Millisecond) + close(startServe) + }() + + ctx := context.WithValue(context.Background(), client.ContextKeyTimeout, 10*time.Millisecond) + stdOut := cmdx.ExecNoErrCtx(ctx, t, newStatusCmd(), "--"+FlagEndpoint, string(serverType), "--"+ts.FlagRemote, l.Addr().String(), "--"+FlagBlock) + assert.True(t, strings.HasPrefix(stdOut, "Context deadline exceeded, going to retry.")) + assert.True(t, strings.HasSuffix(stdOut, "\n"+grpcHealthV1.HealthCheckResponse_SERVING.String()+"\n")) + + s.GracefulStop() + require.NoError(t, serveErr.Wait()) + }) + }) + } +} diff --git a/internal/expand/tree.go b/internal/expand/tree.go index 829145dc2..5a93a747c 100644 --- a/internal/expand/tree.go +++ b/internal/expand/tree.go @@ -109,6 +109,10 @@ func (t *Tree) UnmarshalJSON(v []byte) error { // swagger:ignore func (t *Tree) ToProto() *acl.SubjectTree { + if t == nil { + return nil + } + if t.Type == Leaf { return &acl.SubjectTree{ NodeType: acl.NodeType_NODE_TYPE_LEAF, @@ -130,6 +134,10 @@ func (t *Tree) ToProto() *acl.SubjectTree { // swagger:ignore func TreeFromProto(t *acl.SubjectTree) (*Tree, error) { + if t == nil { + return nil, nil + } + sub, err := relationtuple.SubjectFromProto(t.Subject) if err != nil { return nil, err