From 5b654fb811fec361f18ecc1d55cb47c5a2d0023a Mon Sep 17 00:00:00 2001 From: Knative Prow Robot Date: Thu, 19 Oct 2023 15:26:24 +0100 Subject: [PATCH] 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 --- pkg/rabbit/exchange.go | 4 +--- pkg/rabbit/secret_test.go | 12 +++--------- pkg/rabbit/service.go | 25 ++++++++++++------------- pkg/rabbit/service_test.go | 14 +++++++++++++- pkg/rabbit/types.go | 3 +-- pkg/reconciler/broker/broker.go | 2 +- pkg/reconciler/source/rabbitmqsource.go | 2 +- 7 files changed, 32 insertions(+), 30 deletions(-) diff --git a/pkg/rabbit/exchange.go b/pkg/rabbit/exchange.go index 729c91f910..c8b25caecd 100644 --- a/pkg/rabbit/exchange.go +++ b/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 diff --git a/pkg/rabbit/secret_test.go b/pkg/rabbit/secret_test.go index d8a4444292..0654ac6976 100644 --- a/pkg/rabbit/secret_test.go +++ b/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) } diff --git a/pkg/rabbit/service.go b/pkg/rabbit/service.go index f17bea1668..b6bfedf1b8 100644 --- a/pkg/rabbit/service.go +++ b/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) { diff --git a/pkg/rabbit/service_test.go b/pkg/rabbit/service_test.go index 40e2e5d9e7..56b3c68624 100644 --- a/pkg/rabbit/service_test.go +++ b/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) } }) diff --git a/pkg/rabbit/types.go b/pkg/rabbit/types.go index f05c2577ff..373b4df86a 100644 --- a/pkg/rabbit/types.go +++ b/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) diff --git a/pkg/reconciler/broker/broker.go b/pkg/reconciler/broker/broker.go index 22265dbab3..dfb3c98fe5 100644 --- a/pkg/reconciler/broker/broker.go +++ b/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, ) diff --git a/pkg/reconciler/source/rabbitmqsource.go b/pkg/reconciler/source/rabbitmqsource.go index 4f794745cc..8a7a9555bd 100644 --- a/pkg/reconciler/source/rabbitmqsource.go +++ b/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