Adds linting CI workflow

.bingo/Variables.mk

@@ -35,6 +35,12 @@ $(GINKGO): $(BINGO_DIR)/ginkgo.mod
 	@echo "(re)installing $(GOBIN)/ginkgo-v2.1.4"
 	@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=ginkgo.mod -o=$(GOBIN)/ginkgo-v2.1.4 "github.com/onsi/ginkgo/v2/ginkgo"
 
+GOLANGCI_LINT := $(GOBIN)/golangci-lint-v1.53.3
+$(GOLANGCI_LINT): $(BINGO_DIR)/golangci-lint.mod
+	@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.
+	@echo "(re)installing $(GOBIN)/golangci-lint-v1.53.3"
+	@cd $(BINGO_DIR) && GOWORK=off $(GO) build -mod=mod -modfile=golangci-lint.mod -o=$(GOBIN)/golangci-lint-v1.53.3 "github.com/golangci/golangci-lint/cmd/golangci-lint"
+
 GORELEASER := $(GOBIN)/goreleaser-v1.16.2
 $(GORELEASER): $(BINGO_DIR)/goreleaser.mod
 	@# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies.

.bingo/golangci-lint.mod

@@ -0,0 +1,5 @@
+module _ // Auto generated by https://github.com/bwplotka/bingo. DO NOT EDIT
+
+go 1.19
+
+require github.com/golangci/golangci-lint v1.53.3 // cmd/golangci-lint

.bingo/variables.env

@@ -14,6 +14,8 @@ CONTROLLER_GEN="${GOBIN}/controller-gen-v0.10.0"
 
 GINKGO="${GOBIN}/ginkgo-v2.1.4"
 
+GOLANGCI_LINT="${GOBIN}/golangci-lint-v1.53.3"
+
 GORELEASER="${GOBIN}/goreleaser-v1.16.2"
 
 KIND="${GOBIN}/kind-v0.15.0"

.github/workflows/sanity.yaml

@@ -0,0 +1,31 @@
+name: sanity
+
+on:
+  workflow_dispatch:
+  pull_request:
+  push:
+    branches:
+    - main
+
+jobs:
+  verify:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: actions/setup-go@v4
+        with:
+          go-version-file: "go.mod"
+      - name: Run verification checks
+        run: make verify
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+
+      - uses: actions/setup-go@v4
+        with:
+          go-version-file: "go.mod"
+
+      - name: Run golangci linting checks
+        run: make lint GOLANGCI_LINT_ARGS="--out-format github-actions"

.golangci.yaml

@@ -0,0 +1,51 @@
+run:
+  # Default timeout is 1m, up to give more room
+  timeout: 4m
+
+linters:
+  enable:
+  - asciicheck
+  - bodyclose
+  - errorlint
+  - ginkgolinter
+  - gofmt
+  - goimports
+  - gosec
+  - importas
+  - misspell
+  - nestif
+  - nonamedreturns
+  - prealloc
+  - revive
+  - stylecheck
+  - tparallel
+  - unconvert
+  - unparam
+  - unused
+  - whitespace
+
+linters-settings:
+  errorlint:
+    errorf: false
+
+  importas:
+    alias:
+    - pkg: k8s.io/apimachinery/pkg/apis/meta/v1
+      alias: metav1
+    - pkg: k8s.io/apimachinery/pkg/api/errors
+      alias: apierrors
+    - pkg: k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1
+      alias: apiextensionsv1
+    - pkg: k8s.io/apimachinery/pkg/util/runtime
+      alias: utilruntime
+    - pkg: "^k8s\\.io/api/([^/]+)/(v[^/]+)$"
+      alias: $1$2
+    - pkg: sigs.k8s.io/controller-runtime
+      alias: ctrl
+    - pkg: github.com/operator-framework/rukpak/api/v1alpha1
+      alias: rukpakv1alpha1
+  goimports:
+    local-prefixes: github.com/operator-framework/operator-controller
+
+output:
+  format: tab

Makefile

@@ -61,6 +61,14 @@ help: ## Display this help.
 
 ##@ Development
 
+.PHONY: lint
+lint: $(GOLANGCI_LINT) ## Run golangci linter.
+	$(GOLANGCI_LINT) run --build-tags $(GO_BUILD_TAGS) $(GOLANGCI_LINT_ARGS)
+
+.PHONY: tidy
+tidy: ## Update dependencies.
+	$(Q)go mod tidy
+
 .PHONY: manifests
 manifests: $(CONTROLLER_GEN) ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
 	$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
