feat: Introduced context to the readyz endpoint, added wait to test s…

…uite (#336)

Signed-off-by: James Milligan <james@omnant.co.uk>

main.go

@@ -244,7 +244,7 @@ func main() {
 		setupLog.Error(err, "unable to set up health check")
 		os.Exit(1)
 	}
-	if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil {
+	if err := mgr.AddReadyzCheck("readyz", podMutator.IsReady); err != nil {
 		setupLog.Error(err, "unable to set up ready check")
 		os.Exit(1)
 	}

webhooks/pod_webhook.go

@@ -46,10 +46,21 @@ type PodMutator struct {
 	FlagDResourceRequirements corev1.ResourceRequirements
 	decoder                   *admission.Decoder
 	Log                       logr.Logger
+	ready                     bool
+}
+
+func (m *PodMutator) IsReady(_ *http.Request) error {
+	if m.ready {
+		return nil
+	}
+	return goErr.New("pod mutator is not ready")
 }
 
 // BackfillPermissions recovers the state of the flagd-kubernetes-sync role binding in the event of upgrade
 func (m *PodMutator) BackfillPermissions(ctx context.Context) error {
+	defer func() {
+		m.ready = true
+	}()
 	for i := 0; i < 5; i++ {
 		// fetch all pods with the "openfeature.dev/enabled" annotation set to "true"
 		podList := &corev1.PodList{}

webhooks/pod_webhook_test.go

@@ -1,11 +1,8 @@
 package webhooks
 
 import (
-	"errors"
 	"fmt"
 	"os"
-	"reflect"
-	"time"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -168,55 +165,27 @@ func podMutationWebhookCleanup() {
 
 var _ = Describe("pod mutation webhook", func() {
 	It("should backfill role binding subjects when annotated pods already exist in the cluster", func() {
-		// this integration test confirms the proper execution of the  podMutator.BackfillPermissions method
-		// this method is responsible for backfilling the subjects of the open-feature-operator-flagd-kubernetes-sync
-		// cluster role binding, for previously existing pods on startup
-		// a retry is required on this test as the backfilling occurs asynchronously
-		var finalError error
-		for i := 0; i < 3; i++ {
-			pod1 := getPod(existingPod1Name)
-			pod2 := getPod(existingPod2Name)
-			// Pod 1 and 2 must not have been mutated by the webhook (we want the rolebinding to be updated via BackfillPermissions)
-
-			if len(pod1.Spec.Containers) != 1 {
-				finalError = errors.New("pod1 has had a container injected, it should not be mutated by the webhook")
-				time.Sleep(1 * time.Second)
-				continue
-			}
-			if len(pod2.Spec.Containers) != 1 {
-				finalError = errors.New("pod2 has had a container injected, it should not be mutated by the webhook")
-				time.Sleep(1 * time.Second)
-				continue
-			}
-
-			rb := getRoleBinding(clusterRoleBindingName)
-
-			unexpectedServiceAccount := ""
-			for _, subject := range rb.Subjects {
-				if !reflect.DeepEqual(subject, v1.Subject{
-					Kind:      "ServiceAccount",
-					APIGroup:  "",
-					Name:      existingPod1ServiceAccountName,
-					Namespace: mutatePodNamespace,
-				}) &&
-					!reflect.DeepEqual(subject, v1.Subject{
-						Kind:      "ServiceAccount",
-						APIGroup:  "",
-						Name:      existingPod2ServiceAccountName,
-						Namespace: mutatePodNamespace,
-					}) {
-					unexpectedServiceAccount = subject.Name
-				}
-			}
-			if unexpectedServiceAccount != "" {
-				finalError = fmt.Errorf("unexpected subject found in role binding, name: %s", unexpectedServiceAccount)
-				time.Sleep(1 * time.Second)
-				continue
-			}
-			finalError = nil
-			break
-		}
-		Expect(finalError).ShouldNot(HaveOccurred())
+		pod1 := getPod(existingPod1Name)
+		pod2 := getPod(existingPod2Name)
+		// Pod 1 and 2 must not have been mutated by the webhook (we want the rolebinding to be updated via BackfillPermissions)
+
+		Expect(len(pod1.Spec.Containers)).To(Equal(1))
+		Expect(len(pod2.Spec.Containers)).To(Equal(1))
+
+		rb := getRoleBinding(clusterRoleBindingName)
+
+		Expect(rb.Subjects).To(ContainElement(v1.Subject{
+			Kind:      "ServiceAccount",
+			APIGroup:  "",
+			Name:      existingPod1ServiceAccountName,
+			Namespace: mutatePodNamespace,
+		}))
+		Expect(rb.Subjects).To(ContainElement(v1.Subject{
+			Kind:      "ServiceAccount",
+			APIGroup:  "",
+			Name:      existingPod2ServiceAccountName,
+			Namespace: mutatePodNamespace,
+		}))
 	})
 
 	It("should update cluster role binding's subjects", func() {
@@ -343,7 +312,6 @@ var _ = Describe("pod mutation webhook", func() {
 		})
 		err = k8sClient.Create(testCtx, pod)
 		Expect(err).ShouldNot(HaveOccurred())
-
 		cm := &corev1.ConfigMap{}
 		err = k8sClient.Get(testCtx, client.ObjectKey{
 			Name:      featureFlagConfigurationName,
@@ -414,7 +382,6 @@ var _ = Describe("pod mutation webhook", func() {
 		Expect(err).ShouldNot(HaveOccurred())
 
 		pod = getPod(defaultPodName)
-		fmt.Println(pod.Spec.Containers[1])
 		Expect(pod.Spec.Containers[1].Env).To(Equal([]corev1.EnvVar{
 			{Name: "FLAGD_METRICS_PORT", Value: "8081"},
 			{Name: "FLAGD_PORT", Value: "8080"},
@@ -444,7 +411,6 @@ var _ = Describe("pod mutation webhook", func() {
 		Expect(err).ShouldNot(HaveOccurred())
 
 		pod = getPod(defaultPodName)
-		fmt.Println(pod.Spec.Containers[1])
 		Expect(pod.Spec.Containers[1].Env).To(Equal([]corev1.EnvVar{
 			{Name: "MY_SIDECAR_METRICS_PORT", Value: "10"},
 			{Name: "MY_SIDECAR_PORT", Value: "20"},
@@ -483,7 +449,6 @@ var _ = Describe("pod mutation webhook", func() {
 		Expect(err).ShouldNot(HaveOccurred())
 
 		pod = getPod(defaultPodName)
-		fmt.Println(pod.Spec.Containers[1])
 		Expect(pod.Spec.Containers[1].Env).To(Equal([]corev1.EnvVar{
 			{Name: "FLAGD_METRICS_PORT", Value: "8081"},
 			{Name: "FLAGD_PORT", Value: "8080"},

webhooks/suite_test.go

@@ -10,22 +10,27 @@ import (
 	"time"
 
 	"github.com/open-feature/open-feature-operator/apis/core/v1alpha1"
+	corev1alpha1 "github.com/open-feature/open-feature-operator/apis/core/v1alpha1"
 	"github.com/open-feature/open-feature-operator/apis/core/v1alpha2"
+	corev1alpha2 "github.com/open-feature/open-feature-operator/apis/core/v1alpha2"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
 	admissionv1 "k8s.io/api/admissionregistration/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	// +kubebuilder:scaffold:imports
 )
 
@@ -49,7 +54,7 @@ func strPtr(s string) *string { return &s }
 func TestAPIs(t *testing.T) {
 	RegisterFailHandler(Fail)
 
-	SetDefaultEventuallyTimeout(time.Minute)
+	SetDefaultEventuallyTimeout(time.Second * 15)
 	RunSpecsWithDefaultAndCustomReporters(t,
 		"Controller Suite",
 		[]Reporter{printer.NewlineReporter{}})
@@ -156,16 +161,65 @@ var _ = BeforeSuite(func() {
 	Expect(err).ToNot(HaveOccurred())
 	Expect(k8sClient).ToNot(BeNil())
 
+	// deploy 'before' resources
 	By("setting up previously existing pod (BackfillPermissions test)")
 	setupPreviouslyExistingPods()
 
+	// setup webhook server
+	By("Setup webhook server")
+
+	ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
+
+	mgr, err := ctrl.NewManager(cfg, ctrl.Options{
+		Scheme:             scheme,
+		MetricsBindAddress: "localhost:8999",
+		LeaderElection:     false,
+		Host:               testEnv.WebhookInstallOptions.LocalServingHost,
+		Port:               testEnv.WebhookInstallOptions.LocalServingPort,
+		CertDir:            testEnv.WebhookInstallOptions.LocalServingCertDir,
+	})
+	Expect(err).ToNot(HaveOccurred())
+
+	err = (&corev1alpha1.FeatureFlagConfiguration{}).SetupWebhookWithManager(mgr)
+	Expect(err).ToNot(HaveOccurred())
+
+	err = (&corev1alpha2.FeatureFlagConfiguration{}).SetupWebhookWithManager(mgr)
+	Expect(err).ToNot(HaveOccurred())
+
+	err = mgr.GetFieldIndexer().IndexField(
+		context.Background(),
+		&corev1.Pod{},
+		OpenFeatureEnabledAnnotationPath,
+		OpenFeatureEnabledAnnotationIndex,
+	)
+	Expect(err).ToNot(HaveOccurred())
+
+	// +kubebuilder:scaffold:builder
+	wh := mgr.GetWebhookServer()
+	podMutator := &PodMutator{
+		Client: mgr.GetClient(),
+		Log:    ctrl.Log.WithName("mutating-pod-webhook"),
+	}
+	wh.Register(podMutatingWebhookPath, &webhook.Admission{Handler: podMutator})
+	wh.Register(validatingFeatureFlagConfigurationWebhookPath, &webhook.Admission{
+		Handler: &FeatureFlagConfigurationValidator{
+			Client: mgr.GetClient(),
+			Log:    ctrl.Log.WithName("validating-featureflagconfiguration-webhook"),
+		},
+	})
+
+	// start webhook server
 	By("running webhook server")
+
 	go func() {
-		if err := run(testCtx, cfg, scheme, &testEnv.WebhookInstallOptions); err != nil {
-			logf.Log.Error(err, "run webhook server")
-		}
+		err := mgr.Start(testCtx)
+		Expect(err).ToNot(HaveOccurred())
 	}()
 
+	err = podMutator.BackfillPermissions(testCtx)
+	Expect(err).ToNot(HaveOccurred())
+
+	// wait for webhook to be ready to accept connections
 	d := &net.Dialer{Timeout: time.Second}
 	Eventually(func() error {
 		serverURL := fmt.Sprintf("%s:%d", testEnv.WebhookInstallOptions.LocalServingHost, testEnv.WebhookInstallOptions.LocalServingPort)
@@ -181,9 +235,15 @@ var _ = BeforeSuite(func() {
 		return nil
 	}).Should(Succeed())
 
+	// wait for ready state
+	Eventually(func() error {
+		return podMutator.IsReady(nil)
+	}).Should(Succeed())
+
 	By("setting up resources")
 	setupMutatePodResources()
 	setupValidateFeatureFlagConfigurationResources()
+
 }, 60)
 
 var _ = AfterSuite(func() {