feat: simplify user agent version constraint handling in interceptors

Signed-off-by: Matthieu MOREL <[email protected]>

Author: mmorel-35

util/grpc/useragent.go

@@ -12,15 +12,21 @@ import (
 	"google.golang.org/grpc/status"
 )
 
-// UserAgentUnaryServerInterceptor returns a UnaryServerInterceptor which enforces a minimum client
-// version in the user agent
-func UserAgentUnaryServerInterceptor(clientName, constraintStr string) grpc.UnaryServerInterceptor {
+// parseSemVerConstraint returns a semVer Constraint instance or panic if there is a parsing error with the provided constraint.
+func parseSemVerConstraint(constraintStr string) *semver.Constraints {
 	semVerConstraint, err := semver.NewConstraint(constraintStr)
 	if err != nil {
 		panic(err)
 	}
+	return semVerConstraint
+}
+
+// UserAgentUnaryServerInterceptor returns a UnaryServerInterceptor which enforces a minimum client
+// version in the user agent
+func UserAgentUnaryServerInterceptor(clientName, constraintStr string) grpc.UnaryServerInterceptor {
+	semVerConstraint := parseSemVerConstraint(constraintStr)
 	return func(ctx context.Context, req any, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) {
-		if err := userAgentEnforcer(ctx, clientName, constraintStr, semVerConstraint); err != nil {
+		if err := userAgentEnforcer(ctx, clientName, semVerConstraint); err != nil {
 			return nil, err
 		}
 		return handler(ctx, req)
@@ -30,19 +36,16 @@ func UserAgentUnaryServerInterceptor(clientName, constraintStr string) grpc.Unar
 // UserAgentStreamServerInterceptor returns a StreamServerInterceptor which enforces a minimum client
 // version in the user agent
 func UserAgentStreamServerInterceptor(clientName, constraintStr string) grpc.StreamServerInterceptor {
-	semVerConstraint, err := semver.NewConstraint(constraintStr)
-	if err != nil {
-		panic(err)
-	}
+	semVerConstraint := parseSemVerConstraint(constraintStr)
 	return func(srv any, stream grpc.ServerStream, _ *grpc.StreamServerInfo, handler grpc.StreamHandler) error {
-		if err := userAgentEnforcer(stream.Context(), clientName, constraintStr, semVerConstraint); err != nil {
+		if err := userAgentEnforcer(stream.Context(), clientName, semVerConstraint); err != nil {
 			return err
 		}
 		return handler(srv, stream)
 	}
 }
 
-func userAgentEnforcer(ctx context.Context, clientName, constraintStr string, semVerConstraint *semver.Constraints) error {
+func userAgentEnforcer(ctx context.Context, clientName string, semVerConstraint *semver.Constraints) error {
 	var userAgents []string
 	if md, ok := metadata.FromIncomingContext(ctx); ok {
 		for _, ua := range md["user-agent"] {
@@ -52,7 +55,7 @@ func userAgentEnforcer(ctx context.Context, clientName, constraintStr string, se
 		}
 	}
 	if isLegacyClient(userAgents) {
-		return status.Errorf(codes.FailedPrecondition, "unsatisfied client version constraint: %s", constraintStr)
+		return status.Errorf(codes.FailedPrecondition, "unsatisfied client version constraint: %s", semVerConstraint)
 	}
 
 	for _, userAgent := range userAgents {

util/grpc/useragent_test.go

@@ -9,49 +9,38 @@ import (
 )
 
 func Test_UserAgentEnforcer(t *testing.T) {
+	clientName := "argo-cd"
+	constraintStr := "^1"
+	semverConstraint, _ := semver.NewConstraint(constraintStr)
+
 	t.Run("Test enforcing valid user-agent", func(t *testing.T) {
-		clientName := "argo-cd"
-		constraintStr := "^1"
-		semverConstraint, _ := semver.NewConstraint(constraintStr)
 		md := metadata.New(map[string]string{"user-agent": "argo-cd/1.0"})
 		ctx := metadata.NewIncomingContext(t.Context(), md)
-		err := userAgentEnforcer(ctx, clientName, constraintStr, semverConstraint)
+		err := userAgentEnforcer(ctx, clientName, semverConstraint)
 		require.NoError(t, err)
 	})
 	t.Run("Test enforcing ignored user-agent", func(t *testing.T) {
-		clientName := "argo-cd"
-		constraintStr := "^1"
-		semverConstraint, _ := semver.NewConstraint(constraintStr)
 		md := metadata.New(map[string]string{"user-agent": "flux/3.0"})
 		ctx := metadata.NewIncomingContext(t.Context(), md)
-		err := userAgentEnforcer(ctx, clientName, constraintStr, semverConstraint)
+		err := userAgentEnforcer(ctx, clientName, semverConstraint)
 		require.NoError(t, err)
 	})
 	t.Run("Test enforcing user-agent with version not matching constraint", func(t *testing.T) {
-		clientName := "argo-cd"
-		constraintStr := "^1"
-		semverConstraint, _ := semver.NewConstraint(constraintStr)
 		md := metadata.New(map[string]string{"user-agent": "argo-cd/3.0"})
 		ctx := metadata.NewIncomingContext(t.Context(), md)
-		err := userAgentEnforcer(ctx, clientName, constraintStr, semverConstraint)
+		err := userAgentEnforcer(ctx, clientName, semverConstraint)
 		require.ErrorContains(t, err, "unsatisfied client version constraint")
 	})
 	t.Run("Test legacy user-agent", func(t *testing.T) {
-		clientName := "argo-cd"
-		constraintStr := "^1"
-		semverConstraint, _ := semver.NewConstraint(constraintStr)
 		md := metadata.New(map[string]string{"user-agent": "grpc-go/1.15.0"})
 		ctx := metadata.NewIncomingContext(t.Context(), md)
-		err := userAgentEnforcer(ctx, clientName, constraintStr, semverConstraint)
-		require.ErrorContains(t, err, "unsatisfied client version constraint")
+		err := userAgentEnforcer(ctx, clientName, semverConstraint)
+		require.EqualError(t, err, "rpc error: code = FailedPrecondition desc = unsatisfied client version constraint: ^1")
 	})
 	t.Run("Test invalid version", func(t *testing.T) {
-		clientName := "argo-cd"
-		constraintStr := "^1"
-		semverConstraint, _ := semver.NewConstraint(constraintStr)
 		md := metadata.New(map[string]string{"user-agent": "argo-cd/super"})
 		ctx := metadata.NewIncomingContext(t.Context(), md)
-		err := userAgentEnforcer(ctx, clientName, constraintStr, semverConstraint)
+		err := userAgentEnforcer(ctx, clientName, semverConstraint)
 		require.ErrorContains(t, err, "could not parse version")
 	})
 }