@@ -69,6 +77,10 @@ manifests: $(CONTROLLER_GEN) ## Generate WebhookConfiguration, ClusterRole and C
 generate: $(CONTROLLER_GEN) ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
 	$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."
 
+.PHONY: verify
+verify: tidy fmt generate ## Verify the current code generation.
+	git diff --exit-code
+
 .PHONY: fmt
 fmt: ## Run go fmt against code.
 	go fmt ./...

api/v1alpha1/operator_types.go

@@ -40,7 +40,7 @@ type OperatorSpec struct {
 
 	//+kubebuilder:validation:MaxLength:=48
 	//+kubebuilder:validation:Pattern:=^[a-z0-9]+([\.-][a-z0-9]+)*$
-	// Channel constraint defintion
+	// Channel constraint definition
 	Channel string `json:"channel,omitempty"`
 }
 

cmd/manager/main.go

@@ -20,8 +20,6 @@ import (
 	"flag"
 	"os"
 
-	"github.com/operator-framework/deppy/pkg/deppy/solver"
-	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
 	"github.com/spf13/pflag"
 	"go.uber.org/zap/zapcore"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -33,6 +31,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
 	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
+	"github.com/operator-framework/deppy/pkg/deppy/solver"
+	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
+
 	operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
 	"github.com/operator-framework/operator-controller/internal/controllers"
 	"github.com/operator-framework/operator-controller/internal/resolution/entitysources"

config/crd/bases/operators.operatorframework.io_operators.yaml

@@ -36,7 +36,7 @@ spec:
             description: OperatorSpec defines the desired state of Operator
             properties:
               channel:
-                description: Channel constraint defintion
+                description: Channel constraint definition
                 maxLength: 48
                 pattern: ^[a-z0-9]+([\.-][a-z0-9]+)*$
                 type: string

internal/controllers/operator_controller.go

@@ -41,7 +41,7 @@ import (
 
 	operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
 	"github.com/operator-framework/operator-controller/internal/controllers/validators"
-	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundles_and_dependencies"
+	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundlesanddependencies"
 	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity"
 )
 
@@ -107,6 +107,12 @@ func checkForUnexpectedFieldChange(a, b operatorsv1alpha1.Operator) bool {
 }
 
 // Helper function to do the actual reconcile
+//
+// Today we always return ctrl.Result{} and an error.
+// But in the future we might update this function
+// to return different results (e.g. requeue).
+//
+//nolint:unparam
 func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha1.Operator) (ctrl.Result, error) {
 	// validate spec
 	if err := validators.ValidateOperatorSpec(op); err != nil {
@@ -241,7 +247,7 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph
 func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Solution, packageName string) (*entity.BundleEntity, error) {
 	for _, variable := range solution.SelectedVariables() {
 		switch v := variable.(type) {
-		case *bundles_and_dependencies.BundleVariable:
+		case *bundlesanddependencies.BundleVariable:
 			entityPkgName, err := v.BundleEntity().PackageName()
 			if err != nil {
 				return nil, err
@@ -348,31 +354,6 @@ func (r *OperatorReconciler) existingBundleDeploymentUnstructured(ctx context.Co
 	return &unstructured.Unstructured{Object: unstrExistingBundleDeploymentObj}, nil
 }
 
-// verifyBDStatus reads the various possibilities of status in bundle deployment and translates
-// into corresponding operator condition status and message.
-func verifyBDStatus(dep *rukpakv1alpha1.BundleDeployment) (metav1.ConditionStatus, string) {
-	isValidBundleCond := apimeta.FindStatusCondition(dep.Status.Conditions, rukpakv1alpha1.TypeHasValidBundle)
-	isInstalledCond := apimeta.FindStatusCondition(dep.Status.Conditions, rukpakv1alpha1.TypeInstalled)
-
-	if isValidBundleCond != nil && isValidBundleCond.Status == metav1.ConditionFalse {
-		return metav1.ConditionFalse, isValidBundleCond.Message
-	}
-
-	if isInstalledCond != nil && isInstalledCond.Status == metav1.ConditionFalse {
-		return metav1.ConditionFalse, isInstalledCond.Message
-	}
-
-	if isInstalledCond != nil && isInstalledCond.Status == metav1.ConditionTrue {
-		return metav1.ConditionTrue, "install was successful"
-	}
-	return metav1.ConditionUnknown, fmt.Sprintf("could not determine the state of BundleDeployment %s", dep.Name)
-}
-
-// isBundleDepStale returns true if conditions are out of date.
-func isBundleDepStale(bd *rukpakv1alpha1.BundleDeployment) bool {
-	return bd != nil && bd.Status.ObservedGeneration != bd.GetGeneration()
-}
-
 // mapBundleMediaTypeToBundleProvisioner maps an olm.bundle.mediatype property to a
 // rukpak bundle provisioner class name that is capable of unpacking the bundle type
 func mapBundleMediaTypeToBundleProvisioner(mediaType string) (string, error) {

internal/controllers/operator_controller_test.go

@@ -1061,8 +1061,7 @@ var _ = Describe("Operator Controller Test", func() {
 
 func verifyInvariants(ctx context.Context, c client.Client, op *operatorsv1alpha1.Operator) {
 	key := client.ObjectKeyFromObject(op)
-	err := c.Get(ctx, key, op)
-	Expect(err).To(BeNil())
+	Expect(c.Get(ctx, key, op)).To(Succeed())
 
 	verifyConditionsInvariants(op)
 }

internal/controllers/suite_test.go

@@ -94,10 +94,10 @@ var _ = AfterSuite(func() {
 })
 
 func namesFromList(list client.ObjectList) []string {
-	var names []string
-
 	items, err := meta.ExtractList(list)
 	Expect(err).NotTo(HaveOccurred())
+
+	names := make([]string, 0, len(items))
 	for _, item := range items {
 		names = append(names, item.(client.Object).GetName())
 	}

internal/resolution/entitysources/catalogdsource.go

@@ -5,67 +5,66 @@ import (
 	"encoding/json"
 	"fmt"
 
+	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
 	"github.com/operator-framework/deppy/pkg/deppy"
 	"github.com/operator-framework/deppy/pkg/deppy/input"
-	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity"
 	"github.com/operator-framework/operator-registry/alpha/property"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
+	"github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity"
 )
 
-// catalogdEntitySource is a source for(/collection of) deppy defined input.Entity, built from content
+// CatalogdEntitySource is a source for(/collection of) deppy defined input.Entity, built from content
 // made accessible on-cluster by https://github.com/operator-framework/catalogd.
 // It is an implementation of deppy defined input.EntitySource
-type catalogdEntitySource struct {
+type CatalogdEntitySource struct {
 	client client.Client
 }
 
-func NewCatalogdEntitySource(client client.Client) *catalogdEntitySource {
-
-	return &catalogdEntitySource{client: client}
+func NewCatalogdEntitySource(client client.Client) *CatalogdEntitySource {
+	return &CatalogdEntitySource{client: client}
 }
 
-func (es *catalogdEntitySource) Get(ctx context.Context, id deppy.Identifier) (*input.Entity, error) {
+func (es *CatalogdEntitySource) Get(_ context.Context, _ deppy.Identifier) (*input.Entity, error) {
 	panic("not implemented")
 }
 
-func (es *catalogdEntitySource) Filter(ctx context.Context, filter input.Predicate) (input.EntityList, error) {
+func (es *CatalogdEntitySource) Filter(ctx context.Context, filter input.Predicate) (input.EntityList, error) {
 	resultSet := input.EntityList{}
 	entities, err := getEntities(ctx, es.client)
 	if err != nil {
 		return nil, err
 	}
-	for _, entity := range entities {
-		if filter(&entity) {
-			resultSet = append(resultSet, entity)
+	for i := range entities {
+		if filter(&entities[i]) {
+			resultSet = append(resultSet, entities[i])
 		}
 	}
 	return resultSet, nil
 }
 
-func (es *catalogdEntitySource) GroupBy(ctx context.Context, fn input.GroupByFunction) (input.EntityListMap, error) {
+func (es *CatalogdEntitySource) GroupBy(ctx context.Context, fn input.GroupByFunction) (input.EntityListMap, error) {
 	entities, err := getEntities(ctx, es.client)
 	if err != nil {
 		return nil, err
 	}
 	resultSet := input.EntityListMap{}
-	for _, entity := range entities {
-		keys := fn(&entity)
+	for i := range entities {
+		keys := fn(&entities[i])
 		for _, key := range keys {
-			resultSet[key] = append(resultSet[key], entity)
+			resultSet[key] = append(resultSet[key], entities[i])
 		}
 	}
 	return resultSet, nil
 }
 
-func (es *catalogdEntitySource) Iterate(ctx context.Context, fn input.IteratorFunction) error {
+func (es *CatalogdEntitySource) Iterate(ctx context.Context, fn input.IteratorFunction) error {
 	entities, err := getEntities(ctx, es.client)
 	if err != nil {
 		return err
 	}
-	for _, entity := range entities {
-		if err := fn(&entity); err != nil {
+	for i := range entities {
+		if err := fn(&entities[i]); err != nil {
 			return err
 		}
 	}