removing parse url from the get rabbitmqUrl function so usernames can have special characters needed i.e when pulled from an active directory and the url does not fail to parse (#1259)

Co-authored-by: Gabriel Freites <[email protected]>

Author: knative-prow-robot

pkg/rabbit/exchange.go

@@ -17,8 +17,6 @@ limitations under the License.
 package rabbit
 
 import (
-	"net/url"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/eventing-rabbitmq/pkg/apis/sources/v1alpha1"
 	"knative.dev/pkg/kmeta"
@@ -33,7 +31,7 @@ type ExchangeArgs struct {
 	Namespace                string
 	RabbitMQVhost            string
 	RabbitmqClusterReference *rabbitv1beta1.RabbitmqClusterReference
-	RabbitMQURL              *url.URL
+	RabbitMQURL              string
 	Broker                   *eventingv1.Broker
 	Trigger                  *eventingv1.Trigger
 	Source                   *v1alpha1.RabbitmqSource

pkg/rabbit/secret_test.go

@@ -25,7 +25,6 @@ import (
 	"knative.dev/eventing-rabbitmq/pkg/apis/sources/v1alpha1"
 	eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1"
 
-	"knative.dev/pkg/apis"
 	"knative.dev/pkg/kmeta"
 	_ "knative.dev/pkg/system/testing"
 )
@@ -40,11 +39,6 @@ const (
 
 func TestMakeSecret(t *testing.T) {
 	var TrueValue = true
-	url, err := apis.ParseURL(testRabbitURL)
-	if err != nil {
-		t.Errorf("Failed to parse the test URL: %s", err)
-	}
-
 	for _, tt := range []struct {
 		name string
 		args *ExchangeArgs
@@ -54,7 +48,7 @@ func TestMakeSecret(t *testing.T) {
 			name: "test broker secret name",
 			args: &ExchangeArgs{
 				Broker:      &eventingv1.Broker{ObjectMeta: metav1.ObjectMeta{Name: brokerName, Namespace: ns}},
-				RabbitMQURL: url.URL(),
+				RabbitMQURL: testRabbitURL,
 			},
 			want: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
@@ -79,7 +73,7 @@ func TestMakeSecret(t *testing.T) {
 			name: "test source secret name",
 			args: &ExchangeArgs{
 				Source:      &v1alpha1.RabbitmqSource{ObjectMeta: metav1.ObjectMeta{Name: sourceName, Namespace: ns}},
-				RabbitMQURL: url.URL(),
+				RabbitMQURL: testRabbitURL,
 			},
 			want: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
@@ -115,7 +109,7 @@ func TestMakeSecret(t *testing.T) {
 				owner = tt.args.Source
 				name = tt.args.Source.Name
 			}
-			got := MakeSecret(name, typeString, ns, tt.args.RabbitMQURL.String(), owner)
+			got := MakeSecret(name, typeString, ns, tt.args.RabbitMQURL, owner)
 			if diff := cmp.Diff(tt.want, got); diff != "" {
 				t.Error("unexpected diff (-want, +got) = ", diff)
 			}

pkg/rabbit/service.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"net/url"
 	"strings"
 
 	"go.uber.org/zap"
@@ -254,25 +253,25 @@ func isReady(conditions []rabbitv1beta1.Condition) bool {
 	return numConditions == 0
 }
 
-func (r *Rabbit) RabbitMQURL(ctx context.Context, clusterRef *rabbitv1beta1.RabbitmqClusterReference) (*url.URL, error) {
+func (r *Rabbit) RabbitMQURL(ctx context.Context, clusterRef *rabbitv1beta1.RabbitmqClusterReference) (string, error) {
 	protocol := []byte("amqp")
 	if clusterRef.ConnectionSecret != nil {
 		s, err := r.kubeClientSet.CoreV1().Secrets(clusterRef.Namespace).Get(ctx, clusterRef.ConnectionSecret.Name, metav1.GetOptions{})
 		if err != nil {
-			return nil, err
+			return "", err
 		}
 		uri, ok := s.Data["uri"]
 		if !ok {
-			return nil, fmt.Errorf("rabbit Secret missing key uri")
+			return "", fmt.Errorf("rabbit Secret missing key uri")
 		}
 		uriString := string(uri)
 		password, ok := s.Data["password"]
 		if !ok {
-			return nil, fmt.Errorf("rabbit Secret missing key password")
+			return "", fmt.Errorf("rabbit Secret missing key password")
 		}
 		username, ok := s.Data["username"]
 		if !ok {
-			return nil, fmt.Errorf("rabbit Secret missing key username")
+			return "", fmt.Errorf("rabbit Secret missing key username")
 		}
 		port, ok := s.Data["port"]
 		if !ok {
@@ -286,31 +285,31 @@ func (r *Rabbit) RabbitMQURL(ctx context.Context, clusterRef *rabbitv1beta1.Rabb
 		}
 		uriString = strings.TrimPrefix(uriString, prefix)
 		splittedUri := strings.Split(uriString, ":")
-		return url.Parse(fmt.Sprintf("%s://%s:%s@%s:%s", protocol, username, password, splittedUri[0], port))
+		return fmt.Sprintf("%s://%s:%s@%s:%s", protocol, username, password, splittedUri[0], port), nil
 	}
 
 	rab, err := r.getClusterFromReference(ctx, clusterRef)
 	if err != nil {
-		return nil, err
+		return "", err
 	}
 
 	if rab.Status.DefaultUser == nil || rab.Status.DefaultUser.SecretReference == nil || rab.Status.DefaultUser.ServiceReference == nil {
-		return nil, fmt.Errorf("rabbit \"%s/%s\" not ready", rab.Namespace, rab.Name)
+		return "", fmt.Errorf("rabbit \"%s/%s\" not ready", rab.Namespace, rab.Name)
 	}
 
 	_ = rab.Status.DefaultUser.SecretReference
 
 	s, err := r.kubeClientSet.CoreV1().Secrets(rab.Status.DefaultUser.SecretReference.Namespace).Get(ctx, rab.Status.DefaultUser.SecretReference.Name, metav1.GetOptions{})
 	if err != nil {
-		return nil, err
+		return "", err
 	}
 	password, ok := s.Data[rab.Status.DefaultUser.SecretReference.Keys["password"]]
 	if !ok {
-		return nil, fmt.Errorf("rabbit Secret missing key %s", rab.Status.DefaultUser.SecretReference.Keys["password"])
+		return "", fmt.Errorf("rabbit Secret missing key %s", rab.Status.DefaultUser.SecretReference.Keys["password"])
 	}
 	username, ok := s.Data[rab.Status.DefaultUser.SecretReference.Keys["username"]]
 	if !ok {
-		return nil, fmt.Errorf("rabbit Secret missing key %s", rab.Status.DefaultUser.SecretReference.Keys["username"])
+		return "", fmt.Errorf("rabbit Secret missing key %s", rab.Status.DefaultUser.SecretReference.Keys["username"])
 	}
 	port, ok := s.Data["port"]
 	if !ok {
@@ -320,7 +319,7 @@ func (r *Rabbit) RabbitMQURL(ctx context.Context, clusterRef *rabbitv1beta1.Rabb
 		protocol = []byte("amqps")
 	}
 	host := network.GetServiceHostname(rab.Status.DefaultUser.ServiceReference.Name, rab.Status.DefaultUser.ServiceReference.Namespace)
-	return url.Parse(fmt.Sprintf("%s://%s:%s@%s:%s", protocol, username, password, host, port))
+	return fmt.Sprintf("%s://%s:%s@%s:%s", protocol, username, password, host, port), nil
 }
 
 func (r *Rabbit) GetRabbitMQCASecret(ctx context.Context, clusterRef *rabbitv1beta1.RabbitmqClusterReference) (string, error) {

pkg/rabbit/service_test.go

@@ -125,6 +125,18 @@ func Test_RabbitMQURL(t *testing.T) {
 		secretData: map[string][]byte{"uri": []byte("https://test-uri"), "password": []byte("1234"), "username": []byte("test"), "port": []byte("1234")},
 		wantUrl:    "amqps://test:1234@test-uri:1234",
 		wantErr:    false,
+	}, {
+		name:       "username with /",
+		conSecret:  true,
+		secretData: map[string][]byte{"uri": []byte("https://test-uri:5678"), "password": []byte("1234"), "username": []byte("my//domain/test"), "port": []byte("1234")},
+		wantUrl:    "amqps://my//domain/test:1234@test-uri:1234",
+		wantErr:    false,
+	}, {
+		name:       "username with \\",
+		conSecret:  true,
+		secretData: map[string][]byte{"uri": []byte("https://test-uri:5678"), "password": []byte("1234"), "username": []byte("mydomain\\test"), "port": []byte("1234")},
+		wantUrl:    "amqps://mydomain\\test:1234@test-uri:1234",
+		wantErr:    false,
 	}} {
 		t.Run(tt.name, func(t *testing.T) {
 			tt := tt
@@ -149,7 +161,7 @@ func Test_RabbitMQURL(t *testing.T) {
 			gotUrl, err := r.RabbitMQURL(ctx, cr)
 			if (err != nil && !tt.wantErr) || (err == nil && tt.wantErr) {
 				t.Errorf("unexpected error checking conditions want: %v, got: %v", tt.wantErr, err)
-			} else if !tt.wantErr && gotUrl.String() != tt.wantUrl {
+			} else if !tt.wantErr && gotUrl != tt.wantUrl {
 				t.Errorf("got wrong url want: %s, got: %s", tt.wantUrl, gotUrl)
 			}
 		})

pkg/rabbit/types.go

@@ -18,7 +18,6 @@ package rabbit
 
 import (
 	"context"
-	"net/url"
 
 	rabbitv1beta1 "knative.dev/eventing-rabbitmq/third_party/pkg/apis/rabbitmq.com/v1beta1"
 )
@@ -29,7 +28,7 @@ type Result struct {
 }
 
 type Service interface {
-	RabbitMQURL(context.Context, *rabbitv1beta1.RabbitmqClusterReference) (*url.URL, error)
+	RabbitMQURL(context.Context, *rabbitv1beta1.RabbitmqClusterReference) (string, error)
 	ReconcileExchange(context.Context, *ExchangeArgs) (Result, error)
 	ReconcileQueue(context.Context, *QueueArgs) (Result, error)
 	ReconcileBinding(context.Context, *BindingArgs) (Result, error)

pkg/reconciler/broker/broker.go

@@ -290,7 +290,7 @@ func (r *Reconciler) reconcileRabbitResources(ctx context.Context, b *eventingv1
 
 	return r.reconcileCommonIngressResources(
 		ctx,
-		rabbit.MakeSecret(args.Broker.Name, "broker", args.Namespace, args.RabbitMQURL.String(), args.Broker),
+		rabbit.MakeSecret(args.Broker.Name, "broker", args.Namespace, args.RabbitMQURL, args.Broker),
 		b,
 		args.RabbitMQVhost,
 	)

pkg/reconciler/source/rabbitmqsource.go

@@ -146,7 +146,7 @@ func (r *Reconciler) reconcileRabbitObjects(ctx context.Context, src *v1alpha1.R
 		return err
 	}
 
-	if err := rabbit.ReconcileSecret(ctx, r.secretLister, r.KubeClientSet, rabbit.MakeSecret(src.Name, "source", src.Namespace, rabbitmqURL.String(), src)); err != nil {
+	if err := rabbit.ReconcileSecret(ctx, r.secretLister, r.KubeClientSet, rabbit.MakeSecret(src.Name, "source", src.Namespace, rabbitmqURL, src)); err != nil {
 		logging.FromContext(ctx).Errorw("Problem reconciling Secret", zap.Error(err))
 		src.Status.MarkSecretFailed("SecretFailure", "Failed to reconcile secret: %s", err)
 		return err