Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more linters #2092

Merged
merged 6 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/.cache/buster-for-binary
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
touts zoosporangium viner glucolipin galeproof sanctionment siper galeproof glucolipin fructiculture
1 change: 1 addition & 0 deletions .github/.cache/buster-for-unit-tests
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
tzolkin sacristy hymnwise curative debris preachification suscept spongiculture medicably craniomete
1 change: 1 addition & 0 deletions .github/.cache/buster-for-vars
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
brandenburgs singleheartedly coal-whipper transmutations Tarandian arquebus cropland drumskin intern
9 changes: 9 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ jobs:
uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
with:
go-version: stable
cache-dependency-path: |
go.sum
.github/.cache/buster-for-vars
pleshakov marked this conversation as resolved.
Show resolved Hide resolved

- name: Check for changes
uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
Expand Down Expand Up @@ -81,6 +84,9 @@ jobs:
uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
with:
go-version: stable
cache-dependency-path: |
go.sum
.github/.cache/buster-for-unit-tests

- name: Run Tests
run: make unit-test
Expand Down Expand Up @@ -136,6 +142,9 @@ jobs:
uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
with:
go-version: stable
cache-dependency-path: |
go.sum
.github/.cache/buster-for-binary

- name: Create/Update Draft
uses: lucacome/draft-release@8a63d32c79a171ae6048e614a8988f0ac3ed56d4 # v1.1.0
Expand Down
22 changes: 22 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,33 +37,55 @@ linters-settings:
- fieldalignment
lll:
line-length: 120
dupword:
ignore:
- "test"
linters:
enable:
- asasalint
- asciicheck
- dupword
- errcheck
- errname
- errorlint
- exportloopref
- fatcontext
- ginkgolinter
- gocheckcompilerdirectives
- gocyclo
- godot
- gofmt
- gofumpt
- goimports
- gosec
- gosimple
- gosmopolitan
- govet
- ineffassign
- intrange
- lll
- loggercheck
- makezero
- misspell
- nilerr
- noctx
- nolintlint
- predeclared
- promlinter
- reassign
- revive
- spancheck
- staticcheck
- stylecheck
- tenv
- thelper
- typecheck
- unconvert
- unparam
- unused
- usestdlibvars
- wastedassign
- whitespace
disable-all: true
issues:
max-issues-per-linter: 0
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ check-golangci-lint:

.PHONY: lint
lint: check-golangci-lint ## Run golangci-lint against code
golangci-lint run
golangci-lint run --fix

.PHONY: unit-test
unit-test: ## Run unit tests for the go code
Expand Down
2 changes: 1 addition & 1 deletion apis/v1alpha1/clientsettingspolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ type ClientKeepAlive struct {
}

