Add linter (#272)

* Fix linter

* Pin setup-envtest to compatible version

* Fix linting issues

* Add linter to github workflow

.github/workflows/pr-build-test.yml

@@ -35,4 +35,34 @@ jobs:
       - name: Upload coverage to Codecov
         uses: codecov/codecov-action@v3
         with:
-          verbose: true
+          verbose: true
+
+  lint:
+    name: Code standards (linting)
+    runs-on: ubuntu-22.04
+    steps:
+    - name: Check out code into the Go module directory
+      uses: actions/checkout@v4
+
+    - name: Set up Go
+      uses: actions/setup-go@v5
+      with:
+        go-version: "~1.22.4"
+
+    - name: Cache tools
+      uses: actions/cache@v4
+      id: setup-go
+      with:
+        path: bin
+        key: ${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('Makefile') }}-${{ steps.setup-go.outputs.go-version }}
+
+    - uses: actions/cache@v4
+      with:
+        path: |
+          /home/runner/.cache/golangci-lint
+        key: golangcilint-${{ hashFiles('**/go.sum') }}
+        restore-keys: |
+          golangcilint-
+
+    - name: Lint
+      run: make lint

Makefile

@@ -54,9 +54,11 @@ CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen
 ENVTEST ?= $(LOCALBIN)/setup-envtest
 CHLOGGEN ?= $(LOCALBIN)/chloggen
 ADDLICENSE ?= $(LOCALBIN)/addlicense
+GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint
 
 KUSTOMIZE_VERSION ?= v5.0.3
 CONTROLLER_TOOLS_VERSION ?= v0.14.0
+GOLANGCI_LINT_VERSION ?= v1.57.2
 ALL_SRC := $(shell find . -name '*.go' -type f | sort)
 CW_AGENT_OPERATOR_IMPORT_PATH = "github.com/aws/amazon-cloudwatch-agent-operator"
 
@@ -202,8 +204,8 @@ impi:
 	@echo "Check import order/grouping finished"
 
 .PHONY: lint
-lint: simple-lint
-	$(LOCALBIN)/golangci-lint run ./... ||	(curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(LOCALBIN) v1.51.1)
+lint: golangci-lint simple-lint
+	$(GOLANGCI_LINT) run --timeout 5m
 
 simple-lint: checklicense impi
 
@@ -233,11 +235,14 @@ checklicense: install-addlicense
     			echo "Check License finished successfully"; \
     		fi
 
+.PHONY: golangci-lint
+golangci-lint: ## Download golangci-lint locally if necessary.
+	$(call go-get-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint,$(GOLANGCI_LINT_VERSION))
 
 .PHONY: envtest
 envtest: $(ENVTEST) ## Download envtest-setup locally if necessary.
 $(ENVTEST): $(LOCALBIN)
-	test -s $(LOCALBIN)/setup-envtest || GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
+	test -s $(LOCALBIN)/setup-envtest || GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@release-0.18
 
 # go-get-tool will 'go get' any package $2 and install it to $1.
 PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST))))

cmd/amazon-cloudwatch-agent-target-allocator/watcher/promOperator.go

