Enable lll, nakedret, asciicheck, depguard, dogsled, dupl, exportloop…

…ref golang linters (#2238)

* Enable gocyclo linter

* Enable gocritic linter

* Fix typo for linters-settings, fix gocyclo lint failures

* Add lll linter

* Add nakedret linter

* Enable dupl linter

* Enable exportloopref linter

* Update new lint misses, add TODO for lint disables

* Apply linter to new changes

* Fix merge issues

* Add explanation for ignoring dupl linter

* Address review comments

* Address review comments - move args

* Temporarily disable depguard linter, lint fix

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

.golangci.yml

@@ -14,6 +14,13 @@ linters:
     - unparam
     - gocyclo
     - gocritic
+    - lll
+    - nakedret
+    - asciicheck
+    # - depguard # Disabling temporarily due to https://github.com/golangci/golangci-lint/issues/3906
+    - dogsled
+    - dupl
+    - exportloopref
 
 run:
   timeout: 10m                          # golangci-lint run's timeout.
@@ -27,6 +34,8 @@ issues:
       linters:
         - errcheck                      # Errors may be ignored in tests.
         - unparam                       # Tests might have unused function parameters.
+        - lll
+        - dupl
 
     - text: "`ctx` is unused"           # Context might not be in use in places, but for consistency, we pass it.
       linters:
@@ -35,3 +44,7 @@ issues:
 linters-settings:
   gocyclo:
     min-complexity: 20
+  lll:
+    line-length: 240
+  nakedret:
+    max-func-lines: 2

pkg/app/cassandra.go

@@ -158,7 +158,9 @@ func (cas *CassandraInstance) Ping(ctx context.Context) error {
 // Insert is used to insert the  records into the database
 func (cas *CassandraInstance) Insert(ctx context.Context) error {
 	log.Print("Inserting records into the database.", field.M{"app": cas.name})
-	insertCMD := []string{"sh", "-c", fmt.Sprintf("cqlsh -u cassandra -p $CASSANDRA_PASSWORD -e \"insert into restaurants.guests (id, firstname, lastname, birthday)  values (uuid(), 'Tom', 'Singh', '2015-02-18');\" --request-timeout=%s", cqlTimeout)}
+	insertCMD := []string{"sh", "-c", fmt.Sprintf("cqlsh -u cassandra -p $CASSANDRA_PASSWORD -e \"insert into "+
+		"restaurants.guests (id, firstname, lastname, birthday)  values (uuid(), 'Tom', 'Singh', "+
+		"'2015-02-18');\" --request-timeout=%s", cqlTimeout)}
 	_, stderr, err := cas.execCommand(ctx, insertCMD)
 	if err != nil {
 		return errors.Wrapf(err, "Error %s inserting records into the database.", stderr)
@@ -191,7 +193,8 @@ func (cas *CassandraInstance) Count(ctx context.Context) (int, error) {
 // Reset is used to reset or imitate disaster, in the database
 func (cas *CassandraInstance) Reset(ctx context.Context) error {
 	// delete keyspace and table if they already exist
-	delRes := []string{"sh", "-c", fmt.Sprintf("cqlsh -u cassandra -p $CASSANDRA_PASSWORD -e 'drop table if exists restaurants.guests; drop keyspace if exists restaurants;' --request-timeout=%s", cqlTimeout)}
+	delRes := []string{"sh", "-c", fmt.Sprintf("cqlsh -u cassandra -p $CASSANDRA_PASSWORD -e "+
+		"'drop table if exists restaurants.guests; drop keyspace if exists restaurants;' --request-timeout=%s", cqlTimeout)}
 	_, stderr, err := cas.execCommand(ctx, delRes)
 	if err != nil {
 		return errors.Wrapf(err, "Error %s, deleting resources while reseting application.", stderr)
@@ -202,14 +205,16 @@ func (cas *CassandraInstance) Reset(ctx context.Context) error {
 // Initialize is used to initialize the database or create schema
 func (cas *CassandraInstance) Initialize(ctx context.Context) error {
 	// create the keyspace
-	createKS := []string{"sh", "-c", fmt.Sprintf("cqlsh -u cassandra -p $CASSANDRA_PASSWORD -e \"create keyspace restaurants with replication  = {'class':'SimpleStrategy', 'replication_factor': 3};\" --request-timeout=%s", cqlTimeout)}
+	createKS := []string{"sh", "-c", fmt.Sprintf("cqlsh -u cassandra -p $CASSANDRA_PASSWORD -e \"create keyspace "+
+		"restaurants with replication  = {'class':'SimpleStrategy', 'replication_factor': 3};\" --request-timeout=%s", cqlTimeout)}
 	_, stderr, err := cas.execCommand(ctx, createKS)
 	if err != nil {
 		return errors.Wrapf(err, "Error %s while creating the keyspace for application.", stderr)
 	}
 
 	// create the table
-	createTab := []string{"sh", "-c", fmt.Sprintf("cqlsh -u cassandra -p $CASSANDRA_PASSWORD -e \"create table restaurants.guests (id UUID primary key, firstname text, lastname text, birthday timestamp);\" --request-timeout=%s", cqlTimeout)}
+	createTab := []string{"sh", "-c", fmt.Sprintf("cqlsh -u cassandra -p $CASSANDRA_PASSWORD -e \"create table "+
+		"restaurants.guests (id UUID primary key, firstname text, lastname text, birthday timestamp);\" --request-timeout=%s", cqlTimeout)}
 	_, stderr, err = cas.execCommand(ctx, createTab)
 	if err != nil {
 		return errors.Wrapf(err, "Error %s creating table.", stderr)

pkg/app/cockroachdb.go

@@ -57,7 +57,7 @@ func (c *CockroachDB) Init(context.Context) error {
 	return err
 }
 
-func (c *CockroachDB) Install(ctx context.Context, namespace string) error {
+func (c *CockroachDB) Install(ctx context.Context, namespace string) error { //nolint:dupl // Not a duplicate, common code already extracted
 	log.Info().Print("Installing cockroachdb cluster helm chart.", field.M{"app": c.name})
 	c.namespace = namespace
 

pkg/app/couchbase.go

@@ -80,7 +80,7 @@ func (cb *CouchbaseDB) Init(ctx context.Context) error {
 	return err
 }
 
-func (cb *CouchbaseDB) Install(ctx context.Context, ns string) error {
+func (cb *CouchbaseDB) Install(ctx context.Context, ns string) error { //nolint:dupl // Not a duplicate, common code already extracted
 	log.Info().Print("Installing couchbase operator and cluster helm chart.", field.M{"app": cb.name})
 	cb.namespace = ns
 

pkg/app/mariadb.go

@@ -74,7 +74,7 @@ func (m *MariaDB) Init(context.Context) error {
 	return nil
 }
 
-func (m *MariaDB) Install(ctx context.Context, namespace string) error {
+func (m *MariaDB) Install(ctx context.Context, namespace string) error { //nolint:dupl // Not a duplicate, common code already extracted
 	m.namespace = namespace
 	cli, err := helm.NewCliClient()
 	if err != nil {
@@ -204,7 +204,10 @@ func (m *MariaDB) Reset(ctx context.Context) error {
 
 func (m *MariaDB) Initialize(ctx context.Context) error {
 	// create the database and a pets table
-	createTableCMD := []string{"sh", "-c", "mysql -u root --password=$MARIADB_ROOT_PASSWORD -e 'create database testdb; use testdb;  CREATE TABLE pets (name VARCHAR(20), owner VARCHAR(20), species VARCHAR(20), sex CHAR(1), birth DATE, death DATE);'"}
+	createTableCMD := []string{"sh", "-c", "mysql -u root --password=$MARIADB_ROOT_PASSWORD " +
+		"-e 'create database testdb; use testdb; " +
+		"CREATE TABLE pets (name VARCHAR(20), owner VARCHAR(20), species VARCHAR(20), sex CHAR(1), " +
+		"birth DATE, death DATE);'"}
 	_, stderr, err := m.execCommand(ctx, createTableCMD)
 	if err != nil {
 		return errors.Wrapf(err, "Error while creating the maria table: %s", stderr)

pkg/app/mongodb-deploymentconfig.go

@@ -134,7 +134,9 @@ func (mongo *MongoDBDepConfig) Ping(ctx context.Context) error {
 
 func (mongo *MongoDBDepConfig) Insert(ctx context.Context) error {
 	log.Print("Inserting documents into collection.", field.M{"app": mongo.name})
-	insertCMD := []string{"bash", "-c", fmt.Sprintf("mongo admin --authenticationDatabase admin -u %s -p $MONGODB_ADMIN_PASSWORD --quiet --eval \"db.restaurants.insert({'_id': '%s','name' : 'Tom', 'cuisine' : 'Hawaiian', 'id' : '8675309'})\"", mongo.user, uuid.New())}
+	insertCMD := []string{"bash", "-c", fmt.Sprintf("mongo admin --authenticationDatabase admin -u %s -p "+
+		"$MONGODB_ADMIN_PASSWORD --quiet --eval \"db.restaurants.insert({'_id': '%s','name' : 'Tom', "+
+		"'cuisine' : 'Hawaiian', 'id' : '8675309'})\"", mongo.user, uuid.New())}
 	_, stderr, err := mongo.execCommand(ctx, insertCMD)
 	if err != nil {
 		return errors.Wrapf(err, "Error %s while inserting data data into mongodb collection.", stderr)

pkg/app/mongodb.go

@@ -180,7 +180,9 @@ func (mongo *MongoDB) Ping(ctx context.Context) error {
 
 func (mongo *MongoDB) Insert(ctx context.Context) error {
 	log.Print("Inserting documents into collection.", field.M{"app": mongo.name})
-	insertCMD := []string{"sh", "-c", fmt.Sprintf("mongosh admin --authenticationDatabase admin -u %s -p $MONGODB_ROOT_PASSWORD --quiet --eval \"db.restaurants.insertOne({'_id': '%s','name' : 'Tom', 'cuisine' : 'Hawaiian', 'id' : '8675309'})\"", mongo.username, uuid.New())}
+	insertCMD := []string{"sh", "-c", fmt.Sprintf("mongosh admin --authenticationDatabase admin -u %s -p "+
+		"$MONGODB_ROOT_PASSWORD --quiet --eval \"db.restaurants.insertOne({'_id': '%s','name' : 'Tom', "+
+		"'cuisine' : 'Hawaiian', 'id' : '8675309'})\"", mongo.username, uuid.New())}
 	_, stderr, err := mongo.execCommand(ctx, insertCMD)
 	if err != nil {
 		return errors.Wrapf(err, "Error %s while inserting data data into mongodb collection.", stderr)

pkg/app/mysql.go

@@ -83,7 +83,7 @@ func (mdb *MysqlDB) Init(ctx context.Context) error {
 	return nil
 }
 
-func (mdb *MysqlDB) Install(ctx context.Context, namespace string) error {
+func (mdb *MysqlDB) Install(ctx context.Context, namespace string) error { //nolint:dupl // Not a duplicate, common code already extracted
 	mdb.namespace = namespace
 	cli, err := helm.NewCliClient()
 	if err != nil {
@@ -215,7 +215,9 @@ func (mdb *MysqlDB) Reset(ctx context.Context) error {
 // Initialize is used to initialize the database or create schema
 func (mdb *MysqlDB) Initialize(ctx context.Context) error {
 	// create the database and a pets table
-	createTableCMD := []string{"sh", "-c", "mysql -u root --password=$MYSQL_ROOT_PASSWORD -e 'create database testdb; use testdb;  CREATE TABLE pets (name VARCHAR(20), owner VARCHAR(20), species VARCHAR(20), sex CHAR(1), birth DATE, death DATE);'"}
+	createTableCMD := []string{"sh", "-c", "mysql -u root --password=$MYSQL_ROOT_PASSWORD -e " +
+		"'create database testdb; use testdb;  CREATE TABLE pets (name VARCHAR(20), owner VARCHAR(20), " +
+		"species VARCHAR(20), sex CHAR(1), birth DATE, death DATE);'"}
 	_, stderr, err := mdb.execCommand(ctx, createTableCMD)
 	if err != nil {
 		return errors.Wrapf(err, "Error while creating the mysql table: %s", stderr)

pkg/aws/rds/rds.go

@@ -48,7 +48,19 @@ func NewClient(ctx context.Context, awsConfig *aws.Config, region string) (*RDS,
 }
 
 // CreateDBInstanceWithContext
-func (r RDS) CreateDBInstance(ctx context.Context, storage *int64, instanceClass, instanceID, engine, username, password string, sgIDs []string, publicAccess *bool, restoredClusterID *string, dbSubnetGroup string) (*rds.CreateDBInstanceOutput, error) {
+func (r RDS) CreateDBInstance(
+	ctx context.Context,
+	storage *int64,
+	instanceClass,
+	instanceID,
+	engine,
+	username,
+	password string,
+	sgIDs []string,
+	publicAccess *bool,
+	restoredClusterID *string,
+	dbSubnetGroup string,
+) (*rds.CreateDBInstanceOutput, error) {
 	dbi := &rds.CreateDBInstanceInput{
 		DBInstanceClass:      &instanceClass,
 		DBInstanceIdentifier: &instanceID,
@@ -74,7 +86,18 @@ func (r RDS) CreateDBInstance(ctx context.Context, storage *int64, instanceClass
 	return r.CreateDBInstanceWithContext(ctx, dbi)
 }
 
-func (r RDS) CreateDBCluster(ctx context.Context, storage int64, instanceClass, instanceID, dbSubnetGroup, engine, dbName, username, password string, sgIDs []string) (*rds.CreateDBClusterOutput, error) {
+func (r RDS) CreateDBCluster(
+	ctx context.Context,
+	storage int64,
+	instanceClass,
+	instanceID,
+	dbSubnetGroup,
+	engine,
+	dbName,
+	username,
+	password string,
+	sgIDs []string,
+) (*rds.CreateDBClusterOutput, error) {
 	dbi := &rds.CreateDBClusterInput{
 		DBClusterIdentifier: &instanceID,
 		DatabaseName:        &dbName,
@@ -319,7 +342,8 @@ func (r RDS) RestoreDBClusterFromDBSnapshot(ctx context.Context, instanceID, dbS
 func convertSGIDs(sgIDs []string) []*string {
 	var refSGIDs []*string
 	for _, ID := range sgIDs {
-		refSGIDs = append(refSGIDs, &ID)
+		idPtr := &ID
+		refSGIDs = append(refSGIDs, idPtr)
 	}
 	return refSGIDs
 }

pkg/blockstorage/vmware/vmware.go

@@ -357,10 +357,14 @@ func (p *FcdProvider) SnapshotDelete(ctx context.Context, snapshot *blockstorage
 			if soap.IsVimFault(lerr) {
 				switch soap.ToVimFault(lerr).(type) {
 				case *types.InvalidArgument:
-					log.Error().WithError(lerr).Print("Disk doesn't have given snapshot due to the snapshot stamp being removed in the previous DeleteSnapshot operation which failed with an InvalidState fault. It will be resolved by the next snapshot operation on the same VM. Will NOT retry")
+					log.Error().WithError(lerr).Print("Disk doesn't have given snapshot due to the snapshot stamp being " +
+						"removed in the previous DeleteSnapshot operation which failed with an InvalidState fault. It " +
+						"will be resolved by the next snapshot operation on the same VM. Will NOT retry")
 					return true, nil
 				case *types.NotFound:
-					log.Error().WithError(lerr).Print("There is a temporary catalog mismatch due to a race condition with one another concurrent DeleteSnapshot operation. It will be resolved by the next consolidateDisks operation on the same VM. Will NOT retry")
+					log.Error().WithError(lerr).Print("There is a temporary catalog mismatch due to a race condition with " +
+						"one another concurrent DeleteSnapshot operation. It will be resolved by the next " +
+						"consolidateDisks operation on the same VM. Will NOT retry")
 					return true, nil
 				case *types.InvalidState:
 					log.Error().WithError(lerr).Print("There is some operation, other than this DeleteSnapshot invocation, on the same VM still being protected by its VM state. Will retry")

pkg/chronicle/chronicle_push.go

@@ -142,23 +142,21 @@ func readArtifactPathFile(path string) (string, error) {
 	return t, errors.Wrap(err, "Could not read artifact path file")
 }
 
-func readProfile(path string) (p param.Profile, ok bool, err error) {
+func readProfile(path string) (param.Profile, bool, error) {
 	var buf []byte
-	buf, err = os.ReadFile(path)
+	buf, err := os.ReadFile(path)
+	var p param.Profile
 	switch {
 	case os.IsNotExist(err):
 		err = nil
-		return
+		return p, false, err
 	case err != nil:
-		err = errors.Wrap(err, "Failed to read profile")
-		return
+		return p, false, errors.Wrap(err, "Failed to read profile")
 	}
 	if err = json.Unmarshal(buf, &p); err != nil {
-		err = errors.Wrap(err, "Failed to unmarshal profile")
-	} else {
-		ok = true
+		return p, false, errors.Wrap(err, "Failed to unmarshal profile")
 	}
-	return
+	return p, true, nil
 }
 
 func writeProfile(path string, p param.Profile) error {

pkg/function/export_rds_snapshot_location.go

@@ -81,7 +81,20 @@ func (*exportRDSSnapshotToLocationFunc) Name() string {
 	return ExportRDSSnapshotToLocFuncName
 }
 
-func exportRDSSnapshotToLoc(ctx context.Context, namespace, instanceID, snapshotID, username, password string, databases []string, dbSubnetGroup, backupPrefix string, dbEngine RDSDBEngine, sgIDs []string, profile *param.Profile) (map[string]interface{}, error) {
+func exportRDSSnapshotToLoc(
+	ctx context.Context,
+	namespace,
+	instanceID,
+	snapshotID,
+	username,
+	password string,
+	databases []string,
+	dbSubnetGroup,
+	backupPrefix string,
+	dbEngine RDSDBEngine,
+	sgIDs []string,
+	profile *param.Profile,
+) (map[string]interface{}, error) {
 	// Validate profilextractDumpFromDBe
 	if err := ValidateProfile(profile); err != nil {
 		return nil, errors.Wrap(err, "Profile Validation failed")
@@ -234,7 +247,20 @@ func (d *exportRDSSnapshotToLocationFunc) ExecutionProgress() (crv1alpha1.PhaseP
 	}, nil
 }
 
-func execDumpCommand(ctx context.Context, dbEngine RDSDBEngine, action RDSAction, namespace, dbEndpoint, username, password string, databases []string, backupPrefix, backupID string, profile *param.Profile, dbEngineVersion string) (map[string]interface{}, error) {
+func execDumpCommand(
+	ctx context.Context,
+	dbEngine RDSDBEngine,
+	action RDSAction,
+	namespace,
+	dbEndpoint,
+	username,
+	password string,
+	databases []string,
+	backupPrefix,
+	backupID string,
+	profile *param.Profile,
+	dbEngineVersion string,
+) (map[string]interface{}, error) {
 	// Trim "\n" from creds
 	username = strings.TrimSpace(username)
 	password = strings.TrimSpace(password)

pkg/function/restore_rds_snapshot.go

@@ -164,7 +164,20 @@ func (r *restoreRDSSnapshotFunc) ExecutionProgress() (crv1alpha1.PhaseProgress,
 	}, nil
 }
 
-func restoreRDSSnapshot(ctx context.Context, namespace, instanceID, subnetGroup, snapshotID, backupArtifactPrefix, backupID, username, password string, dbEngine RDSDBEngine, sgIDs []string, profile *param.Profile) (map[string]interface{}, error) {
+func restoreRDSSnapshot(
+	ctx context.Context,
+	namespace,
+	instanceID,
+	subnetGroup,
+	snapshotID,
+	backupArtifactPrefix,
+	backupID,
+	username,
+	password string,
+	dbEngine RDSDBEngine,
+	sgIDs []string,
+	profile *param.Profile,
+) (map[string]interface{}, error) {
 	// Validate profile
 	if err := ValidateProfile(profile); err != nil {
 		return nil, errors.Wrap(err, "Error validating profile")
@@ -324,7 +337,19 @@ func restoreAuroraFromSnapshot(ctx context.Context, rdsCli *rds.RDS, instanceID,
 
 	log.WithContext(ctx).Print("Creating DB instance in the cluster")
 	// After Aurora cluster is created, we will have to explictly create the DB instance
-	dbInsOp, err := rdsCli.CreateDBInstance(ctx, nil, defaultAuroraInstanceClass, fmt.Sprintf("%s-%s", *op.DBCluster.DBClusterIdentifier, restoredAuroraInstanceSuffix), dbEngine, "", "", nil, nil, aws.String(*op.DBCluster.DBClusterIdentifier), subnetGroup)
+	dbInsOp, err := rdsCli.CreateDBInstance(
+		ctx,
+		nil,
+		defaultAuroraInstanceClass,
+		fmt.Sprintf("%s-%s", *op.DBCluster.DBClusterIdentifier, restoredAuroraInstanceSuffix),
+		dbEngine,
+		"",
+		"",
+		nil,
+		nil,
+		aws.String(*op.DBCluster.DBClusterIdentifier),
+		subnetGroup,
+	)
 	if err != nil {
 		return errors.Wrap(err, "Error while creating Aurora DB instance in the cluster.")
 	}

pkg/function/scale_test.go

@@ -315,15 +315,16 @@ func (s *ScaleSuite) TestGetArgs(c *C) {
 			check:            IsNil,
 		},
 	} {
-		namespace, kind, name, replicas, waitForReady, err := getArgs(tc.tp, tc.args)
+		s := scaleWorkloadFunc{}
+		err := s.setArgs(tc.tp, tc.args)
 		c.Assert(err, tc.check)
 		if err != nil {
 			continue
 		}
-		c.Assert(namespace, Equals, tc.wantNamespace)
-		c.Assert(name, Equals, tc.wantName)
-		c.Assert(kind, Equals, tc.wantKind)
-		c.Assert(replicas, Equals, tc.wantReplicas)
-		c.Assert(waitForReady, Equals, tc.wantWaitForReady)
+		c.Assert(s.namespace, Equals, tc.wantNamespace)
+		c.Assert(s.name, Equals, tc.wantName)
+		c.Assert(s.kind, Equals, tc.wantKind)
+		c.Assert(s.replicas, Equals, tc.wantReplicas)
+		c.Assert(s.waitForReady, Equals, tc.wantWaitForReady)
 	}
 }