From 759e22cc23d34e37253f4bd3c6d980a46963cf9e Mon Sep 17 00:00:00 2001 From: Abhinav Nekkanti <10552725+anekkanti@users.noreply.github.com> Date: Wed, 31 Aug 2022 23:05:04 +0000 Subject: [PATCH 01/11] add support for optional sync flag to block on async ops till completion --- app/account.go | 46 +++++++++++---- app/account_test.go | 22 ++++++-- app/app.go | 8 ++- app/namespace.go | 71 ++++++++++++++++++----- app/namespace_test.go | 22 ++++++-- app/request.go | 127 ++++++++++++++++++++++++++++++++++++++++-- app/request_test.go | 70 ++++++++++++++++++++++- go.mod | 4 +- go.sum | 6 ++ 9 files changed, 333 insertions(+), 43 deletions(-) diff --git a/app/account.go b/app/account.go index 89612a5..33c6cb8 100644 --- a/app/account.go +++ b/app/account.go @@ -7,6 +7,7 @@ import ( "github.com/temporalio/tcld/protogen/api/account/v1" "github.com/temporalio/tcld/protogen/api/accountservice/v1" + "github.com/temporalio/tcld/protogen/api/request/v1" "github.com/urfave/cli/v2" "google.golang.org/grpc" ) @@ -47,7 +48,7 @@ func (c *AccountClient) getAccount() (*account.Account, error) { return res.Account, nil } -func (c *AccountClient) updateAccount(ctx *cli.Context, a *account.Account) error { +func (c *AccountClient) updateAccount(ctx *cli.Context, a *account.Account) (*request.RequestStatus, error) { resourceVersion := a.ResourceVersion if v := ctx.String(ResourceVersionFlagName); v != "" { resourceVersion = v @@ -59,10 +60,9 @@ func (c *AccountClient) updateAccount(ctx *cli.Context, a *account.Account) erro Spec: a.Spec, }) if err != nil { - return err + return nil, err } - - return PrintProto(res) + return res.RequestStatus, nil } func (c *AccountClient) parseExistingMetricsCerts(ctx *cli.Context) (account *account.Account, existing caCerts, err error) { @@ -82,8 +82,9 @@ func (c *AccountClient) parseExistingMetricsCerts(ctx *cli.Context) (account *ac return a, existingCerts, nil } -func NewAccountCommand(getAccountClientFn GetAccountClientFn) (CommandOut, error) { +func NewAccountCommand(getAccountClientFn GetAccountClientFn, getRequestClientFn GetRequestClientFn) (CommandOut, error) { var c *AccountClient + var r *RequestClient return CommandOut{ Command: &cli.Command{ Name: "account", @@ -92,6 +93,10 @@ func NewAccountCommand(getAccountClientFn GetAccountClientFn) (CommandOut, error Before: func(ctx *cli.Context) error { var err error c, err = getAccountClientFn(ctx) + if err != nil { + return err + } + r, err = getRequestClientFn(ctx) return err }, Subcommands: []*cli.Command{ @@ -130,7 +135,11 @@ func NewAccountCommand(getAccountClientFn GetAccountClientFn) (CommandOut, error } a.Spec.Metrics.Enabled = true - return c.updateAccount(ctx, a) + status, err := c.updateAccount(ctx, a) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "enable metrics", status) }, }, { @@ -147,7 +156,12 @@ func NewAccountCommand(getAccountClientFn GetAccountClientFn) (CommandOut, error } a.Spec.Metrics.Enabled = false - return c.updateAccount(ctx, a) + status, err := c.updateAccount(ctx, a) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "disable metrics", status) + }, }, { @@ -195,7 +209,11 @@ func NewAccountCommand(getAccountClientFn GetAccountClientFn) (CommandOut, error } a.Spec.Metrics.AcceptedClientCa = bundle - return c.updateAccount(ctx, a) + status, err := c.updateAccount(ctx, a) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "add metrics ca certificate", status) }, }, { @@ -253,7 +271,11 @@ func NewAccountCommand(getAccountClientFn GetAccountClientFn) (CommandOut, error if err != nil || !y { return err } - return c.updateAccount(ctx, a) + status, err := c.updateAccount(ctx, a) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "remove metrics ca certificate", status) }, }, { @@ -288,7 +310,11 @@ func NewAccountCommand(getAccountClientFn GetAccountClientFn) (CommandOut, error a.Spec.Metrics = &account.MetricsSpec{} } a.Spec.Metrics.AcceptedClientCa = cert - return c.updateAccount(ctx, a) + status, err := c.updateAccount(ctx, a) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "set metrics ca certificates", status) }, }, { diff --git a/app/account_test.go b/app/account_test.go index 34583e1..faa62f9 100644 --- a/app/account_test.go +++ b/app/account_test.go @@ -16,6 +16,7 @@ import ( "github.com/temporalio/tcld/protogen/api/account/v1" "github.com/temporalio/tcld/protogen/api/accountservice/v1" "github.com/temporalio/tcld/protogen/api/request/v1" + requestservicemock "github.com/temporalio/tcld/protogen/apimock/requestservice/v1" "github.com/urfave/cli/v2" ) @@ -25,20 +26,31 @@ func TestAccount(t *testing.T) { type AccountTestSuite struct { suite.Suite - cliApp *cli.App - mockCtrl *gomock.Controller - mockService *accountservicemock.MockAccountServiceClient + cliApp *cli.App + mockCtrl *gomock.Controller + mockService *accountservicemock.MockAccountServiceClient + mockReqService *requestservicemock.MockRequestServiceClient } func (s *AccountTestSuite) SetupTest() { s.mockCtrl = gomock.NewController(s.T()) s.mockService = accountservicemock.NewMockAccountServiceClient(s.mockCtrl) - out, err := NewAccountCommand(func(ctx *cli.Context) (*AccountClient, error) { + s.mockReqService = requestservicemock.NewMockRequestServiceClient(s.mockCtrl) + + getAccountClientFn := func(ctx *cli.Context) (*AccountClient, error) { return &AccountClient{ ctx: context.TODO(), client: s.mockService, }, nil - }) + } + getRequestClientFn := func(ctx *cli.Context) (*RequestClient, error) { + return &RequestClient{ + ctx: context.TODO(), + client: s.mockReqService, + }, nil + } + + out, err := NewAccountCommand(getAccountClientFn, getRequestClientFn) s.Require().NoError(err) AutoConfirmFlag.Value = true s.cliApp = &cli.App{ diff --git a/app/app.go b/app/app.go index db8d6ee..6899c3f 100644 --- a/app/app.go +++ b/app/app.go @@ -5,6 +5,10 @@ import ( "go.uber.org/fx" ) +const ( + AppName = "tcld" +) + type AppParams struct { fx.In Commands []*cli.Command `group:"commands"` @@ -17,12 +21,14 @@ type CommandOut struct { func NewApp(params AppParams) (*cli.App, error) { app := &cli.App{ - Name: "tcld", + Name: AppName, Usage: "Temporal Cloud cli", Flags: []cli.Flag{ ServerFlag, ConfigDirFlag, AutoConfirmFlag, + RequestTimeoutFlag, + SyncRequestFlag, }, } app.Commands = params.Commands diff --git a/app/namespace.go b/app/namespace.go index 99c2a06..ee3140b 100644 --- a/app/namespace.go +++ b/app/namespace.go @@ -11,6 +11,7 @@ import ( "github.com/kylelemons/godebug/diff" "github.com/temporalio/tcld/protogen/api/namespace/v1" "github.com/temporalio/tcld/protogen/api/namespaceservice/v1" + "github.com/temporalio/tcld/protogen/api/request/v1" "github.com/urfave/cli/v2" "google.golang.org/grpc" ) @@ -96,7 +97,7 @@ func (c *NamespaceClient) getNamespace(namespace string) (*namespace.Namespace, return res.Namespace, nil } -func (c *NamespaceClient) updateNamespace(ctx *cli.Context, n *namespace.Namespace) error { +func (c *NamespaceClient) updateNamespace(ctx *cli.Context, n *namespace.Namespace) (*request.RequestStatus, error) { resourceVersion := n.ResourceVersion if v := ctx.String(ResourceVersionFlagName); v != "" { resourceVersion = v @@ -109,13 +110,13 @@ func (c *NamespaceClient) updateNamespace(ctx *cli.Context, n *namespace.Namespa Spec: n.Spec, }) if err != nil { - return err + return nil, err } - return PrintProto(res) + return res.RequestStatus, nil } -func (c *NamespaceClient) renameSearchAttribute(ctx *cli.Context, n *namespace.Namespace, existingName string, newName string) error { +func (c *NamespaceClient) renameSearchAttribute(ctx *cli.Context, n *namespace.Namespace, existingName string, newName string) (*request.RequestStatus, error) { resourceVersion := n.ResourceVersion if v := ctx.String(ResourceVersionFlagName); v != "" { resourceVersion = v @@ -128,9 +129,9 @@ func (c *NamespaceClient) renameSearchAttribute(ctx *cli.Context, n *namespace.N NewCustomSearchAttributeName: newName, }) if err != nil { - return err + return nil, err } - return PrintProto(res) + return res.RequestStatus, nil } func (c *NamespaceClient) parseExistingCerts(ctx *cli.Context) (namespace *namespace.Namespace, existing caCerts, err error) { @@ -171,8 +172,9 @@ func ReadCACerts(ctx *cli.Context) (string, error) { return cert, nil } -func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn) (CommandOut, error) { +func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestClientFn GetRequestClientFn) (CommandOut, error) { var c *NamespaceClient + var r *RequestClient return CommandOut{ Command: &cli.Command{ Name: "namespace", @@ -181,6 +183,10 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn) (CommandOut, Before: func(ctx *cli.Context) error { var err error c, err = getNamespaceClientFn(ctx) + if err != nil { + return err + } + r, err = getRequestClientFn(ctx) return err }, Subcommands: []*cli.Command{ @@ -263,7 +269,11 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn) (CommandOut, return errors.New("nothing to change") } n.Spec.AcceptedClientCa = bundle - return c.updateNamespace(ctx, n) + status, err := c.updateNamespace(ctx, n) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "add certificate", status) }, }, { @@ -314,7 +324,11 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn) (CommandOut, if err != nil || !y { return err } - return c.updateNamespace(ctx, n) + status, err := c.updateNamespace(ctx, n) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "remove certificate", status) }, }, { @@ -341,7 +355,11 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn) (CommandOut, return errors.New("nothing to change") } n.Spec.AcceptedClientCa = cert - return c.updateNamespace(ctx, n) + status, err := c.updateNamespace(ctx, n) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "set certificates", status) }, }, }, @@ -417,7 +435,11 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn) (CommandOut, if confirmed { n.Spec.CertificateFilters = replacementFilters.toSpec() - return c.updateNamespace(ctx, n) + status, err := c.updateNamespace(ctx, n) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "import certificate filters", status) } fmt.Println("operation canceled") @@ -491,7 +513,12 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn) (CommandOut, if confirmed { n.Spec.CertificateFilters = nil - return c.updateNamespace(ctx, n) + status, err := c.updateNamespace(ctx, n) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "clear certificate filters", status) + } fmt.Println("operation canceled") @@ -565,7 +592,13 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn) (CommandOut, } n.Spec.CertificateFilters = append(n.Spec.CertificateFilters, newFilters.toSpec()...) - return c.updateNamespace(ctx, n) + + status, err := c.updateNamespace(ctx, n) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "add certificate filters", status) + } fmt.Println("operation canceled") @@ -613,8 +646,12 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn) (CommandOut, n.Spec.SearchAttributes[attrName] = attrType } } + status, err := c.updateNamespace(ctx, n) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "add search attribute", status) - return c.updateNamespace(ctx, n) }, }, { @@ -657,7 +694,11 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn) (CommandOut, if err != nil || !y { return err } - return c.renameSearchAttribute(ctx, n, existingName, newName) + status, err := c.renameSearchAttribute(ctx, n, existingName, newName) + if err != nil { + return err + } + return r.HandleRequestStatus(ctx, "rename search attribute", status) }, }, }, diff --git a/app/namespace_test.go b/app/namespace_test.go index 896c6cd..728ded1 100644 --- a/app/namespace_test.go +++ b/app/namespace_test.go @@ -14,6 +14,7 @@ import ( "github.com/temporalio/tcld/protogen/api/namespaceservice/v1" "github.com/temporalio/tcld/protogen/api/request/v1" namespaceservicemock "github.com/temporalio/tcld/protogen/apimock/namespaceservice/v1" + requestservicemock "github.com/temporalio/tcld/protogen/apimock/requestservice/v1" "github.com/urfave/cli/v2" ) @@ -23,20 +24,31 @@ func TestNamespace(t *testing.T) { type NamespaceTestSuite struct { suite.Suite - cliApp *cli.App - mockCtrl *gomock.Controller - mockService *namespaceservicemock.MockNamespaceServiceClient + cliApp *cli.App + mockCtrl *gomock.Controller + mockService *namespaceservicemock.MockNamespaceServiceClient + mockReqService *requestservicemock.MockRequestServiceClient } func (s *NamespaceTestSuite) SetupTest() { s.mockCtrl = gomock.NewController(s.T()) s.mockService = namespaceservicemock.NewMockNamespaceServiceClient(s.mockCtrl) - out, err := NewNamespaceCommand(func(ctx *cli.Context) (*NamespaceClient, error) { + s.mockReqService = requestservicemock.NewMockRequestServiceClient(s.mockCtrl) + + getNamespaceClientFn := func(ctx *cli.Context) (*NamespaceClient, error) { return &NamespaceClient{ ctx: context.TODO(), client: s.mockService, }, nil - }) + } + getRequestClientFn := func(ctx *cli.Context) (*RequestClient, error) { + return &RequestClient{ + ctx: context.TODO(), + client: s.mockReqService, + }, nil + } + + out, err := NewNamespaceCommand(getNamespaceClientFn, getRequestClientFn) s.Require().NoError(err) AutoConfirmFlag.Value = true s.cliApp = &cli.App{ diff --git a/app/request.go b/app/request.go index d28b2c6..5825061 100644 --- a/app/request.go +++ b/app/request.go @@ -2,12 +2,37 @@ package app import ( "context" + "fmt" + "time" + "github.com/gosuri/uilive" + "github.com/temporalio/tcld/protogen/api/request/v1" "github.com/temporalio/tcld/protogen/api/requestservice/v1" "github.com/urfave/cli/v2" "google.golang.org/grpc" ) +const ( + SyncRequestFlagName = "synchronous-request" + RequestTimeoutFlagName = "request-timeout" +) + +var ( + RequestTimeoutFlag = &cli.DurationFlag{ + Name: RequestTimeoutFlagName, + Usage: "Time to wait for asynchronous requests to finish", + EnvVars: []string{"REQUEST_TIMEOUT"}, + Aliases: []string{"rt"}, + Value: time.Hour, + } + SyncRequestFlag = &cli.BoolFlag{ + Name: SyncRequestFlagName, + Usage: "Block on request to complete", + Aliases: []string{"sync"}, + EnvVars: []string{"SYNCHRONOUS_REQUEST"}, + } +) + type RequestClient struct { client requestservice.RequestServiceClient ctx context.Context @@ -30,14 +55,14 @@ func GetRequestClient(ctx *cli.Context) (*RequestClient, error) { return NewRequestClient(ct, conn), nil } -func (c *RequestClient) getRequestStatus(requestID string) error { +func (c *RequestClient) getRequestStatus(ctx *cli.Context, requestID string) (*request.RequestStatus, error) { res, err := c.client.GetRequestStatus(c.ctx, &requestservice.GetRequestStatusRequest{ RequestId: requestID, }) if err != nil { - return err + return nil, err } - return PrintProto(res) + return res.RequestStatus, nil } func NewRequestCommand(getRequestClientFn GetRequestClientFn) (CommandOut, error) { @@ -66,8 +91,102 @@ func NewRequestCommand(getRequestClientFn GetRequestClientFn) (CommandOut, error }, }, Action: func(ctx *cli.Context) error { - return c.getRequestStatus(ctx.String("request-id")) + res, err := c.getRequestStatus(ctx, ctx.String("request-id")) + if err != nil { + return err + } + return PrintProto(res) + }, + }, { + Name: "wait", + Usage: "wait till the request completes", + Aliases: []string{"w"}, + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: "request-id", + Usage: "the request-id of the asynchronous request", + Aliases: []string{"r"}, + Required: true, + }, + }, + Action: func(ctx *cli.Context) error { + return c.waitOnRequest(ctx, "", ctx.String("request-id")) }, }}, }}, nil } + +func (c *RequestClient) waitOnRequest(ctx *cli.Context, operation string, requestID string) error { + + ticker := time.NewTicker(time.Millisecond) + timer := time.NewTimer(ctx.Duration(RequestTimeoutFlagName)) + + writer := uilive.New() + writer.Start() + defer writer.Stop() + + var status *request.RequestStatus + defer func() { + if status != nil { + PrintProto(status) + } + }() +loop: + for { + select { + case <-timer.C: + return fmt.Errorf("timed out waiting for request to complete, namespace=%s, requestID=%s, timeout=%s", + ctx.String(NamespaceFlagName), + requestID, + ctx.Duration(RequestTimeoutFlagName), + ) + case <-ticker.C: + var err error + status, err = c.getRequestStatus(ctx, requestID) + if err != nil { + return err + } + switch status.State { + case request.STATE_FULFILLED: + break loop + case request.STATE_FAILED: + fmt.Fprintf(writer, "operation failed \n") + return fmt.Errorf("request failed: %s", status.FailureReason) + case request.STATE_CANCELLED: + fmt.Fprintf(writer, "operation failed \n") + return fmt.Errorf("request was cancelled: %s", status.FailureReason) + } + if operation != "" { + fmt.Fprintf(writer, "waiting for %s operation (requestId='%s') to finish, current state: %s\n", + operation, requestID, request.State_name[int32(status.State)]) + } else { + fmt.Fprintf(writer, "waiting for request with id='%s' to finish, current state: %s\n", + requestID, request.State_name[int32(status.State)]) + } + ticker.Reset(time.Second * time.Duration(status.CheckDuration.Seconds)) + } + } + if operation != "" { + fmt.Fprintf(writer, "%s operation completed successfully\n", operation) + } else { + fmt.Fprintf(writer, "request with id='%s' finished successfully\n", requestID) + } + return nil +} + +func (c *RequestClient) HandleRequestStatus( + ctx *cli.Context, + operation string, + status *request.RequestStatus, +) error { + + if ctx.Bool(SyncRequestFlagName) { + return c.waitOnRequest(ctx, operation, status.RequestId) + } + + if err := PrintProto(status); err != nil { + return err + } + fmt.Sprintf("started %s operation with requestId='%s', to monitor its progress use command: `%s request get -r '%s'`", operation, status.RequestId, AppName, status.RequestId) + return nil +} diff --git a/app/request_test.go b/app/request_test.go index 5d4f565..354033c 100644 --- a/app/request_test.go +++ b/app/request_test.go @@ -5,8 +5,10 @@ import ( "errors" "testing" + "github.com/gogo/protobuf/types" "github.com/golang/mock/gomock" "github.com/stretchr/testify/suite" + "github.com/temporalio/tcld/protogen/api/request/v1" "github.com/temporalio/tcld/protogen/api/requestservice/v1" requestservicemock "github.com/temporalio/tcld/protogen/apimock/requestservice/v1" "github.com/urfave/cli/v2" @@ -36,6 +38,9 @@ func (s *RequestTestSuite) SetupTest() { s.cliApp = &cli.App{ Name: "test", Commands: []*cli.Command{out.Command}, + Flags: []cli.Flag{ + RequestTimeoutFlag, + }, } } @@ -48,7 +53,6 @@ func (s *RequestTestSuite) RunCmd(args ...string) error { } func (s *RequestTestSuite) TestGet() { - s.Error(s.RunCmd("request", "get")) s.mockService.EXPECT().GetRequestStatus(gomock.Any(), &requestservice.GetRequestStatusRequest{ @@ -58,6 +62,68 @@ func (s *RequestTestSuite) TestGet() { s.mockService.EXPECT().GetRequestStatus(gomock.Any(), &requestservice.GetRequestStatusRequest{ RequestId: "req1", - }).Return(&requestservice.GetRequestStatusResponse{}, nil).Times(1) + }).Return(&requestservice.GetRequestStatusResponse{ + RequestStatus: &request.RequestStatus{ + State: request.STATE_PENDING, + CheckDuration: &types.Duration{Seconds: 1}, + }, + }, nil).Times(1) s.NoError(s.RunCmd("request", "get", "--request-id", "req1")) } + +func (s *RequestTestSuite) TestWait() { + + s.Error(s.RunCmd("request", "wait")) + + // an error is returned by the api + s.mockService.EXPECT().GetRequestStatus(gomock.Any(), &requestservice.GetRequestStatusRequest{ + RequestId: "req1", + }).Return(nil, errors.New("some error")).Times(1) + s.Error(s.RunCmd("request", "wait", "--request-id", "req1")) + + // call repetatively till a fulfilled is received + s.mockService.EXPECT().GetRequestStatus(gomock.Any(), &requestservice.GetRequestStatusRequest{ + RequestId: "req1", + }).Return(&requestservice.GetRequestStatusResponse{ + RequestStatus: &request.RequestStatus{ + State: request.STATE_PENDING, + CheckDuration: &types.Duration{Seconds: 1}, + }, + }, nil).Times(2) + s.mockService.EXPECT().GetRequestStatus(gomock.Any(), &requestservice.GetRequestStatusRequest{ + RequestId: "req1", + }).Return(&requestservice.GetRequestStatusResponse{ + RequestStatus: &request.RequestStatus{ + State: request.STATE_IN_PROGRESS, + CheckDuration: &types.Duration{Seconds: 1}, + }, + }, nil).Times(2) + s.mockService.EXPECT().GetRequestStatus(gomock.Any(), &requestservice.GetRequestStatusRequest{ + RequestId: "req1", + }).Return(&requestservice.GetRequestStatusResponse{ + RequestStatus: &request.RequestStatus{ + State: request.STATE_FULFILLED, + CheckDuration: &types.Duration{Seconds: 1}, + }, + }, nil).Times(1) + s.NoError(s.RunCmd("request", "wait", "--request-id", "req1")) + + // call repetatively till a state changes to failed is received + s.mockService.EXPECT().GetRequestStatus(gomock.Any(), &requestservice.GetRequestStatusRequest{ + RequestId: "req1", + }).Return(&requestservice.GetRequestStatusResponse{ + RequestStatus: &request.RequestStatus{ + State: request.STATE_PENDING, + CheckDuration: &types.Duration{Seconds: 1}, + }, + }, nil).Times(2) + s.mockService.EXPECT().GetRequestStatus(gomock.Any(), &requestservice.GetRequestStatusRequest{ + RequestId: "req1", + }).Return(&requestservice.GetRequestStatusResponse{ + RequestStatus: &request.RequestStatus{ + State: request.STATE_FAILED, + CheckDuration: &types.Duration{Seconds: 1}, + }, + }, nil).Times(1) + s.Error(s.RunCmd("request", "wait", "--request-id", "req1")) +} diff --git a/go.mod b/go.mod index b3d537c..15ad038 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,8 @@ require ( github.com/cpuguy83/go-md2man/v2 v2.0.1 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/golang/protobuf v1.5.2 // indirect + github.com/gosuri/uilive v0.0.4 // indirect + github.com/mattn/go-isatty v0.0.16 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect @@ -24,7 +26,7 @@ require ( go.uber.org/multierr v1.5.0 // indirect go.uber.org/zap v1.16.0 // indirect golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 // indirect - golang.org/x/sys v0.0.0-20210903071746-97244b99971b // indirect + golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab // indirect golang.org/x/text v0.3.7 // indirect google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013 // indirect google.golang.org/protobuf v1.27.1 // indirect diff --git a/go.sum b/go.sum index f929757..538786d 100644 --- a/go.sum +++ b/go.sum @@ -55,6 +55,8 @@ github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ= github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/gosuri/uilive v0.0.4 h1:hUEBpQDj8D8jXgtCdBu7sWsy5sbW/5GhuO8KBwJ2jyY= +github.com/gosuri/uilive v0.0.4/go.mod h1:V/epo5LjjlDE5RJUcqx8dbw+zc93y5Ya3yg8tfZ74VI= github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= 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= @@ -65,6 +67,8 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= +github.com/mattn/go-isatty v0.0.16 h1:bq3VjFmv/sOjHtdEhmkEV4x1AJtvUvOJ2PFAZ5+peKQ= +github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -149,6 +153,8 @@ golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210903071746-97244b99971b h1:3Dq0eVHn0uaQJmPO+/aYPI/fRMqdrVDbu7MQcku54gg= golang.org/x/sys v0.0.0-20210903071746-97244b99971b/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab h1:2QkjZIsXupsJbJIdSjjUOgWK3aEtzyuh2mPt3l/CkeU= +golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= From 89f42f833fe15cdbcb1ce79bbe071943b76955f1 Mon Sep 17 00:00:00 2001 From: Abhinav Nekkanti <10552725+anekkanti@users.noreply.github.com> Date: Thu, 1 Sep 2022 17:06:20 +0000 Subject: [PATCH 02/11] more fixes --- app/account.go | 14 ++++++++++++++ app/app.go | 2 -- app/namespace.go | 16 ++++++++++++++++ app/request.go | 26 +++++++++++++------------- 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/app/account.go b/app/account.go index 33c6cb8..065d85e 100644 --- a/app/account.go +++ b/app/account.go @@ -120,6 +120,10 @@ func NewAccountCommand(getAccountClientFn GetAccountClientFn, getRequestClientFn { Name: "enable", Usage: "Enables the metrics endpoint. CA Certificates *must* be configured prior to enabling the endpoint", + Flags: []cli.Flag{ + RequestTimeoutFlag, + WaitForRequestFlag, + }, Action: func(ctx *cli.Context) error { a, err := c.getAccount() if err != nil { @@ -145,6 +149,10 @@ func NewAccountCommand(getAccountClientFn GetAccountClientFn, getRequestClientFn { Name: "disable", Usage: "Disables the metrics endpoint", + Flags: []cli.Flag{ + RequestTimeoutFlag, + WaitForRequestFlag, + }, Action: func(ctx *cli.Context) error { a, err := c.getAccount() if err != nil { @@ -178,6 +186,8 @@ func NewAccountCommand(getAccountClientFn GetAccountClientFn, getRequestClientFn ResourceVersionFlag, CaCertificateFlag, CaCertificateFileFlag, + RequestTimeoutFlag, + WaitForRequestFlag, }, Action: func(ctx *cli.Context) error { newCerts, err := readAndParseCACerts(ctx) @@ -226,6 +236,8 @@ func NewAccountCommand(getAccountClientFn GetAccountClientFn, getRequestClientFn CaCertificateFlag, CaCertificateFileFlag, caCertificateFingerprintFlag, + RequestTimeoutFlag, + WaitForRequestFlag, }, Action: func(ctx *cli.Context) error { a, existingCerts, err := c.parseExistingMetricsCerts(ctx) @@ -287,6 +299,8 @@ func NewAccountCommand(getAccountClientFn GetAccountClientFn, getRequestClientFn ResourceVersionFlag, CaCertificateFlag, CaCertificateFileFlag, + RequestTimeoutFlag, + WaitForRequestFlag, }, Action: func(ctx *cli.Context) error { cert, err := ReadCACerts(ctx) diff --git a/app/app.go b/app/app.go index 6899c3f..844c231 100644 --- a/app/app.go +++ b/app/app.go @@ -27,8 +27,6 @@ func NewApp(params AppParams) (*cli.App, error) { ServerFlag, ConfigDirFlag, AutoConfirmFlag, - RequestTimeoutFlag, - SyncRequestFlag, }, } app.Commands = params.Commands diff --git a/app/namespace.go b/app/namespace.go index ee3140b..e321fe8 100644 --- a/app/namespace.go +++ b/app/namespace.go @@ -247,6 +247,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl ResourceVersionFlag, CaCertificateFlag, CaCertificateFileFlag, + RequestTimeoutFlag, + WaitForRequestFlag, }, Action: func(ctx *cli.Context) error { newCerts, err := readAndParseCACerts(ctx) @@ -287,6 +289,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl CaCertificateFlag, CaCertificateFileFlag, caCertificateFingerprintFlag, + RequestTimeoutFlag, + WaitForRequestFlag, }, Action: func(ctx *cli.Context) error { n, existingCerts, err := c.parseExistingCerts(ctx) @@ -341,6 +345,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl ResourceVersionFlag, CaCertificateFlag, CaCertificateFileFlag, + RequestTimeoutFlag, + WaitForRequestFlag, }, Action: func(ctx *cli.Context) error { cert, err := ReadCACerts(ctx) @@ -387,6 +393,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl Usage: `JSON that defines the certificate filters that will be configured on the namespace. This will replace the existing filter configuration. Sample JSON: { "filters": [ { "commonName": "test1" } ] }`, Aliases: []string{"input", "i"}, }, + RequestTimeoutFlag, + WaitForRequestFlag, }, Action: func(ctx *cli.Context) error { fileFlagSet := ctx.Path("certificate-filter-file") != "" @@ -494,6 +502,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl NamespaceFlag, RequestIDFlag, ResourceVersionFlag, + RequestTimeoutFlag, + WaitForRequestFlag, }, Action: func(ctx *cli.Context) error { n, err := c.getNamespace(ctx.String(NamespaceFlagName)) @@ -543,6 +553,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl Usage: `JSON that defines the certificate filters that will be added to the namespace. Sample JSON: { "filters": [ { "commonName": "test1" } ] }`, Aliases: []string{"input", "i"}, }, + RequestTimeoutFlag, + WaitForRequestFlag, }, Action: func(ctx *cli.Context) error { fileFlagSet := ctx.Path("certificate-filter-file") != "" @@ -626,6 +638,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl Aliases: []string{"sa"}, Required: true, }, + RequestTimeoutFlag, + WaitForRequestFlag, }, Action: func(ctx *cli.Context) error { csa, err := toSearchAttributes(ctx.StringSlice("search-attribute")) @@ -674,6 +688,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl Aliases: []string{"nn"}, Required: true, }, + RequestTimeoutFlag, + WaitForRequestFlag, }, Action: func(ctx *cli.Context) error { n, err := c.getNamespace( diff --git a/app/request.go b/app/request.go index 5825061..837b243 100644 --- a/app/request.go +++ b/app/request.go @@ -13,23 +13,23 @@ import ( ) const ( - SyncRequestFlagName = "synchronous-request" + WaitForRequestFlagName = "wait-for-request" RequestTimeoutFlagName = "request-timeout" ) var ( RequestTimeoutFlag = &cli.DurationFlag{ Name: RequestTimeoutFlagName, - Usage: "Time to wait for asynchronous requests to finish", + Usage: "Time to wait for requests to complete", EnvVars: []string{"REQUEST_TIMEOUT"}, Aliases: []string{"rt"}, Value: time.Hour, } - SyncRequestFlag = &cli.BoolFlag{ - Name: SyncRequestFlagName, - Usage: "Block on request to complete", - Aliases: []string{"sync"}, - EnvVars: []string{"SYNCHRONOUS_REQUEST"}, + WaitForRequestFlag = &cli.BoolFlag{ + Name: WaitForRequestFlagName, + Usage: "Wait for request to complete", + Aliases: []string{"wait"}, + EnvVars: []string{"WAIT_FOR_REQUEST"}, } ) @@ -70,7 +70,7 @@ func NewRequestCommand(getRequestClientFn GetRequestClientFn) (CommandOut, error var c *RequestClient return CommandOut{Command: &cli.Command{ Name: "request", - Usage: "Manage asynchronous requests", + Usage: "Manage requests", Aliases: []string{"r"}, Before: func(ctx *cli.Context) error { var err error @@ -85,7 +85,7 @@ func NewRequestCommand(getRequestClientFn GetRequestClientFn) (CommandOut, error Flags: []cli.Flag{ &cli.StringFlag{ Name: "request-id", - Usage: "The request-id of the asynchronous request", + Usage: "The request-id of the request", Aliases: []string{"r"}, Required: true, }, @@ -99,12 +99,12 @@ func NewRequestCommand(getRequestClientFn GetRequestClientFn) (CommandOut, error }, }, { Name: "wait", - Usage: "wait till the request completes", + Usage: "wait for the request complete", Aliases: []string{"w"}, Flags: []cli.Flag{ &cli.StringFlag{ Name: "request-id", - Usage: "the request-id of the asynchronous request", + Usage: "the request-id of the request", Aliases: []string{"r"}, Required: true, }, @@ -153,7 +153,7 @@ loop: fmt.Fprintf(writer, "operation failed \n") return fmt.Errorf("request failed: %s", status.FailureReason) case request.STATE_CANCELLED: - fmt.Fprintf(writer, "operation failed \n") + fmt.Fprintf(writer, "operation cancelled\n") return fmt.Errorf("request was cancelled: %s", status.FailureReason) } if operation != "" { @@ -180,7 +180,7 @@ func (c *RequestClient) HandleRequestStatus( status *request.RequestStatus, ) error { - if ctx.Bool(SyncRequestFlagName) { + if ctx.Bool(WaitForRequestFlagName) { return c.waitOnRequest(ctx, operation, status.RequestId) } From da61afb775e54b5edd7f43414ff46bc9234d5a97 Mon Sep 17 00:00:00 2001 From: Abhinav Nekkanti <10552725+anekkanti@users.noreply.github.com> Date: Mon, 15 May 2023 08:56:44 -0700 Subject: [PATCH 03/11] a fix --- app/namespace_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/namespace_test.go b/app/namespace_test.go index 7c31386..dee5910 100644 --- a/app/namespace_test.go +++ b/app/namespace_test.go @@ -4,12 +4,13 @@ import ( "context" "encoding/base64" "errors" - "github.com/temporalio/tcld/protogen/api/auth/v1" - "github.com/temporalio/tcld/protogen/api/authservice/v1" "os" "strings" "testing" + "github.com/temporalio/tcld/protogen/api/auth/v1" + "github.com/temporalio/tcld/protogen/api/authservice/v1" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/suite" "github.com/temporalio/tcld/protogen/api/namespace/v1" @@ -31,7 +32,7 @@ type NamespaceTestSuite struct { mockCtrl *gomock.Controller mockService *namespaceservicemock.MockNamespaceServiceClient mockAuthService *authservicemock.MockAuthServiceClient - mockReqService *requestservicemock.MockRequestServiceClient + mockReqService *requestservicemock.MockRequestServiceClient } func (s *NamespaceTestSuite) SetupTest() { @@ -45,7 +46,7 @@ func (s *NamespaceTestSuite) SetupTest() { client: s.mockService, authClient: s.mockAuthService, }, nil - } + }) getRequestClientFn := func(ctx *cli.Context) (*RequestClient, error) { return &RequestClient{ ctx: context.TODO(), From 397cbeb1bec78a384d4b51fb20399c0aa3d1c70b Mon Sep 17 00:00:00 2001 From: Abhinav Nekkanti <10552725+anekkanti@users.noreply.github.com> Date: Sun, 25 Jun 2023 18:10:22 -0700 Subject: [PATCH 04/11] more fixes --- app/namespace.go | 20 ++++++++++++++++++++ app/prompt.go | 2 +- app/request.go | 4 +--- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/app/namespace.go b/app/namespace.go index 4d2c38d..ab4a520 100644 --- a/app/namespace.go +++ b/app/namespace.go @@ -286,6 +286,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl Flags: []cli.Flag{ RequestIDFlag, CaCertificateFlag, + WaitForRequestFlag, + RequestTimeoutFlag, &cli.StringFlag{ Name: NamespaceFlagName, Usage: "The namespace hosted on temporal cloud", @@ -418,6 +420,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl Flags: []cli.Flag{ RequestIDFlag, ResourceVersionFlag, + WaitForRequestFlag, + RequestTimeoutFlag, &cli.StringFlag{ Name: NamespaceFlagName, Usage: "The namespace hosted on temporal cloud", @@ -506,6 +510,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl NamespaceFlag, RequestIDFlag, ResourceVersionFlag, + WaitForRequestFlag, + RequestTimeoutFlag, CaCertificateFlag, CaCertificateFileFlag, }, @@ -545,6 +551,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl NamespaceFlag, RequestIDFlag, ResourceVersionFlag, + WaitForRequestFlag, + RequestTimeoutFlag, CaCertificateFlag, CaCertificateFileFlag, caCertificateFingerprintFlag, @@ -600,6 +608,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl NamespaceFlag, RequestIDFlag, ResourceVersionFlag, + WaitForRequestFlag, + RequestTimeoutFlag, CaCertificateFlag, CaCertificateFileFlag, }, @@ -638,6 +648,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl NamespaceFlag, RequestIDFlag, ResourceVersionFlag, + WaitForRequestFlag, + RequestTimeoutFlag, &cli.PathFlag{ Name: certificateFilterFileFlagName, Usage: `Path to a JSON file that defines the certificate filters that will be configured on the namespace. This will replace the existing filter configuration. Sample JSON: { "filters": [ { "commonName": "test1" } ] }`, @@ -793,6 +805,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl NamespaceFlag, RequestIDFlag, ResourceVersionFlag, + WaitForRequestFlag, + RequestTimeoutFlag, &cli.PathFlag{ Name: certificateFilterFileFlagName, Usage: `Path to a JSON file that defines the certificate filters that will be added to the namespace. Sample JSON: { "filters": [ { "commonName": "test1" } ] }`, @@ -877,6 +891,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl Flags: []cli.Flag{ NamespaceFlag, ResourceVersionFlag, + WaitForRequestFlag, + RequestTimeoutFlag, RetentionDaysFlag, RequestIDFlag, }, @@ -934,6 +950,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl NamespaceFlag, RequestIDFlag, ResourceVersionFlag, + WaitForRequestFlag, + RequestTimeoutFlag, &cli.StringSliceFlag{ Name: "search-attribute", Usage: fmt.Sprintf("Flag can be used multiple times; value must be \"name=type\"; valid types are: %v", getSearchAttributeTypes()), @@ -975,6 +993,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl NamespaceFlag, RequestIDFlag, ResourceVersionFlag, + WaitForRequestFlag, + RequestTimeoutFlag, &cli.StringFlag{ Name: "existing-name", Usage: "The name of an existing search attribute", diff --git a/app/prompt.go b/app/prompt.go index acf5b86..c3cdfc5 100644 --- a/app/prompt.go +++ b/app/prompt.go @@ -10,7 +10,7 @@ import ( ) const ( - AutoConfirmFlagName = "auto_confirm" + AutoConfirmFlagName = "auto-confirm" ) var ( diff --git a/app/request.go b/app/request.go index 2333298..0e268d2 100644 --- a/app/request.go +++ b/app/request.go @@ -179,16 +179,14 @@ func (c *RequestClient) HandleRequestStatus( operation string, status *request.RequestStatus, ) error { - if ctx.Bool(WaitForRequestFlagName) { return c.waitOnRequest(ctx, operation, status.RequestId) } - if err := PrintProto(status); err != nil { return err } fmt.Printf( - "started %s operation with requestId='%s', to monitor its progress use command: `%s request get -r '%s'`", + "started %s operation with request id='%s', to monitor its progress use command: `%s request get -r '%s'`", operation, status.RequestId, AppName, status.RequestId, ) return nil From 30c6fca62130fd20dcfc2aaaf1499251ec4f1440 Mon Sep 17 00:00:00 2001 From: Abhinav Nekkanti <10552725+anekkanti@users.noreply.github.com> Date: Sun, 25 Jun 2023 18:24:53 -0700 Subject: [PATCH 05/11] more fixes --- app/user.go | 117 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 77 insertions(+), 40 deletions(-) diff --git a/app/user.go b/app/user.go index 90474c0..a625c4a 100644 --- a/app/user.go +++ b/app/user.go @@ -5,10 +5,12 @@ import ( "errors" "fmt" + "strings" + "github.com/temporalio/tcld/protogen/api/auth/v1" "github.com/temporalio/tcld/protogen/api/authservice/v1" + "github.com/temporalio/tcld/protogen/api/request/v1" "github.com/urfave/cli/v2" - "strings" ) const ( @@ -137,9 +139,9 @@ func (c *UserClient) inviteUsers( emails []string, namespacePermissions []string, accountRole string, -) error { +) (*request.RequestStatus, error) { if len(accountRole) == 0 { - return errors.New("account role required for inviting new users") + return nil, errors.New("account role required for inviting new users") } // the role ids to invite the users for @@ -148,7 +150,7 @@ func (c *UserClient) inviteUsers( // first get the required account role role, err := getAccountRole(c.ctx, c.client, accountRole) if err != nil { - return err + return nil, err } roleIDs = append(roleIDs, role.GetId()) @@ -156,11 +158,11 @@ func (c *UserClient) inviteUsers( if len(namespacePermissions) > 0 { npm, err := toNamespacePermissionsMap(namespacePermissions) if err != nil { - return err + return nil, err } nsRoles, err := getNamespaceRolesBatch(c.ctx, c.client, npm) if err != nil { - return err + return nil, err } for _, nsRole := range nsRoles { roleIDs = append(roleIDs, nsRole.GetId()) @@ -180,19 +182,19 @@ func (c *UserClient) inviteUsers( } resp, err := c.client.InviteUsers(c.ctx, req) if err != nil { - return fmt.Errorf("unable to invite users: %w", err) + return nil, fmt.Errorf("unable to invite users: %w", err) } - return PrintProto(resp.GetRequestStatus()) + return resp.GetRequestStatus(), nil } func (c *UserClient) resendInvitation( ctx *cli.Context, userID string, userEmail string, -) error { +) (*request.RequestStatus, error) { user, err := c.getUser(userID, userEmail) if err != nil { - return err + return nil, err } req := &authservice.ResendUserInviteRequest{ UserId: user.Id, @@ -200,19 +202,19 @@ func (c *UserClient) resendInvitation( } resp, err := c.client.ResendUserInvite(c.ctx, req) if err != nil { - return fmt.Errorf("unable to resend invitation for user: %w", err) + return nil, fmt.Errorf("unable to resend invitation for user: %w", err) } - return PrintProto(resp.GetRequestStatus()) + return resp.GetRequestStatus(), nil } func (c *UserClient) deleteUser( ctx *cli.Context, userID string, userEmail string, -) error { +) (*request.RequestStatus, error) { u, err := c.getUser(userID, userEmail) if err != nil { - return err + return nil, err } req := &authservice.DeleteUserRequest{ UserId: u.Id, @@ -224,12 +226,12 @@ func (c *UserClient) deleteUser( } resp, err := c.client.DeleteUser(c.ctx, req) if err != nil { - return fmt.Errorf("unable to delete user: %w", err) + return nil, fmt.Errorf("unable to delete user: %w", err) } - return PrintProto(resp.GetRequestStatus()) + return resp.GetRequestStatus(), nil } -func (c *UserClient) performUpdate(ctx *cli.Context, user *auth.User) error { +func (c *UserClient) performUpdate(ctx *cli.Context, user *auth.User) (*request.RequestStatus, error) { req := &authservice.UpdateUserRequest{ UserId: user.Id, Spec: user.Spec, @@ -241,9 +243,9 @@ func (c *UserClient) performUpdate(ctx *cli.Context, user *auth.User) error { } resp, err := c.client.UpdateUser(c.ctx, req) if err != nil { - return fmt.Errorf("unable to update user: %w", err) + return nil, fmt.Errorf("unable to update user: %w", err) } - return PrintProto(resp.GetRequestStatus()) + return resp.GetRequestStatus(), nil } func (c *UserClient) setAccountRole( @@ -251,25 +253,25 @@ func (c *UserClient) setAccountRole( userID string, userEmail string, accountRole string, -) error { +) (*request.RequestStatus, error) { user, userRoles, err := c.getUserAndRoles(userID, userEmail) if err != nil { - return err + return nil, err } var newRoleIDs []string accountRoleToSet, err := getAccountRole(c.ctx, c.client, accountRole) if err != nil { - return err + return nil, err } if accountRoleToSet.Spec.AccountRole.ActionGroup == auth.ACCOUNT_ACTION_GROUP_ADMIN { // set the user account admin role y, err := ConfirmPrompt(ctx, "Setting admin role on user. All existing namespace permissions will be replaced, please confirm") if err != nil { - return err + return nil, err } if !y { fmt.Println("operation canceled") - return nil + return nil, nil } // ensure we overwrite all existing roles since the global admin role has permissions to everything newRoleIDs = []string{accountRoleToSet.Id} @@ -293,10 +295,10 @@ func (c *UserClient) setNamespacePermissions( userID string, userEmail string, namespacePermissions []string, -) error { +) (*request.RequestStatus, error) { user, userRoles, err := c.getUserAndRoles(userID, userEmail) if err != nil { - return err + return nil, err } var newRoleIDs []string for _, r := range userRoles { @@ -310,21 +312,21 @@ func (c *UserClient) setNamespacePermissions( if len(namespacePermissions) == 0 { y, err := ConfirmPrompt(ctx, "Looks like you are about to remove all namespace permissions, please confirm") if err != nil { - return err + return nil, err } if !y { fmt.Println("operation canceled") - return nil + return nil, nil } } else { // collect the namespace roles and update user npm, err := toNamespacePermissionsMap(namespacePermissions) if err != nil { - return err + return nil, err } nsRoles, err := getNamespaceRolesBatch(c.ctx, c.client, npm) if err != nil { - return err + return nil, err } for _, nsRole := range nsRoles { newRoleIDs = append(newRoleIDs, nsRole.Id) @@ -393,8 +395,11 @@ func toUserWrapper(u *auth.User, roles []*auth.Role) *auth.UserWrapper { return p } -func NewUserCommand(getUserClientFn GetUserClientFn) (CommandOut, error) { - var c *UserClient +func NewUserCommand(getUserClientFn GetUserClientFn, getRequestClientFn GetRequestClientFn) (CommandOut, error) { + var ( + uc *UserClient + rc *RequestClient + ) return CommandOut{ Command: &cli.Command{ Name: "user", @@ -402,7 +407,11 @@ func NewUserCommand(getUserClientFn GetUserClientFn) (CommandOut, error) { Usage: "User management operations", Before: func(ctx *cli.Context) error { var err error - c, err = getUserClientFn(ctx) + uc, err = getUserClientFn(ctx) + if err != nil { + return err + } + rc, err = getRequestClientFn(ctx) return err }, Subcommands: []*cli.Command{ @@ -429,7 +438,7 @@ func NewUserCommand(getUserClientFn GetUserClientFn) (CommandOut, error) { }, }, Action: func(ctx *cli.Context) error { - return c.listUsers(ctx.String(NamespaceFlagName), ctx.String(pageTokenFlagName), ctx.Int(pageSizeFlagName)) + return uc.listUsers(ctx.String(NamespaceFlagName), ctx.String(pageTokenFlagName), ctx.Int(pageSizeFlagName)) }, }, { @@ -441,7 +450,7 @@ func NewUserCommand(getUserClientFn GetUserClientFn) (CommandOut, error) { userEmailFlag, }, Action: func(ctx *cli.Context) error { - u, roles, err := c.getUserAndRoles(ctx.String(userIDFlagName), ctx.String(userEmailFlagName)) + u, roles, err := uc.getUserAndRoles(ctx.String(userIDFlagName), ctx.String(userEmailFlagName)) if err != nil { return err } @@ -471,14 +480,20 @@ func NewUserCommand(getUserClientFn GetUserClientFn) (CommandOut, error) { Aliases: []string{"p"}, }, RequestIDFlag, + WaitForRequestFlag, + RequestTimeoutFlag, }, Action: func(ctx *cli.Context) error { - return c.inviteUsers( + status, err := uc.inviteUsers( ctx, ctx.StringSlice(userEmailFlagName), ctx.StringSlice(namespacePermissionFlagName), ctx.String(accountRoleFlagName), ) + if err != nil { + return err + } + return rc.HandleRequestStatus(ctx, "invite users", status) }, }, { @@ -489,13 +504,19 @@ func NewUserCommand(getUserClientFn GetUserClientFn) (CommandOut, error) { userIDFlag, userEmailFlag, RequestIDFlag, + WaitForRequestFlag, + RequestTimeoutFlag, }, Action: func(ctx *cli.Context) error { - return c.resendInvitation( + status, err := uc.resendInvitation( ctx, ctx.String(userIDFlagName), ctx.String(userEmailFlagName), ) + if err != nil { + return err + } + return rc.HandleRequestStatus(ctx, "resend user invite", status) }, }, { @@ -507,13 +528,19 @@ func NewUserCommand(getUserClientFn GetUserClientFn) (CommandOut, error) { userEmailFlag, ResourceVersionFlag, RequestIDFlag, + WaitForRequestFlag, + RequestTimeoutFlag, }, Action: func(ctx *cli.Context) error { - return c.deleteUser( + status, err := uc.deleteUser( ctx, ctx.String(userIDFlagName), ctx.String(userEmailFlagName), ) + if err != nil { + return err + } + return rc.HandleRequestStatus(ctx, "delete user", status) }, }, { @@ -525,6 +552,8 @@ func NewUserCommand(getUserClientFn GetUserClientFn) (CommandOut, error) { userEmailFlag, RequestIDFlag, ResourceVersionFlag, + WaitForRequestFlag, + RequestTimeoutFlag, &cli.StringFlag{ Name: accountRoleFlagName, Usage: fmt.Sprintf("The account role to set on the user; valid types are: %v", accountActionGroups), @@ -533,12 +562,16 @@ func NewUserCommand(getUserClientFn GetUserClientFn) (CommandOut, error) { }, }, Action: func(ctx *cli.Context) error { - return c.setAccountRole( + status, err := uc.setAccountRole( ctx, ctx.String(userIDFlagName), ctx.String(userEmailFlagName), ctx.String(accountRoleFlagName), ) + if err != nil { + return err + } + return rc.HandleRequestStatus(ctx, "set account role", status) }, }, { @@ -557,12 +590,16 @@ func NewUserCommand(getUserClientFn GetUserClientFn) (CommandOut, error) { }, }, Action: func(ctx *cli.Context) error { - return c.setNamespacePermissions( + status, err := uc.setNamespacePermissions( ctx, ctx.String(userIDFlagName), ctx.String(userEmailFlagName), ctx.StringSlice(namespacePermissionFlagName), ) + if err != nil { + return err + } + return rc.HandleRequestStatus(ctx, "set namespace permissions", status) }, }, }, From f8fda5190aa36fd91f7a997b877a71fecfa5784e Mon Sep 17 00:00:00 2001 From: Abhinav Nekkanti <10552725+anekkanti@users.noreply.github.com> Date: Sun, 25 Jun 2023 18:32:34 -0700 Subject: [PATCH 06/11] more fixes --- app/namespace_test.go | 5 ++--- app/request.go | 4 ++++ app/user_test.go | 20 ++++++++++++++++---- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/app/namespace_test.go b/app/namespace_test.go index dee5910..2025fd2 100644 --- a/app/namespace_test.go +++ b/app/namespace_test.go @@ -40,20 +40,19 @@ func (s *NamespaceTestSuite) SetupTest() { s.mockService = namespaceservicemock.NewMockNamespaceServiceClient(s.mockCtrl) s.mockReqService = requestservicemock.NewMockRequestServiceClient(s.mockCtrl) s.mockAuthService = authservicemock.NewMockAuthServiceClient(s.mockCtrl) - out, err := NewNamespaceCommand(func(ctx *cli.Context) (*NamespaceClient, error) { + getNamespaceClientFn := func(ctx *cli.Context) (*NamespaceClient, error) { return &NamespaceClient{ ctx: context.TODO(), client: s.mockService, authClient: s.mockAuthService, }, nil - }) + } getRequestClientFn := func(ctx *cli.Context) (*RequestClient, error) { return &RequestClient{ ctx: context.TODO(), client: s.mockReqService, }, nil } - out, err := NewNamespaceCommand(getNamespaceClientFn, getRequestClientFn) s.Require().NoError(err) AutoConfirmFlag.Value = true diff --git a/app/request.go b/app/request.go index 0e268d2..5412d2e 100644 --- a/app/request.go +++ b/app/request.go @@ -179,6 +179,10 @@ func (c *RequestClient) HandleRequestStatus( operation string, status *request.RequestStatus, ) error { + if status == nil { + // status can be empty when the operation is cancelled + return nil + } if ctx.Bool(WaitForRequestFlagName) { return c.waitOnRequest(ctx, operation, status.RequestId) } diff --git a/app/user_test.go b/app/user_test.go index 579fa8d..a04ac9d 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -3,15 +3,17 @@ package app import ( "context" "errors" + "reflect" + "testing" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/suite" "github.com/temporalio/tcld/protogen/api/auth/v1" "github.com/temporalio/tcld/protogen/api/authservice/v1" "github.com/temporalio/tcld/protogen/api/request/v1" authservicemock "github.com/temporalio/tcld/protogen/apimock/authservice/v1" + requestservicemock "github.com/temporalio/tcld/protogen/apimock/requestservice/v1" "github.com/urfave/cli/v2" - "reflect" - "testing" ) func TestUser(t *testing.T) { @@ -23,17 +25,27 @@ type UserTestSuite struct { cliApp *cli.App mockCtrl *gomock.Controller mockAuthService *authservicemock.MockAuthServiceClient + mockReqService *requestservicemock.MockRequestServiceClient } func (s *UserTestSuite) SetupTest() { s.mockCtrl = gomock.NewController(s.T()) s.mockAuthService = authservicemock.NewMockAuthServiceClient(s.mockCtrl) - out, err := NewUserCommand(func(ctx *cli.Context) (*UserClient, error) { + s.mockReqService = requestservicemock.NewMockRequestServiceClient(s.mockCtrl) + + getUserClient := func(ctx *cli.Context) (*UserClient, error) { return &UserClient{ ctx: context.TODO(), client: s.mockAuthService, }, nil - }) + } + getRequestClient := func(ctx *cli.Context) (*RequestClient, error) { + return &RequestClient{ + ctx: context.TODO(), + client: s.mockReqService, + }, nil + } + out, err := NewUserCommand(getUserClient, getRequestClient) s.Require().NoError(err) AutoConfirmFlag.Value = true s.cliApp = &cli.App{ From 7e9322a2423e112b52862f2c6ed82532d8362fc8 Mon Sep 17 00:00:00 2001 From: Abhinav Nekkanti <10552725+anekkanti@users.noreply.github.com> Date: Sun, 25 Jun 2023 18:39:20 -0700 Subject: [PATCH 07/11] another fix --- app/request.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/request.go b/app/request.go index 5412d2e..301cce8 100644 --- a/app/request.go +++ b/app/request.go @@ -128,7 +128,9 @@ func (c *RequestClient) waitOnRequest(ctx *cli.Context, operation string, reques var status *request.RequestStatus defer func() { if status != nil { - PrintProto(status) + if err := PrintProto(status); err != nil { + fmt.Fprintf(writer, "failed to print status: %s", err) + } } }() loop: From 57ef703a6e9398920d198feab1dfa23b1ed66e97 Mon Sep 17 00:00:00 2001 From: Abhinav Nekkanti <10552725+anekkanti@users.noreply.github.com> Date: Tue, 19 Sep 2023 11:26:37 -0700 Subject: [PATCH 08/11] more fixes --- app/request.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/app/request.go b/app/request.go index 301cce8..9556473 100644 --- a/app/request.go +++ b/app/request.go @@ -15,6 +15,8 @@ import ( const ( WaitForRequestFlagName = "wait-for-request" RequestTimeoutFlagName = "request-timeout" + + minCheckDurationTime = time.Second ) var ( @@ -84,14 +86,14 @@ func NewRequestCommand(getRequestClientFn GetRequestClientFn) (CommandOut, error Aliases: []string{"g"}, Flags: []cli.Flag{ &cli.StringFlag{ - Name: "request-id", + Name: RequestIDFlagName, Usage: "The request-id of the request", Aliases: []string{"r"}, Required: true, }, }, Action: func(ctx *cli.Context) error { - res, err := c.getRequestStatus(ctx, ctx.String("request-id")) + res, err := c.getRequestStatus(ctx, ctx.String(RequestIDFlagName)) if err != nil { return err } @@ -103,14 +105,14 @@ func NewRequestCommand(getRequestClientFn GetRequestClientFn) (CommandOut, error Aliases: []string{"w"}, Flags: []cli.Flag{ &cli.StringFlag{ - Name: "request-id", + Name: RequestIDFlagName, Usage: "the request-id of the request", Aliases: []string{"r"}, Required: true, }, }, Action: func(ctx *cli.Context) error { - return c.waitOnRequest(ctx, "", ctx.String("request-id")) + return c.waitOnRequest(ctx, "", ctx.String(RequestIDFlagName)) }, }}, }}, nil @@ -119,7 +121,9 @@ func NewRequestCommand(getRequestClientFn GetRequestClientFn) (CommandOut, error func (c *RequestClient) waitOnRequest(ctx *cli.Context, operation string, requestID string) error { ticker := time.NewTicker(time.Millisecond) + defer ticker.Stop() timer := time.NewTimer(ctx.Duration(RequestTimeoutFlagName)) + defer timer.Stop() writer := uilive.New() writer.Start() @@ -165,7 +169,11 @@ loop: fmt.Fprintf(writer, "waiting for request with id='%s' to finish, current state: %s\n", requestID, request.State_name[int32(status.State)]) } - ticker.Reset(time.Second * time.Duration(status.CheckDuration.Seconds)) + if status.CheckDuration == nil || status.CheckDuration.Seconds == 0 { + ticker.Reset(minCheckDurationTime) // min check duration is 1 second + } else { + ticker.Reset(time.Second * time.Duration(status.CheckDuration.Seconds)) + } } } if operation != "" { From c1315e8fbeb68fbf6de927343da113b98ef629d7 Mon Sep 17 00:00:00 2001 From: Abhinav Nekkanti <10552725+anekkanti@users.noreply.github.com> Date: Tue, 19 Sep 2023 11:31:09 -0700 Subject: [PATCH 09/11] more fixes --- app/namespace.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/namespace.go b/app/namespace.go index 765b08e..dddc5ac 100644 --- a/app/namespace.go +++ b/app/namespace.go @@ -1254,8 +1254,7 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl if err != nil { return err } - - return PrintProto(res.RequestStatus) + return rc.HandleRequestStatus(ctx, "create export sink", res.RequestStatus) }, }, { @@ -1305,8 +1304,8 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl if err != nil { return err } + return rc.HandleRequestStatus(ctx, "create export sink", deleteResp.RequestStatus) - return PrintProto(deleteResp.RequestStatus) }, }, { @@ -1398,8 +1397,7 @@ func NewNamespaceCommand(getNamespaceClientFn GetNamespaceClientFn, getRequestCl if err != nil { return err } - - return PrintProto(resp.RequestStatus) + return rc.HandleRequestStatus(ctx, "update export sink", resp.RequestStatus) }, }, }, From c464fb09017200db836f09622c618f6a78186869 Mon Sep 17 00:00:00 2001 From: Abhinav Nekkanti <10552725+anekkanti@users.noreply.github.com> Date: Tue, 19 Sep 2023 13:36:28 -0700 Subject: [PATCH 10/11] add alias --- app/prompt.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app/prompt.go b/app/prompt.go index c3cdfc5..98fdd3d 100644 --- a/app/prompt.go +++ b/app/prompt.go @@ -16,6 +16,7 @@ const ( var ( AutoConfirmFlag = &cli.BoolFlag{ Name: AutoConfirmFlagName, + Aliases: []string{"auto_confirm"}, Usage: "Automatically confirm all prompts", EnvVars: []string{"AUTO_CONFIRM"}, } From 4e85e54c530e36ab4c6a6d2aa165ce8ba4b9cbcd Mon Sep 17 00:00:00 2001 From: Abhinav Nekkanti <10552725+anekkanti@users.noreply.github.com> Date: Tue, 19 Sep 2023 13:43:05 -0700 Subject: [PATCH 11/11] address review comments --- app/account.go | 1 - app/request.go | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/account.go b/app/account.go index b03be9c..977b543 100644 --- a/app/account.go +++ b/app/account.go @@ -212,7 +212,6 @@ func NewAccountCommand(getAccountClientFn GetAccountClientFn, getRequestClientFn return err } return r.HandleRequestStatus(ctx, "disable metrics", status) - }, }, { diff --git a/app/request.go b/app/request.go index 9556473..f0d8410 100644 --- a/app/request.go +++ b/app/request.go @@ -141,8 +141,7 @@ loop: for { select { case <-timer.C: - return fmt.Errorf("timed out waiting for request to complete, namespace=%s, requestID=%s, timeout=%s", - ctx.String(NamespaceFlagName), + return fmt.Errorf("timed out waiting for request to complete, requestID=%s, timeout=%s", requestID, ctx.Duration(RequestTimeoutFlagName), ) @@ -163,7 +162,7 @@ loop: return fmt.Errorf("request was cancelled: %s", status.FailureReason) } if operation != "" { - fmt.Fprintf(writer, "waiting for %s operation (requestId='%s') to finish, current state: %s\n", + fmt.Fprintf(writer, "waiting for %s operation (id='%s') to finish, current state: %s\n", operation, requestID, request.State_name[int32(status.State)]) } else { fmt.Fprintf(writer, "waiting for request with id='%s' to finish, current state: %s\n",