// ClientKeepAliveTimeout defines the timeouts related to keep-alive client connections.
// Default: Default: https://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_timeout.
lucacome marked this conversation as resolved.
Show resolved Hide resolved
// Default: https://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_timeout.
type ClientKeepAliveTimeout struct {
// Server sets the timeout during which a keep-alive client connection will stay open on the server side.
// Setting this value to 0 disables keep-alive client connections.
Expand Down
4 changes: 2 additions & 2 deletions apis/v1alpha1/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import (
// GroupName specifies the group name used to register the objects.
const GroupName = "gateway.nginx.org"

// SchemeGroupVersion is group version used to register these objects
// SchemeGroupVersion is group version used to register these objects.
var SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1alpha1"}

// Resource takes an unqualified resource and returns a Group qualified GroupResource
// Resource takes an unqualified resource and returns a Group qualified GroupResource.
func Resource(resource string) schema.GroupResource {
return SchemeGroupVersion.WithResource(resource).GroupResource()
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/gateway/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,8 +444,9 @@ func createProvisionerModeCommand() *cobra.Command {

// FIXME(pleshakov): Remove this command once NGF min supported Kubernetes version supports sleep action in
// preStop hook.
// nolint:lll
// See https://github.com/kubernetes/enhancements/tree/4ec371d92dcd4f56a2ab18c8ba20bb85d8d20efe/keps/sig-node/3960-pod-lifecycle-sleep-action
//
//nolint:lll
func createSleepCommand() *cobra.Command {
// flag names
const durationFlag = "duration"
Expand Down
1 change: 1 addition & 0 deletions cmd/gateway/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type flagTestCase struct {
}

func testFlag(t *testing.T, cmd *cobra.Command, test flagTestCase) {
t.Helper()
g := NewWithT(t)
// discard any output generated by cobra
cmd.SetOut(io.Discard)
Expand Down
2 changes: 1 addition & 1 deletion cmd/gateway/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"os"
)

// Set during go build
// Set during go build.
var (
version string
commit string
Expand Down
5 changes: 2 additions & 3 deletions cmd/gateway/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
)

const (
// nolint:lll
// Regex from: https://github.com/kubernetes-sigs/gateway-api/blob/v1.1.0/apis/v1/shared_types.go#L647
controllerNameRegex = `^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/[A-Za-z0-9\/\-._~%!$&'()*+,;=:]+$` //nolint:lll
)
Expand Down Expand Up @@ -163,15 +162,15 @@ func validateEndpoint(endpoint string) error {
return fmt.Errorf("%q must be in the format <host>:<port>", endpoint)
}

// validatePort makes sure a given port is inside the valid port range for its usage
// validatePort makes sure a given port is inside the valid port range for its usage.
func validatePort(port int) error {
if port < 1024 || port > 65535 {
return fmt.Errorf("port outside of valid port range [1024 - 65535]: %v", port)
}
return nil
}

// ensureNoPortCollisions checks if the same port has been defined multiple times
// ensureNoPortCollisions checks if the same port has been defined multiple times.
func ensureNoPortCollisions(ports ...int) error {
seen := make(map[int]struct{})

Expand Down
2 changes: 1 addition & 1 deletion internal/framework/controller/predicate/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (ServicePortsChangedPredicate) Update(e event.UpdateEvent) bool {
oldPortSet := make(map[ports]struct{})
newPortSet := make(map[ports]struct{})

for i := 0; i < len(oldSvc.Spec.Ports); i++ {
for i := range len(oldSvc.Spec.Ports) {
oldPortSet[ports{servicePort: oldPorts[i].Port, targetPort: oldPorts[i].TargetPort}] = struct{}{}
newPortSet[ports{servicePort: newPorts[i].Port, targetPort: newPorts[i].TargetPort}] = struct{}{}
}
Expand Down
10 changes: 7 additions & 3 deletions internal/framework/controller/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
}
}

func (r *Reconciler) newObject(objectType ngftypes.ObjectType) ngftypes.ObjectType {
func (r *Reconciler) mustCreateNewObject(objectType ngftypes.ObjectType) ngftypes.ObjectType {
if r.cfg.OnlyMetadata {
partialObj := &metav1.PartialObjectMetadata{}
partialObj.SetGroupVersionKind(objectType.GetObjectKind().GroupVersionKind())
Expand All @@ -65,7 +65,11 @@
t := reflect.TypeOf(objectType).Elem()

// We could've used objectType.DeepCopyObject() here, but it's a bit slower confirmed by benchmarks.
return reflect.New(t).Interface().(client.Object)
obj, ok := reflect.New(t).Interface().(client.Object)
lucacome marked this conversation as resolved.
Show resolved Hide resolved
if !ok {
panic("failed to create a new object")

Check warning on line 70 in internal/framework/controller/reconciler.go

View check run for this annotation

Codecov / codecov/patch

internal/framework/controller/reconciler.go#L70

Added line #L70 was not covered by tests
}
return obj
}

// Reconcile implements the reconcile.Reconciler Reconcile method.
Expand All @@ -83,7 +87,7 @@
}
}

obj := r.newObject(r.cfg.ObjectType)
obj := r.mustCreateNewObject(r.cfg.ObjectType)

if err := r.cfg.Getter.Get(ctx, req.NamespacedName, obj); err != nil {
if !apierrors.IsNotFound(err) {
Expand Down
4 changes: 2 additions & 2 deletions internal/framework/kinds/kinds.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
)

// Gateway API Kinds
// Gateway API Kinds.
const (
// Gateway is the Gateway Kind
// Gateway is the Gateway Kind.
Gateway = "Gateway"
// GatewayClass is the GatewayClass Kind.
GatewayClass = "GatewayClass"
Expand Down
2 changes: 1 addition & 1 deletion internal/framework/runnables/cronjob.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func NewCronJob(cfg CronJobConfig) *CronJob {
}

// Start starts the cronjob.
// Implements controller-runtime manager.Runnable
// Implements controller-runtime manager.Runnable.
func (j *CronJob) Start(ctx context.Context) error {
select {
case <-j.cfg.ReadyCh:
Expand Down
2 changes: 0 additions & 2 deletions internal/framework/status/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,6 @@ func (u *Updater) writeStatuses(
//
// Note: this function is public because fake dependencies require us to test this function from the test package
// to avoid import cycles.
//
//nolint:nilerr
func NewRetryUpdateFunc(
getter controller.Getter,
updater K8sUpdater,
Expand Down
17 changes: 8 additions & 9 deletions internal/mode/static/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/go-logr/logr"
ngxclient "github.com/nginxinc/nginx-plus-go-client/client"
apiv1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand All @@ -20,7 +19,7 @@ import (
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/events"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
frameworkStatus "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config"

ngfConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config"
ngxConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file"
Expand Down Expand Up @@ -75,7 +74,7 @@ type eventHandlerConfig struct {
// eventRecorder records events for Kubernetes resources.
eventRecorder record.EventRecorder
// usageReportConfig contains the configuration for NGINX Plus usage reporting.
usageReportConfig *config.UsageReportConfig
usageReportConfig *ngfConfig.UsageReportConfig
// nginxConfiguredOnStartChecker sets the health of the Pod to Ready once we've written out our initial config.
nginxConfiguredOnStartChecker *nginxConfiguredOnStartChecker
// gatewayPodConfig contains information about this Pod.
Expand All @@ -89,7 +88,7 @@ type eventHandlerConfig struct {
}

const (
// groups for GroupStatusUpdater
// groups for GroupStatusUpdater.
groupAllExceptGateways = "all-graphs-except-gateways"
groupGateways = "gateways"
groupControlPlane = "control-plane"
Expand Down Expand Up @@ -308,7 +307,7 @@ func (h *eventHandlerImpl) parseAndCaptureEvent(ctx context.Context, logger logr
}
}

// updateNginxConf updates nginx conf files and reloads nginx
// updateNginxConf updates nginx conf files and reloads nginx.
func (h *eventHandlerImpl) updateNginxConf(ctx context.Context, conf dataplane.Configuration) error {
files := h.cfg.generator.Generate(conf)
if err := h.cfg.nginxFileMgr.ReplaceFiles(files); err != nil {
Expand All @@ -324,7 +323,7 @@ func (h *eventHandlerImpl) updateNginxConf(ctx context.Context, conf dataplane.C

// updateUpstreamServers is called only when endpoints have changed. It updates nginx conf files and then:
// - if using NGINX Plus, determines which servers have changed and uses the N+ API to update them;
// - otherwise if not using NGINX Plus, or an error was returned from the API, reloads nginx
// - otherwise if not using NGINX Plus, or an error was returned from the API, reloads nginx.
func (h *eventHandlerImpl) updateUpstreamServers(
ctx context.Context,
logger logr.Logger,
Expand Down Expand Up @@ -410,7 +409,7 @@ func serversEqual(newServers []ngxclient.UpstreamServer, oldServers []ngxclient.
}

// updateControlPlaneAndSetStatus updates the control plane configuration and then sets the status
// based on the outcome
// based on the outcome.
func (h *eventHandlerImpl) updateControlPlaneAndSetStatus(
ctx context.Context,
logger logr.Logger,
Expand All @@ -429,7 +428,7 @@ func (h *eventHandlerImpl) updateControlPlaneAndSetStatus(
logger.Error(err, msg)
h.cfg.eventRecorder.Eventf(
cfg,
apiv1.EventTypeWarning,
v1.EventTypeWarning,
"UpdateFailed",
msg+": %s",
err.Error(),
Expand All @@ -454,7 +453,7 @@ func getGatewayAddresses(
ctx context.Context,
k8sClient client.Client,
svc *v1.Service,
podConfig config.GatewayPodConfig,
podConfig ngfConfig.GatewayPodConfig,
) ([]gatewayv1.GatewayStatusAddress, error) {
podAddress := []gatewayv1.GatewayStatusAddress{
{
Expand Down
7 changes: 3 additions & 4 deletions internal/mode/static/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ import (
)

const (
// clusterTimeout is a timeout for connections to the Kubernetes API
// clusterTimeout is a timeout for connections to the Kubernetes API.
clusterTimeout = 10 * time.Second
)

Expand All @@ -80,7 +80,7 @@ func init() {
utilruntime.Must(appsv1.AddToScheme(scheme))
}

// nolint:gocyclo
//nolint:gocyclo
func StartManager(cfg config.Config) error {
nginxChecker := newNginxConfiguredOnStartChecker()
mgr, err := createManager(cfg, nginxChecker)
Expand Down Expand Up @@ -530,7 +530,7 @@ func registerControllers(
}

// 10 min jitter is enough per telemetry destination recommendation
// For the default period of 24 hours, jitter will be 10min /(24*60)min = 0.0069
// For the default period of 24 hours, jitter will be 10min /(24*60)min = 0.0069.
sjberman marked this conversation as resolved.
Show resolved Hide resolved
const telemetryJitterFactor = 10.0 / (24 * 60) // added jitter is bound by jitterFactor * period

func createTelemetryJob(
Expand Down Expand Up @@ -566,7 +566,6 @@ func createTelemetryJob(
if err != nil {
return nil, fmt.Errorf("cannot create telemetry exporter: %w", err)
}

} else {
exporter = telemetry.NewLoggingExporter(cfg.Logger.WithName("telemetryExporter").V(1 /* debug */))
}
Expand Down
2 changes: 1 addition & 1 deletion internal/mode/static/metrics/collectors/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type ControllerCollector struct {
eventBatchProcessDuration prometheus.Histogram
}

// NewControllerCollector creates a new ControllerCollector
// NewControllerCollector creates a new ControllerCollector.
func NewControllerCollector(constLabels map[string]string) *ControllerCollector {
nc := &ControllerCollector{
eventBatchProcessDuration: prometheus.NewHistogram(
Expand Down
4 changes: 2 additions & 2 deletions internal/mode/static/metrics/collectors/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ const (
nginxStatusURI = "http://config-status/stub_status"
)

// NewNginxMetricsCollector creates an NginxCollector which fetches stats from NGINX over a unix socket
// NewNginxMetricsCollector creates an NginxCollector which fetches stats from NGINX over a unix socket.
func NewNginxMetricsCollector(constLabels map[string]string, logger log.Logger) prometheus.Collector {
httpClient := runtime.GetSocketClient(nginxStatusSock)
ngxClient := prometheusClient.NewNginxClient(&httpClient, nginxStatusURI)

return nginxCollector.NewNginxCollector(ngxClient, metrics.Namespace, constLabels, logger)
}

// NewNginxPlusMetricsCollector creates an NginxCollector which fetches stats from NGINX Plus API over a unix socket
// NewNginxPlusMetricsCollector creates an NginxCollector which fetches stats from NGINX Plus API over a unix socket.
func NewNginxPlusMetricsCollector(
plusClient *client.NginxClient,
constLabels map[string]string,
Expand Down
Loading
Loading