Upgrade to Operator SDK 1.41.1 (#211)

* Add missing improvements from Operator SDK 1.34.1

These were not mentioned in the upgrade documentation for version 1.34.x (https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.34.0/), but I've found them by compating the release with the previous one (https://github.com/operator-framework/operator-sdk/compare/v1.33.0...v1.34.1).

* Upgrade to Operator SDK 1.36.0

Source of upgrade steps: https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.36.0/
Key differences:
- Go packages `k8s.io/*` are already at a version higher than the one in the upgrade.
- `ENVTEST_K8S_VERSION` is at a version higher than the one in the upgrade
- We didn't have the golangci-lint make command before, thus we only needed to add things.

* Upgrade to Operator SDK 1.38.0

Source of upgrade steps: https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.38.0/

* Upgrade to Operator SDK 1.39.0

Source of upgrade steps: https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.39.0/

* Upgrade to Operator SDK 1.40.0

Source of upgrade steps: https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.40.0/

I didn't do the "Add app.kubernetes.io/name label to your manifests" since it seems that we have it already, and it's customized.

* Address lint errors

* Update golangci-lint version used to support Go 1.24

* Improve workflows

- Make workflow targets more specific.
- Make build workflow only build (i.e. remove test part of it).
- Rearrange steps and improve naming for build workflow.

* Add back deleted test

Initially the test has been removed due to lint saying that it was duplicate code, but it falsely errored since the values are different.

* Improve code and add missing upgrade pieces

* Upgrade to Operator SDK 1.41.1

Source of upgrade steps: https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.41.0/

Upgrading to 1.41.1 from 1.40.0 doesn't have any migration steps.

Key elements:
- Upgrade to golangci-lint v2
- Made the manifests using the updated controller tools

* Address linter errors

golanci-lint v2 seems to be more robust than the previous one, which is beneficial. Thus, we address the linter errors thrown by v2 and improve our code even further.

* Add Makefile improvements

These were brought in by comparing the Makefile of a freshly created operator using the latest operator-sdk with ours.

* Add missing default kustomization for 1.40.0 upgrade

* Bring default kustomization to latest version

This is done by putting the file's content from a newly-generated operator.

* Switch metrics-bind-address default value back to 8080

This ensures that the upgrade is backwards-compatible.

* Add webhook-related scaffolding

This enables us to easily add support for webhooks by running `operator-sdk create webhook` whenever we want to add them.

* Fix typo

pkg/kubernetessecrets/kubernetes_secrets_builder.go

@@ -2,7 +2,7 @@ package kubernetessecrets
 
 import (
 	"context"
-	errs "errors"
+	"errors"
 	"fmt"
 	"reflect"
 	"regexp"
@@ -11,7 +11,7 @@ import (
 	"github.com/1Password/onepassword-operator/pkg/onepassword/model"
 	"github.com/1Password/onepassword-operator/pkg/utils"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	kubeValidate "k8s.io/apimachinery/pkg/util/validation"
@@ -26,11 +26,20 @@ const VersionAnnotation = OnepasswordPrefix + "/item-version"
 const ItemPathAnnotation = OnepasswordPrefix + "/item-path"
 const RestartDeploymentsAnnotation = OnepasswordPrefix + "/auto-restart"
 
-var ErrCannotUpdateSecretType = errs.New("Cannot change secret type. Secret type is immutable")
+var ErrCannotUpdateSecretType = errors.New("cannot change secret type: secret type is immutable")
 
 var log = logf.Log
 
-func CreateKubernetesSecretFromItem(ctx context.Context, kubeClient kubernetesClient.Client, secretName, namespace string, item *model.Item, autoRestart string, labels map[string]string, secretType string, ownerRef *metav1.OwnerReference) error {
+func CreateKubernetesSecretFromItem(
+	ctx context.Context,
+	kubeClient kubernetesClient.Client,
+	secretName, namespace string,
+	item *model.Item,
+	autoRestart string,
+	labels map[string]string,
+	secretType string,
+	ownerRef *metav1.OwnerReference,
+) error {
 	itemVersion := fmt.Sprint(item.Version)
 	secretAnnotations := map[string]string{
 		VersionAnnotation:  itemVersion,
@@ -40,17 +49,20 @@ func CreateKubernetesSecretFromItem(ctx context.Context, kubeClient kubernetesCl
 	if autoRestart != "" {
 		_, err := utils.StringToBool(autoRestart)
 		if err != nil {
-			return fmt.Errorf("Error parsing %v annotation on Secret %v. Must be true or false. Defaulting to false.", RestartDeploymentsAnnotation, secretName)
+			return fmt.Errorf("error parsing %v annotation on Secret %v. Must be true or false. Defaulting to false",
+				RestartDeploymentsAnnotation, secretName,
+			)
 		}
 		secretAnnotations[RestartDeploymentsAnnotation] = autoRestart
 	}
 
 	// "Opaque" and "" secret types are treated the same by Kubernetes.
-	secret := BuildKubernetesSecretFromOnePasswordItem(secretName, namespace, secretAnnotations, labels, secretType, *item, ownerRef)
+	secret := BuildKubernetesSecretFromOnePasswordItem(secretName, namespace, secretAnnotations, labels,
+		secretType, *item, ownerRef)
 
 	currentSecret := &corev1.Secret{}
 	err := kubeClient.Get(ctx, types.NamespacedName{Name: secret.Name, Namespace: secret.Namespace}, currentSecret)
-	if err != nil && errors.IsNotFound(err) {
+	if err != nil && apierrors.IsNotFound(err) {
 		log.Info(fmt.Sprintf("Creating Secret %v at namespace '%v'", secret.Name, secret.Namespace))
 		return kubeClient.Create(ctx, secret)
 	} else if err != nil {
@@ -75,20 +87,29 @@ func CreateKubernetesSecretFromItem(ctx context.Context, kubeClient kubernetesCl
 	currentLabels := currentSecret.Labels
 	if !reflect.DeepEqual(currentAnnotations, secretAnnotations) || !reflect.DeepEqual(currentLabels, labels) {
 		log.Info(fmt.Sprintf("Updating Secret %v at namespace '%v'", secret.Name, secret.Namespace))
-		currentSecret.ObjectMeta.Annotations = secretAnnotations
-		currentSecret.ObjectMeta.Labels = labels
+		currentSecret.Annotations = secretAnnotations
+		currentSecret.Labels = labels
 		currentSecret.Data = secret.Data
 		if err := kubeClient.Update(ctx, currentSecret); err != nil {
-			return fmt.Errorf("Kubernetes secret update failed: %w", err)
+			return fmt.Errorf("kubernetes secret update failed: %w", err)
 		}
 		return nil
 	}
 
-	log.Info(fmt.Sprintf("Secret with name %v and version %v already exists", secret.Name, secret.Annotations[VersionAnnotation]))
+	log.Info(fmt.Sprintf("Secret with name %v and version %v already exists",
+		secret.Name, secret.Annotations[VersionAnnotation],
+	))
 	return nil
 }
 
-func BuildKubernetesSecretFromOnePasswordItem(name, namespace string, annotations map[string]string, labels map[string]string, secretType string, item model.Item, ownerRef *metav1.OwnerReference) *corev1.Secret {
+func BuildKubernetesSecretFromOnePasswordItem(
+	name, namespace string,
+	annotations map[string]string,
+	labels map[string]string,
+	secretType string,
+	item model.Item,
+	ownerRef *metav1.OwnerReference,
+) *corev1.Secret {
 	var ownerRefs []metav1.OwnerReference
 	if ownerRef != nil {
 		ownerRefs = []metav1.OwnerReference{*ownerRef}

pkg/kubernetessecrets/kubernetes_secrets_builder_test.go

@@ -3,7 +3,6 @@ package kubernetessecrets
 import (
 	"context"
 	"fmt"
-	"github.com/1Password/onepassword-operator/pkg/onepassword/model"
 	"strings"
 	"testing"
 
@@ -12,26 +11,34 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	kubeValidate "k8s.io/apimachinery/pkg/util/validation"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	"github.com/1Password/onepassword-operator/pkg/onepassword/model"
 )
 
-const restartDeploymentAnnotation = "false"
+const (
+	restartDeploymentAnnotation = "false"
+	testNamespace               = "test"
+	testItemUUID                = "h46bb3jddvay7nxopfhvlwg35q"
+	testVaultUUID               = "hfnjvi6aymbsnfc2xeeoheizda"
+)
 
 func TestCreateKubernetesSecretFromOnePasswordItem(t *testing.T) {
 	ctx := context.Background()
 	secretName := "test-secret-name"
-	namespace := "test"
+	namespace := testNamespace
 
 	item := model.Item{}
 	item.Fields = generateFields(5)
 	item.Version = 123
-	item.VaultID = "hfnjvi6aymbsnfc2xeeoheizda"
-	item.ID = "h46bb3jddvay7nxopfhvlwg35q"
+	item.VaultID = testVaultUUID
+	item.ID = testItemUUID
 
 	kubeClient := fake.NewClientBuilder().Build()
 	secretLabels := map[string]string{}
 	secretType := ""
 
-	err := CreateKubernetesSecretFromItem(ctx, kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, nil)
+	err := CreateKubernetesSecretFromItem(ctx, kubeClient, secretName, namespace, &item, restartDeploymentAnnotation,
+		secretLabels, secretType, nil)
 	if err != nil {
 		t.Errorf("Unexpected error: %v", err)
 	}
@@ -48,13 +55,13 @@ func TestCreateKubernetesSecretFromOnePasswordItem(t *testing.T) {
 func TestKubernetesSecretFromOnePasswordItemOwnerReferences(t *testing.T) {
 	ctx := context.Background()
 	secretName := "test-secret-name"
-	namespace := "test"
+	namespace := testNamespace
 
 	item := model.Item{}
 	item.Fields = generateFields(5)
 	item.Version = 123
-	item.VaultID = "hfnjvi6aymbsnfc2xeeoheizda"
-	item.ID = "h46bb3jddvay7nxopfhvlwg35q"
+	item.VaultID = testVaultUUID
+	item.ID = testItemUUID
 
 	kubeClient := fake.NewClientBuilder().Build()
 	secretLabels := map[string]string{}
@@ -66,15 +73,19 @@ func TestKubernetesSecretFromOnePasswordItemOwnerReferences(t *testing.T) {
 		Name:       "test-deployment",
 		UID:        types.UID("test-uid"),
 	}
-	err := CreateKubernetesSecretFromItem(ctx, kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, ownerRef)
+	err := CreateKubernetesSecretFromItem(ctx, kubeClient, secretName, namespace, &item, restartDeploymentAnnotation,
+		secretLabels, secretType, ownerRef)
 	if err != nil {
 		t.Errorf("Unexpected error: %v", err)
 	}
 	createdSecret := &corev1.Secret{}
 	err = kubeClient.Get(ctx, types.NamespacedName{Name: secretName, Namespace: namespace}, createdSecret)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
 
 	// Check owner references.
-	gotOwnerRefs := createdSecret.ObjectMeta.OwnerReferences
+	gotOwnerRefs := createdSecret.OwnerReferences
 	if len(gotOwnerRefs) != 1 {
 		t.Errorf("Expected owner references length: 1 but got: %d", len(gotOwnerRefs))
 	}
@@ -94,19 +105,20 @@ func TestKubernetesSecretFromOnePasswordItemOwnerReferences(t *testing.T) {
 func TestUpdateKubernetesSecretFromOnePasswordItem(t *testing.T) {
 	ctx := context.Background()
 	secretName := "test-secret-update"
-	namespace := "test"
+	namespace := testNamespace
 
 	item := model.Item{}
 	item.Fields = generateFields(5)
 	item.Version = 123
-	item.VaultID = "hfnjvi6aymbsnfc2xeeoheizda"
-	item.ID = "h46bb3jddvay7nxopfhvlwg35q"
+	item.VaultID = testVaultUUID
+	item.ID = testItemUUID
 
 	kubeClient := fake.NewClientBuilder().Build()
 	secretLabels := map[string]string{}
 	secretType := ""
 
-	err := CreateKubernetesSecretFromItem(ctx, kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, nil)
+	err := CreateKubernetesSecretFromItem(ctx, kubeClient, secretName, namespace, &item, restartDeploymentAnnotation,
+		secretLabels, secretType, nil)
 
 	if err != nil {
 		t.Errorf("Unexpected error: %v", err)
@@ -116,9 +128,10 @@ func TestUpdateKubernetesSecretFromOnePasswordItem(t *testing.T) {
 	newItem := model.Item{}
 	newItem.Fields = generateFields(6)
 	newItem.Version = 456
-	newItem.VaultID = "hfnjvi6aymbsnfc2xeeoheizda"
-	newItem.ID = "h46bb3jddvay7nxopfhvlwg35q"
-	err = CreateKubernetesSecretFromItem(ctx, kubeClient, secretName, namespace, &newItem, restartDeploymentAnnotation, secretLabels, secretType, nil)
+	newItem.VaultID = testVaultUUID
+	newItem.ID = testItemUUID
+	err = CreateKubernetesSecretFromItem(ctx, kubeClient, secretName, namespace, &newItem, restartDeploymentAnnotation,
+		secretLabels, secretType, nil)
 	if err != nil {
 		t.Errorf("Unexpected error: %v", err)
 	}
@@ -210,19 +223,20 @@ func TestBuildKubernetesSecretFixesInvalidLabels(t *testing.T) {
 func TestCreateKubernetesTLSSecretFromOnePasswordItem(t *testing.T) {
 	ctx := context.Background()
 	secretName := "tls-test-secret-name"
-	namespace := "test"
+	namespace := testNamespace
 
 	item := model.Item{}
 	item.Fields = generateFields(5)
 	item.Version = 123
-	item.VaultID = "hfnjvi6aymbsnfc2xeeoheizda"
-	item.ID = "h46bb3jddvay7nxopfhvlwg35q"
+	item.VaultID = testVaultUUID
+	item.ID = testItemUUID
 
 	kubeClient := fake.NewClientBuilder().Build()
 	secretLabels := map[string]string{}
 	secretType := "kubernetes.io/tls"
 
-	err := CreateKubernetesSecretFromItem(ctx, kubeClient, secretName, namespace, &item, restartDeploymentAnnotation, secretLabels, secretType, nil)
+	err := CreateKubernetesSecretFromItem(ctx, kubeClient, secretName, namespace, &item, restartDeploymentAnnotation,
+		secretLabels, secretType, nil)
 	if err != nil {
 		t.Errorf("Unexpected error: %v", err)
 	}
@@ -254,7 +268,9 @@ func compareAnnotationsToItem(annotations map[string]string, item model.Item, t
 	}
 
 	if annotations[RestartDeploymentsAnnotation] != "false" {
-		t.Errorf("Expected restart deployments annotation to be %v but was %v", restartDeploymentAnnotation, RestartDeploymentsAnnotation)
+		t.Errorf("Expected restart deployments annotation to be %v but was %v",
+			restartDeploymentAnnotation, RestartDeploymentsAnnotation,
+		)
 	}
 }
 
@@ -286,7 +302,10 @@ func ParseVaultIdAndItemIdFromPath(path string) (string, string, error) {
 	if len(splitPath) == 4 && splitPath[0] == "vaults" && splitPath[2] == "items" {
 		return splitPath[1], splitPath[3], nil
 	}
-	return "", "", fmt.Errorf("%q is not an acceptable path for One Password item. Must be of the format: `vaults/{vault_id}/items/{item_id}`", path)
+	return "", "", fmt.Errorf(
+		"%q is not an acceptable path for One Password item. Must be of the format: `vaults/{vault_id}/items/{item_id}`",
+		path,
+	)
 }
 
 func validLabel(v string) bool {

pkg/logs/log_levels.go

@@ -1,7 +1,7 @@
 package logs
 
 // A Level is a logging priority. Lower levels are more important.
-// All levels have been multipled by -1 to ensure compatibilty
+// All levels have been multiplied by -1 to ensure compatibility
 // between zapcore and logr
 const (
 	ErrorLevel = -2

pkg/mocks/mocksecretserver.go

@@ -2,6 +2,7 @@ package mocks
 
 import (
 	"context"
+
 	"github.com/stretchr/testify/mock"
 
 	"github.com/1Password/onepassword-operator/pkg/onepassword/model"

pkg/onepassword/annotations.go

@@ -45,13 +45,14 @@ func FilterAnnotations(annotations map[string]string, regex *regexp.Regexp) map[
 
 func AreAnnotationsUsingSecrets(annotations map[string]string, secrets map[string]*corev1.Secret) bool {
 	_, ok := secrets[annotations[NameAnnotation]]
-	if ok {
-		return true
-	}
-	return false
+	return ok
 }
 
-func AppendAnnotationUpdatedSecret(annotations map[string]string, secrets map[string]*corev1.Secret, updatedDeploymentSecrets map[string]*corev1.Secret) map[string]*corev1.Secret {
+func AppendAnnotationUpdatedSecret(
+	annotations map[string]string,
+	secrets map[string]*corev1.Secret,
+	updatedDeploymentSecrets map[string]*corev1.Secret,
+) map[string]*corev1.Secret {
 	secret, ok := secrets[annotations[NameAnnotation]]
 	if ok {
 		updatedDeploymentSecrets[secret.Name] = secret

pkg/onepassword/annotations_test.go

@@ -80,7 +80,7 @@ func TestGetNoAnnotationsForDeployment(t *testing.T) {
 	}
 
 	numAnnotations := len(filteredAnnotations)
-	if 0 != numAnnotations {
+	if numAnnotations != 0 {
 		t.Errorf("Expected %v annotations got %v", 0, numAnnotations)
 	}
 }

pkg/onepassword/client/connect/connect.go

@@ -58,7 +58,8 @@ func (c *Connect) GetItemsByTitle(ctx context.Context, vaultID, itemTitle string
 }
 
 // GetFileContent retrieves the content of a file from a 1Password item.
-// As the Connect has a delay when synchronizing files and returns a 500 error in this case, this function implements a retry mechanism.
+// As the Connect has a delay when synchronizing files and returns a 500 error in this case,
+// this function implements a retry mechanism.
 func (c *Connect) GetFileContent(ctx context.Context, vaultID, itemID, fileID string) ([]byte, error) {
 	const maxRetries = 5
 	const delay = 1 * time.Second

pkg/onepassword/client/testing/item.go

@@ -1,7 +1,6 @@
 package testing
 
 import (
-	sdk "github.com/1password/onepassword-sdk-go"
 	"testing"
 	"time"
 
@@ -9,6 +8,7 @@ import (
 
 	"github.com/1Password/connect-sdk-go/onepassword"
 	"github.com/1Password/onepassword-operator/pkg/onepassword/model"
+	sdk "github.com/1password/onepassword-sdk-go"
 )
 
 func CreateConnectItem() *onepassword.Item {

pkg/onepassword/client/testing/mock/connect.go

@@ -104,7 +104,11 @@ func (c *ConnectClientMock) GetFileContent(file *onepassword.File) ([]byte, erro
 	return args.Get(0).([]byte), args.Error(1)
 }
 
-func (c *ConnectClientMock) DownloadFile(file *onepassword.File, targetDirectory string, overwrite bool) (string, error) {
+func (c *ConnectClientMock) DownloadFile(
+	file *onepassword.File,
+	targetDirectory string,
+	overwrite bool,
+) (string, error) {
 	// Only implement this if mocking is needed
 	panic("implement me")
 }

pkg/onepassword/client/testing/mock/sdk.go

@@ -23,7 +23,7 @@ type ItemAPIMock struct {
 }
 
 func (i *ItemAPIMock) Create(ctx context.Context, params sdk.ItemCreateParams) (sdk.Item, error) {
-	//TODO implement me
+	// TODO implement me
 	panic("implement me")
 }
 
@@ -33,27 +33,31 @@ func (i *ItemAPIMock) Get(ctx context.Context, vaultID string, itemID string) (s
 }
 
 func (i *ItemAPIMock) Put(ctx context.Context, item sdk.Item) (sdk.Item, error) {
-	//TODO implement me
+	// TODO implement me
 	panic("implement me")
 }
 
 func (i *ItemAPIMock) Delete(ctx context.Context, vaultID string, itemID string) error {
-	//TODO implement me
+	// TODO implement me
 	panic("implement me")
 }
 
 func (i *ItemAPIMock) Archive(ctx context.Context, vaultID string, itemID string) error {
-	//TODO implement me
+	// TODO implement me
 	panic("implement me")
 }
 
-func (i *ItemAPIMock) List(ctx context.Context, vaultID string, filters ...sdk.ItemListFilter) ([]sdk.ItemOverview, error) {
+func (i *ItemAPIMock) List(
+	ctx context.Context,
+	vaultID string,
+	filters ...sdk.ItemListFilter,
+) ([]sdk.ItemOverview, error) {
 	args := i.Called(ctx, vaultID, filters)
 	return args.Get(0).([]sdk.ItemOverview), args.Error(1)
 }
 
 func (i *ItemAPIMock) Shares() sdk.ItemsSharesAPI {
-	//TODO implement me
+	// TODO implement me
 	panic("implement me")
 }
 
@@ -66,17 +70,21 @@ type FileAPIMock struct {
 }
 
 func (f *FileAPIMock) Attach(ctx context.Context, item sdk.Item, fileParams sdk.FileCreateParams) (sdk.Item, error) {
-	//TODO implement me
+	// TODO implement me
 	panic("implement me")
 }
 
 func (f *FileAPIMock) Delete(ctx context.Context, item sdk.Item, sectionID string, fieldID string) (sdk.Item, error) {
-	//TODO implement me
+	// TODO implement me
 	panic("implement me")
 }
 
-func (f *FileAPIMock) ReplaceDocument(ctx context.Context, item sdk.Item, docParams sdk.DocumentCreateParams) (sdk.Item, error) {
-	//TODO implement me
+func (f *FileAPIMock) ReplaceDocument(
+	ctx context.Context,
+	item sdk.Item,
+	docParams sdk.DocumentCreateParams,
+) (sdk.Item, error) {
+	// TODO implement me
 	panic("implement me")
 }
 

pkg/onepassword/connect_setup.go

@@ -32,11 +32,19 @@ func SetupConnect(ctx context.Context, kubeClient client.Client, deploymentNames
 	return nil
 }
 
-func setupDeployment(ctx context.Context, kubeClient client.Client, deploymentPath string, deploymentNamespace string) error {
+func setupDeployment(
+	ctx context.Context,
+	kubeClient client.Client,
+	deploymentPath string,
+	deploymentNamespace string,
+) error {
 	existingDeployment := &appsv1.Deployment{}
 
 	// check if deployment has already been created
-	err := kubeClient.Get(ctx, types.NamespacedName{Name: "onepassword-connect", Namespace: deploymentNamespace}, existingDeployment)
+	err := kubeClient.Get(ctx, types.NamespacedName{
+		Name:      "onepassword-connect",
+		Namespace: deploymentNamespace,
+	}, existingDeployment)
 	if err != nil {
 		if errors.IsNotFound(err) {
 			logConnectSetup.Info("No existing Connect deployment found. Creating Deployment")
@@ -46,7 +54,12 @@ func setupDeployment(ctx context.Context, kubeClient client.Client, deploymentPa
 	return err
 }
 
-func createDeployment(ctx context.Context, kubeClient client.Client, deploymentPath string, deploymentNamespace string) error {
+func createDeployment(
+	ctx context.Context,
+	kubeClient client.Client,
+	deploymentPath string,
+	deploymentNamespace string,
+) error {
 	deployment, err := getDeploymentToCreate(deploymentPath, deploymentNamespace)
 	if err != nil {
 		return err
@@ -81,8 +94,11 @@ func getDeploymentToCreate(deploymentPath string, deploymentNamespace string) (*
 func setupService(ctx context.Context, kubeClient client.Client, servicePath string, deploymentNamespace string) error {
 	existingService := &corev1.Service{}
 
-	//check if service has already been created
-	err := kubeClient.Get(ctx, types.NamespacedName{Name: "onepassword-connect", Namespace: deploymentNamespace}, existingService)
+	// check if service has already been created
+	err := kubeClient.Get(ctx, types.NamespacedName{
+		Name:      "onepassword-connect",
+		Namespace: deploymentNamespace,
+	}, existingService)
 	if err != nil {
 		if errors.IsNotFound(err) {
 			logConnectSetup.Info("No existing Connect service found. Creating Service")
@@ -92,7 +108,12 @@ func setupService(ctx context.Context, kubeClient client.Client, servicePath str
 	return err
 }
 
-func createService(ctx context.Context, kubeClient client.Client, servicePath string, deploymentNamespace string) error {
+func createService(
+	ctx context.Context,
+	kubeClient client.Client,
+	servicePath string,
+	deploymentNamespace string,
+) error {
 	f, err := os.Open(servicePath)
 	if err != nil {
 		return err

pkg/onepassword/containers.go

@@ -28,7 +28,11 @@ func AreContainersUsingSecrets(containers []corev1.Container, secrets map[string
 	return false
 }
 
-func AppendUpdatedContainerSecrets(containers []corev1.Container, secrets map[string]*corev1.Secret, updatedDeploymentSecrets map[string]*corev1.Secret) map[string]*corev1.Secret {
+func AppendUpdatedContainerSecrets(
+	containers []corev1.Container,
+	secrets map[string]*corev1.Secret,
+	updatedDeploymentSecrets map[string]*corev1.Secret,
+) map[string]*corev1.Secret {
 	for i := 0; i < len(containers); i++ {
 		envVariables := containers[i].Env
 		for j := 0; j < len(envVariables); j++ {
@@ -42,7 +46,7 @@ func AppendUpdatedContainerSecrets(containers []corev1.Container, secrets map[st
 		envFromVariables := containers[i].EnvFrom
 		for j := 0; j < len(envFromVariables); j++ {
 			if envFromVariables[j].SecretRef != nil {
-				secret, ok := secrets[envFromVariables[j].SecretRef.LocalObjectReference.Name]
+				secret, ok := secrets[envFromVariables[j].SecretRef.Name]
 				if ok {
 					updatedDeploymentSecrets[secret.Name] = secret
 				}

pkg/onepassword/deployments.go

@@ -9,10 +9,15 @@ func IsDeploymentUsingSecrets(deployment *appsv1.Deployment, secrets map[string]
 	volumes := deployment.Spec.Template.Spec.Volumes
 	containers := deployment.Spec.Template.Spec.Containers
 	containers = append(containers, deployment.Spec.Template.Spec.InitContainers...)
-	return AreAnnotationsUsingSecrets(deployment.Annotations, secrets) || AreContainersUsingSecrets(containers, secrets) || AreVolumesUsingSecrets(volumes, secrets)
+	return AreAnnotationsUsingSecrets(deployment.Annotations, secrets) ||
+		AreContainersUsingSecrets(containers, secrets) ||
+		AreVolumesUsingSecrets(volumes, secrets)
 }
 
-func GetUpdatedSecretsForDeployment(deployment *appsv1.Deployment, secrets map[string]*corev1.Secret) map[string]*corev1.Secret {
+func GetUpdatedSecretsForDeployment(
+	deployment *appsv1.Deployment,
+	secrets map[string]*corev1.Secret,
+) map[string]*corev1.Secret {
 	volumes := deployment.Spec.Template.Spec.Volumes
 	containers := deployment.Spec.Template.Spec.Containers
 	containers = append(containers, deployment.Spec.Template.Spec.InitContainers...)

pkg/onepassword/items.go

@@ -49,7 +49,10 @@ func ParseVaultAndItemFromPath(path string) (string, string, error) {
 	if len(splitPath) == 4 && splitPath[0] == "vaults" && splitPath[2] == "items" {
 		return splitPath[1], splitPath[3], nil
 	}
-	return "", "", fmt.Errorf("%q is not an acceptable path for One Password item. Must be of the format: `vaults/{vault_id}/items/{item_id}`", path)
+	return "", "", fmt.Errorf(
+		"%q is not an acceptable path for One Password item. Must be of the format: `vaults/{vault_id}/items/{item_id}`",
+		path,
+	)
 }
 
 func getVaultID(ctx context.Context, client opclient.Client, vaultNameOrID string) (string, error) {
@@ -60,7 +63,7 @@ func getVaultID(ctx context.Context, client opclient.Client, vaultNameOrID strin
 		}
 
 		if len(vaults) == 0 {
-			return "", fmt.Errorf("No vaults found with identifier %q", vaultNameOrID)
+			return "", fmt.Errorf("no vaults found with identifier %q", vaultNameOrID)
 		}
 
 		oldestVault := vaults[0]
@@ -70,7 +73,9 @@ func getVaultID(ctx context.Context, client opclient.Client, vaultNameOrID strin
 					oldestVault = returnedVault
 				}
 			}
-			logger.Info(fmt.Sprintf("%v 1Password vaults found with the title %q. Will use vault %q as it is the oldest.", len(vaults), vaultNameOrID, oldestVault.ID))
+			logger.Info(fmt.Sprintf("%v 1Password vaults found with the title %q. Will use vault %q as it is the oldest.",
+				len(vaults), vaultNameOrID, oldestVault.ID,
+			))
 		}
 		vaultNameOrID = oldestVault.ID
 	}
@@ -85,7 +90,7 @@ func getItemID(ctx context.Context, client opclient.Client, vaultId, itemNameOrI
 		}
 
 		if len(items) == 0 {
-			return "", fmt.Errorf("No items found with identifier %q", itemNameOrID)
+			return "", fmt.Errorf("no items found with identifier %q", itemNameOrID)
 		}
 
 		oldestItem := items[0]
@@ -95,7 +100,9 @@ func getItemID(ctx context.Context, client opclient.Client, vaultId, itemNameOrI
 					oldestItem = returnedItem
 				}
 			}
-			logger.Info(fmt.Sprintf("%v 1Password items found with the title %q. Will use item %q as it is the oldest.", len(items), itemNameOrID, oldestItem.ID))
+			logger.Info(fmt.Sprintf("%v 1Password items found with the title %q. Will use item %q as it is the oldest.",
+				len(items), itemNameOrID, oldestItem.ID,
+			))
 		}
 		itemNameOrID = oldestItem.ID
 	}

pkg/onepassword/model/item.go

@@ -24,9 +24,7 @@ func (i *Item) FromConnectItem(item *connect.Item) {
 	i.VaultID = item.Vault.ID
 	i.Version = item.Version
 
-	for _, tag := range item.Tags {
-		i.Tags = append(i.Tags, tag)
-	}
+	i.Tags = append(i.Tags, item.Tags...)
 
 	for _, field := range item.Fields {
 		i.Fields = append(i.Fields, ItemField{

pkg/onepassword/secret_update_handler.go

@@ -18,12 +18,16 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
-const envHostVariable = "OP_HOST"
+// const envHostVariable = "OP_HOST"
 const lockTag = "operator.1password.io:ignore-secret"
 
 var log = logf.Log.WithName("update_op_kubernetes_secrets_task")
 
-func NewManager(kubernetesClient client.Client, opClient opclient.Client, shouldAutoRestartDeploymentsGlobal bool) *SecretUpdateHandler {
+func NewManager(
+	kubernetesClient client.Client,
+	opClient opclient.Client,
+	shouldAutoRestartDeploymentsGlobal bool,
+) *SecretUpdateHandler {
 	return &SecretUpdateHandler{
 		client:                             kubernetesClient,
 		opClient:                           opClient,
@@ -46,7 +50,10 @@ func (h *SecretUpdateHandler) UpdateKubernetesSecretsTask(ctx context.Context) e
 	return h.restartDeploymentsWithUpdatedSecrets(ctx, updatedKubernetesSecrets)
 }
 
-func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(ctx context.Context, updatedSecretsByNamespace map[string]map[string]*corev1.Secret) error {
+func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(
+	ctx context.Context,
+	updatedSecretsByNamespace map[string]map[string]*corev1.Secret,
+) error {
 	// No secrets to update. Exit
 	if len(updatedSecretsByNamespace) == 0 || updatedSecretsByNamespace == nil {
 		return nil
@@ -83,14 +90,18 @@ func (h *SecretUpdateHandler) restartDeploymentsWithUpdatedSecrets(ctx context.C
 			}
 		}
 
-		log.V(logs.DebugLevel).Info(fmt.Sprintf("Deployment %q at namespace %q is up to date", deployment.GetName(), deployment.Namespace))
+		log.V(logs.DebugLevel).Info(fmt.Sprintf("Deployment %q at namespace %q is up to date",
+			deployment.GetName(), deployment.Namespace,
+		))
 
 	}
 	return nil
 }
 
 func (h *SecretUpdateHandler) restartDeployment(ctx context.Context, deployment *appsv1.Deployment) {
-	log.Info(fmt.Sprintf("Deployment %q at namespace %q references an updated secret. Restarting", deployment.GetName(), deployment.Namespace))
+	log.Info(fmt.Sprintf("Deployment %q at namespace %q references an updated secret. Restarting",
+		deployment.GetName(), deployment.Namespace,
+	))
 	if deployment.Spec.Template.Annotations == nil {
 		deployment.Spec.Template.Annotations = map[string]string{}
 	}
@@ -101,7 +112,9 @@ func (h *SecretUpdateHandler) restartDeployment(ctx context.Context, deployment
 	}
 }
 
-func (h *SecretUpdateHandler) updateKubernetesSecrets(ctx context.Context) (map[string]map[string]*corev1.Secret, error) {
+func (h *SecretUpdateHandler) updateKubernetesSecrets(ctx context.Context) (
+	map[string]map[string]*corev1.Secret, error,
+) {
 	secrets := &corev1.SecretList{}
 	err := h.client.List(ctx, secrets)
 	if err != nil {
@@ -123,7 +136,9 @@ func (h *SecretUpdateHandler) updateKubernetesSecrets(ctx context.Context) (map[
 
 		item, err := GetOnePasswordItemByPath(ctx, h.opClient, OnePasswordItemPath)
 		if err != nil {
-			log.Error(err, fmt.Sprintf("failed to retrieve 1Password item at path %s for secret %s", secret.Annotations[ItemPathAnnotation], secret.Name))
+			log.Error(err, fmt.Sprintf("failed to retrieve 1Password item at path %s for secret %s",
+				secret.Annotations[ItemPathAnnotation], secret.Name,
+			))
 			continue
 		}
 
@@ -132,7 +147,11 @@ func (h *SecretUpdateHandler) updateKubernetesSecrets(ctx context.Context) (map[
 
 		if currentVersion != itemVersion || secret.Annotations[ItemPathAnnotation] != itemPathString {
 			if isItemLockedForForcedRestarts(item) {
-				log.V(logs.DebugLevel).Info(fmt.Sprintf("Secret '%v' has been updated in 1Password but is set to be ignored. Updates to an ignored secret will not trigger an update to a kubernetes secret or a rolling restart.", secret.GetName()))
+				log.V(logs.DebugLevel).Info(fmt.Sprintf(
+					"Secret '%v' has been updated in 1Password but is set to be ignored. "+
+						"Updates to an ignored secret will not trigger an update to a kubernetes secret or a rolling restart.",
+					secret.GetName(),
+				))
 				secret.Annotations[VersionAnnotation] = itemVersion
 				secret.Annotations[ItemPathAnnotation] = itemPathString
 				if err := h.client.Update(ctx, &secret); err != nil {
@@ -145,7 +164,9 @@ func (h *SecretUpdateHandler) updateKubernetesSecrets(ctx context.Context) (map[
 			secret.Annotations[VersionAnnotation] = itemVersion
 			secret.Annotations[ItemPathAnnotation] = itemPathString
 			secret.Data = kubeSecrets.BuildKubernetesSecretData(item.Fields, item.Files)
-			log.V(logs.DebugLevel).Info(fmt.Sprintf("New secret path: %v and version: %v", secret.Annotations[ItemPathAnnotation], secret.Annotations[VersionAnnotation]))
+			log.V(logs.DebugLevel).Info(fmt.Sprintf("New secret path: %v and version: %v",
+				secret.Annotations[ItemPathAnnotation], secret.Annotations[VersionAnnotation],
+			))
 			if err := h.client.Update(ctx, &secret); err != nil {
 				log.Error(err, fmt.Sprintf("failed to update secret %s to version %s", secret.Name, itemVersion))
 				continue
@@ -171,10 +192,7 @@ func isItemLockedForForcedRestarts(item *model.Item) bool {
 
 func isUpdatedSecret(secretName string, updatedSecrets map[string]*corev1.Secret) bool {
 	_, ok := updatedSecrets[secretName]
-	if ok {
-		return true
-	}
-	return false
+	return ok
 }
 
 func (h *SecretUpdateHandler) getIsSetForAutoRestartByNamespaceMap(ctx context.Context) (map[string]bool, error) {
@@ -209,16 +227,22 @@ func (h *SecretUpdateHandler) getPathFromOnePasswordItem(secret corev1.Secret) s
 	return secret.Annotations[ItemPathAnnotation]
 }
 
-func isSecretSetForAutoRestart(secret *corev1.Secret, deployment *appsv1.Deployment, setForAutoRestartByNamespace map[string]bool) bool {
+func isSecretSetForAutoRestart(
+	secret *corev1.Secret,
+	deployment *appsv1.Deployment,
+	setForAutoRestartByNamespace map[string]bool,
+) bool {
 	restartDeployment := secret.Annotations[RestartDeploymentsAnnotation]
-	//If annotation for auto restarts for deployment is not set. Check for the annotation on its namepsace
+	// If annotation for auto restarts for deployment is not set. Check for the annotation on its namepsace
 	if restartDeployment == "" {
 		return isDeploymentSetForAutoRestart(deployment, setForAutoRestartByNamespace)
 	}
 
 	restartDeploymentBool, err := utils.StringToBool(restartDeployment)
 	if err != nil {
-		log.Error(err, fmt.Sprintf("Error parsing %s annotation on Secret %s. Must be true or false. Defaulting to false.", RestartDeploymentsAnnotation, secret.Name))
+		log.Error(err, fmt.Sprintf("Error parsing %s annotation on Secret %s. Must be true or false. Defaulting to false.",
+			RestartDeploymentsAnnotation, secret.Name,
+		))
 		return false
 	}
 	return restartDeploymentBool
@@ -226,14 +250,17 @@ func isSecretSetForAutoRestart(secret *corev1.Secret, deployment *appsv1.Deploym
 
 func isDeploymentSetForAutoRestart(deployment *appsv1.Deployment, setForAutoRestartByNamespace map[string]bool) bool {
 	restartDeployment := deployment.Annotations[RestartDeploymentsAnnotation]
-	//If annotation for auto restarts for deployment is not set. Check for the annotation on its namepsace
+	// If annotation for auto restarts for deployment is not set. Check for the annotation on its namepsace
 	if restartDeployment == "" {
 		return setForAutoRestartByNamespace[deployment.Namespace]
 	}
 
 	restartDeploymentBool, err := utils.StringToBool(restartDeployment)
 	if err != nil {
-		log.Error(err, fmt.Sprintf("Error parsing %s annotation on Deployment %s. Must be true or false. Defaulting to false.", RestartDeploymentsAnnotation, deployment.Name))
+		log.Error(err, fmt.Sprintf(
+			"Error parsing %s annotation on Deployment %s. Must be true or false. Defaulting to false.",
+			RestartDeploymentsAnnotation, deployment.Name,
+		))
 		return false
 	}
 	return restartDeploymentBool
@@ -241,14 +268,16 @@ func isDeploymentSetForAutoRestart(deployment *appsv1.Deployment, setForAutoRest
 
 func (h *SecretUpdateHandler) isNamespaceSetToAutoRestart(namespace *corev1.Namespace) bool {
 	restartDeployment := namespace.Annotations[RestartDeploymentsAnnotation]
-	//If annotation for auto restarts for deployment is not set. Check environment variable set on the operator
+	// If annotation for auto restarts for deployment is not set. Check environment variable set on the operator
 	if restartDeployment == "" {
 		return h.shouldAutoRestartDeploymentsGlobal
 	}
 
 	restartDeploymentBool, err := utils.StringToBool(restartDeployment)
 	if err != nil {
-		log.Error(err, fmt.Sprintf("Error parsing %s annotation on Namespace %s. Must be true or false. Defaulting to false.", RestartDeploymentsAnnotation, namespace.Name))
+		log.Error(err, fmt.Sprintf("Error parsing %s annotation on Namespace %s. Must be true or false. Defaulting to false.",
+			RestartDeploymentsAnnotation, namespace.Name,
+		))
 		return false
 	}
 	return restartDeploymentBool

pkg/onepassword/secret_update_handler_test.go

@@ -43,7 +43,6 @@ type testUpdateSecretTask struct {
 	existingSecret           *corev1.Secret
 	expectedError            error
 	expectedResultSecret     *corev1.Secret
-	expectedEvents           []string
 	opItem                   map[string]string
 	expectedRestart          bool
 	globalAutoRestartEnabled bool
@@ -63,6 +62,9 @@ var defaultNamespace = &corev1.Namespace{
 	},
 }
 
+// TODO: Refactor test cases to avoid duplication.
+//
+//nolint:dupl
 var tests = []testUpdateSecretTask{
 	{
 		testName:          "Test unrelated deployment is not restarted with an updated secret",
@@ -838,9 +840,10 @@ func TestUpdateSecretHandler(t *testing.T) {
 				assert.Equal(t, testData.expectedResultSecret.Annotations[VersionAnnotation], secret.Annotations[VersionAnnotation])
 			}
 
-			//check if deployment has been restarted
+			// check if deployment has been restarted
 			deployment := &appsv1.Deployment{}
 			err = cl.Get(ctx, types.NamespacedName{Name: testData.existingDeployment.Name, Namespace: namespace}, deployment)
+			assert.NoError(t, err)
 
 			_, ok := deployment.Spec.Template.Annotations[RestartAnnotation]
 			if ok {
@@ -849,7 +852,7 @@ func TestUpdateSecretHandler(t *testing.T) {
 				assert.False(t, testData.expectedRestart, "Deployment was restarted but should not have been.")
 			}
 
-			oldPodTemplateAnnotations := testData.existingDeployment.Spec.Template.ObjectMeta.Annotations
+			oldPodTemplateAnnotations := testData.existingDeployment.Spec.Template.Annotations
 			newPodTemplateAnnotations := deployment.Spec.Template.Annotations
 			for name, expected := range oldPodTemplateAnnotations {
 				actual, ok := newPodTemplateAnnotations[name]

pkg/onepassword/volumes.go

@@ -10,13 +10,14 @@ func AreVolumesUsingSecrets(volumes []corev1.Volume, secrets map[string]*corev1.
 			return false
 		}
 	}
-	if len(volumes) == 0 {
-		return false
-	}
-	return true
+	return len(volumes) > 0
 }
 
-func AppendUpdatedVolumeSecrets(volumes []corev1.Volume, secrets map[string]*corev1.Secret, updatedDeploymentSecrets map[string]*corev1.Secret) map[string]*corev1.Secret {
+func AppendUpdatedVolumeSecrets(
+	volumes []corev1.Volume,
+	secrets map[string]*corev1.Secret,
+	updatedDeploymentSecrets map[string]*corev1.Secret,
+) map[string]*corev1.Secret {
 	for i := 0; i < len(volumes); i++ {
 		secret := IsVolumeUsingSecret(volumes[i], secrets)
 		if secret != nil {