Fix v2 adapter and grpc diff lint

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

providers/v2/adapter/store/client.go

@@ -52,6 +52,7 @@ func (w *Client) GetSecret(ctx context.Context, ref esv1.ExternalSecretDataRemot
 	return w.v2Provider.GetSecret(ctx, ref, w.providerRef, w.sourceNamespace)
 }
 
+// GetSecretMap retrieves a secret object and returns its key/value pairs.
 func (w *Client) GetSecretMap(ctx context.Context, ref esv1.ExternalSecretDataRemoteRef) (map[string][]byte, error) {
 	return w.v2Provider.GetSecretMap(ctx, ref, w.providerRef, w.sourceNamespace)
 }

providers/v2/adapter/store/client_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package store
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"testing"
@@ -29,6 +30,12 @@ import (
 	pb "github.com/external-secrets/external-secrets/proto/provider"
 )
 
+const (
+	testProperty        = "property"
+	testSourceNamespace = "tenant-a"
+	testValue           = "value"
+)
+
 type fakeV2Provider struct {
 	getSecretResponse    []byte
 	getSecretErr         error
@@ -168,7 +175,7 @@ func (f fakePushSecretRemoteRef) GetProperty() string {
 func TestClientGetSecretDelegatesProviderReferenceAndNamespace(t *testing.T) {
 	providerRef := &pb.ProviderReference{Name: "provider", Namespace: "config-ns"}
 	provider := &fakeV2Provider{getSecretResponse: []byte("secret-value")}
-	client := NewClient(provider, providerRef, "tenant-a")
+	client := NewClient(provider, providerRef, testSourceNamespace)
 
 	ref := esv1.ExternalSecretDataRemoteRef{Key: "sample", Version: "v1", Property: "password"}
 	value, err := client.GetSecret(context.Background(), ref)
@@ -185,7 +192,7 @@ func TestClientGetSecretDelegatesProviderReferenceAndNamespace(t *testing.T) {
 	if provider.getSecretProviderRef != providerRef {
 		t.Fatalf("unexpected provider ref: %#v", provider.getSecretProviderRef)
 	}
-	if provider.getSecretNamespace != "tenant-a" {
+	if provider.getSecretNamespace != testSourceNamespace {
 		t.Fatalf("unexpected source namespace: %q", provider.getSecretNamespace)
 	}
 }
@@ -197,7 +204,7 @@ func TestClientGetSecretMapDelegatesProviderReferenceAndNamespace(t *testing.T)
 		"baz": []byte("qux"),
 	}
 	provider := &fakeV2Provider{getSecretMapResponse: expected}
-	client := NewClient(provider, providerRef, "tenant-a")
+	client := NewClient(provider, providerRef, testSourceNamespace)
 
 	ref := esv1.ExternalSecretDataRemoteRef{Key: "sample"}
 	secretMap, err := client.GetSecretMap(context.Background(), ref)
@@ -214,7 +221,7 @@ func TestClientGetSecretMapDelegatesProviderReferenceAndNamespace(t *testing.T)
 	if provider.getSecretProviderRef != providerRef {
 		t.Fatalf("unexpected provider ref: %#v", provider.getSecretProviderRef)
 	}
-	if provider.getSecretNamespace != "tenant-a" {
+	if provider.getSecretNamespace != testSourceNamespace {
 		t.Fatalf("unexpected source namespace: %q", provider.getSecretNamespace)
 	}
 }
@@ -222,9 +229,9 @@ func TestClientGetSecretMapDelegatesProviderReferenceAndNamespace(t *testing.T)
 func TestClientGetAllSecretsDelegatesFindCriteria(t *testing.T) {
 	providerRef := &pb.ProviderReference{Name: "provider", Namespace: "config-ns"}
 	path := "/team-a"
-	expected := map[string][]byte{"db-password": []byte("value")}
+	expected := map[string][]byte{"db-password": []byte(testValue)}
 	provider := &fakeV2Provider{getAllSecretsResponse: expected}
-	client := NewClient(provider, providerRef, "tenant-a")
+	client := NewClient(provider, providerRef, testSourceNamespace)
 
 	find := esv1.ExternalSecretFind{
 		Tags: map[string]string{
@@ -239,7 +246,7 @@ func TestClientGetAllSecretsDelegatesFindCriteria(t *testing.T) {
 		t.Fatalf("GetAllSecrets() error = %v", err)
 	}
 
-	if string(secrets["db-password"]) != "value" {
+	if string(secrets["db-password"]) != testValue {
 		t.Fatalf("unexpected secret value: %#v", secrets)
 	}
 	if provider.getAllSecretsFind.Tags["team"] != "a" {
@@ -254,7 +261,7 @@ func TestClientGetAllSecretsDelegatesFindCriteria(t *testing.T) {
 	if provider.getSecretProviderRef != providerRef {
 		t.Fatalf("unexpected provider ref: %#v", provider.getSecretProviderRef)
 	}
-	if provider.getSecretNamespace != "tenant-a" {
+	if provider.getSecretNamespace != testSourceNamespace {
 		t.Fatalf("unexpected source namespace: %q", provider.getSecretNamespace)
 	}
 }
@@ -262,18 +269,18 @@ func TestClientGetAllSecretsDelegatesFindCriteria(t *testing.T) {
 func TestClientPushSecretConvertsPayloadAndMetadata(t *testing.T) {
 	providerRef := &pb.ProviderReference{Name: "provider", Namespace: "config-ns"}
 	provider := &fakeV2Provider{}
-	client := NewClient(provider, providerRef, "tenant-a")
+	client := NewClient(provider, providerRef, testSourceNamespace)
 
 	metadata := []byte(`{"owner":"eso"}`)
 	secret := &corev1.Secret{
 		Data: map[string][]byte{
-			"token": []byte("value"),
+			"token": []byte(testValue),
 		},
 	}
 	pushData := fakePushSecretData{
-		property:  "property",
+		property:  testProperty,
 		secretKey: "token",
-		remoteKey: "remote/path",
+		remoteKey: serverTestRemoteKey,
 		metadata:  &apiextensionsv1.JSON{Raw: metadata},
 	}
 
@@ -282,22 +289,22 @@ func TestClientPushSecretConvertsPayloadAndMetadata(t *testing.T) {
 		t.Fatalf("PushSecret() error = %v", err)
 	}
 
-	if string(provider.pushSecretData["token"]) != "value" {
+	if string(provider.pushSecretData["token"]) != testValue {
 		t.Fatalf("unexpected secret data: %#v", provider.pushSecretData)
 	}
 	if provider.pushSecretPayload == nil {
 		t.Fatal("expected push payload to be recorded")
 	}
-	if provider.pushSecretPayload.RemoteKey != "remote/path" || provider.pushSecretPayload.SecretKey != "token" || provider.pushSecretPayload.Property != "property" {
+	if provider.pushSecretPayload.RemoteKey != serverTestRemoteKey || provider.pushSecretPayload.SecretKey != "token" || provider.pushSecretPayload.Property != testProperty {
 		t.Fatalf("unexpected push payload: %#v", provider.pushSecretPayload)
 	}
-	if string(provider.pushSecretPayload.Metadata) != string(metadata) {
+	if !bytes.Equal(provider.pushSecretPayload.Metadata, metadata) {
 		t.Fatalf("unexpected metadata: %q", string(provider.pushSecretPayload.Metadata))
 	}
 	if provider.pushSecretProviderRef != providerRef {
 		t.Fatalf("unexpected provider ref: %#v", provider.pushSecretProviderRef)
 	}
-	if provider.pushSecretNamespace != "tenant-a" {
+	if provider.pushSecretNamespace != testSourceNamespace {
 		t.Fatalf("unexpected source namespace: %q", provider.pushSecretNamespace)
 	}
 }
@@ -305,7 +312,7 @@ func TestClientPushSecretConvertsPayloadAndMetadata(t *testing.T) {
 func TestClientPushSecretForwardsKubernetesSecretShape(t *testing.T) {
 	providerRef := &pb.ProviderReference{Name: "provider", Namespace: "config-ns"}
 	provider := &fakeV2Provider{}
-	client := NewClient(provider, providerRef, "tenant-a")
+	client := NewClient(provider, providerRef, testSourceNamespace)
 
 	metadata := []byte(`{"mergePolicy":"replace"}`)
 	secret := &corev1.Secret{
@@ -319,9 +326,9 @@ func TestClientPushSecretForwardsKubernetesSecretShape(t *testing.T) {
 		},
 	}
 	pushData := fakePushSecretData{
-		property:  "property",
+		property:  testProperty,
 		secretKey: ".dockerconfigjson",
-		remoteKey: "remote/path",
+		remoteKey: serverTestRemoteKey,
 		metadata:  &apiextensionsv1.JSON{Raw: metadata},
 	}
 
@@ -348,7 +355,7 @@ func TestClientPushSecretForwardsKubernetesSecretShape(t *testing.T) {
 	if provider.pushSecretPayload == nil {
 		t.Fatal("expected push payload to be recorded")
 	}
-	if string(provider.pushSecretPayload.Metadata) != string(metadata) {
+	if !bytes.Equal(provider.pushSecretPayload.Metadata, metadata) {
 		t.Fatalf("unexpected metadata: %q", string(provider.pushSecretPayload.Metadata))
 	}
 }
@@ -356,11 +363,11 @@ func TestClientPushSecretForwardsKubernetesSecretShape(t *testing.T) {
 func TestClientDeleteSecretConvertsRemoteRef(t *testing.T) {
 	providerRef := &pb.ProviderReference{Name: "provider", Namespace: "config-ns"}
 	provider := &fakeV2Provider{}
-	client := NewClient(provider, providerRef, "tenant-a")
+	client := NewClient(provider, providerRef, testSourceNamespace)
 
 	err := client.DeleteSecret(context.Background(), fakePushSecretRemoteRef{
-		remoteKey: "remote/path",
-		property:  "property",
+		remoteKey: serverTestRemoteKey,
+		property:  testProperty,
 	})
 	if err != nil {
 		t.Fatalf("DeleteSecret() error = %v", err)
@@ -369,13 +376,13 @@ func TestClientDeleteSecretConvertsRemoteRef(t *testing.T) {
 	if provider.deleteSecretRemoteRef == nil {
 		t.Fatal("expected delete remote ref to be recorded")
 	}
-	if provider.deleteSecretRemoteRef.RemoteKey != "remote/path" || provider.deleteSecretRemoteRef.Property != "property" {
+	if provider.deleteSecretRemoteRef.RemoteKey != serverTestRemoteKey || provider.deleteSecretRemoteRef.Property != testProperty {
 		t.Fatalf("unexpected remote ref: %#v", provider.deleteSecretRemoteRef)
 	}
 	if provider.deleteSecretProviderRef != providerRef {
 		t.Fatalf("unexpected provider ref: %#v", provider.deleteSecretProviderRef)
 	}
-	if provider.deleteSecretNamespace != "tenant-a" {
+	if provider.deleteSecretNamespace != testSourceNamespace {
 		t.Fatalf("unexpected source namespace: %q", provider.deleteSecretNamespace)
 	}
 }
@@ -383,11 +390,11 @@ func TestClientDeleteSecretConvertsRemoteRef(t *testing.T) {
 func TestClientSecretExistsConvertsRemoteRef(t *testing.T) {
 	providerRef := &pb.ProviderReference{Name: "provider", Namespace: "config-ns"}
 	provider := &fakeV2Provider{secretExistsResponse: true}
-	client := NewClient(provider, providerRef, "tenant-a")
+	client := NewClient(provider, providerRef, testSourceNamespace)
 
 	exists, err := client.SecretExists(context.Background(), fakePushSecretRemoteRef{
-		remoteKey: "remote/path",
-		property:  "property",
+		remoteKey: serverTestRemoteKey,
+		property:  testProperty,
 	})
 	if err != nil {
 		t.Fatalf("SecretExists() error = %v", err)
@@ -399,13 +406,13 @@ func TestClientSecretExistsConvertsRemoteRef(t *testing.T) {
 	if provider.secretExistsRemoteRef == nil {
 		t.Fatal("expected exists remote ref to be recorded")
 	}
-	if provider.secretExistsRemoteRef.RemoteKey != "remote/path" || provider.secretExistsRemoteRef.Property != "property" {
+	if provider.secretExistsRemoteRef.RemoteKey != serverTestRemoteKey || provider.secretExistsRemoteRef.Property != testProperty {
 		t.Fatalf("unexpected remote ref: %#v", provider.secretExistsRemoteRef)
 	}
 	if provider.secretExistsProviderRef != providerRef {
 		t.Fatalf("unexpected provider ref: %#v", provider.secretExistsProviderRef)
 	}
-	if provider.secretExistsNamespace != "tenant-a" {
+	if provider.secretExistsNamespace != testSourceNamespace {
 		t.Fatalf("unexpected source namespace: %q", provider.secretExistsNamespace)
 	}
 }
@@ -414,7 +421,7 @@ func TestClientValidateMapsProviderErrors(t *testing.T) {
 	t.Run("success", func(t *testing.T) {
 		providerRef := &pb.ProviderReference{Name: "provider", Namespace: "config-ns"}
 		provider := &fakeV2Provider{}
-		client := NewClient(provider, providerRef, "tenant-a")
+		client := NewClient(provider, providerRef, testSourceNamespace)
 
 		result, err := client.Validate()
 		if err != nil {
@@ -426,7 +433,7 @@ func TestClientValidateMapsProviderErrors(t *testing.T) {
 		if provider.validateProviderRef != providerRef {
 			t.Fatalf("unexpected provider ref: %#v", provider.validateProviderRef)
 		}
-		if provider.validateNamespace != "tenant-a" {
+		if provider.validateNamespace != testSourceNamespace {
 			t.Fatalf("unexpected source namespace: %q", provider.validateNamespace)
 		}
 	})
@@ -434,7 +441,7 @@ func TestClientValidateMapsProviderErrors(t *testing.T) {
 	t.Run("error", func(t *testing.T) {
 		validateErr := errors.New("invalid credentials")
 		provider := &fakeV2Provider{validateErr: validateErr}
-		client := NewClient(provider, &pb.ProviderReference{Name: "provider"}, "tenant-a")
+		client := NewClient(provider, &pb.ProviderReference{Name: "provider"}, testSourceNamespace)
 
 		result, err := client.Validate()
 		if !errors.Is(err, validateErr) {
@@ -449,7 +456,7 @@ func TestClientValidateMapsProviderErrors(t *testing.T) {
 func TestClientCloseDelegates(t *testing.T) {
 	closeErr := errors.New("close failed")
 	provider := &fakeV2Provider{closeErr: closeErr}
-	client := NewClient(provider, &pb.ProviderReference{Name: "provider"}, "tenant-a")
+	client := NewClient(provider, &pb.ProviderReference{Name: "provider"}, testSourceNamespace)
 
 	err := client.Close(context.Background())
 	if !errors.Is(err, closeErr) {

providers/v2/adapter/store/server.go

@@ -104,32 +104,49 @@ func (s *Server) getClient(ctx context.Context, ref *pb.ProviderReference, names
 	return provider.NewClient(ctx, syntheticStore, s.kubeClient, namespace)
 }
 
-// GetSecret retrieves a single secret from the provider.
-func (s *Server) GetSecret(ctx context.Context, req *pb.GetSecretRequest) (*pb.GetSecretResponse, error) {
-	if req == nil || req.RemoteRef == nil {
-		return nil, fmt.Errorf("request or remote ref is nil")
+func (s *Server) getClientForRemoteRef(
+	ctx context.Context,
+	providerRef *pb.ProviderReference,
+	sourceNamespace string,
+	remoteRef *pb.ExternalSecretDataRemoteRef,
+) (esv1.SecretsClient, esv1.ExternalSecretDataRemoteRef, error) {
+	if remoteRef == nil {
+		return nil, esv1.ExternalSecretDataRemoteRef{}, fmt.Errorf("request or remote ref is nil")
 	}
-	if err := validateSourceNamespace(req.SourceNamespace); err != nil {
-		return nil, err
+	if err := validateSourceNamespace(sourceNamespace); err != nil {
+		return nil, esv1.ExternalSecretDataRemoteRef{}, err
 	}
-	client, err := s.getClient(ctx, req.ProviderRef, req.SourceNamespace)
+
+	client, err := s.getClient(ctx, providerRef, sourceNamespace)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get client: %w", err)
+		return nil, esv1.ExternalSecretDataRemoteRef{}, fmt.Errorf("failed to get client: %w", err)
 	}
-	defer func() { _ = client.Close(ctx) }()
 
-	// Convert protobuf remote ref to v1 remote ref
 	ref := esv1.ExternalSecretDataRemoteRef{
-		Key:      req.RemoteRef.Key,
-		Version:  req.RemoteRef.Version,
-		Property: req.RemoteRef.Property,
+		Key:      remoteRef.Key,
+		Version:  remoteRef.Version,
+		Property: remoteRef.Property,
+	}
+	if remoteRef.DecodingStrategy != "" {
+		ref.DecodingStrategy = esv1.ExternalSecretDecodingStrategy(remoteRef.DecodingStrategy)
+	}
+	if remoteRef.MetadataPolicy != "" {
+		ref.MetadataPolicy = esv1.ExternalSecretMetadataPolicy(remoteRef.MetadataPolicy)
 	}
-	if req.RemoteRef.DecodingStrategy != "" {
-		ref.DecodingStrategy = esv1.ExternalSecretDecodingStrategy(req.RemoteRef.DecodingStrategy)
+
+	return client, ref, nil
+}
+
+// GetSecret retrieves a single secret from the provider.
+func (s *Server) GetSecret(ctx context.Context, req *pb.GetSecretRequest) (*pb.GetSecretResponse, error) {
+	if req == nil {
+		return nil, fmt.Errorf("request or remote ref is nil")
 	}
-	if req.RemoteRef.MetadataPolicy != "" {
-		ref.MetadataPolicy = esv1.ExternalSecretMetadataPolicy(req.RemoteRef.MetadataPolicy)
+	client, ref, err := s.getClientForRemoteRef(ctx, req.ProviderRef, req.SourceNamespace, req.RemoteRef)
+	if err != nil {
+		return nil, err
 	}
+	defer func() { _ = client.Close(ctx) }()
 
 	value, err := client.GetSecret(ctx, ref)
 	if err != nil {
@@ -143,31 +160,15 @@ func (s *Server) GetSecret(ctx context.Context, req *pb.GetSecretRequest) (*pb.G
 
 // GetSecretMap retrieves multiple key/value pairs from a single secret object.
 func (s *Server) GetSecretMap(ctx context.Context, req *pb.GetSecretMapRequest) (*pb.GetSecretMapResponse, error) {
-	if req == nil || req.RemoteRef == nil {
+	if req == nil {
 		return nil, fmt.Errorf("request or remote ref is nil")
 	}
-	if err := validateSourceNamespace(req.SourceNamespace); err != nil {
-		return nil, err
-	}
-
-	client, err := s.getClient(ctx, req.ProviderRef, req.SourceNamespace)
+	client, ref, err := s.getClientForRemoteRef(ctx, req.ProviderRef, req.SourceNamespace, req.RemoteRef)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get client: %w", err)
+		return nil, err
 	}
 	defer func() { _ = client.Close(ctx) }()
 
-	ref := esv1.ExternalSecretDataRemoteRef{
-		Key:      req.RemoteRef.Key,
-		Version:  req.RemoteRef.Version,
-		Property: req.RemoteRef.Property,
-	}
-	if req.RemoteRef.DecodingStrategy != "" {
-		ref.DecodingStrategy = esv1.ExternalSecretDecodingStrategy(req.RemoteRef.DecodingStrategy)
-	}
-	if req.RemoteRef.MetadataPolicy != "" {
-		ref.MetadataPolicy = esv1.ExternalSecretMetadataPolicy(req.RemoteRef.MetadataPolicy)
-	}
-
 	secrets, err := client.GetSecretMap(ctx, ref)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get secret map: %w", err)

providers/v2/adapter/store/server_test.go

@@ -30,6 +30,13 @@ import (
 	pb "github.com/external-secrets/external-secrets/proto/provider"
 )
 
+const (
+	serverTestRemoteKey       = "remote/path"
+	serverTestProperty        = "property"
+	serverTestSourceNamespace = "tenant-a"
+	serverTestValue           = "value"
+)
+
 type fakeProviderInterface struct {
 	newClient func(context.Context, esv1.GenericStore, client.Client, string) (esv1.SecretsClient, error)
 	caps      esv1.SecretStoreCapabilities
@@ -152,13 +159,13 @@ func TestServerGetSecretMapsRemoteRefAndSyntheticStoreNamespace(t *testing.T) {
 
 	req := &pb.GetSecretRequest{
 		ProviderRef: &pb.ProviderReference{
-			ApiVersion: "provider.external-secrets.io/v2alpha1",
-			Kind:       "Fake",
-			Name:       "backend",
-			Namespace:  "provider-config-ns",
+			ApiVersion:   "provider.external-secrets.io/v2alpha1",
+			Kind:         "Fake",
+			Name:         "backend",
+			Namespace:    "provider-config-ns",
 			StoreRefKind: esv1.ProviderKindStr,
 		},
-		SourceNamespace: "tenant-a",
+		SourceNamespace: serverTestSourceNamespace,
 		RemoteRef: &pb.ExternalSecretDataRemoteRef{
 			Key:              "sample",
 			Version:          "v1",
@@ -176,17 +183,17 @@ func TestServerGetSecretMapsRemoteRefAndSyntheticStoreNamespace(t *testing.T) {
 	if string(resp.Value) != "secret-value" {
 		t.Fatalf("expected secret-value, got %q", string(resp.Value))
 	}
-	if mapper.ref != req.ProviderRef || mapper.sourceNamespace != "tenant-a" {
+	if mapper.ref != req.ProviderRef || mapper.sourceNamespace != serverTestSourceNamespace {
 		t.Fatalf("unexpected spec mapper input: ref=%#v namespace=%q", mapper.ref, mapper.sourceNamespace)
 	}
-	if receivedNamespace != "tenant-a" {
+	if receivedNamespace != serverTestSourceNamespace {
 		t.Fatalf("unexpected new client namespace: %q", receivedNamespace)
 	}
 	syntheticStore, ok := receivedStore.(*SyntheticStore)
 	if !ok {
 		t.Fatalf("expected SyntheticStore, got %T", receivedStore)
 	}
-	if syntheticStore.Namespace != "tenant-a" {
+	if syntheticStore.Namespace != serverTestSourceNamespace {
 		t.Fatalf("unexpected synthetic store namespace: %q", syntheticStore.Namespace)
 	}
 	if syntheticStore.Kind != esv1.SecretStoreKind {
@@ -238,13 +245,13 @@ func TestServerPushSecretMapsClusterProviderStoreKindToClusterSecretStore(t *tes
 			Name:         "backend",
 			StoreRefKind: esv1.ClusterProviderKindStr,
 		},
-		SourceNamespace: "tenant-a",
+		SourceNamespace: serverTestSourceNamespace,
 		SecretData: map[string][]byte{
-			"value": []byte("secret-value"),
+			serverTestValue: []byte("secret-value"),
 		},
 		PushSecretData: &pb.PushSecretData{
 			RemoteKey: "remote-secret",
-			SecretKey: "value",
+			SecretKey: serverTestValue,
 		},
 	})
 	if err != nil {
@@ -283,7 +290,7 @@ func TestServerGetSecretMapDelegates(t *testing.T) {
 			Kind:       "Fake",
 			Name:       "backend",
 		},
-		SourceNamespace: "tenant-a",
+		SourceNamespace: serverTestSourceNamespace,
 		RemoteRef:       &pb.ExternalSecretDataRemoteRef{Key: "sample"},
 	})
 	if err != nil {
@@ -303,7 +310,7 @@ func TestServerGetAllSecretsMapsFindCriteria(t *testing.T) {
 		spec: &esv1.SecretStoreSpec{Provider: &esv1.SecretStoreProvider{Fake: &esv1.FakeProvider{}}},
 	}
 	fakeClient := &fakeSecretsClient{
-		getAllSecretsResponse: map[string][]byte{"db-password": []byte("value")},
+		getAllSecretsResponse: map[string][]byte{"db-password": []byte(serverTestValue)},
 	}
 
 	server := NewServer(nil, ProviderMapping{
@@ -321,7 +328,7 @@ func TestServerGetAllSecretsMapsFindCriteria(t *testing.T) {
 			Kind:       "Fake",
 			Name:       "backend",
 		},
-		SourceNamespace: "tenant-a",
+		SourceNamespace: serverTestSourceNamespace,
 		Find: &pb.ExternalSecretFind{
 			Tags:               map[string]string{"team": "a"},
 			Path:               "/team-a",
@@ -334,7 +341,7 @@ func TestServerGetAllSecretsMapsFindCriteria(t *testing.T) {
 		t.Fatalf("GetAllSecrets() error = %v", err)
 	}
 
-	if string(resp.Secrets["db-password"]) != "value" {
+	if string(resp.Secrets["db-password"]) != serverTestValue {
 		t.Fatalf("unexpected response: %#v", resp.Secrets)
 	}
 	if fakeClient.getAllSecretsFind.Tags["team"] != "a" {
@@ -369,14 +376,14 @@ func TestServerPushDeleteAndExistsMapWriteRequests(t *testing.T) {
 			Kind:       "Fake",
 			Name:       "backend",
 		},
-		SourceNamespace: "tenant-a",
+		SourceNamespace: serverTestSourceNamespace,
 		SecretData: map[string][]byte{
-			"token": []byte("value"),
+			"token": []byte(serverTestValue),
 		},
 		PushSecretData: &pb.PushSecretData{
-			RemoteKey: "remote/path",
+			RemoteKey: serverTestRemoteKey,
 			SecretKey: "token",
-			Property:  "property",
+			Property:  serverTestProperty,
 			Metadata:  []byte(`{"owner":"eso"}`),
 		},
 	})
@@ -384,13 +391,13 @@ func TestServerPushDeleteAndExistsMapWriteRequests(t *testing.T) {
 		t.Fatalf("PushSecret() error = %v", err)
 	}
 
-	if fakeClient.pushSecretSecret == nil || string(fakeClient.pushSecretSecret.Data["token"]) != "value" {
+	if fakeClient.pushSecretSecret == nil || string(fakeClient.pushSecretSecret.Data["token"]) != serverTestValue {
 		t.Fatalf("unexpected pushed secret: %#v", fakeClient.pushSecretSecret)
 	}
 	if fakeClient.pushSecretSecret.Type != "" {
 		t.Fatalf("unexpected secret type: %q", fakeClient.pushSecretSecret.Type)
 	}
-	if fakeClient.pushSecretData.GetRemoteKey() != "remote/path" || fakeClient.pushSecretData.GetSecretKey() != "token" || fakeClient.pushSecretData.GetProperty() != "property" {
+	if fakeClient.pushSecretData.GetRemoteKey() != serverTestRemoteKey || fakeClient.pushSecretData.GetSecretKey() != "token" || fakeClient.pushSecretData.GetProperty() != serverTestProperty {
 		t.Fatalf("unexpected push data: %#v", fakeClient.pushSecretData)
 	}
 	if got := fakeClient.pushSecretData.GetMetadata(); got == nil || string(got.Raw) != `{"owner":"eso"}` {
@@ -403,17 +410,17 @@ func TestServerPushDeleteAndExistsMapWriteRequests(t *testing.T) {
 			Kind:       "Fake",
 			Name:       "backend",
 		},
-		SourceNamespace: "tenant-a",
+		SourceNamespace: serverTestSourceNamespace,
 		RemoteRef: &pb.PushSecretRemoteRef{
-			RemoteKey: "remote/path",
-			Property:  "property",
+			RemoteKey: serverTestRemoteKey,
+			Property:  serverTestProperty,
 		},
 	})
 	if err != nil {
 		t.Fatalf("DeleteSecret() error = %v", err)
 	}
 
-	if fakeClient.deleteSecretRef.GetRemoteKey() != "remote/path" || fakeClient.deleteSecretRef.GetProperty() != "property" {
+	if fakeClient.deleteSecretRef.GetRemoteKey() != serverTestRemoteKey || fakeClient.deleteSecretRef.GetProperty() != serverTestProperty {
 		t.Fatalf("unexpected delete ref: %#v", fakeClient.deleteSecretRef)
 	}
 
@@ -423,10 +430,10 @@ func TestServerPushDeleteAndExistsMapWriteRequests(t *testing.T) {
 			Kind:       "Fake",
 			Name:       "backend",
 		},
-		SourceNamespace: "tenant-a",
+		SourceNamespace: serverTestSourceNamespace,
 		RemoteRef: &pb.PushSecretRemoteRef{
-			RemoteKey: "remote/path",
-			Property:  "property",
+			RemoteKey: serverTestRemoteKey,
+			Property:  serverTestProperty,
 		},
 	})
 	if err != nil {
@@ -436,7 +443,7 @@ func TestServerPushDeleteAndExistsMapWriteRequests(t *testing.T) {
 	if !resp.Exists {
 		t.Fatal("expected exists response to be true")
 	}
-	if fakeClient.secretExistsRef.GetRemoteKey() != "remote/path" || fakeClient.secretExistsRef.GetProperty() != "property" {
+	if fakeClient.secretExistsRef.GetRemoteKey() != serverTestRemoteKey || fakeClient.secretExistsRef.GetProperty() != serverTestProperty {
 		t.Fatalf("unexpected exists ref: %#v", fakeClient.secretExistsRef)
 	}
 }
@@ -462,7 +469,7 @@ func TestServerPushSecretForwardsKubernetesSecretMetadata(t *testing.T) {
 			Kind:       "Fake",
 			Name:       "backend",
 		},
-		SourceNamespace: "tenant-a",
+		SourceNamespace: serverTestSourceNamespace,
 		SecretData: map[string][]byte{
 			".dockerconfigjson": []byte("payload"),
 		},
@@ -470,9 +477,9 @@ func TestServerPushSecretForwardsKubernetesSecretMetadata(t *testing.T) {
 		SecretLabels:      map[string]string{"team": "platform"},
 		SecretAnnotations: map[string]string{"owner": "app-team"},
 		PushSecretData: &pb.PushSecretData{
-			RemoteKey: "remote/path",
+			RemoteKey: serverTestRemoteKey,
 			SecretKey: ".dockerconfigjson",
-			Property:  "property",
+			Property:  serverTestProperty,
 			Metadata:  []byte(`{"mergePolicy":"replace"}`),
 		},
 	}
@@ -599,7 +606,7 @@ func TestServerRejectsInvalidRequests(t *testing.T) {
 			name: "push_secret_nil_payload",
 			call: func() error {
 				_, err := server.PushSecret(context.Background(), &pb.PushSecretRequest{
-					SourceNamespace: "tenant-a",
+					SourceNamespace: serverTestSourceNamespace,
 				})
 				return err
 			},

providers/v2/common/grpc/server/http.go

@@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+// Package server provides gRPC server infrastructure for v2 providers.
 package server
 
 import (
@@ -28,21 +29,21 @@ import (
 )
 
 const (
-	// DefaultMetricsPort is the default port for the HTTP metrics server
+	// DefaultMetricsPort is the default port for the HTTP metrics server.
 	DefaultMetricsPort = 8081
-	// DefaultMetricsPath is the default path for the metrics endpoint
+	// DefaultMetricsPath is the default path for the metrics endpoint.
 	DefaultMetricsPath = "/metrics"
 )
 
 var metricsLog = ctrl.Log.WithName("metrics-server")
 
-// MetricsServer serves Prometheus metrics via HTTP
+// MetricsServer serves Prometheus metrics via HTTP.
 type MetricsServer struct {
 	server   *http.Server
 	registry *prometheus.Registry
 }
 
-// NewMetricsServer creates a new HTTP metrics server
+// NewMetricsServer creates a new HTTP metrics server.
 func NewMetricsServer(port int, registry *prometheus.Registry) *MetricsServer {
 	if registry == nil {
 		registry = prometheus.NewRegistry()
@@ -54,9 +55,11 @@ func NewMetricsServer(port int, registry *prometheus.Registry) *MetricsServer {
 	}))
 
 	// Add health check endpoint
-	mux.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) {
+	mux.HandleFunc("/healthz", func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusOK)
-		w.Write([]byte("ok"))
+		if _, err := w.Write([]byte("ok")); err != nil {
+			metricsLog.Error(err, "failed to write health response")
+		}
 	})
 
 	server := &http.Server{
@@ -74,7 +77,7 @@ func NewMetricsServer(port int, registry *prometheus.Registry) *MetricsServer {
 	}
 }
 
-// Start starts the HTTP metrics server
+// Start starts the HTTP metrics server.
 func (m *MetricsServer) Start(ctx context.Context) error {
 	metricsLog.Info("Starting metrics server", "addr", m.server.Addr, "path", DefaultMetricsPath)
 
@@ -101,8 +104,7 @@ func (m *MetricsServer) Start(ctx context.Context) error {
 	}
 }
 
-// GetRegistry returns the Prometheus registry
+// GetRegistry returns the Prometheus registry.
 func (m *MetricsServer) GetRegistry() *prometheus.Registry {
 	return m.registry
 }
-

providers/v2/common/grpc/server/interceptors.go

@@ -29,7 +29,7 @@ import (
 
 // LoggingUnaryInterceptor logs all RPC calls with connection details.
 func LoggingUnaryInterceptor(verbose bool) grpc.UnaryServerInterceptor {
-	return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
+	return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) {
 		start := time.Now()
 
 		// Get peer information

providers/v2/common/grpc/server/metrics.go

@@ -18,19 +18,19 @@ package server
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
 	"google.golang.org/grpc"
-	"google.golang.org/grpc/status"
 )
 
 var (
-	// gRPC latency buckets optimized for typical RPC call durations
+	// gRPC latency buckets optimized for typical RPC call durations.
 	grpcLatencyBuckets = []float64{0.01, 0.05, 0.1, 0.5, 1, 2, 5, 10, 30}
 
-	// gRPC Server Counters
+	// gRPC Server Counters.
 	serverRequestsTotal = prometheus.NewCounterVec(
 		prometheus.CounterOpts{
 			Name: "grpc_server_requests_total",
@@ -39,7 +39,7 @@ var (
 		[]string{"method", "status"},
 	)
 
-	// gRPC Server Histograms
+	// gRPC Server Histograms.
 	serverRequestDuration = prometheus.NewHistogramVec(
 		prometheus.HistogramOpts{
 			Name:    "grpc_server_request_duration_seconds",
@@ -49,7 +49,7 @@ var (
 		[]string{"method"},
 	)
 
-	// gRPC Server Gauges
+	// gRPC Server Gauges.
 	serverActiveConnections = prometheus.NewGauge(
 		prometheus.GaugeOpts{
 			Name: "grpc_server_active_connections",
@@ -58,17 +58,17 @@ var (
 	)
 )
 
-// ServerMetrics interface for testability
-type ServerMetrics interface {
+// Metrics provides the hooks used by the server interceptors.
+type Metrics interface {
 	RecordRequest(method string, err error, duration time.Duration)
 	IncrementActiveConnections()
 	DecrementActiveConnections()
 }
 
-// defaultServerMetrics implements ServerMetrics using Prometheus
+// defaultServerMetrics implements Metrics using Prometheus.
 type defaultServerMetrics struct{}
 
-// RecordRequest records metrics for a server request
+// RecordRequest records metrics for a server request.
 func (m *defaultServerMetrics) RecordRequest(method string, err error, duration time.Duration) {
 	status := "success"
 	if err != nil {
@@ -79,22 +79,22 @@ func (m *defaultServerMetrics) RecordRequest(method string, err error, duration
 	serverRequestsTotal.WithLabelValues(method, status).Inc()
 }
 
-// IncrementActiveConnections increments the active connections gauge
+// IncrementActiveConnections increments the active connections gauge.
 func (m *defaultServerMetrics) IncrementActiveConnections() {
 	serverActiveConnections.Inc()
 }
 
-// DecrementActiveConnections decrements the active connections gauge
+// DecrementActiveConnections decrements the active connections gauge.
 func (m *defaultServerMetrics) DecrementActiveConnections() {
 	serverActiveConnections.Dec()
 }
 
-// Global instance
-var serverMetrics ServerMetrics = &defaultServerMetrics{}
+// Global instance.
+var serverMetrics Metrics = &defaultServerMetrics{}
 
-// MetricsUnaryInterceptor returns a gRPC unary server interceptor that records metrics
+// MetricsUnaryInterceptor returns a gRPC unary server interceptor that records metrics.
 func MetricsUnaryInterceptor() grpc.UnaryServerInterceptor {
-	return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
+	return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) {
 		start := time.Now()
 
 		// Call the handler
@@ -108,9 +108,9 @@ func MetricsUnaryInterceptor() grpc.UnaryServerInterceptor {
 	}
 }
 
-// ConnectionCountingStreamServerInterceptor returns a gRPC stream server interceptor that tracks active connections
+// ConnectionCountingStreamServerInterceptor returns a gRPC stream server interceptor that tracks active connections.
 func ConnectionCountingStreamServerInterceptor() grpc.StreamServerInterceptor {
-	return func(srv interface{}, ss grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error {
+	return func(srv any, ss grpc.ServerStream, _ *grpc.StreamServerInfo, handler grpc.StreamHandler) error {
 		serverMetrics.IncrementActiveConnections()
 		defer serverMetrics.DecrementActiveConnections()
 
@@ -118,7 +118,7 @@ func ConnectionCountingStreamServerInterceptor() grpc.StreamServerInterceptor {
 	}
 }
 
-// RegisterMetrics registers all server metrics with Prometheus
+// RegisterMetrics registers all server metrics with Prometheus.
 func RegisterMetrics(registry prometheus.Registerer) error {
 	collectors := []prometheus.Collector{
 		serverRequestsTotal,
@@ -128,8 +128,9 @@ func RegisterMetrics(registry prometheus.Registerer) error {
 
 	for _, collector := range collectors {
 		if err := registry.Register(collector); err != nil {
-			// Check if already registered
-			if _, ok := err.(prometheus.AlreadyRegisteredError); ok {
+			// Ignore duplicate registration so shared registries can reuse the collectors.
+			var alreadyRegistered prometheus.AlreadyRegisteredError
+			if errors.As(err, &alreadyRegistered) {
 				continue
 			}
 			return fmt.Errorf("failed to register server metric: %w", err)
@@ -139,20 +140,7 @@ func RegisterMetrics(registry prometheus.Registerer) error {
 	return nil
 }
 
-// GetServerMetrics returns the server metrics instance (for testing)
-func GetServerMetrics() ServerMetrics {
+// GetServerMetrics returns the server metrics instance for testing.
+func GetServerMetrics() Metrics {
 	return serverMetrics
 }
-
-// getStatusCode extracts the gRPC status code from an error
-func getStatusCode(err error) string {
-	if err == nil {
-		return "OK"
-	}
-	st, ok := status.FromError(err)
-	if !ok {
-		return "Unknown"
-	}
-	return st.Code().String()
-}
-

providers/v2/common/grpc/server/server.go

@@ -25,19 +25,19 @@ import (
 	"google.golang.org/grpc/keepalive"
 )
 
-// ServerOptions holds configuration options for creating a gRPC server.
-type ServerOptions struct {
+// Options holds configuration options for creating a gRPC server.
+type Options struct {
 	EnableTLS bool
 	Verbose   bool
 }
 
+// ServerOptions is kept as a compatibility alias for existing callers.
+//
+//nolint:revive // Compatibility alias for the previous exported API.
+type ServerOptions = Options
+
 // NewGRPCServer creates a new gRPC server with standard configuration.
-// It includes:
-// - TLS/mTLS if enabled
-// - Keepalive parameters
-// - Connection tap handler (if verbose)
-// - RPC logging interceptor
-func NewGRPCServer(opts ServerOptions) (*grpc.Server, error) {
+func NewGRPCServer(opts Options) (*grpc.Server, error) {
 	var grpcOpts []grpc.ServerOption
 
 	// Add keepalive parameters for better connection diagnostics

providers/v2/common/grpc/server/tls.go

@@ -24,11 +24,14 @@ import (
 )
 
 const (
-	// Default paths for certificate files
-	DefaultCertDir    = "/etc/provider/certs"
+	// DefaultCertDir is the default directory for provider TLS assets.
+	DefaultCertDir = "/etc/provider/certs"
+	// DefaultCACertFile is the default CA certificate filename.
 	DefaultCACertFile = "ca.crt"
-	DefaultCertFile   = "tls.crt"
-	DefaultKeyFile    = "tls.key"
+	// DefaultCertFile is the default server certificate filename.
+	DefaultCertFile = "tls.crt"
+	// DefaultKeyFile is the default server key filename.
+	DefaultKeyFile = "tls.key"
 )
 
 // TLSConfig holds configuration for provider server TLS.
@@ -40,11 +43,7 @@ type TLSConfig struct {
 }
 
 // DefaultTLSConfig returns a TLSConfig with default values.
-// Values can be overridden via environment variables:
-// - TLS_CERT_DIR
-// - TLS_CA_CERT_FILE
-// - TLS_CERT_FILE
-// - TLS_KEY_FILE
+// Values can be overridden via TLS_CERT_DIR, TLS_CA_CERT_FILE, TLS_CERT_FILE, and TLS_KEY_FILE.
 func DefaultTLSConfig() *TLSConfig {
 	return &TLSConfig{
 		CertDir:    getEnvOrDefault("TLS_CERT_DIR", DefaultCertDir),
@@ -68,6 +67,7 @@ func LoadTLSConfig(config *TLSConfig) (*tls.Config, error) {
 
 	// Load CA certificate for client verification
 	caPath := fmt.Sprintf("%s/%s", config.CertDir, config.CACertFile)
+	// #nosec G304 -- TLSConfig paths are explicit operator-provided mount points.
 	caCert, err := os.ReadFile(caPath)
 	if err != nil {
 		return nil, fmt.Errorf("failed to load CA certificate: %w", err)