Enables default lint checks, fixes lint and bugs (argoproj#1330)

.golangci.yml

@@ -19,4 +19,3 @@ linters:
     - ineffassign
     - unconvert
     - misspell
-  disable-all: true

cmd/argocd-util/main.go

@@ -10,8 +10,6 @@ import (
 	"os/exec"
 	"syscall"
 
-	"github.com/argoproj/argo-cd/common"
-	"github.com/argoproj/argo-cd/util"
 	"github.com/ghodss/yaml"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
@@ -25,6 +23,9 @@ import (
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 
+	"github.com/argoproj/argo-cd/common"
+	"github.com/argoproj/argo-cd/util"
+
 	"github.com/argoproj/argo-cd/errors"
 	"github.com/argoproj/argo-cd/util/cli"
 	"github.com/argoproj/argo-cd/util/db"
@@ -188,7 +189,7 @@ func NewGenDexConfigCommand() *cobra.Command {
 				errors.CheckError(err)
 				maskedDexCfgBytes, err := yaml.Marshal(dexCfg)
 				errors.CheckError(err)
-				fmt.Printf(string(maskedDexCfgBytes))
+				fmt.Print(string(maskedDexCfgBytes))
 			} else {
 				err = ioutil.WriteFile(out, dexCfgBytes, 0644)
 				errors.CheckError(err)

cmd/argocd/commands/app.go

@@ -790,7 +790,7 @@ func printDiff(name string, live *unstructured.Unstructured, target *unstructure
 	tempDir, err := ioutil.TempDir("", "argocd-diff")
 	errors.CheckError(err)
 
-	targetFile := path.Join(tempDir, fmt.Sprintf("%s", name))
+	targetFile := path.Join(tempDir, name)
 	targetData := []byte("")
 	if target != nil {
 		targetData, err = yaml.Marshal(target)

cmd/argocd/commands/login.go

@@ -233,7 +233,7 @@ func oauth2Login(ctx context.Context, port int, oauth2conf *oauth2.Config, provi
 		<p style="margin-top:20px; font-size:18; text-align:center">Authentication was successful, you can now return to CLI. This page will close automatically</p>
 		<script>window.onload=function(){setTimeout(this.close, 4000)}</script>
 		`
-		fmt.Fprintf(w, successPage)
+		fmt.Fprint(w, successPage)
 		completionChan <- ""
 	}
 	srv := &http.Server{Addr: "localhost:" + strconv.Itoa(port)}

common/installer.go

@@ -138,9 +138,8 @@ func CreateClusterRoleBinding(
 // InstallClusterManagerRBAC installs RBAC resources for a cluster manager to operate a cluster. Returns a token
 func InstallClusterManagerRBAC(clientset kubernetes.Interface) (string, error) {
 	const ns = "kube-system"
-	var err error
 
-	err = CreateServiceAccount(clientset, ArgoCDManagerServiceAccount, ns)
+	err := CreateServiceAccount(clientset, ArgoCDManagerServiceAccount, ns)
 	if err != nil {
 		return "", err
 	}

controller/appcontroller.go

@@ -11,8 +11,6 @@ import (
 	"time"
 
 	log "github.com/sirupsen/logrus"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 	v1 "k8s.io/api/core/v1"
 	apierr "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -127,21 +125,6 @@ func NewApplicationController(
 	return &ctrl, nil
 }
 
-func (ctrl *ApplicationController) getApp(name string) (*appv1.Application, error) {
-	obj, exists, err := ctrl.appInformer.GetStore().GetByKey(fmt.Sprintf("%s/%s", ctrl.namespace, name))
-	if err != nil {
-		return nil, err
-	}
-	if !exists {
-		return nil, status.Error(codes.NotFound, fmt.Sprintf("unable to find application with name %s", name))
-	}
-	a, ok := (obj).(*appv1.Application)
-	if !ok {
-		return nil, status.Errorf(codes.Internal, fmt.Sprintf("unexpected object type in app informer"))
-	}
-	return a, nil
-}
-
 func (ctrl *ApplicationController) setAppManagedResources(a *appv1.Application, comparisonResult *comparisonResult) error {
 	managedResources, err := ctrl.managedResources(a, comparisonResult)
 	if err != nil {
@@ -594,7 +577,7 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo
 
 	startTime := time.Now()
 	defer func() {
-		reconcileDuration := time.Now().Sub(startTime)
+		reconcileDuration := time.Since(startTime)
 		ctrl.metricsServer.IncReconcile(origApp, reconcileDuration)
 		logCtx := log.WithFields(log.Fields{"application": origApp.Name, "time_ms": reconcileDuration.Seconds() * 1e3, "full": fullRefresh})
 		logCtx.Info("Reconciliation completed")
@@ -871,10 +854,7 @@ func alreadyAttemptedSync(app *appv1.Application, commitSHA string) bool {
 	specSource.TargetRevision = ""
 	syncResSource := app.Status.OperationState.SyncResult.Source.DeepCopy()
 	syncResSource.TargetRevision = ""
-	if !reflect.DeepEqual(app.Spec.Source, app.Status.OperationState.SyncResult.Source) {
-		return false
-	}
-	return true
+	return reflect.DeepEqual(app.Spec.Source, app.Status.OperationState.SyncResult.Source)
 }
 
 func (ctrl *ApplicationController) newApplicationInformerAndLister() (cache.SharedIndexInformer, applisters.ApplicationLister) {

controller/cache/cache.go

@@ -64,14 +64,6 @@ type liveStateCache struct {
 	settings     *settings.ArgoCDSettings
 }
 
-func (c *liveStateCache) processEvent(event watch.EventType, obj *unstructured.Unstructured, url string) error {
-	info, err := c.getSyncedCluster(url)
-	if err != nil {
-		return err
-	}
-	return info.processEvent(event, obj)
-}
-
 func (c *liveStateCache) getCluster(server string) (*clusterInfo, error) {
 	c.lock.Lock()
 	defer c.lock.Unlock()

controller/sync.go

@@ -329,10 +329,7 @@ func (sc *syncContext) generateSyncTasks() ([]syncTask, bool) {
 // startedPreSyncPhase detects if we already started the PreSync stage of a sync operation.
 // This is equal to if we have anything in our resource or hook list
 func (sc *syncContext) startedPreSyncPhase() bool {
-	if len(sc.syncRes.Resources) > 0 {
-		return true
-	}
-	return false
+	return len(sc.syncRes.Resources) > 0
 }
 
 // startedSyncPhase detects if we have already started the Sync stage of a sync operation.
@@ -464,8 +461,7 @@ func (sc *syncContext) doApplySync(syncTasks []syncTask, dryRun, force, update b
 		wg.Add(1)
 		go func(t syncTask) {
 			defer wg.Done()
-			var resDetails appv1.ResourceResult
-			resDetails = sc.pruneObject(t.liveObj, sc.syncOp.Prune, dryRun)
+			resDetails := sc.pruneObject(t.liveObj, sc.syncOp.Prune, dryRun)
 			if !resDetails.Status.Successful() {
 				syncSuccessful = false
 			}

pkg/apiclient/grpcproxy.go

@@ -128,10 +128,8 @@ func (c *client) startGRPCProxy() (*grpc.Server, net.Listener, error) {
 			}
 
 			go func() {
-				select {
-				case <-stream.Context().Done():
-					util.Close(resp.Body)
-				}
+				<-stream.Context().Done()
+				util.Close(resp.Body)
 			}()
 			defer util.Close(resp.Body)
 

server/application/application_test.go

@@ -136,7 +136,7 @@ func newTestAppServer(objects ...runtime.Object) *Server {
 	fakeProjLister := factory.Argoproj().V1alpha1().AppProjects().Lister().AppProjects(testNamespace)
 
 	enforcer := rbac.NewEnforcer(kubeclientset, testNamespace, common.ArgoCDRBACConfigMapName, nil)
-	enforcer.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
+	_ = enforcer.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
 	enforcer.SetDefaultRole("role:admin")
 	enforcer.SetClaimsEnforcerFunc(rbacpolicy.NewRBACPolicyEnforcer(enforcer, fakeProjLister).EnforceClaims)
 
@@ -337,7 +337,7 @@ func TestUpdateAppProject(t *testing.T) {
 	appServer.enf.SetDefaultRole("")
 
 	// Verify normal update works (without changing project)
-	appServer.enf.SetBuiltinPolicy(`p, admin, applications, update, default/test-app, allow`)
+	_ = appServer.enf.SetBuiltinPolicy(`p, admin, applications, update, default/test-app, allow`)
 	_, err := appServer.Update(ctx, &ApplicationUpdateRequest{Application: testApp})
 	assert.NoError(t, err)
 
@@ -347,31 +347,31 @@ func TestUpdateAppProject(t *testing.T) {
 	assert.Equal(t, status.Code(err), codes.PermissionDenied)
 
 	// Verify inability to change projects without create privileges in new project
-	appServer.enf.SetBuiltinPolicy(`
+	_ = appServer.enf.SetBuiltinPolicy(`
 p, admin, applications, update, default/test-app, allow
 p, admin, applications, update, my-proj/test-app, allow
 `)
 	_, err = appServer.Update(ctx, &ApplicationUpdateRequest{Application: testApp})
 	assert.Equal(t, status.Code(err), codes.PermissionDenied)
 
 	// Verify inability to change projects without update privileges in new project
-	appServer.enf.SetBuiltinPolicy(`
+	_ = appServer.enf.SetBuiltinPolicy(`
 p, admin, applications, update, default/test-app, allow
 p, admin, applications, create, my-proj/test-app, allow
 `)
 	_, err = appServer.Update(ctx, &ApplicationUpdateRequest{Application: testApp})
 	assert.Equal(t, status.Code(err), codes.PermissionDenied)
 
 	// Verify inability to change projects without update privileges in old project
-	appServer.enf.SetBuiltinPolicy(`
+	_ = appServer.enf.SetBuiltinPolicy(`
 p, admin, applications, create, my-proj/test-app, allow
 p, admin, applications, update, my-proj/test-app, allow
 `)
 	_, err = appServer.Update(ctx, &ApplicationUpdateRequest{Application: testApp})
 	assert.Equal(t, status.Code(err), codes.PermissionDenied)
 
 	// Verify can update project with proper permissions
-	appServer.enf.SetBuiltinPolicy(`
+	_ = appServer.enf.SetBuiltinPolicy(`
 p, admin, applications, update, default/test-app, allow
 p, admin, applications, create, my-proj/test-app, allow
 p, admin, applications, update, my-proj/test-app, allow

server/project/project.go

@@ -182,7 +182,7 @@ func getRemovedDestination(oldProj, newProj *v1alpha1.AppProject) map[string]v1a
 		newDest[fmt.Sprintf("%s/%s", dest.Server, dest.Namespace)] = dest
 	}
 
-	removed := make(map[string]v1alpha1.ApplicationDestination, 0)
+	removed := make(map[string]v1alpha1.ApplicationDestination)
 	for key, dest := range oldDest {
 		if _, ok := newDest[key]; !ok {
 			removed[key] = dest

server/project/project_test.go

@@ -6,10 +6,10 @@ import (
 	"strings"
 	"testing"
 
-	jwt "github.com/dgrijalva/jwt-go"
+	"github.com/dgrijalva/jwt-go"
 	"github.com/stretchr/testify/assert"
-	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/fake"
@@ -42,7 +42,7 @@ func TestProjectServer(t *testing.T) {
 	})
 	settingsMgr := settings.NewSettingsManager(context.Background(), kubeclientset, testNamespace)
 	enforcer := rbac.NewEnforcer(kubeclientset, testNamespace, common.ArgoCDRBACConfigMapName, nil)
-	enforcer.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
+	_ = enforcer.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
 	enforcer.SetDefaultRole("role:admin")
 	enforcer.SetClaimsEnforcerFunc(func(claims jwt.Claims, rvals ...interface{}) bool {
 		return true
@@ -90,7 +90,8 @@ func TestProjectServer(t *testing.T) {
 		_, err := projectServer.Update(context.Background(), &ProjectUpdateRequest{Project: updatedProj})
 
 		assert.NotNil(t, err)
-		assert.Equal(t, codes.InvalidArgument, grpc.Code(err))
+		statusCode, _ := status.FromError(err)
+		assert.Equal(t, codes.InvalidArgument, statusCode.Code())
 	})
 
 	t.Run("TestRemoveSourceSuccessful", func(t *testing.T) {
@@ -123,7 +124,8 @@ func TestProjectServer(t *testing.T) {
 		_, err := projectServer.Update(context.Background(), &ProjectUpdateRequest{Project: updatedProj})
 
 		assert.NotNil(t, err)
-		assert.Equal(t, codes.InvalidArgument, grpc.Code(err))
+		statusCode, _ := status.FromError(err)
+		assert.Equal(t, codes.InvalidArgument, statusCode.Code())
 	})
 
 	t.Run("TestDeleteProjectSuccessful", func(t *testing.T) {
@@ -142,7 +144,8 @@ func TestProjectServer(t *testing.T) {
 		projectServer := NewServer("default", fake.NewSimpleClientset(), apps.NewSimpleClientset(&defaultProj), enforcer, util.NewKeyLock(), nil)
 
 		_, err := projectServer.Delete(context.Background(), &ProjectQuery{Name: defaultProj.Name})
-		assert.Equal(t, codes.InvalidArgument, grpc.Code(err))
+		statusCode, _ := status.FromError(err)
+		assert.Equal(t, codes.InvalidArgument, statusCode.Code())
 	})
 
 	t.Run("TestDeleteProjectReferencedByApp", func(t *testing.T) {
@@ -156,7 +159,8 @@ func TestProjectServer(t *testing.T) {
 		_, err := projectServer.Delete(context.Background(), &ProjectQuery{Name: "test"})
 
 		assert.NotNil(t, err)
-		assert.Equal(t, codes.InvalidArgument, grpc.Code(err))
+		statusCode, _ := status.FromError(err)
+		assert.Equal(t, codes.InvalidArgument, statusCode.Code())
 	})
 
 	t.Run("TestCreateTokenSuccesfully", func(t *testing.T) {

server/rbacpolicy/rbacpolicy.go

@@ -110,15 +110,6 @@ func (p *RBACPolicyEnforcer) getProjectFromRequest(rvals ...interface{}) *v1alph
 	return nil
 }
 
-// enforceNoClaims handles the RBAC enforcement case when there are no or invalid JWT claims
-func (p *RBACPolicyEnforcer) enforceNoClaims(rvals ...interface{}) bool {
-	if rvals[0] == nil {
-		vals := append([]interface{}{""}, rvals[1:]...)
-		return p.enf.Enforce(vals...)
-	}
-	return p.enf.Enforce(rvals...)
-}
-
 // enforceProjectToken will check to see the valid token has not yet been revoked in the project
 func (p *RBACPolicyEnforcer) enforceProjectToken(subject string, claims jwt.MapClaims, proj *v1alpha1.AppProject, rvals ...interface{}) bool {
 	subjectSplit := strings.Split(subject, ":")
@@ -143,13 +134,3 @@ func (p *RBACPolicyEnforcer) enforceProjectToken(subject string, claims jwt.MapC
 	return p.enf.EnforceRuntimePolicy(proj.ProjectPoliciesString(), vals...)
 
 }
-
-// enforceApplicationResource handles enforcement of application resources, taking into
-// consideration OIDC group bindings defined in the project
-func (p *RBACPolicyEnforcer) getProjectPolicy(projName string) string {
-	proj, err := p.projLister.Get(projName)
-	if err != nil {
-		return ""
-	}
-	return proj.ProjectPoliciesString()
-}

server/rbacpolicy/rbacpolicy_test.go

@@ -46,13 +46,12 @@ func TestEnforceAllPolicies(t *testing.T) {
 	projLister := test.NewFakeProjLister(newFakeProj())
 	enf := rbac.NewEnforcer(kubeclientset, test.FakeArgoCDNamespace, common.ArgoCDConfigMapName, nil)
 	enf.EnableLog(true)
-	enf.SetBuiltinPolicy(`p, alice, applications, create, my-proj/*, allow`)
-	enf.SetUserPolicy(`p, bob, applications, create, my-proj/*, allow`)
+	_ = enf.SetBuiltinPolicy(`p, alice, applications, create, my-proj/*, allow`)
+	_ = enf.SetUserPolicy(`p, bob, applications, create, my-proj/*, allow`)
 	rbacEnf := NewRBACPolicyEnforcer(enf, projLister)
 	enf.SetClaimsEnforcerFunc(rbacEnf.EnforceClaims)
 
-	var claims jwt.MapClaims
-	claims = jwt.MapClaims{"sub": "alice"}
+	claims := jwt.MapClaims{"sub": "alice"}
 	assert.True(t, enf.Enforce(claims, "applications", "create", "my-proj/my-app"))
 	claims = jwt.MapClaims{"sub": "bob"}
 	assert.True(t, enf.Enforce(claims, "applications", "create", "my-proj/my-app"))

server/repository/repository.go

@@ -79,11 +79,9 @@ func (s *Server) List(ctx context.Context, q *RepoQuery) (*appsv1.RepositoryList
 		return nil, err
 	}
 	items := make([]appsv1.Repository, 0)
-	if urls != nil {
-		for _, url := range urls {
-			if s.enf.Enforce(ctx.Value("claims"), rbacpolicy.ResourceRepositories, rbacpolicy.ActionGet, url) {
-				items = append(items, appsv1.Repository{Repo: url})
-			}
+	for _, url := range urls {
+		if s.enf.Enforce(ctx.Value("claims"), rbacpolicy.ResourceRepositories, rbacpolicy.ActionGet, url) {
+			items = append(items, appsv1.Repository{Repo: url})
 		}
 	}
 	err = util.RunAllAsync(len(items), func(i int) error {
@@ -152,8 +150,9 @@ func getKustomizationRes(ctx context.Context, repoClient repository.RepoServerSe
 		kustomizationRes, err := repoClient.ListDir(ctx, &request)
 		if err != nil {
 			return nil, err
+		} else {
+			return kustomizationRes, nil
 		}
-		return kustomizationRes, nil
 	}
 	return nil, errors.New("could not find kustomization")
 }