Enable gofumpt linter (projectcontour#6093)

A stricter form of gofmt, added rules can be found here: https://github.com/mvdan/gofumpt?tab=readme-ov-file#added-rules

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>

.golangci.yml

@@ -4,7 +4,7 @@ run:
 linters:
   enable:
   - bodyclose
-  - gofmt
+  - gofumpt
   - goimports
   - revive
   - gosec
@@ -23,8 +23,6 @@ linters-settings:
     - clas
     - cancelled
     locale: US
-  gofmt:
-    simplify: true
   unparam:
     check-exported: false
   goheader:
@@ -62,8 +60,12 @@ linters-settings:
     enable-all: true
   ginkgolinter:
     forbid-focus-container: true
+  gofumpt:
+    extra-rules: true
 
 issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
   exclude-rules:
   - linters: ["unparam"]
     text: "always receives"

CONTRIBUTING.md

@@ -311,6 +311,13 @@ At a maintainer's discretion, pull requests with multiple commits can be merged
 Merging pull requests with multiple commits can make sense in cases where a change involves code generation or mechanical changes that can be cleanly separated from semantic changes.
 The maintainer should review commit messages for each commit and make sure that each commit builds and passes tests.
 
+### Code formatting
+
+Contour utilizes [`gofumpt`](https://github.com/mvdan/gofumpt) for strict Golang formatting of the contour codebase.
+The `lint` CI job checks this to ensure all commits are formatted as expected.
+
+The `make format` target can be used to run `gofumpt` locally before making a PR.
+
 ### Import Aliases
 
 Naming is one of the most difficult things in software engineering.

Makefile

@@ -223,6 +223,11 @@ lint-flags:
 		exit 2; \
 	fi
 
+.PHONY: format
+format: ## Run gofumpt to format the codebase.
+	@echo Running gofumpt...
+	@./hack/gofumpt -l -w -extra .
+
 .PHONY: generate
 generate: ## Re-generate generated code and documentation
 generate: generate-rbac generate-crd-deepcopy generate-crd-yaml generate-gateway-yaml generate-deployment generate-api-docs generate-metrics-docs generate-uml generate-go

apis/projectcontour/v1/helpers.go

@@ -179,7 +179,6 @@ func (dc *DetailedCondition) IsPositivePolarity() bool {
 // getIndex checks if a SubCondition of type condType exists in the
 // slice, and returns its index if so. If not, returns -1.
 func getIndex(condType string, subconds []SubCondition) int {
-
 	for i, cond := range subconds {
 		if cond.Type == condType {
 			return i
@@ -191,7 +190,6 @@ func getIndex(condType string, subconds []SubCondition) int {
 // GetConditionFor returns the a pointer to the condition for a given type,
 // or nil if there are none currently present.
 func (status *HTTPProxyStatus) GetConditionFor(condType string) *DetailedCondition {
-
 	for i, cond := range status.Conditions {
 		if cond.Type == condType {
 			return &status.Conditions[i]

apis/projectcontour/v1/helpers_test.go

@@ -30,7 +30,6 @@ type subConditionDetails struct {
 }
 
 func TestAddErrorConditions(t *testing.T) {
-
 	tests := map[string]struct {
 		dc            *DetailedCondition
 		subconditions []subConditionDetails
@@ -274,7 +273,6 @@ func TestAddErrorConditions(t *testing.T) {
 }
 
 func TestAddWarningConditions(t *testing.T) {
-
 	tests := map[string]struct {
 		dc            *DetailedCondition
 		subconditions []subConditionDetails
@@ -544,7 +542,6 @@ func TestGetConditionFor(t *testing.T) {
 }
 
 func TestGetError(t *testing.T) {
-
 	dcWithErrors := &DetailedCondition{
 		Errors: []SubCondition{
 			{
@@ -571,11 +568,9 @@ func TestGetError(t *testing.T) {
 	emptySubCond, ok := dcEmpty.GetError(ConditionTypeServiceError)
 	assert.False(t, ok)
 	assert.Equal(t, SubCondition{}, emptySubCond)
-
 }
 
 func TestGetWarning(t *testing.T) {
-
 	dcWithErrors := &DetailedCondition{
 		Warnings: []SubCondition{
 			{
@@ -612,18 +607,16 @@ func TestGetWarning(t *testing.T) {
 	emptySubCond, ok := dcEmpty.GetWarning("SimpleTest1")
 	assert.False(t, ok)
 	assert.Equal(t, SubCondition{}, emptySubCond)
-
 }
-func TestTruncateLongMessage(t *testing.T) {
 
+func TestTruncateLongMessage(t *testing.T) {
 	shortmessage := "This is a message shorter than the max length"
 
 	assert.Equal(t, shortmessage, truncateLongMessage(shortmessage))
 
 	truncatedLongMessage := longMessage[:LongMessageLength]
 
 	assert.Equal(t, truncatedLongMessage, truncateLongMessage(longMessage))
-
 }
 
 // nolint:misspell

apis/projectcontour/v1/register.go

@@ -28,8 +28,10 @@ const (
 // New code should use GroupVersion.
 var SchemeGroupVersion = GroupVersion
 
-var HTTPProxyGVR = GroupVersion.WithResource("httpproxies")
-var TLSCertificateDelegationGVR = GroupVersion.WithResource("tlscertificatedelegations")
+var (
+	HTTPProxyGVR                = GroupVersion.WithResource("httpproxies")
+	TLSCertificateDelegationGVR = GroupVersion.WithResource("tlscertificatedelegations")
+)
 
 // Resource gets an Contour GroupResource for a specified resource
 func Resource(resource string) schema.GroupResource {

apis/projectcontour/v1/tlscertificatedelegation.go

@@ -25,7 +25,6 @@ type TLSCertificateDelegationSpec struct {
 // CertificateDelegation maps the authority to reference a secret
 // in the current namespace to a set of namespaces.
 type CertificateDelegation struct {
-
 	// required, the name of a secret in the current namespace.
 	SecretName string `json:"secretName"`
 

apis/projectcontour/v1alpha1/accesslog.go

@@ -191,7 +191,6 @@ const (
 type AccessLogJSONFields []string
 
 func (a AccessLogJSONFields) Validate() error {
-
 	for key, val := range a.AsFieldMap() {
 		if val == "" {
 			return fmt.Errorf("invalid JSON log field name %s", key)

apis/projectcontour/v1alpha1/contourconfig_helpers_test.go

@@ -230,7 +230,6 @@ func TestContourConfigurationSpecValidate(t *testing.T) {
 		}
 		c.Tracing.CustomTags = customTags
 		require.Error(t, c.Validate())
-
 	})
 }
 

cmd/contour/certgen.go

@@ -53,7 +53,6 @@ func registerCertGen(app *kingpin.Application) (*kingpin.CmdClause, *certgenConf
 
 // certgenConfig holds the configuration for the certificate generation process.
 type certgenConfig struct {
-
 	// KubeConfig is the path to the Kubeconfig file if we're not running in a cluster
 	KubeConfig string
 
@@ -154,5 +153,4 @@ func doCertgen(config *certgenConfig, log logrus.FieldLogger) {
 	if oerr := OutputCerts(config, coreClient, generatedCerts); oerr != nil {
 		log.WithError(oerr).Fatalf("failed output certificates")
 	}
-
 }

cmd/contour/certgen_test.go

@@ -276,7 +276,7 @@ func TestOutputFileMode(t *testing.T) {
 
 			err = filepath.Walk(outputDir, func(path string, info os.FileInfo, err error) error {
 				if !info.IsDir() {
-					assert.Equal(t, os.FileMode(0600), info.Mode(), "incorrect mode for file "+path)
+					assert.Equal(t, os.FileMode(0o600), info.Mode(), "incorrect mode for file "+path)
 				}
 				return nil
 			})

cmd/contour/contour.go

@@ -187,5 +187,4 @@ func main() {
 		app.Usage(args)
 		os.Exit(2)
 	}
-
 }

cmd/contour/serve.go

@@ -91,7 +91,6 @@ func registerServe(app *kingpin.Application) (*kingpin.CmdClause, *serveContext)
 	ctx := newServeContext()
 
 	parseConfig := func(_ *kingpin.ParseContext) error {
-
 		if ctx.contourConfigurationName != "" && configFile != "" {
 			return fmt.Errorf("cannot specify both %s and %s", "--contour-config", "-c/--config-path")
 		}
@@ -200,7 +199,6 @@ type EndpointsTranslator interface {
 // NewServer returns a Server object which contains the initial configuration
 // objects required to start an instance of Contour.
 func NewServer(log logrus.FieldLogger, ctx *serveContext) (*Server, error) {
-
 	var restConfigOpts []func(*rest.Config)
 
 	if qps := ctx.Config.KubeClientQPS; qps > 0 {
@@ -260,7 +258,8 @@ func NewServer(log logrus.FieldLogger, ctx *serveContext) (*Server, error) {
 						secret.SetAnnotations(nil)
 
 						return secret, nil
-					}},
+					},
+				},
 			},
 			// DefaultTransform is called for objects that do not have a TransformByObject function.
 			DefaultTransform: func(obj any) (any, error) {
@@ -742,7 +741,7 @@ func (s *Server) doServe() error {
 	return s.mgr.Start(signals.SetupSignalHandler())
 }
 
-func (s *Server) getExtensionSvcConfig(name string, namespace string) (xdscache_v3.ExtensionServiceConfig, error) {
+func (s *Server) getExtensionSvcConfig(name, namespace string) (xdscache_v3.ExtensionServiceConfig, error) {
 	extensionSvc := &contour_api_v1alpha1.ExtensionService{}
 	key := client.ObjectKey{
 		Namespace: namespace,
@@ -822,7 +821,6 @@ func (s *Server) setupTracingService(tracingConfig *contour_api_v1alpha1.Tracing
 		MaxPathTagLength:       ref.Val(tracingConfig.MaxPathTagLength, 256),
 		CustomTags:             customTags,
 	}, nil
-
 }
 
 func (s *Server) setupRateLimitService(contourConfiguration contour_api_v1alpha1.ContourConfigurationSpec) (*xdscache_v3.RateLimitConfig, error) {
@@ -954,8 +952,8 @@ func (x *xdsServer) Start(ctx context.Context) error {
 
 // setupMetrics creates metrics service for Contour.
 func (s *Server) setupMetrics(metricsConfig contour_api_v1alpha1.MetricsConfig, healthConfig contour_api_v1alpha1.HealthConfig,
-	registry *prometheus.Registry) error {
-
+	registry *prometheus.Registry,
+) error {
 	// Create metrics service and register with mgr.
 	metricsvc := &httpsvc.Service{
 		Addr:        metricsConfig.Address,
@@ -982,8 +980,8 @@ func (s *Server) setupMetrics(metricsConfig contour_api_v1alpha1.MetricsConfig,
 }
 
 func (s *Server) setupHealth(healthConfig contour_api_v1alpha1.HealthConfig,
-	metricsConfig contour_api_v1alpha1.MetricsConfig) error {
-
+	metricsConfig contour_api_v1alpha1.MetricsConfig,
+) error {
 	if healthConfig.Address != metricsConfig.Address || healthConfig.Port != metricsConfig.Port {
 		healthsvc := &httpsvc.Service{
 			Addr:        healthConfig.Address,
@@ -1002,8 +1000,8 @@ func (s *Server) setupHealth(healthConfig contour_api_v1alpha1.HealthConfig,
 }
 
 func (s *Server) setupGatewayAPI(contourConfiguration contour_api_v1alpha1.ContourConfigurationSpec,
-	mgr manager.Manager, eventHandler *contour.EventRecorder, sh *k8s.StatusUpdateHandler) []leadership.NeedLeaderElectionNotification {
-
+	mgr manager.Manager, eventHandler *contour.EventRecorder, sh *k8s.StatusUpdateHandler,
+) []leadership.NeedLeaderElectionNotification {
 	needLeadershipNotification := []leadership.NeedLeaderElectionNotification{}
 
 	// Check if GatewayAPI is configured.
@@ -1129,7 +1127,6 @@ type dagBuilderConfig struct {
 }
 
 func (s *Server) getDAGBuilder(dbc dagBuilderConfig) *dag.Builder {
-
 	var (
 		requestHeadersPolicy       dag.HeadersPolicy
 		responseHeadersPolicy      dag.HeadersPolicy
@@ -1273,7 +1270,6 @@ func (s *Server) informOnResource(obj client.Object, handler cache.ResourceEvent
 	}
 
 	registration, err := inf.AddEventHandler(handler)
-
 	if err != nil {
 		return err
 	}

cmd/contour/servecontext_test.go

@@ -340,7 +340,8 @@ func TestParseHTTPVersions(t *testing.T) {
 		"http/1.1+http/2 duplicated": {
 			versions: []contour_api_v1alpha1.HTTPVersionType{
 				contour_api_v1alpha1.HTTPVersion1, contour_api_v1alpha1.HTTPVersion2,
-				contour_api_v1alpha1.HTTPVersion1, contour_api_v1alpha1.HTTPVersion2},
+				contour_api_v1alpha1.HTTPVersion1, contour_api_v1alpha1.HTTPVersion2,
+			},
 			parseVersions: []envoy_v3.HTTPVersionType{envoy_v3.HTTPVersion1, envoy_v3.HTTPVersion2},
 		},
 	}

cmd/contour/shutdownmanager.go

@@ -198,7 +198,6 @@ func (s *shutdownContext) shutdownHandler() {
 
 // shutdownEnvoy sends a POST request to /healthcheck/fail to tell Envoy to start draining connections
 func shutdownEnvoy(adminAddress string) error {
-
 	httpClient := http.Client{
 		Transport: &http.Transport{
 			DialContext: func(_ context.Context, _, _ string) (net.Conn, error) {
@@ -221,7 +220,6 @@ func shutdownEnvoy(adminAddress string) error {
 
 // getOpenConnections parses a http request to a prometheus endpoint returning the sum of values found
 func getOpenConnections(adminAddress string) (int, error) {
-
 	httpClient := http.Client{
 		Transport: &http.Transport{
 			DialContext: func(_ context.Context, _, _ string) (net.Conn, error) {
@@ -280,7 +278,6 @@ func parseOpenConnections(stats io.Reader) (int, error) {
 }
 
 func doShutdownManager(config *shutdownmanagerContext) {
-
 	config.Info("started envoy shutdown manager")
 
 	http.HandleFunc("/healthz", config.healthzHandler)

hack/actions/check-changefile-exists.go

@@ -52,7 +52,6 @@ Error: %s
 	
 Please see the "Commit message and PR guidelines" section of CONTRIBUTING.md,
 or https://github.com/projectcontour/contour/blob/main/design/changelog.md for background.`, errorMessage))
-
 	}
 
 	// We need a GITHUB_TOKEN and PR_NUMBER in the environment.
@@ -62,7 +61,6 @@ or https://github.com/projectcontour/contour/blob/main/design/changelog.md for b
 	token, ok := os.LookupEnv("GITHUB_TOKEN")
 	if !ok {
 		log.Fatal("No GITHUB_TOKEN set, check the Action config.")
-
 	}
 	prEnv, ok := os.LookupEnv("PR_NUMBER")
 	if !ok {

hack/generate-metrics-doc.go

@@ -79,7 +79,7 @@ func main() {
 		log.Fatalf("%s", err)
 	}
 
-	f, err := os.OpenFile("table.md", os.O_CREATE|os.O_RDWR, 0644)
+	f, err := os.OpenFile("table.md", os.O_CREATE|os.O_RDWR, 0o644)
 	if err != nil {
 		log.Fatalf("%s", err)
 	}

hack/gofumpt

@@ -0,0 +1,3 @@
+#! /usr/bin/env bash
+
+go run mvdan.cc/gofumpt@v0.5.0 "$@"

hack/release/prepare-release.go

@@ -66,7 +66,7 @@ func capture(cmd []string) string {
 	return out.String()
 }
 
-func updateMappingForTOC(filePath string, vers string, toc string) error {
+func updateMappingForTOC(filePath, vers, toc string) error {
 	data, err := os.ReadFile(filePath)
 	if err != nil {
 		return err
@@ -86,7 +86,7 @@ func updateMappingForTOC(filePath string, vers string, toc string) error {
 		},
 	)
 
-	return os.WriteFile(filePath, []byte(rn.MustString()), 0600)
+	return os.WriteFile(filePath, []byte(rn.MustString()), 0o600)
 }
 
 // InsertAfter is like yaml.ElementAppender except it inserts after the named node.
@@ -114,7 +114,7 @@ func (a InsertAfter) Filter(rn *yaml.RNode) (*yaml.RNode, error) {
 	return rn, nil
 }
 
-func updateConfigForSite(filePath string, vers string) error {
+func updateConfigForSite(filePath, vers string) error {
 	data, err := os.ReadFile(filePath)
 	if err != nil {
 		return err
@@ -148,7 +148,7 @@ func updateConfigForSite(filePath string, vers string) error {
 		log.Fatalf("%s", err)
 	}
 
-	return os.WriteFile(filePath, []byte(rn.MustString()), 0600)
+	return os.WriteFile(filePath, []byte(rn.MustString()), 0o600)
 }
 
 func updateIndexFile(filePath, newVers string) error {
@@ -160,7 +160,7 @@ func updateIndexFile(filePath, newVers string) error {
 	upd := strings.ReplaceAll(string(data), "version: main", fmt.Sprintf("version: \"%s\"", newVers))
 	upd = strings.ReplaceAll(upd, "branch: main", "branch: release-"+newVers)
 
-	return os.WriteFile(filePath, []byte(upd), 0600)
+	return os.WriteFile(filePath, []byte(upd), 0o600)
 }
 
 func main() {