From 49489209511ae7c1ea917ec17e912c3f60f56228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Tue, 2 Apr 2024 14:05:07 +0100 Subject: [PATCH 1/2] updates bulk-delete to use DeleteRelationships new optional-limit former implementation of bulk-delete was designed to support deleting large amounts of relationships in a batched way. This commit replaces the implementation that relied on ReadRelationships with the new native DeleteRelationships optional-limit. The contract is kept mostly intact, except the interactive prompt is removed as it was considered bad UX. Instead, the same limits of 1000 before the user is prompted is retained, and instead zed will fail, indicating the user to bump the limit with the new --optional-limit flag, or to use --force. The --optional-limit flag sets the corresponding `DeleteRelationship` request field, and `--force` now sets the `OptionalAllowPartialDeletions` field. If set to true, all relationships will be deleted in batches defined by `--optional-limit`, which continues to default to 1000 elements. It also adds a progress bar to provide visual feedback to the user while deletion takes place. --- go.mod | 6 +- go.sum | 18 --- internal/cmd/backup.go | 36 +----- internal/cmd/backup_test.go | 51 ++++---- internal/cmd/helpers_test.go | 99 ---------------- internal/cmd/restorer.go | 3 +- internal/commands/relationship.go | 86 +++++++------- internal/commands/relationship_nowasm.go | 31 ----- internal/commands/relationship_test.go | 143 +++++++++++++++++++++++ internal/commands/relationship_wasm.go | 5 - internal/console/console.go | 29 +++++ internal/testing/test_helpers.go | 112 ++++++++++++++++++ 12 files changed, 363 insertions(+), 256 deletions(-) create mode 100644 internal/testing/test_helpers.go diff --git a/go.mod b/go.mod index 5521bf0e..d01b771d 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,6 @@ toolchain go1.21.3 require ( github.com/99designs/keyring v1.2.2 - github.com/AlecAivazis/survey/v2 v2.3.7 github.com/TylerBrock/colorjson v0.0.0-20200706003622-8a50f05110d2 github.com/authzed/authzed-go v0.11.2-0.20240320204618-9622b72a72c6 github.com/authzed/grpcutil v0.0.0-20240123194739-2ea1e3d2d98b @@ -34,6 +33,7 @@ require ( golang.org/x/mod v0.15.0 golang.org/x/sync v0.6.0 golang.org/x/term v0.17.0 + google.golang.org/genproto/googleapis/rpc v0.0.0-20240205150955-31a09d347014 google.golang.org/grpc v1.62.1 google.golang.org/protobuf v1.33.0 gopkg.in/yaml.v3 v3.0.1 @@ -66,7 +66,6 @@ require ( github.com/cloudspannerecosystem/spanner-change-streams-tail v0.3.1 // indirect github.com/cncf/udpa/go v0.0.0-20220112060539-c52dc94e7fbe // indirect github.com/cncf/xds/go v0.0.0-20231128003011-0fa0005c9caa // indirect - github.com/creack/pty v1.1.18 // indirect github.com/creasty/defaults v1.7.0 // indirect github.com/dalzilio/rudd v1.1.1-0.20230806153452-9e08a6ea8170 // indirect github.com/danieljoos/wincred v1.2.1 // indirect @@ -129,7 +128,6 @@ require ( github.com/joho/godotenv v1.5.1 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect - github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect github.com/klauspost/compress v1.17.6 // indirect github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect @@ -139,7 +137,6 @@ require ( github.com/mailru/easyjson v0.7.7 // indirect github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-runewidth v0.0.15 // indirect - github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d // indirect github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db // indirect github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect @@ -205,7 +202,6 @@ require ( google.golang.org/appengine v1.6.8 // indirect google.golang.org/genproto v0.0.0-20240205150955-31a09d347014 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240205150955-31a09d347014 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20240205150955-31a09d347014 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect diff --git a/go.sum b/go.sum index 5bddc3e9..f689549a 100644 --- a/go.sum +++ b/go.sum @@ -53,8 +53,6 @@ github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 h1:/vQbFIOMb github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4/go.mod h1:hN7oaIRCjzsZ2dE+yG5k+rsdt3qcwykqK6HVGcKwsw4= github.com/99designs/keyring v1.2.2 h1:pZd3neh/EmUzWONb35LxQfvuY7kiSXAq3HQd97+XBn0= github.com/99designs/keyring v1.2.2/go.mod h1:wes/FrByc8j7lFOAGLGSNEg8f/PaI3cgTBqhFkHUrPk= -github.com/AlecAivazis/survey/v2 v2.3.7 h1:6I/u8FvytdGsgonrYsVn2t8t4QiRnh6QSTqkkhIiSjQ= -github.com/AlecAivazis/survey/v2 v2.3.7/go.mod h1:xUTIdE4KCOIjsBAE1JYsUPoCqYdZ1reCfTwbto0Fduo= github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78 h1:w+iIsaOQNcT7OZ575w+acHgRric5iCyQh+xv+KJ4HB8= github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= @@ -65,8 +63,6 @@ github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8 github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10= github.com/Microsoft/go-winio v0.6.0 h1:slsWYD/zyx7lCXoZVlvQrj0hPTM1HI4+v1sIda2yDvg= github.com/Microsoft/go-winio v0.6.0/go.mod h1:cTAf44im0RAYeL23bpB+fzCyDH2MJiz2BO69KH/soAE= -github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2 h1:+vx7roKuyA63nhn5WAunQHLTznkw5W8b1Xc0dNjp83s= -github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2/go.mod h1:HBCaDeC1lPdgDeDbhX8XFpy1jqjK0IBG8W5K+xYqA0w= github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 h1:TngWCqHvy9oXAN6lEVMRuU21PR1EtLVZJmdB18Gu3Rw= github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5/go.mod h1:lmUJ/7eu/Q8D7ML55dXQrVaamCz2vxCfdQBasLZfHKk= github.com/TylerBrock/colorjson v0.0.0-20200706003622-8a50f05110d2 h1:ZBbLwSJqkHBuFDA6DUhhse0IGJ7T5bemHyNILUjvOq4= @@ -135,9 +131,6 @@ github.com/containerd/continuity v0.3.0/go.mod h1:wJEAIwKOm/pBZuBd0JmeTvnLquTB1A github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= -github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= -github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= -github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/creasty/defaults v1.7.0 h1:eNdqZvc5B509z18lD8yc212CAqJNvfT1Jq6L8WowdBA= github.com/creasty/defaults v1.7.0/go.mod h1:iGzKe6pbEHnpMPtfDXZEr0NVxWnPTjb1bbDy08fPzYM= github.com/dalzilio/rudd v1.1.1-0.20230806153452-9e08a6ea8170 h1:bHEN1z3EOO/IXHTQ8ZcmGoW4gTJt+mSrH2Sd458uo0E= @@ -354,8 +347,6 @@ github.com/hashicorp/golang-lru v1.0.2 h1:dV3g9Z/unq5DpblPpw+Oqcv4dU/1omnb4Ok8iP github.com/hashicorp/golang-lru v1.0.2/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4= github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= -github.com/hinshun/vt10x v0.0.0-20220119200601-820417d04eec h1:qv2VnGeEQHchGaZ/u7lxST/RaJw+cv273q79D81Xbog= -github.com/hinshun/vt10x v0.0.0-20220119200601-820417d04eec/go.mod h1:Q48J4R4DvxnHolD5P8pOtXigYlRuPLGl6moFx3ulM68= github.com/hokaccha/go-prettyjson v0.0.0-20210113012101-fb4e108d2519 h1:nqAlWFEdqI0ClbTDrhDvE/8LeQ4pftrqKUX9w5k0j3s= github.com/hokaccha/go-prettyjson v0.0.0-20210113012101-fb4e108d2519/go.mod h1:pFlLw2CfqZiIBOx6BuCeRLCrfxBJipTY0nIOF/VbGcI= github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= @@ -397,8 +388,6 @@ github.com/jzelinskie/cobrautil/v2 v2.0.0-20231016191810-9f8a4f6d038a/go.mod h1: github.com/jzelinskie/stringz v0.0.3 h1:0GhG3lVMYrYtIvRbxvQI6zqRTT1P1xyQlpa0FhfUXas= github.com/jzelinskie/stringz v0.0.3/go.mod h1:hHYbgxJuNLRw91CmpuFsYEOyQqpDVFg8pvEh23vy4P0= github.com/k0kubun/go-ansi v0.0.0-20180517002512-3bf9e2903213/go.mod h1:vNUNkEQ1e29fT/6vq2aBdFsgNPmy8qMdSay1npru+Sw= -github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= -github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.17.6 h1:60eq2E/jlfwQXtvZEeBUYADs+BwKBWURIY+Gj2eRGjI= @@ -426,10 +415,8 @@ github.com/magiconair/properties v1.8.7 h1:IeQXZAiQcpL9mgcAe1Nu6cX9LLw6ExEHKjN0V github.com/magiconair/properties v1.8.7/go.mod h1:Dhd985XPs7jluiymwWYZ0G4Z61jb3vdS329zhj2hYo0= github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= -github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= -github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= github.com/mattn/go-isatty v0.0.19/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= @@ -441,9 +428,6 @@ github.com/mattn/go-runewidth v0.0.15/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh github.com/mattn/go-sqlite3 v1.14.6 h1:dNPt6NO46WmLVt2DLNpwczCmdV5boIZ6g/tlDrlRUbg= github.com/mattn/go-sqlite3 v1.14.6/go.mod h1:NyWgC/yNuGj7Q9rpYnZvas74GogHl5/Z4A/KQRfk6bU= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= -github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE= -github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d h1:5PJl274Y63IEHC+7izoQE9x6ikvDFZS2mDVS3drnohI= -github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE= github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db h1:62I3jR2EmQ4l5rM/4FEfDWcRD+abF5XlKShorW5LRoQ= github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db/go.mod h1:l0dey0ia/Uv7NcFFVbCLtqEBQbrT4OCwCSKTEv6enCw= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= @@ -780,7 +764,6 @@ golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190422165155-953cdadca894/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -839,7 +822,6 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ= -golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= diff --git a/internal/cmd/backup.go b/internal/cmd/backup.go index 7956763c..eaf53d5e 100644 --- a/internal/cmd/backup.go +++ b/internal/cmd/backup.go @@ -19,13 +19,13 @@ import ( "github.com/mattn/go-isatty" "github.com/rodaine/table" "github.com/rs/zerolog/log" - "github.com/schollz/progressbar/v3" "github.com/spf13/cobra" "golang.org/x/exp/constraints" "golang.org/x/exp/maps" "github.com/authzed/zed/internal/client" "github.com/authzed/zed/internal/commands" + "github.com/authzed/zed/internal/console" "github.com/authzed/zed/pkg/backupformat" ) @@ -231,30 +231,6 @@ func hasRelPrefix(rel *v1.Relationship, prefix string) bool { strings.HasPrefix(rel.Subject.Object.ObjectType, prefix) } -func relProgressBar(description string) *progressbar.ProgressBar { - bar := progressbar.NewOptions(-1, - progressbar.OptionSetWidth(10), - progressbar.OptionSetRenderBlankState(true), - progressbar.OptionSetVisibility(false), - ) - if isatty.IsTerminal(os.Stderr.Fd()) { - bar = progressbar.NewOptions64(-1, - progressbar.OptionSetDescription(description), - progressbar.OptionSetWriter(os.Stderr), - progressbar.OptionSetWidth(10), - progressbar.OptionThrottle(65*time.Millisecond), - progressbar.OptionShowCount(), - progressbar.OptionShowIts(), - progressbar.OptionSetItsString("relationship"), - progressbar.OptionOnCompletion(func() { _, _ = fmt.Fprint(os.Stderr, "\n") }), - progressbar.OptionSpinnerType(14), - progressbar.OptionFullWidth(), - progressbar.OptionSetRenderBlankState(true), - ) - } - return bar -} - func backupCreateCmdFunc(cmd *cobra.Command, args []string) (err error) { f, err := createBackupFile(args[0]) if err != nil { @@ -264,13 +240,13 @@ func backupCreateCmdFunc(cmd *cobra.Command, args []string) (err error) { defer func(e *error) { *e = errors.Join(*e, f.Close()) }(&err) defer func(e *error) { *e = errors.Join(*e, f.Sync()) }(&err) - client, err := client.NewClient(cmd) + c, err := client.NewClient(cmd) if err != nil { return fmt.Errorf("unable to initialize client: %w", err) } ctx := cmd.Context() - schemaResp, err := client.ReadSchema(ctx, &v1.ReadSchemaRequest{}) + schemaResp, err := c.ReadSchema(ctx, &v1.ReadSchemaRequest{}) if err != nil { return fmt.Errorf("error reading schema: %w", err) } else if schemaResp.ReadAt == nil { @@ -299,7 +275,7 @@ func backupCreateCmdFunc(cmd *cobra.Command, args []string) (err error) { } defer func(e *error) { *e = errors.Join(*e, encoder.Close()) }(&err) - relationshipStream, err := client.BulkExportRelationships(ctx, &v1.BulkExportRelationshipsRequest{ + relationshipStream, err := c.BulkExportRelationships(ctx, &v1.BulkExportRelationshipsRequest{ Consistency: &v1.Consistency{ Requirement: &v1.Consistency_AtExactSnapshot{ AtExactSnapshot: schemaResp.ReadAt, @@ -312,7 +288,7 @@ func backupCreateCmdFunc(cmd *cobra.Command, args []string) (err error) { relationshipReadStart := time.Now() - bar := relProgressBar("processing backup") + bar := console.CreateProgressBar("processing backup") var relsEncoded, relsProcessed uint for { if err := ctx.Err(); err != nil { @@ -520,7 +496,7 @@ func backupRedactCmdFunc(cmd *cobra.Command, args []string) error { } defer func(e *error) { *e = errors.Join(*e, redactor.Close()) }(&err) - bar := relProgressBar("redacting backup") + bar := console.CreateProgressBar("redacting backup") var written int64 for { if err := cmd.Context().Err(); err != nil { diff --git a/internal/cmd/backup_test.go b/internal/cmd/backup_test.go index 5168aa55..ee4b2ca8 100644 --- a/internal/cmd/backup_test.go +++ b/internal/cmd/backup_test.go @@ -6,9 +6,9 @@ import ( "path/filepath" "strings" "testing" - "time" "github.com/authzed/zed/internal/client" + zedtesting "github.com/authzed/zed/internal/testing" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" "github.com/authzed/spicedb/pkg/tuple" @@ -162,7 +162,7 @@ func TestBackupParseRelsCmdFunc(t *testing.T) { tt := tt t.Parallel() - cmd := createTestCobraCommandWithFlagValue(t, stringFlag{"prefix-filter", tt.filter}) + cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, zedtesting.StringFlag{FlagName: "prefix-filter", FlagValue: tt.filter}) backupName := createTestBackup(t, tt.schema, tt.relationships) f, err := os.CreateTemp("", "parse-output") require.NoError(t, err) @@ -183,7 +183,7 @@ func TestBackupParseRelsCmdFunc(t *testing.T) { } func TestBackupParseRevisionCmdFunc(t *testing.T) { - cmd := createTestCobraCommandWithFlagValue(t, stringFlag{"prefix-filter", "test"}) + cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, zedtesting.StringFlag{FlagName: "prefix-filter", FlagValue: "test"}) backupName := createTestBackup(t, testSchema, testRelationships) f, err := os.CreateTemp("", "parse-output") require.NoError(t, err) @@ -241,9 +241,9 @@ func TestBackupParseSchemaCmdFunc(t *testing.T) { tt := tt t.Parallel() - cmd := createTestCobraCommandWithFlagValue(t, - stringFlag{"prefix-filter", tt.filter}, - boolFlag{"rewrite-legacy", tt.rewriteLegacy}) + cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, + zedtesting.StringFlag{FlagName: "prefix-filter", FlagValue: tt.filter}, + zedtesting.BoolFlag{FlagName: "rewrite-legacy", FlagValue: tt.rewriteLegacy}) backupName := createTestBackup(t, tt.schema, nil) f, err := os.CreateTemp("", "parse-output") require.NoError(t, err) @@ -264,9 +264,9 @@ func TestBackupParseSchemaCmdFunc(t *testing.T) { } func TestBackupCreateCmdFunc(t *testing.T) { - cmd := createTestCobraCommandWithFlagValue(t, - stringFlag{"prefix-filter", ""}, - boolFlag{"rewrite-legacy", false}) + cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, + zedtesting.StringFlag{FlagName: "prefix-filter"}, + zedtesting.BoolFlag{FlagName: "rewrite-legacy"}) f := filepath.Join(os.TempDir(), uuid.NewString()) _, err := os.Stat(f) require.Error(t, err) @@ -276,7 +276,7 @@ func TestBackupCreateCmdFunc(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - srv := newServer(ctx, t) + srv := zedtesting.NewTestServer(ctx, t) go func() { require.NoError(t, srv.Run(ctx)) }() @@ -288,9 +288,9 @@ func TestBackupCreateCmdFunc(t *testing.T) { client.NewClient = originalClient }() - client.NewClient = clientFromConn(conn) + client.NewClient = zedtesting.ClientFromConn(conn) - c, err := clientFromConn(conn)(cmd) + c, err := zedtesting.ClientFromConn(conn)(cmd) require.NoError(t, err) _, err = c.WriteSchema(ctx, &v1.WriteSchemaRequest{Schema: testSchema}) @@ -324,26 +324,21 @@ func TestBackupCreateCmdFunc(t *testing.T) { require.Equal(t, resp.WrittenAt.Token, d.ZedToken().Token) } -type durationFlag struct { - flagName string - flagValue time.Duration -} - func TestBackupRestoreCmdFunc(t *testing.T) { - cmd := createTestCobraCommandWithFlagValue(t, - stringFlag{"prefix-filter", "test"}, - boolFlag{"rewrite-legacy", false}, - stringFlag{"conflict-strategy", "fail"}, - boolFlag{"disable-retries", false}, - intFlag{"batch-size", 100}, - uintFlag{"batches-per-transaction", 10}, - durationFlag{"request-timeout", 0}, + cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, + zedtesting.StringFlag{FlagName: "prefix-filter", FlagValue: "test"}, + zedtesting.BoolFlag{FlagName: "rewrite-legacy"}, + zedtesting.StringFlag{FlagName: "conflict-strategy", FlagValue: "fail"}, + zedtesting.BoolFlag{FlagName: "disable-retries"}, + zedtesting.IntFlag{FlagName: "batch-size", FlagValue: 100}, + zedtesting.UintFlag{FlagName: "batches-per-transaction", FlagValue: 10}, + zedtesting.DurationFlag{FlagName: "request-timeout"}, ) backupName := createTestBackup(t, testSchema, testRelationships) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - srv := newServer(ctx, t) + srv := zedtesting.NewTestServer(ctx, t) go func() { require.NoError(t, srv.Run(ctx)) }() @@ -355,9 +350,9 @@ func TestBackupRestoreCmdFunc(t *testing.T) { client.NewClient = originalClient }() - client.NewClient = clientFromConn(conn) + client.NewClient = zedtesting.ClientFromConn(conn) - c, err := clientFromConn(conn)(cmd) + c, err := zedtesting.ClientFromConn(conn)(cmd) require.NoError(t, err) err = backupRestoreCmdFunc(cmd, []string{backupName}) require.NoError(t, err) diff --git a/internal/cmd/helpers_test.go b/internal/cmd/helpers_test.go index bb37c647..feaccdb1 100644 --- a/internal/cmd/helpers_test.go +++ b/internal/cmd/helpers_test.go @@ -2,22 +2,12 @@ package cmd import ( "bufio" - "context" "os" "testing" - "github.com/authzed/authzed-go/v1" - "github.com/authzed/spicedb/pkg/cmd/datastore" - "github.com/authzed/spicedb/pkg/cmd/server" - "github.com/authzed/spicedb/pkg/cmd/util" - "google.golang.org/grpc" - - "github.com/authzed/zed/internal/client" - v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" "github.com/authzed/spicedb/pkg/tuple" "github.com/samber/lo" - "github.com/spf13/cobra" "github.com/stretchr/testify/require" "github.com/authzed/zed/pkg/backupformat" @@ -49,51 +39,6 @@ func readLines(t *testing.T, fileName string) []string { return lines } -type stringFlag struct { - flagName string - flagValue string -} - -type boolFlag struct { - flagName string - flagValue bool -} - -type intFlag struct { - flagName string - flagValue int -} - -type uintFlag struct { - flagName string - flagValue uint -} - -func createTestCobraCommandWithFlagValue(t *testing.T, flagAndValues ...any) *cobra.Command { - t.Helper() - - c := cobra.Command{} - for _, flagAndValue := range flagAndValues { - switch f := flagAndValue.(type) { - case stringFlag: - c.Flags().String(f.flagName, f.flagValue, "") - case boolFlag: - c.Flags().Bool(f.flagName, f.flagValue, "") - case intFlag: - c.Flags().Int(f.flagName, f.flagValue, "") - case uintFlag: - c.Flags().Uint(f.flagName, f.flagValue, "") - case durationFlag: - c.Flags().Duration(f.flagName, f.flagValue, "") - default: - t.Fatalf("unknown flag type: %T", f) - } - } - - c.SetContext(context.Background()) - return &c -} - func createTestBackup(t *testing.T, schema string, relationships []string) string { t.Helper() @@ -118,47 +63,3 @@ func createTestBackup(t *testing.T, schema string, relationships []string) strin return f.Name() } - -func clientFromConn(conn *grpc.ClientConn) func(cmd *cobra.Command) (client.Client, error) { - return func(_ *cobra.Command) (client.Client, error) { - return &authzed.ClientWithExperimental{ - Client: authzed.Client{ - SchemaServiceClient: v1.NewSchemaServiceClient(conn), - PermissionsServiceClient: v1.NewPermissionsServiceClient(conn), - WatchServiceClient: v1.NewWatchServiceClient(conn), - }, - ExperimentalServiceClient: v1.NewExperimentalServiceClient(conn), - }, nil - } -} - -func newServer(ctx context.Context, t *testing.T) server.RunnableServer { - t.Helper() - - ds, err := datastore.NewDatastore(ctx, - datastore.DefaultDatastoreConfig().ToOption(), - datastore.WithRequestHedgingEnabled(false), - ) - require.NoError(t, err, "unable to start memdb datastore") - - configOpts := []server.ConfigOption{ - server.WithGRPCServer(util.GRPCServerConfig{ - Network: util.BufferedNetwork, - Enabled: true, - }), - server.WithGRPCAuthFunc(func(ctx context.Context) (context.Context, error) { - return ctx, nil - }), - server.WithHTTPGateway(util.HTTPServerConfig{HTTPEnabled: false}), - server.WithMetricsAPI(util.HTTPServerConfig{HTTPEnabled: false}), - server.WithDispatchCacheConfig(server.CacheConfig{Enabled: false, Metrics: false}), - server.WithNamespaceCacheConfig(server.CacheConfig{Enabled: false, Metrics: false}), - server.WithClusterDispatchCacheConfig(server.CacheConfig{Enabled: false, Metrics: false}), - server.WithDatastore(ds), - } - - srv, err := server.NewConfigWithOptionsAndDefaults(configOpts...).Complete(ctx) - require.NoError(t, err) - - return srv -} diff --git a/internal/cmd/restorer.go b/internal/cmd/restorer.go index c1bd444f..8b852086 100644 --- a/internal/cmd/restorer.go +++ b/internal/cmd/restorer.go @@ -18,6 +18,7 @@ import ( "google.golang.org/grpc/status" "github.com/authzed/zed/internal/client" + "github.com/authzed/zed/internal/console" "github.com/authzed/zed/pkg/backupformat" ) @@ -88,7 +89,7 @@ func newRestorer(schema string, decoder *backupformat.Decoder, client client.Cli batchesPerTransaction: batchesPerTransaction, conflictStrategy: conflictStrategy, disableRetryErrors: disableRetryErrors, - bar: relProgressBar("restoring from backup"), + bar: console.CreateProgressBar("restoring from backup"), } } diff --git a/internal/commands/relationship.go b/internal/commands/relationship.go index 4f2b59f6..cbc5d149 100644 --- a/internal/commands/relationship.go +++ b/internal/commands/relationship.go @@ -19,6 +19,8 @@ import ( "github.com/jzelinskie/stringz" "github.com/rs/zerolog/log" "github.com/spf13/cobra" + "google.golang.org/genproto/googleapis/rpc/errdetails" + "google.golang.org/grpc/status" ) func RegisterRelationshipCmd(rootCmd *cobra.Command) *cobra.Command { @@ -47,10 +49,11 @@ func RegisterRelationshipCmd(rootCmd *cobra.Command) *cobra.Command { registerConsistencyFlags(readCmd.Flags()) relationshipCmd.AddCommand(bulkDeleteCmd) - bulkDeleteCmd.Flags().Bool("force", false, "force deletion immediately without confirmation") + bulkDeleteCmd.Flags().Bool("force", false, "force deletion of all elements in batches defined by ") bulkDeleteCmd.Flags().String("subject-filter", "", "optional subject filter") + bulkDeleteCmd.Flags().Uint("optional-limit", 1000, "the max amount of elements to delete. If you want to delete all in batches of size , set --force to true") bulkDeleteCmd.Flags().Bool("estimate-count", true, "estimate the count of relationships to be deleted") - + _ = bulkDeleteCmd.Flags().MarkDeprecated("estimate-count", "no longer used, make use of --optional-limit instead") return relationshipCmd } @@ -124,57 +127,62 @@ func bulkDeleteRelationships(cmd *cobra.Command, args []string) error { return err } - request := &v1.ReadRelationshipsRequest{RelationshipFilter: filter} - - counter := -1 - if cobrautil.MustGetBool(cmd, "estimate-count") { - request.Consistency = &v1.Consistency{Requirement: &v1.Consistency_FullyConsistent{FullyConsistent: true}} - - ctx, cancel := context.WithCancel(cmd.Context()) - defer cancel() + bar := console.CreateProgressBar("deleting relationships") + defer func() { + _ = bar.Finish() + }() - log.Trace().Interface("request", request).Send() - resp, err := spicedbClient.ReadRelationships(ctx, request) + allowPartialDeletions := cobrautil.MustGetBool(cmd, "force") + optionalLimit := cobrautil.MustGetUint(cmd, "optional-limit") + var resp *v1.DeleteRelationshipsResponse + for { + delRequest := &v1.DeleteRelationshipsRequest{ + RelationshipFilter: filter, + OptionalLimit: uint32(optionalLimit), + OptionalAllowPartialDeletions: allowPartialDeletions, + } + log.Trace().Interface("request", delRequest).Msg("deleting relationships") + + resp, err = spicedbClient.DeleteRelationships(cmd.Context(), delRequest) + if errorInfo, ok := grpcErrorInfoFrom(err); ok { + if errorInfo.GetReason() == v1.ErrorReason_ERROR_REASON_TOO_MANY_RELATIONSHIPS_FOR_TRANSACTIONAL_DELETE.String() { + return fmt.Errorf("could not delete %s, as more than %s relationships were found. Consider increasing --optional-limit or deleting all relationships using --force", + errorInfo.GetMetadata()["filter_resource_type"], + errorInfo.GetMetadata()["limit"]) + } + } if err != nil { return err } - counter = 0 - for { - _, err := resp.Recv() - if errors.Is(err, io.EOF) { - break - } - - if err != nil { - return err - } - - counter++ - if counter > 1000 { - cancel() - break - } + if resp.DeletionProgress == v1.DeleteRelationshipsResponse_DELETION_PROGRESS_COMPLETE { + break } - } - if !cobrautil.MustGetBool(cmd, "force") { - err := performBulkDeletionConfirmation(counter) - if err != nil { + if err := bar.Add(int(optionalLimit)); err != nil { return err } } - delRequest := &v1.DeleteRelationshipsRequest{RelationshipFilter: request.RelationshipFilter} - log.Trace().Interface("request", delRequest).Msg("deleting relationships") + _ = bar.Finish() + console.Println(resp.DeletedAt.GetToken()) + return nil +} + +func grpcErrorInfoFrom(err error) (*errdetails.ErrorInfo, bool) { + if err == nil { + return nil, false + } - resp, err := spicedbClient.DeleteRelationships(cmd.Context(), delRequest) - if err != nil { - return err + if s, ok := status.FromError(err); ok { + for _, d := range s.Details() { + if errInfo, ok := d.(*errdetails.ErrorInfo); ok { + return errInfo, true + } + } } - console.Println(resp.DeletedAt.GetToken()) - return nil + return nil, false } func buildRelationshipsFilter(cmd *cobra.Command, args []string) (*v1.RelationshipFilter, error) { diff --git a/internal/commands/relationship_nowasm.go b/internal/commands/relationship_nowasm.go index f415fa41..ea94114e 100644 --- a/internal/commands/relationship_nowasm.go +++ b/internal/commands/relationship_nowasm.go @@ -4,40 +4,9 @@ package commands import ( - "errors" - "fmt" "os" - "github.com/AlecAivazis/survey/v2" - "github.com/AlecAivazis/survey/v2/terminal" "golang.org/x/term" ) var isFileTerminal = func(f *os.File) bool { return term.IsTerminal(int(f.Fd())) } - -func performBulkDeletionConfirmation(counter int) error { - message := fmt.Sprintf("Will delete %d relationships. Continue?", counter) - if counter > 1000 { - message = "Will delete 1000+ relationships. Continue?" - } - if counter < 0 { - message = "Will delete all matching relationships. Continue?" - } - - response := false - err := survey.AskOne(&survey.Confirm{ - Message: message, - }, &response) - if err != nil { - if errors.Is(err, terminal.InterruptErr) { - os.Exit(0) - } - - return err - } - - if !response { - os.Exit(1) - } - return nil -} diff --git a/internal/commands/relationship_test.go b/internal/commands/relationship_test.go index b5305379..4b28f491 100644 --- a/internal/commands/relationship_test.go +++ b/internal/commands/relationship_test.go @@ -2,11 +2,14 @@ package commands import ( "context" + "errors" + "io" "os" "strings" "testing" "github.com/authzed/zed/internal/client" + zedtesting "github.com/authzed/zed/internal/testing" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" "github.com/authzed/spicedb/pkg/tuple" @@ -18,6 +21,13 @@ import ( "google.golang.org/protobuf/types/known/structpb" ) +const testSchema = `definition test/resource { + relation reader: test/user + relation writer: test/user +} + +definition test/user {}` + func init() { zerolog.SetGlobalLevel(zerolog.Disabled) } @@ -549,3 +559,136 @@ func (m *mockClient) WriteRelationships(_ context.Context, in *v1.WriteRelations require.True(m.t, proto.Equal(expectedWrite, in)) return &v1.WriteRelationshipsResponse{WrittenAt: &v1.ZedToken{Token: "test"}}, nil } + +func TestBulkDeleteForcing(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + srv := zedtesting.NewTestServer(ctx, t) + go func() { + require.NoError(t, srv.Run(ctx)) + }() + conn, err := srv.GRPCDialContext(ctx) + require.NoError(t, err) + + originalClient := client.NewClient + defer func() { + client.NewClient = originalClient + }() + + client.NewClient = zedtesting.ClientFromConn(conn) + testCmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, + zedtesting.StringFlag{FlagName: "subject-filter"}, + zedtesting.UintFlag{FlagName: "optional-limit", FlagValue: 1}, + zedtesting.BoolFlag{FlagName: "force", FlagValue: true}) + c, err := client.NewClient(testCmd) + require.NoError(t, err) + + _, err = c.WriteSchema(ctx, &v1.WriteSchemaRequest{Schema: testSchema}) + require.NoError(t, err) + + _, err = c.WriteRelationships(ctx, &v1.WriteRelationshipsRequest{ + Updates: []*v1.RelationshipUpdate{ + { + Operation: v1.RelationshipUpdate_OPERATION_TOUCH, + Relationship: tuple.ParseRel("test/resource:1#reader@test/user:1"), + }, + { + Operation: v1.RelationshipUpdate_OPERATION_TOUCH, + Relationship: tuple.ParseRel("test/resource:1#writer@test/user:2"), + }, + { + Operation: v1.RelationshipUpdate_OPERATION_TOUCH, + Relationship: tuple.ParseRel("test/resource:1#writer@test/user:3"), + }, + }, + }) + require.NoError(t, err) + + err = bulkDeleteRelationships(testCmd, []string{"test/resource:1"}) + require.NoError(t, err) + + assertRelationshipsEmpty(ctx, t, c, &v1.RelationshipFilter{ResourceType: "test/resource"}) +} + +func TestBulkDeleteNotForcing(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + srv := zedtesting.NewTestServer(ctx, t) + go func() { + require.NoError(t, srv.Run(ctx)) + }() + conn, err := srv.GRPCDialContext(ctx) + require.NoError(t, err) + + originalClient := client.NewClient + defer func() { + client.NewClient = originalClient + }() + + client.NewClient = zedtesting.ClientFromConn(conn) + testCmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, + zedtesting.StringFlag{FlagName: "subject-filter"}, + zedtesting.UintFlag{FlagName: "optional-limit", FlagValue: 1}, + zedtesting.BoolFlag{FlagName: "force", FlagValue: false}) + c, err := client.NewClient(testCmd) + require.NoError(t, err) + + _, err = c.WriteSchema(ctx, &v1.WriteSchemaRequest{Schema: testSchema}) + require.NoError(t, err) + + _, err = c.WriteRelationships(ctx, &v1.WriteRelationshipsRequest{ + Updates: []*v1.RelationshipUpdate{ + { + Operation: v1.RelationshipUpdate_OPERATION_TOUCH, + Relationship: tuple.ParseRel("test/resource:1#reader@test/user:1"), + }, + { + Operation: v1.RelationshipUpdate_OPERATION_TOUCH, + Relationship: tuple.ParseRel("test/resource:1#writer@test/user:2"), + }, + { + Operation: v1.RelationshipUpdate_OPERATION_TOUCH, + Relationship: tuple.ParseRel("test/resource:1#writer@test/user:3"), + }, + }, + }) + require.NoError(t, err) + + err = bulkDeleteRelationships(testCmd, []string{"test/resource:1"}) + require.ErrorContains(t, err, "could not delete test/resource") + assertRelationshipCount(ctx, t, c, &v1.RelationshipFilter{ResourceType: "test/resource"}, 3) +} + +func assertRelationshipsEmpty(ctx context.Context, t *testing.T, c client.Client, filter *v1.RelationshipFilter) { + t.Helper() + + assertRelationshipCount(ctx, t, c, filter, 0) +} + +func assertRelationshipCount(ctx context.Context, t *testing.T, c client.Client, filter *v1.RelationshipFilter, count int) { + t.Helper() + + rrCli, err := c.ReadRelationships(ctx, &v1.ReadRelationshipsRequest{ + Consistency: &v1.Consistency{ + Requirement: &v1.Consistency_FullyConsistent{ + FullyConsistent: true, + }, + }, + RelationshipFilter: filter, + }) + require.NoError(t, err) + + relCount := 0 + for { + _, err = rrCli.Recv() + if errors.Is(err, io.EOF) { + break + } + + require.NoError(t, err) + relCount++ + } + + require.NoError(t, rrCli.CloseSend()) + require.Equal(t, count, relCount) +} diff --git a/internal/commands/relationship_wasm.go b/internal/commands/relationship_wasm.go index 5ce80e8e..4388932d 100644 --- a/internal/commands/relationship_wasm.go +++ b/internal/commands/relationship_wasm.go @@ -3,8 +3,3 @@ package commands import "os" var isFileTerminal = func(f *os.File) bool { return true } - -func performBulkDeletionConfirmation(counter int) error { - // Nothing to do. - return nil -} diff --git a/internal/console/console.go b/internal/console/console.go index 57ff0591..84eef870 100644 --- a/internal/console/console.go +++ b/internal/console/console.go @@ -3,6 +3,10 @@ package console import ( "fmt" "os" + "time" + + "github.com/mattn/go-isatty" + "github.com/schollz/progressbar/v3" ) // Printf defines an (overridable) function for printing to the console via stdout. @@ -24,3 +28,28 @@ func Println(values ...any) { Printf("%v\n", value) } } + +func CreateProgressBar(description string) *progressbar.ProgressBar { + bar := progressbar.NewOptions(-1, + progressbar.OptionSetWidth(10), + progressbar.OptionSetRenderBlankState(true), + progressbar.OptionSetVisibility(false), + ) + if isatty.IsTerminal(os.Stderr.Fd()) { + bar = progressbar.NewOptions64(-1, + progressbar.OptionSetDescription(description), + progressbar.OptionSetWriter(os.Stderr), + progressbar.OptionSetWidth(10), + progressbar.OptionThrottle(65*time.Millisecond), + progressbar.OptionShowCount(), + progressbar.OptionShowIts(), + progressbar.OptionSetItsString("relationship"), + progressbar.OptionOnCompletion(func() { _, _ = fmt.Fprint(os.Stderr, "\n") }), + progressbar.OptionSpinnerType(14), + progressbar.OptionFullWidth(), + progressbar.OptionSetRenderBlankState(true), + ) + } + + return bar +} diff --git a/internal/testing/test_helpers.go b/internal/testing/test_helpers.go new file mode 100644 index 00000000..aa180c97 --- /dev/null +++ b/internal/testing/test_helpers.go @@ -0,0 +1,112 @@ +package testing + +import ( + "context" + "testing" + "time" + + v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + "github.com/authzed/authzed-go/v1" + "github.com/authzed/spicedb/pkg/cmd/datastore" + "github.com/authzed/spicedb/pkg/cmd/server" + "github.com/authzed/spicedb/pkg/cmd/util" + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + + "github.com/authzed/zed/internal/client" +) + +func ClientFromConn(conn *grpc.ClientConn) func(cmd *cobra.Command) (client.Client, error) { + return func(_ *cobra.Command) (client.Client, error) { + return &authzed.ClientWithExperimental{ + Client: authzed.Client{ + SchemaServiceClient: v1.NewSchemaServiceClient(conn), + PermissionsServiceClient: v1.NewPermissionsServiceClient(conn), + WatchServiceClient: v1.NewWatchServiceClient(conn), + }, + ExperimentalServiceClient: v1.NewExperimentalServiceClient(conn), + }, nil + } +} + +func NewTestServer(ctx context.Context, t *testing.T) server.RunnableServer { + t.Helper() + + ds, err := datastore.NewDatastore(ctx, + datastore.DefaultDatastoreConfig().ToOption(), + datastore.WithRequestHedgingEnabled(false), + ) + require.NoError(t, err, "unable to start memdb datastore") + + configOpts := []server.ConfigOption{ + server.WithGRPCServer(util.GRPCServerConfig{ + Network: util.BufferedNetwork, + Enabled: true, + }), + server.WithGRPCAuthFunc(func(ctx context.Context) (context.Context, error) { + return ctx, nil + }), + server.WithHTTPGateway(util.HTTPServerConfig{HTTPEnabled: false}), + server.WithMetricsAPI(util.HTTPServerConfig{HTTPEnabled: false}), + server.WithDispatchCacheConfig(server.CacheConfig{Enabled: false, Metrics: false}), + server.WithNamespaceCacheConfig(server.CacheConfig{Enabled: false, Metrics: false}), + server.WithClusterDispatchCacheConfig(server.CacheConfig{Enabled: false, Metrics: false}), + server.WithDatastore(ds), + } + + srv, err := server.NewConfigWithOptionsAndDefaults(configOpts...).Complete(ctx) + require.NoError(t, err) + + return srv +} + +type StringFlag struct { + FlagName string + FlagValue string +} + +type BoolFlag struct { + FlagName string + FlagValue bool +} + +type IntFlag struct { + FlagName string + FlagValue int +} + +type UintFlag struct { + FlagName string + FlagValue uint +} + +type DurationFlag struct { + FlagName string + FlagValue time.Duration +} + +func CreateTestCobraCommandWithFlagValue(t *testing.T, flagAndValues ...any) *cobra.Command { + t.Helper() + + c := cobra.Command{} + for _, flagAndValue := range flagAndValues { + switch f := flagAndValue.(type) { + case StringFlag: + c.Flags().String(f.FlagName, f.FlagValue, "") + case BoolFlag: + c.Flags().Bool(f.FlagName, f.FlagValue, "") + case IntFlag: + c.Flags().Int(f.FlagName, f.FlagValue, "") + case UintFlag: + c.Flags().Uint(f.FlagName, f.FlagValue, "") + case DurationFlag: + c.Flags().Duration(f.FlagName, f.FlagValue, "") + default: + t.Fatalf("unknown flag type: %T", f) + } + } + + c.SetContext(context.Background()) + return &c +} From 0adf9e54ca5427f08bed47904b978ae0804577a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Thu, 4 Apr 2024 16:44:41 +0100 Subject: [PATCH 2/2] introduces support for optional resource type in ResourceFilter This introduces support for optional resource type, recently introduced in SpiceDB 1.30.0. since the CLI contract has ambiguity, as a tradeoff, the contract will break by requiring clients to specify colon after the resource type, in order to disambiguate with _only specifying the relation_. The subject-type continues to not require colon as suffix. Tests were also added for missing permutations and the optional resource id prefix. --- internal/commands/relationship.go | 48 +++++--- internal/commands/relationship_test.go | 161 ++++++++++++++++++++++--- 2 files changed, 171 insertions(+), 38 deletions(-) diff --git a/internal/commands/relationship.go b/internal/commands/relationship.go index cbc5d149..20e86004 100644 --- a/internal/commands/relationship.go +++ b/internal/commands/relationship.go @@ -95,7 +95,7 @@ var readCmd = &cobra.Command{ } var bulkDeleteCmd = &cobra.Command{ - Use: "bulk-delete ", + Use: "bulk-delete ", Short: "Deletes relationships matching the provided pattern en masse", Args: cobra.RangeArgs(1, 3), ValidArgsFunction: GetArgs(ResourceID, Permission, SubjectTypeWithOptionalRelation), @@ -146,8 +146,13 @@ func bulkDeleteRelationships(cmd *cobra.Command, args []string) error { resp, err = spicedbClient.DeleteRelationships(cmd.Context(), delRequest) if errorInfo, ok := grpcErrorInfoFrom(err); ok { if errorInfo.GetReason() == v1.ErrorReason_ERROR_REASON_TOO_MANY_RELATIONSHIPS_FOR_TRANSACTIONAL_DELETE.String() { + resourceType := "relationships" + if returnedResourceType, ok := errorInfo.GetMetadata()["filter_resource_type"]; ok { + resourceType = returnedResourceType + } + return fmt.Errorf("could not delete %s, as more than %s relationships were found. Consider increasing --optional-limit or deleting all relationships using --force", - errorInfo.GetMetadata()["filter_resource_type"], + resourceType, errorInfo.GetMetadata()["limit"]) } } @@ -186,32 +191,39 @@ func grpcErrorInfoFrom(err error) (*errdetails.ErrorInfo, bool) { } func buildRelationshipsFilter(cmd *cobra.Command, args []string) (*v1.RelationshipFilter, error) { - filter := &v1.RelationshipFilter{ResourceType: args[0]} + filter := &v1.RelationshipFilter{} - if strings.Contains(args[0], ":") { - var resourceID string - err := stringz.SplitExact(args[0], ":", &filter.ResourceType, &resourceID) - if err != nil { - return nil, err - } + expectedSubjectIndex := 3 + if len(args) > 0 { + if strings.Contains(args[0], ":") { + var resourceID string + err := stringz.SplitExact(args[0], ":", &filter.ResourceType, &resourceID) + if err != nil { + return nil, err + } + + if strings.HasSuffix(resourceID, "%") { + filter.OptionalResourceIdPrefix = strings.TrimSuffix(resourceID, "%") + } else { + filter.OptionalResourceId = resourceID + } - if strings.HasSuffix(resourceID, "%") { - filter.OptionalResourceIdPrefix = strings.TrimSuffix(resourceID, "%") + if len(args) > 1 { + filter.OptionalRelation = args[1] + } } else { - filter.OptionalResourceId = resourceID + // if the first argument does not contain :, we assume the resource_type/resource_id has been omitted + filter.OptionalRelation = args[0] + expectedSubjectIndex = 2 } } - if len(args) > 1 { - filter.OptionalRelation = args[1] - } - subjectFilter := cobrautil.MustGetString(cmd, "subject-filter") - if len(args) == 3 { + if len(args) == expectedSubjectIndex { if subjectFilter != "" { return nil, errors.New("cannot specify subject filter both positionally and via --subject-filter") } - subjectFilter = args[2] + subjectFilter = args[expectedSubjectIndex-1] } if subjectFilter != "" { diff --git a/internal/commands/relationship_test.go b/internal/commands/relationship_test.go index 4b28f491..c03856a2 100644 --- a/internal/commands/relationship_test.go +++ b/internal/commands/relationship_test.go @@ -21,12 +21,16 @@ import ( "google.golang.org/protobuf/types/known/structpb" ) -const testSchema = `definition test/resource { - relation reader: test/user - relation writer: test/user +const testSchema = `definition resource { + relation reader: user + relation writer: user } -definition test/user {}` +definition folder { + relation parent: folder +} + +definition user {}` func init() { zerolog.SetGlobalLevel(zerolog.Disabled) @@ -478,13 +482,14 @@ func fileFromStrings(t *testing.T, strings []string) *os.File { func TestBuildRelationshipsFilter(t *testing.T) { tests := []struct { - name string - args []string - expected *v1.RelationshipFilter + name string + args []string + subjectFlag string + expected *v1.RelationshipFilter }{ { name: "resource type", - args: []string{"res"}, + args: []string{"res:"}, expected: &v1.RelationshipFilter{ResourceType: "res"}, }, { @@ -517,6 +522,121 @@ func TestBuildRelationshipsFilter(t *testing.T) { OptionalSubjectFilter: &v1.SubjectFilter{SubjectType: "sub", OptionalSubjectId: "321"}, }, }, + { + name: "resource ID", + args: []string{":123"}, + expected: &v1.RelationshipFilter{ + OptionalResourceId: "123", + }, + }, + { + name: "resource ID prefix", + args: []string{":123%"}, + expected: &v1.RelationshipFilter{ + OptionalResourceIdPrefix: "123", + }, + }, + { + name: "resource ID, relation", + args: []string{":123", "view"}, + expected: &v1.RelationshipFilter{ + OptionalResourceId: "123", + OptionalRelation: "view", + }, + }, + { + name: "resource ID, relation, subject type", + args: []string{":123", "view", "sub"}, + expected: &v1.RelationshipFilter{ + OptionalResourceId: "123", + OptionalRelation: "view", + OptionalSubjectFilter: &v1.SubjectFilter{SubjectType: "sub"}, + }, + }, + { + name: "resource ID, relation, subject type, subject ID", + args: []string{":123", "view", "sub:321"}, + expected: &v1.RelationshipFilter{ + OptionalResourceId: "123", + OptionalRelation: "view", + OptionalSubjectFilter: &v1.SubjectFilter{SubjectType: "sub", OptionalSubjectId: "321"}, + }, + }, + { + name: "relation", + args: []string{"view"}, + expected: &v1.RelationshipFilter{ + OptionalRelation: "view", + }, + }, + { + name: "relation, subject type", + args: []string{"view", "sub"}, + expected: &v1.RelationshipFilter{ + OptionalRelation: "view", + OptionalSubjectFilter: &v1.SubjectFilter{SubjectType: "sub"}, + }, + }, + { + name: "relation, subject type, subject id", + args: []string{"view", "sub:321"}, + expected: &v1.RelationshipFilter{ + OptionalRelation: "view", + OptionalSubjectFilter: &v1.SubjectFilter{SubjectType: "sub", OptionalSubjectId: "321"}, + }, + }, + { + name: "resource type, subject type", + args: []string{"res:"}, + subjectFlag: "sub", + expected: &v1.RelationshipFilter{ + ResourceType: "res", + OptionalSubjectFilter: &v1.SubjectFilter{SubjectType: "sub"}, + }, + }, + { + name: "resource type, subject type, subject id", + args: []string{"res:"}, + subjectFlag: "sub:321", + expected: &v1.RelationshipFilter{ + ResourceType: "res", + OptionalSubjectFilter: &v1.SubjectFilter{SubjectType: "sub", OptionalSubjectId: "321"}, + }, + }, + { + name: "resource type, resource id, subject type", + args: []string{"res:123"}, + subjectFlag: "sub", + expected: &v1.RelationshipFilter{ + ResourceType: "res", + OptionalResourceId: "123", + OptionalSubjectFilter: &v1.SubjectFilter{SubjectType: "sub"}, + }, + }, + { + name: "resource type, resource id, subject type, subject id", + args: []string{"res:123"}, + subjectFlag: "sub:321", + expected: &v1.RelationshipFilter{ + ResourceType: "res", + OptionalResourceId: "123", + OptionalSubjectFilter: &v1.SubjectFilter{SubjectType: "sub", OptionalSubjectId: "321"}, + }, + }, + { + name: "subject type", + subjectFlag: "sub", + expected: &v1.RelationshipFilter{ + OptionalSubjectFilter: &v1.SubjectFilter{SubjectType: "sub"}, + }, + }, + { + name: "subject type, subject id", + subjectFlag: "sub:321", + expected: &v1.RelationshipFilter{ + OptionalSubjectFilter: &v1.SubjectFilter{SubjectType: "sub", OptionalSubjectId: "321"}, + }, + }, } for _, tt := range tests { tt := tt @@ -524,7 +644,7 @@ func TestBuildRelationshipsFilter(t *testing.T) { t.Parallel() cmd := &cobra.Command{} - cmd.Flags().String("subject-filter", "", "") + cmd.Flags().String("subject-filter", tt.subjectFlag, "") filter, err := buildRelationshipsFilter(cmd, tt.args) require.NoError(t, err) @@ -533,6 +653,7 @@ func TestBuildRelationshipsFilter(t *testing.T) { require.Equal(t, tt.expected.OptionalRelation, filter.OptionalRelation, "relations do not match") if tt.expected.OptionalSubjectFilter != nil { + require.NotNil(t, filter.OptionalSubjectFilter, "subject filter is nil") require.Equal(t, tt.expected.OptionalSubjectFilter.SubjectType, filter.OptionalSubjectFilter.SubjectType, "subject types do not match") require.Equal(t, tt.expected.OptionalSubjectFilter.OptionalSubjectId, filter.OptionalSubjectFilter.OptionalSubjectId, "subject IDs do not match") } @@ -590,24 +711,24 @@ func TestBulkDeleteForcing(t *testing.T) { Updates: []*v1.RelationshipUpdate{ { Operation: v1.RelationshipUpdate_OPERATION_TOUCH, - Relationship: tuple.ParseRel("test/resource:1#reader@test/user:1"), + Relationship: tuple.ParseRel("resource:1#reader@user:1"), }, { Operation: v1.RelationshipUpdate_OPERATION_TOUCH, - Relationship: tuple.ParseRel("test/resource:1#writer@test/user:2"), + Relationship: tuple.ParseRel("resource:1#writer@user:2"), }, { Operation: v1.RelationshipUpdate_OPERATION_TOUCH, - Relationship: tuple.ParseRel("test/resource:1#writer@test/user:3"), + Relationship: tuple.ParseRel("folder:1#parent@folder:2"), }, }, }) require.NoError(t, err) - err = bulkDeleteRelationships(testCmd, []string{"test/resource:1"}) + err = bulkDeleteRelationships(testCmd, []string{":1"}) require.NoError(t, err) - assertRelationshipsEmpty(ctx, t, c, &v1.RelationshipFilter{ResourceType: "test/resource"}) + assertRelationshipsEmpty(ctx, t, c, &v1.RelationshipFilter{OptionalResourceId: "1"}) } func TestBulkDeleteNotForcing(t *testing.T) { @@ -640,23 +761,23 @@ func TestBulkDeleteNotForcing(t *testing.T) { Updates: []*v1.RelationshipUpdate{ { Operation: v1.RelationshipUpdate_OPERATION_TOUCH, - Relationship: tuple.ParseRel("test/resource:1#reader@test/user:1"), + Relationship: tuple.ParseRel("resource:1#reader@user:1"), }, { Operation: v1.RelationshipUpdate_OPERATION_TOUCH, - Relationship: tuple.ParseRel("test/resource:1#writer@test/user:2"), + Relationship: tuple.ParseRel("resource:1#writer@user:2"), }, { Operation: v1.RelationshipUpdate_OPERATION_TOUCH, - Relationship: tuple.ParseRel("test/resource:1#writer@test/user:3"), + Relationship: tuple.ParseRel("resource:1#writer@user:3"), }, }, }) require.NoError(t, err) - err = bulkDeleteRelationships(testCmd, []string{"test/resource:1"}) - require.ErrorContains(t, err, "could not delete test/resource") - assertRelationshipCount(ctx, t, c, &v1.RelationshipFilter{ResourceType: "test/resource"}, 3) + err = bulkDeleteRelationships(testCmd, []string{"resource:1"}) + require.ErrorContains(t, err, "could not delete resource") + assertRelationshipCount(ctx, t, c, &v1.RelationshipFilter{ResourceType: "resource"}, 3) } func assertRelationshipsEmpty(ctx context.Context, t *testing.T, c client.Client, filter *v1.RelationshipFilter) {