From c30272cc7ed94507370a75c5803acc421f09eb97 Mon Sep 17 00:00:00 2001 From: omris94 <46892443+omris94@users.noreply.github.com> Date: Tue, 30 Jul 2024 18:55:55 +0300 Subject: [PATCH] Make the conversion whebhook of `v1alpha3` and `v1beta1` preserve the way a service is addressed in `clientIntent.Spec.call[x].name` instead of changing it to an explicit form (#460) --- src/operator/api/v1alpha3/webhooks.go | 14 +++++++- src/operator/api/v1alpha3/webhooks_test.go | 41 ++++++++++++++++++++++ src/operator/api/v1beta1/webhooks.go | 14 +++++++- src/operator/api/v1beta1/webhooks_test.go | 41 ++++++++++++++++++++++ 4 files changed, 108 insertions(+), 2 deletions(-) diff --git a/src/operator/api/v1alpha3/webhooks.go b/src/operator/api/v1alpha3/webhooks.go index 6aeee5ea8..154f3a63e 100644 --- a/src/operator/api/v1alpha3/webhooks.go +++ b/src/operator/api/v1alpha3/webhooks.go @@ -192,7 +192,13 @@ func (in *ClientIntents) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Targets = make([]v2alpha1.Target, len(in.Spec.Calls)) for i, call := range in.Spec.Calls { if call.IsTargetInCluster() && call.Type != IntentTypeKafka { - kubernetesTarget := v2alpha1.KubernetesTarget{Name: call.GetServerFullyQualifiedName(in.Namespace), Kind: call.GetTargetServerKind()} + var name string + if call.Name == call.GetTargetServerName() { + name = call.Name + } else { + name = call.GetServerFullyQualifiedName(in.Namespace) + } + kubernetesTarget := v2alpha1.KubernetesTarget{Name: name, Kind: call.GetTargetServerKind()} if kubernetesTarget.Kind == serviceidentity.KindOtterizeLegacy { // This is an internal kind the user doesn't need to see it kubernetesTarget.Kind = "" @@ -267,6 +273,12 @@ func (in *ClientIntents) ConvertFrom(srcRaw conversion.Hub) error { in.Spec.Service.Kind = src.Spec.Workload.Kind in.Spec.Calls = make([]Intent, len(src.Spec.Targets)) for i, target := range src.Spec.Targets { + if target.IsTargetTheKubernetesAPIServer(src.Namespace) { + // Using "svc:kubernetes.default" was a common use case in v1alpha3 - + // therefore we prefer to convert to this form. + in.Spec.Calls[i] = Intent{Name: "svc:" + target.Kubernetes.Name} + continue + } if target.Kubernetes != nil { in.Spec.Calls[i] = Intent{Name: target.Kubernetes.Name, Kind: target.Kubernetes.Kind} if target.Kubernetes.HTTP != nil { diff --git a/src/operator/api/v1alpha3/webhooks_test.go b/src/operator/api/v1alpha3/webhooks_test.go index 32e715380..fef425de4 100644 --- a/src/operator/api/v1alpha3/webhooks_test.go +++ b/src/operator/api/v1alpha3/webhooks_test.go @@ -146,6 +146,47 @@ func (t *WebhooksTestSuite) TestProtectedServiceConversion() { t.Require().Equal(original.Spec, converted.Spec) } +func (t *WebhooksTestSuite) TestClientIntentsKubernetes() { + // Create a ClientIntents with random data + original := &ClientIntents{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + }, + Spec: &IntentsSpec{ + Service: Service{ + Name: "test", + Kind: "test", + }, + Calls: []Intent{ + { + Name: "test.test", + }, + { + Name: "test2", + }, + { + Name: "test3.other-namespace", + }, + { + Name: "svc:kubernetes.default", + }, + }, + }} + + // ConvertTo + dstRaw := &v2alpha1.ClientIntents{} + err := original.ConvertTo(dstRaw) + t.Require().NoError(err) + + // ConvertFrom + converted := &ClientIntents{} + err = converted.ConvertFrom(dstRaw) + t.Require().NoError(err) + + t.Require().Equal(original.Spec, converted.Spec) +} + func TestWebhooksTestSuite(t *testing.T) { suite.Run(t, new(WebhooksTestSuite)) } diff --git a/src/operator/api/v1beta1/webhooks.go b/src/operator/api/v1beta1/webhooks.go index 362be76c5..91c04ef19 100644 --- a/src/operator/api/v1beta1/webhooks.go +++ b/src/operator/api/v1beta1/webhooks.go @@ -192,7 +192,13 @@ func (in *ClientIntents) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Targets = make([]v2alpha1.Target, len(in.Spec.Calls)) for i, call := range in.Spec.Calls { if call.IsTargetInCluster() && call.Type != IntentTypeKafka { - kubernetesTarget := v2alpha1.KubernetesTarget{Name: call.GetServerFullyQualifiedName(in.Namespace), Kind: call.GetTargetServerKind()} + var name string + if call.Name == call.GetTargetServerName() { + name = call.Name + } else { + name = call.GetServerFullyQualifiedName(in.Namespace) + } + kubernetesTarget := v2alpha1.KubernetesTarget{Name: name, Kind: call.GetTargetServerKind()} if kubernetesTarget.Kind == serviceidentity.KindOtterizeLegacy { // This is an internal kind the user doesn't need to see it kubernetesTarget.Kind = "" @@ -267,6 +273,12 @@ func (in *ClientIntents) ConvertFrom(srcRaw conversion.Hub) error { in.Spec.Service.Kind = src.Spec.Workload.Kind in.Spec.Calls = make([]Intent, len(src.Spec.Targets)) for i, target := range src.Spec.Targets { + if target.IsTargetTheKubernetesAPIServer(src.Namespace) { + // Using "svc:kubernetes.default" was a common use case in v1alpha3 - + // therefore we prefer to convert to this form. + in.Spec.Calls[i] = Intent{Name: "svc:" + target.Kubernetes.Name} + continue + } if target.Kubernetes != nil { in.Spec.Calls[i] = Intent{Name: target.Kubernetes.Name, Kind: target.Kubernetes.Kind} if target.Kubernetes.HTTP != nil { diff --git a/src/operator/api/v1beta1/webhooks_test.go b/src/operator/api/v1beta1/webhooks_test.go index b67c635e3..0a57731c1 100644 --- a/src/operator/api/v1beta1/webhooks_test.go +++ b/src/operator/api/v1beta1/webhooks_test.go @@ -146,6 +146,47 @@ func (t *WebhooksTestSuite) TestProtectedServiceConversion() { t.Require().Equal(original.Spec, converted.Spec) } +func (t *WebhooksTestSuite) TestClientIntentsKubernetes() { + // Create a ClientIntents with random data + original := &ClientIntents{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + }, + Spec: &IntentsSpec{ + Service: Service{ + Name: "test", + Kind: "test", + }, + Calls: []Intent{ + { + Name: "test.test", + }, + { + Name: "test2", + }, + { + Name: "test3.other-namespace", + }, + { + Name: "svc:kubernetes.default", + }, + }, + }} + + // ConvertTo + dstRaw := &v2alpha1.ClientIntents{} + err := original.ConvertTo(dstRaw) + t.Require().NoError(err) + + // ConvertFrom + converted := &ClientIntents{} + err = converted.ConvertFrom(dstRaw) + t.Require().NoError(err) + + t.Require().Equal(original.Spec, converted.Spec) +} + func TestWebhooksTestSuite(t *testing.T) { suite.Run(t, new(WebhooksTestSuite)) }