Add go-vet shadow variable checking.

cmd/ecs-service-logs/main.go

@@ -71,7 +71,7 @@ const (
 
 	defaultAWSRegion string = "us-west-2"
 
-	logLevel                  string = "level"
+	filterLogLevel            string = "level"
 	logTaskDefinitionFamily   string = "ecs_task_def_family"
 	logTaskDefinitionRevision string = "ecs_task_def_revision"
 	logGitBranch              string = "git_branch"
@@ -388,9 +388,9 @@ func showFunction(cmd *cobra.Command, args []string) error {
 		keychainName := v.GetString(flagAWSVaultKeychainName)
 		keychainProfile := v.GetString(flagAWSProfile)
 		if len(keychainName) > 0 && len(keychainProfile) > 0 {
-			creds, err := getAWSCredentials(keychainName, keychainProfile)
-			if err != nil {
-				return errors.Wrap(err, fmt.Sprintf("Unable to get AWS credentials from the keychain %s and profile %s", keychainName, keychainProfile))
+			creds, credsErr := getAWSCredentials(keychainName, keychainProfile)
+			if credsErr != nil {
+				return errors.Wrap(credsErr, fmt.Sprintf("Unable to get AWS credentials from the keychain %s and profile %s", keychainName, keychainProfile))
 			}
 			awsConfig.CredentialsChainVerboseErrors = aws.Bool(verbose)
 			awsConfig.Credentials = creds
@@ -582,7 +582,7 @@ func showFunction(cmd *cobra.Command, args []string) error {
 	}
 
 	if level := strings.ToLower(v.GetString(flagLogLevel)); len(level) > 0 {
-		filters[logLevel] = level
+		filters[filterLogLevel] = level
 	}
 
 	filterPattern := ""

cmd/generate_shipment_edi/main.go

@@ -95,9 +95,9 @@ func main() {
 
 	var sendToGexHTTP services.GexSender
 	if sendToGex {
-		certificates, rootCAs, err := initDODCertificates(v, logger)
-		if certificates == nil || rootCAs == nil || err != nil {
-			log.Fatal("Error in getting tls certs", err)
+		certificates, rootCAs, initDODCertificatesErr := initDODCertificates(v, logger)
+		if certificates == nil || rootCAs == nil || initDODCertificatesErr != nil {
+			log.Fatal("Error in getting tls certs", initDODCertificatesErr)
 		}
 		tlsConfig := &tls.Config{Certificates: certificates, RootCAs: rootCAs}
 		url := v.GetString("gex-url")
@@ -164,13 +164,13 @@ func processInvoice(db *pop.Connection, shipment models.Shipment, invoiceModel m
 
 	if sendToGex {
 		fmt.Println("Sending to GEX. . .")
-		invoice858CString, err := invoice858C.EDIString()
-		if err != nil {
-			return nil, err
+		invoice858CString, invoice858CErr := invoice858C.EDIString()
+		if invoice858CErr != nil {
+			return nil, invoice858CErr
 		}
-		resp, err := gexSender.SendToGex(invoice858CString, *transactionName)
-		if resp == nil || err != nil {
-			logger.Fatal("Gex Sender had no response", zap.Error(err))
+		resp, sendToGexErr := gexSender.SendToGex(invoice858CString, *transactionName)
+		if resp == nil || sendToGexErr != nil {
+			logger.Fatal("Gex Sender had no response", zap.Error(sendToGexErr))
 		}
 
 		fmt.Printf("status code: %v, error: %v\n", resp.StatusCode, err)

cmd/generate_test_data/main.go

@@ -66,18 +66,18 @@ func main() {
 		numShipmentOfferSplit := []int{15, 10}
 		// TSPs should never be able to see DRAFT or SUBMITTED or AWARDING shipments.
 		status := []models.ShipmentStatus{"AWARDED", "ACCEPTED", "APPROVED", "IN_TRANSIT", "DELIVERED", "COMPLETED"}
-		_, _, _, err := testdatagen.CreateShipmentOfferData(db, numTspUsers, numShipments, numShipmentOfferSplit, status, models.SelectedMoveTypeHHG)
-		if err != nil {
-			log.Panic(err)
+		_, _, _, createShipmentOfferDataErr := testdatagen.CreateShipmentOfferData(db, numTspUsers, numShipments, numShipmentOfferSplit, status, models.SelectedMoveTypeHHG)
+		if createShipmentOfferDataErr != nil {
+			log.Panic(createShipmentOfferDataErr)
 		}
 		// Create an office user
 		testdatagen.MakeDefaultOfficeUser(db)
 		log.Print("Success! Created TSP test data.")
 	} else if *namedScenario == tdgs.E2eBasicScenario.Name {
 		// Initialize logger
-		logger, err := zap.NewDevelopment()
-		if err != nil {
-			log.Panic(err)
+		logger, newDevelopmentErr := zap.NewDevelopment()
+		if newDevelopmentErr != nil {
+			log.Panic(newDevelopmentErr)
 		}
 
 		// Initialize storage and uploader

cmd/health_checker/main.go

@@ -208,14 +208,14 @@ func createHTTPClient(v *viper.Viper, logger *zap.Logger) (*http.Client, error)
 
 	if len(clientKeyEncoded) > 0 && len(clientCertEncoded) > 0 {
 
-		clientKey, err := base64.StdEncoding.DecodeString(clientKeyEncoded)
-		if err != nil {
-			return nil, errors.Wrap(err, "error decoding client key")
+		clientKey, clientKeyErr := base64.StdEncoding.DecodeString(clientKeyEncoded)
+		if clientKeyErr != nil {
+			return nil, errors.Wrap(clientKeyErr, "error decoding client key")
 		}
 
-		clientCert, err := base64.StdEncoding.DecodeString(clientCertEncoded)
-		if err != nil {
-			return nil, errors.Wrap(err, "error decoding client cert")
+		clientCert, clientCertErr := base64.StdEncoding.DecodeString(clientCertEncoded)
+		if clientCertErr != nil {
+			return nil, errors.Wrap(clientCertErr, "error decoding client cert")
 		}
 
 		caBytes := make([]byte, 0)
@@ -227,9 +227,10 @@ func createHTTPClient(v *viper.Viper, logger *zap.Logger) (*http.Client, error)
 			caBytes = []byte(caString)
 		}
 
-		tlsConfig, err = createTLSConfig([]byte(clientKey), []byte(clientCert), caBytes, false)
-		if err != nil {
-			return nil, errors.Wrap(err, "error creating TLS config")
+		var tlsConfigErr error
+		tlsConfig, tlsConfigErr = createTLSConfig([]byte(clientKey), []byte(clientCert), caBytes, false)
+		if tlsConfigErr != nil {
+			return nil, errors.Wrap(tlsConfigErr, "error creating TLS config")
 		}
 
 	} else {
@@ -239,14 +240,14 @@ func createHTTPClient(v *viper.Viper, logger *zap.Logger) (*http.Client, error)
 
 		if len(clientKeyFile) > 0 && len(clientCertFile) > 0 {
 
-			clientKey, err := ioutil.ReadFile(clientKeyFile) // #nosec b/c we need to read a file from a user-defined path
-			if err != nil {
-				return nil, errors.Wrap(err, "error reading client key file at "+clientKeyFile)
+			clientKey, clientKeyErr := ioutil.ReadFile(clientKeyFile) // #nosec b/c we need to read a file from a user-defined path
+			if clientKeyErr != nil {
+				return nil, errors.Wrap(clientKeyErr, "error reading client key file at "+clientKeyFile)
 			}
 
-			clientCert, err := ioutil.ReadFile(clientCertFile) // #nosec b/c we need to read a file from a user-defined path
-			if err != nil {
-				return nil, errors.Wrap(err, "error reading client cert file at "+clientKeyFile)
+			clientCert, clientCertErr := ioutil.ReadFile(clientCertFile) // #nosec b/c we need to read a file from a user-defined path
+			if clientCertErr != nil {
+				return nil, errors.Wrap(clientCertErr, "error reading client cert file at "+clientKeyFile)
 			}
 
 			caBytes := make([]byte, 0)
@@ -257,10 +258,10 @@ func createHTTPClient(v *viper.Viper, logger *zap.Logger) (*http.Client, error)
 				}
 				caBytes = content
 			}
-
-			tlsConfig, err = createTLSConfig(clientKey, clientCert, caBytes, false)
-			if err != nil {
-				return nil, errors.Wrap(err, "error creating TLS config")
+			var tlsConfigErr error
+			tlsConfig, tlsConfigErr = createTLSConfig(clientKey, clientCert, caBytes, false)
+			if tlsConfigErr != nil {
+				return nil, errors.Wrap(tlsConfigErr, "error creating TLS config")
 			}
 		}
 	}

cmd/milmove/main.go

@@ -263,18 +263,18 @@ func serveFunction(cmd *cobra.Command, args []string) error {
 	logger = logger.With(fields...)
 
 	if v.GetBool(cli.LogTaskMetadataFlag) {
-		resp, err := http.Get("http://169.254.170.2/v2/metadata")
-		if err != nil {
-			logger.Error(errors.Wrap(err, "could not fetch task metadata").Error())
+		resp, httpGetErr := http.Get("http://169.254.170.2/v2/metadata")
+		if httpGetErr != nil {
+			logger.Error(errors.Wrap(httpGetErr, "could not fetch task metadata").Error())
 		} else {
-			body, err := ioutil.ReadAll(resp.Body)
-			if err != nil {
-				logger.Error(errors.Wrap(err, "could not read task metadata").Error())
+			body, readAllErr := ioutil.ReadAll(resp.Body)
+			if readAllErr != nil {
+				logger.Error(errors.Wrap(readAllErr, "could not read task metadata").Error())
 			} else {
 				taskMetadata := &ecs.TaskMetadata{}
-				err := json.Unmarshal(body, taskMetadata)
-				if err != nil {
-					logger.Error(errors.Wrap(err, "could not parse task metadata").Error())
+				unmarshallErr := json.Unmarshal(body, taskMetadata)
+				if unmarshallErr != nil {
+					logger.Error(errors.Wrap(unmarshallErr, "could not parse task metadata").Error())
 				} else {
 					logger = logger.With(
 						zap.String("ecs_cluster", taskMetadata.Cluster),
@@ -314,11 +314,11 @@ func serveFunction(cmd *cobra.Command, args []string) error {
 
 	// Assert that our secret keys can be parsed into actual private keys
 	// TODO: Store the parsed key in handlers/AppContext instead of parsing every time
-	if _, err := jwt.ParseRSAPrivateKeyFromPEM([]byte(loginGovSecretKey)); err != nil {
-		logger.Fatal("Login.gov private key", zap.Error(err))
+	if _, parseRSAPrivateKeyFromPEMErr := jwt.ParseRSAPrivateKeyFromPEM([]byte(loginGovSecretKey)); parseRSAPrivateKeyFromPEMErr != nil {
+		logger.Fatal("Login.gov private key", zap.Error(parseRSAPrivateKeyFromPEMErr))
 	}
-	if _, err := jwt.ParseRSAPrivateKeyFromPEM([]byte(clientAuthSecretKey)); err != nil {
-		logger.Fatal("Client auth private key", zap.Error(err))
+	if _, parseRSAPrivateKeyFromPEMErr := jwt.ParseRSAPrivateKeyFromPEM([]byte(clientAuthSecretKey)); parseRSAPrivateKeyFromPEMErr != nil {
+		logger.Fatal("Client auth private key", zap.Error(parseRSAPrivateKeyFromPEMErr))
 	}
 	if len(loginGovHostname) == 0 {
 		logger.Fatal("Must provide the Login.gov hostname parameter, exiting")
@@ -488,9 +488,9 @@ func serveFunction(cmd *cobra.Command, args []string) error {
 			data["database"] = dbErr == nil
 		}
 
-		err := json.NewEncoder(w).Encode(data)
-		if err != nil {
-			logger.Error("Failed encoding health check response", zap.Error(err))
+		newEncoderErr := json.NewEncoder(w).Encode(data)
+		if newEncoderErr != nil {
+			logger.Error("Failed encoding health check response", zap.Error(newEncoderErr))
 		}
 
 		// We are not using request middleware here so logging directly in the check
@@ -665,9 +665,9 @@ func serveFunction(cmd *cobra.Command, args []string) error {
 		localAuthMux.Handle(pat.Post("/create"), authentication.NewCreateUserHandler(authContext, dbConnection, appnames, clientAuthSecretKey, noSessionTimeout, useSecureCookie))
 
 		devlocalCAPath := v.GetString(cli.DevlocalCAFlag)
-		devlocalCa, err := ioutil.ReadFile(devlocalCAPath) // #nosec
-		if err != nil {
-			logger.Error(fmt.Sprintf("Unable to read devlocal CA from path %s", devlocalCAPath), zap.Error(err))
+		devlocalCa, readFileErr := ioutil.ReadFile(devlocalCAPath) // #nosec
+		if readFileErr != nil {
+			logger.Error(fmt.Sprintf("Unable to read devlocal CA from path %s", devlocalCAPath), zap.Error(readFileErr))
 		} else {
 			rootCAs.AppendCertsFromPEM(devlocalCa)
 		}

cmd/renderer/main.go

@@ -35,8 +35,8 @@ func main() {
 
 	if len(variablesFile) > 0 {
 		// Read contents of variables file into vars
-		vars, err := ioutil.ReadFile(variablesFile)
-		if err != nil {
+		vars, readFileErr := ioutil.ReadFile(variablesFile)
+		if readFileErr != nil {
 			log.Fatal(errors.New("error reading variables file"))
 		}
 

pkg/auth/cookie.go

@@ -117,8 +117,8 @@ func sessionClaimsFromRequest(logger Logger, secret string, appName Application,
 	}
 
 	token, err := jwt.ParseWithClaims(cookie.Value, &SessionClaims{}, func(token *jwt.Token) (interface{}, error) {
-		rsaKey, err := jwt.ParseRSAPrivateKeyFromPEM([]byte(secret))
-		return &rsaKey.PublicKey, err
+		rsaKey, parseRSAPrivateKeyFromPEMErr := jwt.ParseRSAPrivateKeyFromPEM([]byte(secret))
+		return &rsaKey.PublicKey, parseRSAPrivateKeyFromPEMErr
 	})
 
 	if err != nil || token == nil || !token.Valid {

pkg/awardqueue/awardqueue.go

@@ -91,18 +91,20 @@ func (aq *AwardQueue) attemptShipmentOffer(ctx context.Context, shipment models.
 		loopCount++
 
 		tsp := models.TransportationServiceProvider{}
-		if err := aq.db.Find(&tsp, tspPerformance.TransportationServiceProviderID); err == nil {
+		if findErr := aq.db.Find(&tsp, tspPerformance.TransportationServiceProviderID); findErr == nil {
 			aq.logger.TraceInfo(ctx, "Attempting to offer to TSP", zap.String("tsp_id", tsp.ID.String()))
 
-			isAdministrativeShipment, err := aq.ShipmentWithinBlackoutDates(tsp.ID, shipment)
-			if err != nil {
-				aq.logger.TraceError(ctx, "Failed to determine if shipment is within TSP blackout dates", zap.Error(err))
-				return nil, err
+			isAdministrativeShipment, shipmentWithinBlackoutDatesErr := aq.ShipmentWithinBlackoutDates(tsp.ID, shipment)
+			if shipmentWithinBlackoutDatesErr != nil {
+				aq.logger.TraceError(ctx, "Failed to determine if shipment is within TSP blackout dates", zap.Error(shipmentWithinBlackoutDatesErr))
+				return nil, shipmentWithinBlackoutDatesErr
 			}
 
-			shipmentOffer, err = models.CreateShipmentOffer(aq.db, shipment.ID, tsp.ID, tspPerformance.ID, isAdministrativeShipment)
-			if err == nil {
-				if tspPerformance, err = models.IncrementTSPPerformanceOfferCount(aq.db, tspPerformance.ID); err == nil {
+			var createShipmentOfferErr error
+			shipmentOffer, createShipmentOfferErr = models.CreateShipmentOffer(aq.db, shipment.ID, tsp.ID, tspPerformance.ID, isAdministrativeShipment)
+			if createShipmentOfferErr == nil {
+				var tspPerformanceErr error
+				if tspPerformance, tspPerformanceErr = models.IncrementTSPPerformanceOfferCount(aq.db, tspPerformance.ID); tspPerformanceErr == nil {
 					if isAdministrativeShipment == true {
 						aq.logger.TraceInfo(ctx, "Shipment pickup date is during a blackout period. Awarding Administrative Shipment to TSP.")
 					} else {
@@ -117,16 +119,16 @@ func (aq *AwardQueue) attemptShipmentOffer(ctx context.Context, shipment models.
 						foundAvailableTSP = true
 
 						// Award the shipment
-						if err := models.AwardShipment(aq.db, shipment.ID); err != nil {
-							aq.logger.TraceError(ctx, "Failed to set shipment as awarded", zap.Error(err))
-							return nil, err
+						if awardShipmentErr := models.AwardShipment(aq.db, shipment.ID); awardShipmentErr != nil {
+							aq.logger.TraceError(ctx, "Failed to set shipment as awarded", zap.Error(awardShipmentErr))
+							return nil, awardShipmentErr
 						}
 					}
 				} else {
-					aq.logger.TraceError(ctx, "Failed to increment offer count", zap.Error(err))
+					aq.logger.TraceError(ctx, "Failed to increment offer count", zap.Error(tspPerformanceErr))
 				}
 			} else {
-				aq.logger.TraceError(ctx, "Failed to offer to TSP", zap.Error(err))
+				aq.logger.TraceError(ctx, "Failed to offer to TSP", zap.Error(createShipmentOfferErr))
 			}
 		}
 

pkg/cli/email.go

@@ -63,11 +63,11 @@ func InitEmail(v *viper.Viper, logger Logger) notifications.NotificationSender {
 		logger.Info("Using ses email backend",
 			zap.String("region", awsSESRegion),
 			zap.String("domain", awsSESDomain))
-		sesSession, err := awssession.NewSession(&aws.Config{
+		sesSession, newSessionErr := awssession.NewSession(&aws.Config{
 			Region: aws.String(awsSESRegion),
 		})
-		if err != nil {
-			logger.Fatal("Failed to create a new AWS client config provider", zap.Error(err))
+		if newSessionErr != nil {
+			logger.Fatal("Failed to create a new AWS client config provider", zap.Error(newSessionErr))
 		}
 		sesService := ses.New(sesSession)
 		return notifications.NewNotificationSender(sesService, awsSESDomain, logger)

pkg/handlers/internalapi/generic_move_documents.go

@@ -67,9 +67,9 @@ func (h CreateGenericMoveDocumentHandler) Handle(params movedocop.CreateGenericM
 	uploads := models.Uploads{}
 	for _, id := range uploadIds {
 		converted := uuid.Must(uuid.FromString(id.String()))
-		upload, err := models.FetchUpload(ctx, h.DB(), session, converted)
-		if err != nil {
-			return handlers.ResponseForError(h.Logger(), err)
+		upload, fetchUploadErr := models.FetchUpload(ctx, h.DB(), session, converted)
+		if fetchUploadErr != nil {
+			return handlers.ResponseForError(h.Logger(), fetchUploadErr)
 		}
 		uploads = append(uploads, upload)
 	}
@@ -79,9 +79,9 @@ func (h CreateGenericMoveDocumentHandler) Handle(params movedocop.CreateGenericM
 		id := uuid.Must(uuid.FromString(payload.PersonallyProcuredMoveID.String()))
 
 		// Enforce that the ppm's move_id matches our move
-		ppm, err := models.FetchPersonallyProcuredMove(h.DB(), session, id)
-		if err != nil {
-			return handlers.ResponseForError(h.Logger(), err)
+		ppm, fetchPPMErr := models.FetchPersonallyProcuredMove(h.DB(), session, id)
+		if fetchPPMErr != nil {
+			return handlers.ResponseForError(h.Logger(), fetchPPMErr)
 		}
 		if ppm.MoveID != moveID {
 			return movedocop.NewCreateGenericMoveDocumentBadRequest()

pkg/handlers/internalapi/move_documents.go

@@ -161,9 +161,9 @@ func (h UpdateMoveDocumentHandler) Handle(params movedocop.UpdateMoveDocumentPar
 			// (because the document has been toggled between OK and HAS_ISSUE and back)
 			// then don't complete it again.
 			if ppm.Status != models.PPMStatusCOMPLETED {
-				err := ppm.Complete()
-				if err != nil {
-					return handlers.ResponseForError(h.Logger(), err)
+				completeErr := ppm.Complete()
+				if completeErr != nil {
+					return handlers.ResponseForError(h.Logger(), completeErr)
 				}
 			}
 		}

pkg/handlers/internalapi/moving_expense_documents.go

@@ -70,9 +70,9 @@ func (h CreateMovingExpenseDocumentHandler) Handle(params movedocop.CreateMoving
 	uploads := models.Uploads{}
 	for _, id := range uploadIds {
 		converted := uuid.Must(uuid.FromString(id.String()))
-		upload, err := models.FetchUpload(ctx, h.DB(), session, converted)
-		if err != nil {
-			return handlers.ResponseForError(h.Logger(), err)
+		upload, fetchUploadErr := models.FetchUpload(ctx, h.DB(), session, converted)
+		if fetchUploadErr != nil {
+			return handlers.ResponseForError(h.Logger(), fetchUploadErr)
 		}
 		uploads = append(uploads, upload)
 	}
@@ -82,9 +82,9 @@ func (h CreateMovingExpenseDocumentHandler) Handle(params movedocop.CreateMoving
 		id := uuid.Must(uuid.FromString(payload.PersonallyProcuredMoveID.String()))
 
 		// Enforce that the ppm's move_id matches our move
-		ppm, err := models.FetchPersonallyProcuredMove(h.DB(), session, id)
-		if err != nil {
-			return handlers.ResponseForError(h.Logger(), err)
+		ppm, fetchPPMErr := models.FetchPersonallyProcuredMove(h.DB(), session, id)
+		if fetchPPMErr != nil {
+			return handlers.ResponseForError(h.Logger(), fetchPPMErr)
 		}
 		if ppm.MoveID != moveID {
 			return movedocop.NewCreateMovingExpenseDocumentBadRequest()

pkg/handlers/internalapi/personally_procured_move.go

@@ -406,9 +406,9 @@ func (h PatchPersonallyProcuredMoveHandler) updateEstimates(ppm *models.Personal
 	if ppm.HasSit != nil && *ppm.HasSit == true {
 		cwtWeight := unit.Pound(*ppm.WeightEstimate).ToCWT()
 		sitZip3 := rateengine.Zip5ToZip3(*ppm.DestinationPostalCode)
-		sitComputation, err := re.SitCharge(cwtWeight, daysInSIT, sitZip3, *ppm.OriginalMoveDate, true)
-		if err != nil {
-			return err
+		sitComputation, sitChargeErr := re.SitCharge(cwtWeight, daysInSIT, sitZip3, *ppm.OriginalMoveDate, true)
+		if sitChargeErr != nil {
+			return sitChargeErr
 		}
 		sitCharge := float64(sitComputation.ApplyDiscount(lhDiscount, sitDiscount))
 		reimbursementString := fmt.Sprintf("$%.2f", sitCharge/100)