Require a user ACK or --force flag for update commands #minor (flyteo…

…rg#423)

* WIP. Marked places where an acknowledgement before an update is needed.

Signed-off-by: Kamal Eybov <[email protected]>

* (1) Added error handling for methods fetching matchable attributes when attributes do not exist. (2) Added fetching data that is needed for diffing during updates.

Signed-off-by: Kamal Eybov <[email protected]>

* Diff and ask for ack.

Signed-off-by: Kamal Eybov <[email protected]>

* Fixed some of the TODOs.

Signed-off-by: Kamal Eybov <[email protected]>

* Cleaned up error handling.

Signed-off-by: Kamal Eybov <[email protected]>

* Updated tests.

Signed-off-by: Kamal Eybov <[email protected]>

* More tests.

Signed-off-by: Kamal Eybov <[email protected]>

* Fix case for No string to no (flyteorg#419)

Signed-off-by: Future Outlier <[email protected]>
Co-authored-by: Future Outlier <[email protected]>
Signed-off-by: Kamal Eybov <[email protected]>

* Replaced diffing implementation.

Signed-off-by: Kamal Eybov <[email protected]>

* Addressed pull request comments.

Signed-off-by: Kamal Eybov <[email protected]>

* Fixed linter errors.

Signed-off-by: Kamal Eybov <[email protected]>

---------

Signed-off-by: Kamal Eybov <[email protected]>
Signed-off-by: Future Outlier <[email protected]>
Co-authored-by: Future-Outlier <[email protected]>
Co-authored-by: Future Outlier <[email protected]>

flytectl/clierrors/errors.go

@@ -6,11 +6,11 @@ var (
 	ErrProjectNotPassed     = "project id wasn't passed\n" // #nosec
 	ErrProjectIDBothPassed  = "both project and id are passed\n"
 	ErrProjectNameNotPassed = "project name is a required flag"
-	ErrFailedProjectUpdate  = "Project %v failed to update due to %v\n"
+	ErrFailedProjectUpdate  = "Project %v failed to update due to %w\n"
 
 	ErrLPNotPassed        = "launch plan name wasn't passed\n"
 	ErrLPVersionNotPassed = "launch plan version wasn't passed\n" //nolint
-	ErrFailedLPUpdate     = "launch plan %v failed to update due to %v\n"
+	ErrFailedLPUpdate     = "launch plan %v failed to update due to %w\n"
 
 	ErrExecutionNotPassed    = "execution name wasn't passed\n"
 	ErrFailedExecutionUpdate = "execution %v failed to update due to %v\n"

flytectl/cmd/config/subcommand/clusterresourceattribute/update_config.go

@@ -6,6 +6,7 @@ package clusterresourceattribute
 type AttrUpdateConfig struct {
 	AttrFile string `json:"attrFile" pflag:",attribute file name to be used for updating attribute for the resource type."`
 	DryRun   bool   `json:"dryRun" pflag:",execute command without making any modifications."`
+	Force    bool   `json:"force" pflag:",do not ask for an acknowledgement during updates."`
 }
 
 var DefaultUpdateConfig = &AttrUpdateConfig{}

flytectl/cmd/config/subcommand/execution/update_config.go

@@ -10,4 +10,5 @@ type UpdateConfig struct {
 	Archive  bool `json:"archive" pflag:",archive execution."`
 	Activate bool `json:"activate" pflag:",activate execution."`
 	DryRun   bool `json:"dryRun" pflag:",execute command without making any modifications."`
+	Force    bool `json:"force" pflag:",do not ask for an acknowledgement during updates."`
 }

flytectl/cmd/config/subcommand/executionclusterlabel/update_config.go

@@ -6,6 +6,7 @@ package executionclusterlabel
 type AttrUpdateConfig struct {
 	AttrFile string `json:"attrFile" pflag:",attribute file name to be used for updating attribute for the resource type."`
 	DryRun   bool   `json:"dryRun" pflag:",execute command without making any modifications."`
+	Force    bool   `json:"force" pflag:",do not ask for an acknowledgement during updates."`
 }
 
 var DefaultUpdateConfig = &AttrUpdateConfig{}

flytectl/cmd/config/subcommand/executionqueueattribute/update_config.go

@@ -6,6 +6,7 @@ package executionqueueattribute
 type AttrUpdateConfig struct {
 	AttrFile string `json:"attrFile" pflag:",attribute file name to be used for updating attribute for the resource type."`
 	DryRun   bool   `json:"dryRun" pflag:",execute command without making any modifications."`
+	Force    bool   `json:"force" pflag:",do not ask for an acknowledgement during updates."`
 }
 
 var DefaultUpdateConfig = &AttrUpdateConfig{}

flytectl/cmd/config/subcommand/launchplan/updateconfig.go

@@ -10,5 +10,6 @@ type UpdateConfig struct {
 	Archive  bool   `json:"archive" pflag:",disable the launch plan schedule (if it has an active schedule associated with it)."`
 	Activate bool   `json:"activate" pflag:",activate launchplan."`
 	DryRun   bool   `json:"dryRun" pflag:",execute command without making any modifications."`
+	Force    bool   `json:"force" pflag:",do not ask for an acknowledgement during updates."`
 	Version  string `json:"version" pflag:",version of the launchplan to be fetched."`
 }

flytectl/cmd/config/subcommand/plugin_override/update_config.go

@@ -6,6 +6,7 @@ package pluginoverride
 type AttrUpdateConfig struct {
 	AttrFile string `json:"attrFile" pflag:",attribute file name to be used for updating attribute for the resource type."`
 	DryRun   bool   `json:"dryRun" pflag:",execute command without making any modifications."`
+	Force    bool   `json:"force" pflag:",do not ask for an acknowledgement during updates."`
 }
 
 var DefaultUpdateConfig = &AttrUpdateConfig{}

flytectl/cmd/config/subcommand/project/project_config.go

@@ -33,6 +33,7 @@ type ConfigProject struct {
 	ArchiveProject  bool              `json:"archiveProject" pflag:",(Deprecated) Archives the project specified as argument. Only used in update"`
 	Activate        bool              `json:"activate" pflag:",Activates the project specified as argument. Only used in update"`
 	Archive         bool              `json:"archive" pflag:",Archives the project specified as argument. Only used in update"`
+	Force           bool              `json:"force" pflag:",Skips asking for an acknowledgement during an update operation. Only used in update"`
 	Name            string            `json:"name" pflag:",name for the project specified as argument."`
 	DryRun          bool              `json:"dryRun" pflag:",execute command without making any modifications."`
 	Description     string            `json:"description" pflag:",description for the project specified as argument."`

flytectl/cmd/config/subcommand/taskresourceattribute/update_config.go

@@ -6,6 +6,7 @@ package taskresourceattribute
 type AttrUpdateConfig struct {
 	AttrFile string `json:"attrFile" pflag:",attribute file name to be used for updating attribute for the resource type."`
 	DryRun   bool   `json:"dryRun" pflag:",execute command without making any modifications."`
+	Force    bool   `json:"force" pflag:",do not ask for an acknowledgement during updates."`
 }
 
 var DefaultUpdateConfig = &AttrUpdateConfig{}

flytectl/cmd/config/subcommand/workflowexecutionconfig/update_config.go

@@ -6,6 +6,7 @@ package workflowexecutionconfig
 type AttrUpdateConfig struct {
 	AttrFile string `json:"attrFile" pflag:",attribute file name to be used for updating attribute for the resource type."`
 	DryRun   bool   `json:"dryRun" pflag:",execute command without making any modifications."`
+	Force    bool   `json:"force" pflag:",do not ask for an acknowledgement during updates."`
 }
 
 var DefaultUpdateConfig = &AttrUpdateConfig{}

flytectl/cmd/testutils/test_utils.go

@@ -6,20 +6,19 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"math/rand"
 	"os"
 	"regexp"
 	"strings"
 	"testing"
 
-	"github.com/flyteorg/flyteidl/clients/go/admin/mocks"
-
-	"github.com/flyteorg/flyteidl/clients/go/admin"
-
 	"github.com/stretchr/testify/assert"
 
 	"github.com/flyteorg/flytectl/cmd/config"
 	cmdCore "github.com/flyteorg/flytectl/cmd/core"
 	extMocks "github.com/flyteorg/flytectl/pkg/ext/mocks"
+	"github.com/flyteorg/flyteidl/clients/go/admin"
+	"github.com/flyteorg/flyteidl/clients/go/admin/mocks"
 )
 
 const projectValue = "dummyProject"
@@ -112,6 +111,34 @@ func (s *TestStruct) TearDownAndVerify(t *testing.T, expectedLog string) {
 	assert.Equal(t, sanitizeString(expectedLog), sanitizeString(buf.String()))
 }
 
+func (s *TestStruct) TearDownAndVerifyContains(t *testing.T, expectedLog string) {
+	if err := s.Writer.Close(); err != nil {
+		panic(fmt.Errorf("could not close test context writer: %w", err))
+	}
+
+	var buf bytes.Buffer
+	if _, err := io.Copy(&buf, s.Reader); err != nil {
+		panic(fmt.Errorf("could not read from test context reader: %w", err))
+	}
+
+	assert.Contains(t, sanitizeString(buf.String()), sanitizeString(expectedLog))
+}
+
+// RandomName returns a string composed of random lowercase English letters of specified length.
+func RandomName(length int) string {
+	if length < 0 {
+		panic("length should be a non-negative number")
+	}
+
+	var b strings.Builder
+	for i := 0; i < length; i++ {
+		c := rune('a' + rand.Intn('z'-'a')) // #nosec G404 - we use this function for testing only, do not need a cryptographically secure random number generator
+		b.WriteRune(c)
+	}
+
+	return b.String()
+}
+
 func sanitizeString(str string) string {
 	// Not the most comprehensive ANSI pattern, but this should capture common color operations
 	// such as \x1b[107;0m and \x1b[0m. Expand if needed (insert regex 2 problems joke here).