Skip to content

Commit

Permalink
Enable more linters for standardizing imports (projectcontour#6094)
Browse files Browse the repository at this point in the history
Enable gci linter, for deterministic import order

Enable importas linter for consistent import aliases
- group imports by standard, external, contour
- enforce alias for all occurrences that have a rule

Signed-off-by: Sunjay Bhatia <[email protected]>
  • Loading branch information
sunjayBhatia authored Feb 12, 2024
1 parent 3d78210 commit fa1d380
Show file tree
Hide file tree
Showing 328 changed files with 18,822 additions and 18,551 deletions.
37 changes: 37 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ linters:
- bodyclose
- gofumpt
- goimports
- gci
- importas
- revive
- gosec
- misspell
Expand Down Expand Up @@ -62,6 +64,41 @@ linters-settings:
forbid-focus-container: true
gofumpt:
extra-rules: true
gci:
custom-order: true
sections:
- standard
- default
- prefix(github.com/projectcontour/contour)
importas:
no-unaliased: true
alias:
- pkg: github.com/projectcontour/contour/apis/projectcontour/(v\w+)
alias: contour_${1}
- pkg: sigs.k8s.io/gateway-api/apis/(v\w+)
alias: gatewayapi_${1}
- pkg: k8s.io.*/apis?/(\w+)/(v\w+)
alias: ${1}_${2}
- pkg: github.com/envoyproxy/go-control-plane/envoy/config/(\w+)/(v\w+)
alias: envoy_config_${1}_${2}
- pkg: github.com/envoyproxy/go-control-plane/envoy/service/(\w+)/(v\w+)
alias: envoy_service_${1}_${2}
- pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/filters/(\w+)/(\w+)/(v\w+)
alias: envoy_filter_${1}_${2}_${3}
- pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/(\w+)/(v\w+)
alias: envoy_transport_socket_${1}_${2}
- pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/compression/(\w+)/(\w+)/(v\w+)
alias: envoy_compression_${1}_${2}_${3}
- pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/access_loggers/(\w+)/(v\w+)
alias: envoy_access_logger_${1}_${2}
- pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/formatter/(\w+)/(v\w+)
alias: envoy_formatter_${1}_${2}
- pkg: github.com/envoyproxy/go-control-plane/envoy/extensions/upstreams/(\w+)/(v\w+)
alias: envoy_upstream_${1}_${2}
- pkg: github.com/envoyproxy/go-control-plane/envoy/type/(v\w+)
alias: envoy_type_${1}
- pkg: github.com/envoyproxy/go-control-plane/envoy/type/matcher/(v\w+)
alias: envoy_matcher_${1}

issues:
max-issues-per-linter: 0
Expand Down
8 changes: 5 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -321,19 +321,21 @@ 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.
Contour uses the following pattern to name imports when referencing packages from other packages.
Contour uses the following general pattern to name imports when referencing internal packages and packages from other projects.
> thing_version: The name+package path of the thing and then the version separated by underscores
Examples:
```
contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
contour_api_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
contour_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
contour_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
envoy_v3 "github.com/projectcontour/contour/internal/envoy/v3"
xdscache_v3 "github.com/projectcontour/contour/internal/xdscache/v3"
```
Exact patterns for import paths can be found in the `importas` linter settings in `.golangci.yml`
### Pre commit CI
Before a change is submitted it should pass all the pre commit CI jobs.
Expand Down
4 changes: 3 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ lint-flags:
.PHONY: format
format: ## Run gofumpt to format the codebase.
@echo Running gofumpt...
@./hack/gofumpt -l -w -extra .
@go run mvdan.cc/[email protected] -l -w -extra .
@echo Running gci...
@go run github.com/daixiang0/[email protected] write . --skip-generated -s standard -s default -s "prefix(github.com/projectcontour/contour)" --custom-order

.PHONY: generate
generate: ## Re-generate generated code and documentation
Expand Down
12 changes: 6 additions & 6 deletions apis/projectcontour/v1/detailedconditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,28 @@
package v1

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// ConditionStatus is a type alias for the k8s.io/apimachinery/pkg/apis/meta/v1
// ConditionStatus type to maintain API compatibility.
// +k8s:deepcopy-gen=false
type ConditionStatus = metav1.ConditionStatus
type ConditionStatus = meta_v1.ConditionStatus

// These are valid condition statuses. "ConditionTrue" means a resource is in the condition.
// "ConditionFalse" means a resource is not in the condition. "ConditionUnknown" means kubernetes
// can't decide if a resource is in the condition or not. In the future, we could add other
// intermediate conditions, e.g. ConditionDegraded. These are retained here for API compatibility.
const (
ConditionTrue ConditionStatus = metav1.ConditionTrue
ConditionFalse ConditionStatus = metav1.ConditionFalse
ConditionUnknown ConditionStatus = metav1.ConditionUnknown
ConditionTrue ConditionStatus = meta_v1.ConditionTrue
ConditionFalse ConditionStatus = meta_v1.ConditionFalse
ConditionUnknown ConditionStatus = meta_v1.ConditionUnknown
)

// Condition is a type alias for the k8s.io/apimachinery/pkg/apis/meta/v1
// Condition type to maintain API compatibility.
// +k8s:deepcopy-gen=false
type Condition = metav1.Condition
type Condition = meta_v1.Condition

// SubCondition is a Condition-like type intended for use as a subcondition inside a DetailedCondition.
//
Expand Down
16 changes: 8 additions & 8 deletions apis/projectcontour/v1/httpproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
package v1

import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
core_v1 "k8s.io/api/core/v1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// HTTPProxySpec defines the spec of the CRD.
Expand Down Expand Up @@ -1431,7 +1431,7 @@ type HTTPProxyStatus struct {
Description string `json:"description,omitempty"`
// +optional
// LoadBalancer contains the current status of the load balancer.
LoadBalancer corev1.LoadBalancerStatus `json:"loadBalancer,omitempty"`
LoadBalancer core_v1.LoadBalancerStatus `json:"loadBalancer,omitempty"`
// +optional
// Conditions contains information about the current status of the HTTPProxy,
// in an upstream-friendly container.
Expand Down Expand Up @@ -1464,8 +1464,8 @@ type HTTPProxyStatus struct {
// +kubebuilder:resource:scope=Namespaced,path=httpproxies,shortName=proxy;proxies,singular=httpproxy
// +kubebuilder:subresource:status
type HTTPProxy struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata"`
meta_v1.TypeMeta `json:",inline"`
meta_v1.ObjectMeta `json:"metadata"`

Spec HTTPProxySpec `json:"spec"`
// Status is a container for computed information about the HTTPProxy.
Expand All @@ -1478,9 +1478,9 @@ type HTTPProxy struct {

// HTTPProxyList is a list of HTTPProxies.
type HTTPProxyList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata"`
Items []HTTPProxy `json:"items"`
meta_v1.TypeMeta `json:",inline"`
meta_v1.ListMeta `json:"metadata"`
Items []HTTPProxy `json:"items"`
}

// SlowStartPolicy will gradually increase amount of traffic to a newly added endpoint.
Expand Down
4 changes: 2 additions & 2 deletions apis/projectcontour/v1/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
package v1

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
)
Expand Down Expand Up @@ -49,7 +49,7 @@ func AddKnownTypes(scheme *runtime.Scheme) error {
&TLSCertificateDelegation{},
&TLSCertificateDelegationList{},
)
metav1.AddToGroupVersion(scheme, GroupVersion)
meta_v1.AddToGroupVersion(scheme, GroupVersion)
return nil
}

Expand Down
12 changes: 6 additions & 6 deletions apis/projectcontour/v1/tlscertificatedelegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
package v1

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// TLSCertificateDelegationSpec defines the spec of the CRD
Expand Down Expand Up @@ -68,8 +68,8 @@ type TLSCertificateDelegationStatus struct {
// +kubebuilder:resource:scope=Namespaced,path=tlscertificatedelegations,shortName=tlscerts,singular=tlscertificatedelegation
// +kubebuilder:subresource:status
type TLSCertificateDelegation struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata"`
meta_v1.TypeMeta `json:",inline"`
meta_v1.ObjectMeta `json:"metadata"`

Spec TLSCertificateDelegationSpec `json:"spec"`
// +optional
Expand All @@ -80,7 +80,7 @@ type TLSCertificateDelegation struct {

// TLSCertificateDelegationList is a list of TLSCertificateDelegations.
type TLSCertificateDelegationList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata"`
Items []TLSCertificateDelegation `json:"items"`
meta_v1.TypeMeta `json:",inline"`
meta_v1.ListMeta `json:"metadata"`
Items []TLSCertificateDelegation `json:"items"`
}
31 changes: 16 additions & 15 deletions apis/projectcontour/v1alpha1/accesslog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,27 @@ package v1alpha1_test
import (
"testing"

"github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
"github.com/stretchr/testify/require"

contour_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
)

func TestValidateAccessLogType(t *testing.T) {
require.Error(t, v1alpha1.AccessLogType("").Validate())
require.Error(t, v1alpha1.AccessLogType("foo").Validate())
require.Error(t, contour_v1alpha1.AccessLogType("").Validate())
require.Error(t, contour_v1alpha1.AccessLogType("foo").Validate())

require.NoError(t, v1alpha1.EnvoyAccessLog.Validate())
require.NoError(t, v1alpha1.JSONAccessLog.Validate())
require.NoError(t, contour_v1alpha1.EnvoyAccessLog.Validate())
require.NoError(t, contour_v1alpha1.JSONAccessLog.Validate())
}

func TestValidateAccessLogLevel(t *testing.T) {
require.Error(t, v1alpha1.AccessLogLevel("").Validate())
require.Error(t, v1alpha1.AccessLogLevel("foo").Validate())
require.Error(t, contour_v1alpha1.AccessLogLevel("").Validate())
require.Error(t, contour_v1alpha1.AccessLogLevel("foo").Validate())

require.NoError(t, v1alpha1.LogLevelInfo.Validate())
require.NoError(t, v1alpha1.LogLevelError.Validate())
require.NoError(t, v1alpha1.LogLevelCritical.Validate())
require.NoError(t, v1alpha1.LogLevelDisabled.Validate())
require.NoError(t, contour_v1alpha1.LogLevelInfo.Validate())
require.NoError(t, contour_v1alpha1.LogLevelError.Validate())
require.NoError(t, contour_v1alpha1.LogLevelCritical.Validate())
require.NoError(t, contour_v1alpha1.LogLevelDisabled.Validate())
}

func TestValidateAccessLogJSONFields(t *testing.T) {
Expand All @@ -59,7 +60,7 @@ func TestValidateAccessLogJSONFields(t *testing.T) {
}

for _, c := range errorCases {
require.Error(t, v1alpha1.AccessLogJSONFields(c).Validate(), c)
require.Error(t, contour_v1alpha1.AccessLogJSONFields(c).Validate(), c)
}

successCases := [][]string{
Expand All @@ -82,7 +83,7 @@ func TestValidateAccessLogJSONFields(t *testing.T) {
}

for _, c := range successCases {
require.NoError(t, v1alpha1.AccessLogJSONFields(c).Validate(), c)
require.NoError(t, contour_v1alpha1.AccessLogJSONFields(c).Validate(), c)
}
}

Expand All @@ -103,7 +104,7 @@ func TestAccessLogFormatString(t *testing.T) {
}

for _, c := range errorCases {
require.Error(t, v1alpha1.AccessLogFormatString(c).Validate(), c)
require.Error(t, contour_v1alpha1.AccessLogFormatString(c).Validate(), c)
}

successCases := []string{
Expand Down Expand Up @@ -135,6 +136,6 @@ func TestAccessLogFormatString(t *testing.T) {
}

for _, c := range successCases {
require.NoError(t, v1alpha1.AccessLogFormatString(c).Validate(), c)
require.NoError(t, contour_v1alpha1.AccessLogFormatString(c).Validate(), c)
}
}
20 changes: 10 additions & 10 deletions apis/projectcontour/v1alpha1/contourconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
package v1alpha1

import (
contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
contour_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
)

// ContourConfigurationSpec represents a configuration of a Contour controller.
Expand Down Expand Up @@ -65,7 +65,7 @@ type ContourConfigurationSpec struct {
// GlobalExternalAuthorization allows envoys external authorization filter
// to be enabled for all virtual hosts.
// +optional
GlobalExternalAuthorization *contour_api_v1.AuthorizationServer `json:"globalExtAuth,omitempty"`
GlobalExternalAuthorization *contour_v1.AuthorizationServer `json:"globalExtAuth,omitempty"`

// RateLimitService optionally holds properties of the Rate Limit Service
// to be used for global rate limiting.
Expand Down Expand Up @@ -800,7 +800,7 @@ type RateLimitServiceConfig struct {
// HTTPProxy can overwrite this configuration.
//
// +optional
DefaultGlobalRateLimitPolicy *contour_api_v1.GlobalRateLimitPolicy `json:"defaultGlobalRateLimitPolicy,omitempty"`
DefaultGlobalRateLimitPolicy *contour_v1.GlobalRateLimitPolicy `json:"defaultGlobalRateLimitPolicy,omitempty"`
}

// TracingConfig defines properties for exporting trace data to OpenTelemetry.
Expand Down Expand Up @@ -899,7 +899,7 @@ type ContourConfigurationStatus struct {
// +patchStrategy=merge
// +listType=map
// +listMapKey=type
Conditions []contour_api_v1.DetailedCondition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
Conditions []contour_v1.DetailedCondition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
}

// +genclient
Expand All @@ -909,8 +909,8 @@ type ContourConfigurationStatus struct {

// ContourConfiguration is the schema for a Contour instance.
type ContourConfiguration struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
meta_v1.TypeMeta `json:",inline"`
meta_v1.ObjectMeta `json:"metadata,omitempty"`

Spec ContourConfigurationSpec `json:"spec"`

Expand All @@ -923,7 +923,7 @@ type ContourConfiguration struct {

// ContourConfigurationList contains a list of Contour configuration resources.
type ContourConfigurationList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []ContourConfiguration `json:"items"`
meta_v1.TypeMeta `json:",inline"`
meta_v1.ListMeta `json:"metadata,omitempty"`
Items []ContourConfiguration `json:"items"`
}
Loading

0 comments on commit fa1d380

Please sign in to comment.