diff --git a/pkg/controller/keyvault/ingress_secret_provider_class.go b/pkg/controller/keyvault/ingress_secret_provider_class.go index 7571694b..f3f27b0d 100644 --- a/pkg/controller/keyvault/ingress_secret_provider_class.go +++ b/pkg/controller/keyvault/ingress_secret_provider_class.go @@ -95,7 +95,7 @@ func (i *IngressSecretProviderClassReconciler) Reconcile(ctx context.Context, re Labels: manifests.GetTopLevelLabels(), OwnerReferences: []metav1.OwnerReference{{ APIVersion: ing.APIVersion, - Controller: util.BoolPtr(true), + Controller: util.ToPtr(true), Kind: ing.Kind, Name: ing.Name, UID: ing.UID, diff --git a/pkg/controller/keyvault/ingress_secret_provider_class_test.go b/pkg/controller/keyvault/ingress_secret_provider_class_test.go index b8d6c57b..210ab191 100644 --- a/pkg/controller/keyvault/ingress_secret_provider_class_test.go +++ b/pkg/controller/keyvault/ingress_secret_provider_class_test.go @@ -201,7 +201,7 @@ func TestIngressSecretProviderClassReconcilerIntegrationWithoutSPCLabels(t *test Labels: manifests.GetTopLevelLabels(), OwnerReferences: []metav1.OwnerReference{{ APIVersion: ing.APIVersion, - Controller: util.BoolPtr(true), + Controller: util.ToPtr(true), Kind: ing.Kind, Name: ing.Name, UID: ing.UID, diff --git a/pkg/controller/keyvault/placeholder_pod.go b/pkg/controller/keyvault/placeholder_pod.go index 723e104b..c2967a3c 100644 --- a/pkg/controller/keyvault/placeholder_pod.go +++ b/pkg/controller/keyvault/placeholder_pod.go @@ -99,7 +99,7 @@ func (p *PlaceholderPodController) Reconcile(ctx context.Context, req ctrl.Reque Labels: spc.Labels, OwnerReferences: []metav1.OwnerReference{{ APIVersion: spc.APIVersion, - Controller: util.BoolPtr(true), + Controller: util.ToPtr(true), Kind: spc.Kind, Name: spc.Name, UID: spc.UID, @@ -194,7 +194,7 @@ func (p *PlaceholderPodController) buildDeployment(ctx context.Context, dep *app }, }, Spec: *manifests.WithPreferSystemNodes(&corev1.PodSpec{ - AutomountServiceAccountToken: util.BoolPtr(false), + AutomountServiceAccountToken: util.ToPtr(false), Containers: []corev1.Container{{ Name: "placeholder", Image: path.Join(p.config.Registry, "/oss/kubernetes/pause:3.6-hotfix.20220114"), @@ -215,7 +215,7 @@ func (p *PlaceholderPodController) buildDeployment(ctx context.Context, dep *app VolumeSource: corev1.VolumeSource{ CSI: &corev1.CSIVolumeSource{ Driver: "secrets-store.csi.k8s.io", - ReadOnly: util.BoolPtr(true), + ReadOnly: util.ToPtr(true), VolumeAttributes: map[string]string{"secretProviderClass": spc.Name}, }, }, diff --git a/pkg/controller/keyvault/placeholder_pod_test.go b/pkg/controller/keyvault/placeholder_pod_test.go index 54ae45cb..048a9d07 100644 --- a/pkg/controller/keyvault/placeholder_pod_test.go +++ b/pkg/controller/keyvault/placeholder_pod_test.go @@ -123,7 +123,7 @@ func TestPlaceholderPodControllerIntegration(t *testing.T) { }, }, Spec: *manifests.WithPreferSystemNodes(&corev1.PodSpec{ - AutomountServiceAccountToken: util.BoolPtr(false), + AutomountServiceAccountToken: util.ToPtr(false), Containers: []corev1.Container{{ Name: "placeholder", Image: "test-registry/oss/kubernetes/pause:3.6-hotfix.20220114", @@ -144,7 +144,7 @@ func TestPlaceholderPodControllerIntegration(t *testing.T) { VolumeSource: corev1.VolumeSource{ CSI: &corev1.CSIVolumeSource{ Driver: "secrets-store.csi.k8s.io", - ReadOnly: util.BoolPtr(true), + ReadOnly: util.ToPtr(true), VolumeAttributes: map[string]string{"secretProviderClass": spc.Name}, }, }, @@ -279,7 +279,7 @@ func TestPlaceholderPodControllerNoManagedByLabels(t *testing.T) { }, }, Spec: *manifests.WithPreferSystemNodes(&corev1.PodSpec{ - AutomountServiceAccountToken: util.BoolPtr(false), + AutomountServiceAccountToken: util.ToPtr(false), Containers: []corev1.Container{{ Name: "placeholder", Image: "test-registry/oss/kubernetes/pause:3.6-hotfix.20220114", @@ -300,7 +300,7 @@ func TestPlaceholderPodControllerNoManagedByLabels(t *testing.T) { VolumeSource: corev1.VolumeSource{ CSI: &corev1.CSIVolumeSource{ Driver: "secrets-store.csi.k8s.io", - ReadOnly: util.BoolPtr(true), + ReadOnly: util.ToPtr(true), VolumeAttributes: map[string]string{"secretProviderClass": spc.Name}, }, }, diff --git a/pkg/controller/osm/ingress_backend_reconciler.go b/pkg/controller/osm/ingress_backend_reconciler.go index 2d383178..2a4c4ccf 100644 --- a/pkg/controller/osm/ingress_backend_reconciler.go +++ b/pkg/controller/osm/ingress_backend_reconciler.go @@ -109,7 +109,7 @@ func (i *IngressBackendReconciler) Reconcile(ctx context.Context, req ctrl.Reque Labels: manifests.GetTopLevelLabels(), OwnerReferences: []metav1.OwnerReference{{ APIVersion: ing.APIVersion, - Controller: util.BoolPtr(true), + Controller: util.ToPtr(true), Kind: ing.Kind, Name: ing.Name, UID: ing.UID, diff --git a/pkg/controller/osm/ingress_backend_reconciler_test.go b/pkg/controller/osm/ingress_backend_reconciler_test.go index e4d330ec..16d7661a 100644 --- a/pkg/controller/osm/ingress_backend_reconciler_test.go +++ b/pkg/controller/osm/ingress_backend_reconciler_test.go @@ -43,7 +43,7 @@ var ( Annotations: map[string]string{"kubernetes.azure.com/use-osm-mtls": "true"}, }, Spec: netv1.IngressSpec{ - IngressClassName: util.StringPtr("test-ingress-class"), + IngressClassName: util.ToPtr("test-ingress-class"), Rules: []netv1.IngressRule{{}, { IngressRuleValue: netv1.IngressRuleValue{ HTTP: &netv1.HTTPIngressRuleValue{ diff --git a/pkg/controller/service/ingress_reconciler.go b/pkg/controller/service/ingress_reconciler.go index 4118fa72..a0cc2750 100644 --- a/pkg/controller/service/ingress_reconciler.go +++ b/pkg/controller/service/ingress_reconciler.go @@ -106,7 +106,7 @@ func (i *NginxIngressReconciler) Reconcile(ctx context.Context, req ctrl.Request Namespace: svc.Namespace, OwnerReferences: []metav1.OwnerReference{{ APIVersion: svc.APIVersion, - Controller: util.BoolPtr(true), + Controller: util.ToPtr(true), Kind: svc.Kind, Name: svc.Name, UID: svc.UID, @@ -116,7 +116,7 @@ func (i *NginxIngressReconciler) Reconcile(ctx context.Context, req ctrl.Request }, }, Spec: netv1.IngressSpec{ - IngressClassName: util.StringPtr(i.ingConfig.IcName), + IngressClassName: util.ToPtr(i.ingConfig.IcName), Rules: []netv1.IngressRule{{ Host: svc.Annotations["kubernetes.azure.com/ingress-host"], IngressRuleValue: netv1.IngressRuleValue{ diff --git a/pkg/controller/service/ingress_reconciler_test.go b/pkg/controller/service/ingress_reconciler_test.go index dd4e6f5c..c51cbb95 100644 --- a/pkg/controller/service/ingress_reconciler_test.go +++ b/pkg/controller/service/ingress_reconciler_test.go @@ -5,9 +5,10 @@ package service import ( "context" + "testing" + "github.com/Azure/aks-app-routing-operator/pkg/controller/metrics" "github.com/Azure/aks-app-routing-operator/pkg/controller/testutils" - "testing" "github.com/Azure/aks-app-routing-operator/pkg/manifests" "github.com/go-logr/logr" @@ -86,7 +87,7 @@ func TestIngressReconcilerIntegration(t *testing.T) { ResourceVersion: "1", OwnerReferences: []metav1.OwnerReference{{ APIVersion: "v1", - Controller: util.BoolPtr(true), + Controller: util.ToPtr(true), Kind: "Service", Name: svc.Name, UID: svc.UID, @@ -101,7 +102,7 @@ func TestIngressReconcilerIntegration(t *testing.T) { }, }, Spec: netv1.IngressSpec{ - IngressClassName: util.StringPtr(icName), + IngressClassName: util.ToPtr(icName), Rules: []netv1.IngressRule{{ Host: "test-host", IngressRuleValue: netv1.IngressRuleValue{ @@ -189,7 +190,7 @@ func TestIngressReconcilerIntegrationNoOSM(t *testing.T) { ResourceVersion: "1", OwnerReferences: []metav1.OwnerReference{{ APIVersion: "v1", - Controller: util.BoolPtr(true), + Controller: util.ToPtr(true), Kind: "Service", Name: svc.Name, UID: svc.UID, @@ -199,7 +200,7 @@ func TestIngressReconcilerIntegrationNoOSM(t *testing.T) { }, }, Spec: netv1.IngressSpec{ - IngressClassName: util.StringPtr(icName), + IngressClassName: util.ToPtr(icName), Rules: []netv1.IngressRule{{ Host: "test-host", IngressRuleValue: netv1.IngressRuleValue{ diff --git a/pkg/util/util.go b/pkg/util/util.go index 2c2f2e62..21a9a53a 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -7,6 +7,7 @@ import ( "context" "flag" "math/rand" + "strings" "time" "k8s.io/apimachinery/pkg/api/errors" @@ -37,15 +38,13 @@ func UseServerSideApply() { patchType = client.Apply } -func Int32Ptr(i int32) *int32 { return &i } -func Int64Ptr(i int64) *int64 { return &i } -func BoolPtr(b bool) *bool { return &b } -func StringPtr(str string) *string { return &str } -func ToPtr[T any](t T) *T { return &t } +func Int32Ptr(i int32) *int32 { return &i } +func Int64Ptr(i int64) *int64 { return &i } +func ToPtr[T any](t T) *T { return &t } func FindOwnerKind(owners []metav1.OwnerReference, kind string) string { for _, cur := range owners { - if cur.Kind == kind { + if strings.EqualFold(cur.Kind, kind) { return cur.Name } } @@ -53,9 +52,10 @@ func FindOwnerKind(owners []metav1.OwnerReference, kind string) string { } func Jitter(base time.Duration, ratio float64) time.Duration { - if ratio >= 1 || ratio == 0 { + if ratio >= 1 || ratio <= 0 { return base } + jitter := (rand.Float64() * float64(base) * ratio) - (float64(base) * (ratio / 2)) return base + time.Duration(jitter) } diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index d4050edd..44624e9c 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -1,11 +1,185 @@ package util import ( + "fmt" "testing" + "time" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +func TestToPtr(t *testing.T) { + intVar := 2 + require.Equal(t, &intVar, ToPtr(intVar)) + + stringVar := "string" + require.Equal(t, &stringVar, ToPtr(stringVar)) +} + +func TestFindOwnerKind(t *testing.T) { + cases := []struct { + name string + owners []metav1.OwnerReference + kind string + expected string + }{ + { + name: "nil owners", + owners: nil, + kind: "kind", + expected: "", + }, + { + name: "empty owners", + owners: []metav1.OwnerReference{}, + kind: "kind", + expected: "", + }, + { + name: "non-existent owner", + owners: []metav1.OwnerReference{{ + Kind: "Kind", + Name: "Name", + }}, + kind: "kind2", + expected: "", + }, + { + name: "existent owner", + owners: []metav1.OwnerReference{{ + Kind: "Kind", + Name: "Name", + }}, + kind: "Kind", + expected: "Name", + }, + { + name: "existent owner different casing", + owners: []metav1.OwnerReference{{ + Kind: "kind", + Name: "name", + }}, + kind: "Kind", + expected: "name", + }, + { + name: "existent owner multiple owners", + owners: []metav1.OwnerReference{ + { + Kind: "kind", + Name: "name", + }, + { + Kind: "kind2", + Name: "name2", + }, + }, + kind: "Kind2", + expected: "name2", + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := FindOwnerKind(c.owners, c.kind) + require.Equal(t, c.expected, got) + }) + } +} + +func TestJitter(t *testing.T) { + base := time.Minute + + // out of bounds ratio testing + require.Equal(t, base, Jitter(base, 0)) + require.Equal(t, base, Jitter(base, 1.2)) + require.Equal(t, base, Jitter(base, 3)) + require.Equal(t, base, Jitter(base, -0.2)) + require.Equal(t, base, Jitter(base, -2)) + + // ensure jitter is within bounds + cases := []float64{0.2, 0.3, 0.75, 0.9, 0.543} + for _, ratio := range cases { + t.Run(fmt.Sprintf("ratio-%f", ratio), func(t *testing.T) { + for i := 0; i < 100; i++ { // run a few times to get the full "range" + got := Jitter(base, ratio) + upper := base + time.Duration((float64(base)*ratio)-(float64(base)*(ratio/2))) + lower := (base + time.Duration(float64(base)*(ratio/2))) * -1 + require.LessOrEqual(t, got, upper) + require.GreaterOrEqual(t, got, lower) + } + }) + } +} + +func TestMergeMaps(t *testing.T) { + cases := []struct { + name string + m1 map[string]string + m2 map[string]string + m3 map[string]string + expected map[string]string + }{ + { + name: "nil maps", + m1: nil, + m2: nil, + m3: nil, + expected: map[string]string{}, + }, + { + name: "empty maps", + m1: map[string]string{}, + m2: map[string]string{}, + m3: map[string]string{}, + expected: map[string]string{}, + }, + { + name: "some nil maps", + m1: nil, + m2: map[string]string{"one": "two"}, + m3: nil, + expected: map[string]string{"one": "two"}, + }, + { + name: "equivalent maps", + m1: map[string]string{"one": "two"}, + m2: map[string]string{"one": "two"}, + m3: map[string]string{"one": "two"}, + expected: map[string]string{"one": "two"}, + }, + { + name: "different maps", + m1: map[string]string{"one": "two"}, + m2: map[string]string{"three": "four"}, + m3: map[string]string{"five": "six"}, + expected: map[string]string{"one": "two", "three": "four", "five": "six"}, + }, + { + name: "some overlap", + m1: map[string]string{"one": "two"}, + m2: map[string]string{"one": "two"}, + m3: map[string]string{"three": "four"}, + expected: map[string]string{"one": "two", "three": "four"}, + }, + { + name: "multiple keys", + m1: map[string]string{"one": "two", "three": "four"}, + m2: map[string]string{"three": "four", "five": "six"}, + m3: map[string]string{"seven": "eight"}, + expected: map[string]string{"one": "two", "three": "four", "five": "six", "seven": "eight"}, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := MergeMaps(c.m1, c.m2, c.m3) + require.Equal(t, c.expected, got) + }) + } +} + func TestKeys(t *testing.T) { type Case[K comparable, v any] struct { name string diff --git a/testing/e2e/clients/application.go b/testing/e2e/clients/application.go index 4ea2256f..e02d5639 100644 --- a/testing/e2e/clients/application.go +++ b/testing/e2e/clients/application.go @@ -33,7 +33,7 @@ func GetServicePrincipalOptions(ctx context.Context, applicationObjectID string, // add new password credential addPasswordReq := graphapplications.NewItemAddPasswordPostRequestBody() newCreds := graphmodels.NewPasswordCredential() - newCreds.SetDisplayName(util.StringPtr(credName)) + newCreds.SetDisplayName(util.ToPtr(credName)) newCreds.SetEndDateTime(to.Ptr(time.Now().Add(2 * time.Hour))) addPasswordReq.SetPasswordCredential(newCreds) addPasswordCredResp, err := graphClient.Applications().ByApplicationId(applicationObjectID).AddPassword().Post(ctx, addPasswordReq, nil)