@@ -286,7 +286,7 @@ func (w *PrometheusCRWatcher) addStoreAssetsForServiceMonitor(
 	for i, endp := range endps {
 		objKey := fmt.Sprintf("serviceMonitor/%s/%s/%d", smNamespace, smName, i)
 
-		if err = store.AddBearerToken(ctx, smNamespace, endp.BearerTokenSecret, objKey); err != nil {
+		if err = store.AddSafeAuthorizationCredentials(ctx, smNamespace, endp.Authorization, objKey); err != nil {
 			break
 		}
 
@@ -329,7 +329,7 @@ func (w *PrometheusCRWatcher) addStoreAssetsForPodMonitor(
 	for i, endp := range podMetricsEndps {
 		objKey := fmt.Sprintf("podMonitor/%s/%s/%d", pmNamespace, pmName, i)
 
-		if err = store.AddBearerToken(ctx, pmNamespace, &endp.BearerTokenSecret, objKey); err != nil {
+		if err = store.AddSafeAuthorizationCredentials(ctx, pmNamespace, endp.Authorization, objKey); err != nil {
 			break
 		}
 

cmd/amazon-cloudwatch-agent-target-allocator/watcher/promOperator_test.go

@@ -189,11 +189,14 @@ func TestLoadConfig(t *testing.T) {
 					PodMetricsEndpoints: []monitoringv1.PodMetricsEndpoint{
 						{
 							Port: "web",
-							BearerTokenSecret: v1.SecretKeySelector{
-								LocalObjectReference: v1.LocalObjectReference{
-									Name: "bearer",
+							Authorization: &monitoringv1.SafeAuthorization{
+								Type: "Bearer",
+								Credentials: &v1.SecretKeySelector{
+									LocalObjectReference: v1.LocalObjectReference{
+										Name: "bearer",
+									},
+									Key: "token",
 								},
-								Key: "token",
 							},
 						},
 					},

controllers/common.go

@@ -122,6 +122,9 @@ func reconcileDesiredObjectsWPrune(ctx context.Context, kubeClient client.Client
 	}
 
 	desiredObjectMap, err := reconcileDesiredObjectUIDs(ctx, kubeClient, logger, &owner, scheme, desiredObjects...)
+	if err != nil {
+		return fmt.Errorf("failed to reconcile desired objects: %w", err)
+	}
 
 	// Pruning owned objects in the cluster which are not should not be present after the reconciliation.
 	err = pruneStaleObjects(ctx, kubeClient, logger, previouslyOwnedObjects, desiredObjectMap)

integration-tests/manifests/annotations/validate_annotation_methods.go

@@ -1,6 +1,5 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
-
 package annotations
 
 import (
@@ -186,7 +185,7 @@ func checkNameSpaceAnnotations(t *testing.T, clientSet *kubernetes.Clientset, ex
 			}
 		}
 
-		if correct == true {
+		if correct {
 			fmt.Println("Namespace annotations are correct!")
 			return true
 		}
@@ -201,6 +200,10 @@ func updateOperator(t *testing.T, clientSet *kubernetes.Clientset, deployment *a
 	args := deployment.Spec.Template.Spec.Containers[0].Args
 
 	deployment, err = clientSet.AppsV1().Deployments(amazonCloudwatchNamespace).Get(context.TODO(), amazonControllerManager, metav1.GetOptions{})
+	if err != nil {
+		t.Errorf("Failed to get deployment: %v\n", err)
+		return false
+	}
 	deployment.Spec.Template.Spec.Containers[0].Args = args
 
 	_, err = clientSet.AppsV1().Deployments(amazonCloudwatchNamespace).Update(context.TODO(), deployment, metav1.UpdateOptions{})
@@ -293,7 +296,6 @@ func updateAnnotationConfig(deployment *appsV1.Deployment, jsonStr string) *apps
 	indexOfAutoAnnotationConfigString := findIndexOfPrefix("--auto-annotation-config=", args)
 	if indexOfAutoAnnotationConfigString < 0 {
 		deployment.Spec.Template.Spec.Containers[0].Args = append(deployment.Spec.Template.Spec.Containers[0].Args, "--auto-annotation-config="+jsonStr)
-		indexOfAutoAnnotationConfigString = len(deployment.Spec.Template.Spec.Containers[0].Args) - 1
 	} else {
 		deployment.Spec.Template.Spec.Containers[0].Args[indexOfAutoAnnotationConfigString] = "--auto-annotation-config=" + jsonStr
 	}

integration-tests/manifests/annotations/validate_annotations_namespace_test.go

@@ -267,7 +267,6 @@ func TestAnnotationsOnMultipleResources(t *testing.T) {
 }
 
 func TestAutoAnnotationForManualAnnotationRemoval(t *testing.T) {
-	startTime := time.Now()
 	clientSet, uniqueNamespace := setupFunction(t, "manual-annotation-removal", []string{sampleDeploymentYamlNameRelPath})
 	annotationConfig := auto.AnnotationConfig{
 		Java: auto.AnnotationResources{
@@ -279,7 +278,7 @@ func TestAutoAnnotationForManualAnnotationRemoval(t *testing.T) {
 	if err != nil {
 		t.Error("Error:", err)
 	}
-	startTime = time.Now()
+	startTime := time.Now()
 	updateTheOperator(t, clientSet, string(jsonStr))
 	if err != nil {
 		t.Errorf("Failed to get deployment app: %s", err.Error())
@@ -308,6 +307,9 @@ func TestAutoAnnotationForManualAnnotationRemoval(t *testing.T) {
 		time.Sleep(5 * time.Second)
 	}
 	deployment, err := clientSet.AppsV1().Deployments(uniqueNamespace).Get(ctx, deploymentName, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("Error getting deployment: %v\n", err)
+	}
 
 	//Removing all annotations
 	deployment.ObjectMeta.Annotations = nil

integration-tests/manifests/cmd/validate_instrumentation_vars.go

@@ -7,7 +7,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -85,7 +84,7 @@ func verifyInstrumentationEnvVariables(clientset *kubernetes.Clientset, namespac
 	}
 	fmt.Println("Pod environment variables:", envMap)
 
-	fileData, err := ioutil.ReadFile(jsonPath)
+	fileData, err := os.ReadFile(jsonPath)
 	if err != nil {
 		fmt.Println("Error reading JSON file:", err)
 		return false

internal/manifests/collector/ports.go

@@ -70,9 +70,7 @@ var AppSignalsPortToServicePortMap = map[int32][]corev1.ServicePort{
 func PortMapToServicePortList(portMap map[int32][]corev1.ServicePort) []corev1.ServicePort {
 	ports := make([]corev1.ServicePort, 0, len(portMap))
 	for _, plist := range portMap {
-		for _, p := range plist {
-			ports = append(ports, p)
-		}
+		ports = append(ports, plist...)
 	}
 	sort.Slice(ports, func(i, j int) bool {
 		return ports[i].Name < ports[j].Name

internal/manifests/collector/ports_test.go

@@ -125,10 +125,10 @@ func TestXRayGetContainerPorts(t *testing.T) {
 
 func TestXRayWithBindAddressDefaultGetContainerPorts(t *testing.T) {
 	cfg := getStringFromFile("./test-resources/xrayAgentConfig.json")
-	strings.Replace(cfg, "2800", "2000", 1)
+	cfg = strings.Replace(cfg, "2800", "2000", 1) // set Xray trace port to 2000
 	containerPorts := getContainerPorts(logger, cfg, "", []corev1.ServicePort{})
 	assert.Equal(t, 2, len(containerPorts))
-	assert.Equal(t, int32(2800), containerPorts[CWA+XrayTraces].ContainerPort)
+	assert.Equal(t, int32(2000), containerPorts[CWA+XrayTraces].ContainerPort)
 	assert.Equal(t, CWA+XrayTraces, containerPorts[CWA+XrayTraces].Name)
 	assert.Equal(t, int32(2900), containerPorts[CWA+XrayProxy].ContainerPort)
 	assert.Equal(t, CWA+XrayProxy, containerPorts[CWA+XrayProxy].Name)
@@ -137,11 +137,13 @@ func TestXRayWithBindAddressDefaultGetContainerPorts(t *testing.T) {
 
 func TestXRayWithTCPProxyBindAddressDefaultGetContainerPorts(t *testing.T) {
 	cfg := getStringFromFile("./test-resources/xrayAgentConfig.json")
-	strings.Replace(cfg, "2900", "2000", 1)
+	cfg = strings.Replace(cfg, "2900", "2000", 1) // set Xray proxy port to 2000
 	containerPorts := getContainerPorts(logger, cfg, "", []corev1.ServicePort{})
 	assert.Equal(t, 2, len(containerPorts))
 	assert.Equal(t, int32(2800), containerPorts[CWA+XrayTraces].ContainerPort)
 	assert.Equal(t, CWA+XrayTraces, containerPorts[CWA+XrayTraces].Name)
+	assert.Equal(t, int32(2000), containerPorts[CWA+XrayProxy].ContainerPort)
+	assert.Equal(t, CWA+XrayProxy, containerPorts[CWA+XrayProxy].Name)
 	assert.Equal(t, corev1.ProtocolUDP, containerPorts[CWA+XrayTraces].Protocol)
 }
 
@@ -153,7 +155,7 @@ func TestNilMetricsGetContainerPorts(t *testing.T) {
 
 func TestMultipleReceiversGetContainerPorts(t *testing.T) {
 	cfg := getStringFromFile("./test-resources/multipleReceiversAgentConfig.json")
-	strings.Replace(cfg, "2900", "2000", 1)
+	cfg = strings.Replace(cfg, "2900", "2000", 1) // set Xray proxy to port 2000
 	wantPorts := []corev1.ContainerPort{
 		{
 			Name:          CWA + Server,
@@ -200,11 +202,6 @@ func TestMultipleReceiversGetContainerPorts(t *testing.T) {
 			Protocol:      corev1.ProtocolUDP,
 			ContainerPort: int32(2800),
 		},
-		{
-			Name:          CWA + XrayProxy,
-			Protocol:      corev1.ProtocolTCP,
-			ContainerPort: int32(2900),
-		},
 		{
 			Name:          OtlpGrpc + "-4327",
 			Protocol:      corev1.ProtocolTCP,

internal/manifests/collector/suite_test.go

@@ -118,52 +118,3 @@ func paramsWithOtelConfig(otelCfg string) manifests.Params {
 		Recorder: record.NewFakeRecorder(10),
 	}
 }
-
-func newParams(taContainerImage string, file string) (manifests.Params, error) {
-	replicas := int32(1)
-	var configJSON []byte
-	var err error
-
-	if file == "" {
-		configJSON, err = os.ReadFile("testdata/test.json")
-	} else {
-		configJSON, err = os.ReadFile(file)
-	}
-	if err != nil {
-		return manifests.Params{}, fmt.Errorf("Error getting json file: %w", err)
-	}
-
-	cfg := config.New(
-		config.WithCollectorImage(defaultCollectorImage),
-	)
-
-	return manifests.Params{
-		Config: cfg,
-		OtelCol: v1alpha1.AmazonCloudWatchAgent{
-			TypeMeta: metav1.TypeMeta{
-				Kind:       "cloudwatch.aws.amazon.com",
-				APIVersion: "v1",
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test",
-				Namespace: "default",
-				UID:       instanceUID,
-			},
-			Spec: v1alpha1.AmazonCloudWatchAgentSpec{
-				Mode: v1alpha1.ModeStatefulSet,
-				Ports: []v1.ServicePort{{
-					Name: "web",
-					Port: 80,
-					TargetPort: intstr.IntOrString{
-						Type:   intstr.Int,
-						IntVal: 80,
-					},
-					NodePort: 0,
-				}},
-				Replicas: &replicas,
-				Config:   string(configJSON),
-			},
-		},
-		Log: logger,
-	}, nil
-}

pkg/instrumentation/podmutator.go

@@ -29,7 +29,6 @@ const (
 
 var (
 	errMultipleInstancesPossible = errors.New("multiple OpenTelemetry Instrumentation instances available, cannot determine which one to select")
-	errNoInstancesAvailable      = errors.New("no OpenTelemetry Instrumentation instances available")
 )
 
 type instPodMutator struct {