added unit test to webhook

nit

nit

nit

pkg/apis/agones/v1/gameserver.go

@@ -732,8 +732,6 @@ func (gs *GameServer) ApplyToPodContainer(pod *corev1.Pod, containerName string,
 // Pod creates a new Pod from the PodTemplateSpec
 // attached to the GameServer resource
 func (gs *GameServer) Pod(apiHooks APIHooks, sidecars ...corev1.Container) (*corev1.Pod, error) {
-	logger := runtime.NewLoggerWithSource("gke")
-
 	pod := &corev1.Pod{
 		ObjectMeta: *gs.Spec.Template.ObjectMeta.DeepCopy(),
 		Spec:       *gs.Spec.Template.Spec.DeepCopy(),
@@ -766,26 +764,21 @@ func (gs *GameServer) Pod(apiHooks APIHooks, sidecars ...corev1.Container) (*cor
 			return nil, err
 		}
 	}
-
 	// Put the sidecars at the start of the list of containers so that the kubelet starts them first.
 	containers := make([]corev1.Container, 0, len(sidecars)+len(pod.Spec.Containers))
 	containers = append(containers, sidecars...)
 	containers = append(containers, pod.Spec.Containers...)
 	pod.Spec.Containers = containers
-	pod.Spec.Containers[1].Ports[0].ContainerPort = 6000
-
-	logger.Info("[VICENTE]After for loop for ports", pod.Spec.Containers)
 
 	gs.podScheduling(pod)
 
 	if err := apiHooks.MutateGameServerPod(&gs.Spec, pod); err != nil {
-		logger.Info("[VICENTE]After for loop for ERROR MTUATAING", err)
 		return nil, err
 	}
 	if err := apiHooks.SetEviction(gs.Status.Eviction, pod); err != nil {
 		return nil, err
 	}
-	logger.Info("[VICENTE]After for loop NO ERROR", pod.Spec.Containers)
+
 	return pod, nil
 }
 

pkg/cloudproduct/gke/gke.go

@@ -263,13 +263,11 @@ func (apa *autopilotPortAllocator) Allocate(gs *agonesv1.GameServer) *agonesv1.G
 
 	var ports []agonesv1.GameServerPort
 	for i, p := range gs.Spec.Ports {
-
 		if p.PortPolicy != agonesv1.Dynamic && !runtime.FeatureEnabled(runtime.FeatureAutopilotPassthroughPort) {
 			logger.WithField("gs", gs.Name).WithField("portPolicy", p.PortPolicy).Error(
 				"GameServer has invalid PortPolicy for Autopilot - this should have been rejected by webhooks. Refusing to assign ports.")
 			return gs
 		}
-
 		p.HostPort = int32(i + 1) // Autopilot expects _some_ host port - use a value unique to this GameServer Port.
 
 		if p.Protocol == agonesv1.ProtocolTCPUDP {

pkg/gameservers/controller.go

@@ -59,9 +59,10 @@ import (
 )
 
 const (
-	sdkserverSidecarName = "agones-gameserver-sidecar"
-	grpcPortEnvVar       = "AGONES_SDK_GRPC_PORT"
-	httpPortEnvVar       = "AGONES_SDK_HTTP_PORT"
+	sdkserverSidecarName  = "agones-gameserver-sidecar"
+	grpcPortEnvVar        = "AGONES_SDK_GRPC_PORT"
+	httpPortEnvVar        = "AGONES_SDK_HTTP_PORT"
+	passthroughPortEnvVar = "PASSTHROUGH"
 )
 
 // Extensions struct contains what is needed to bind webhook handlers
@@ -205,21 +206,15 @@ func NewController(
 // initializes a new logger for extensions.
 func NewExtensions(apiHooks agonesv1.APIHooks, wh *webhooks.WebHook) *Extensions {
 	ext := &Extensions{apiHooks: apiHooks}
-	logger := runtime.NewLoggerWithSource("gke")
+
 	ext.baseLogger = runtime.NewLoggerWithType(ext)
 
 	wh.AddHandler("/mutate", agonesv1.Kind("GameServer"), admissionv1.Create, ext.creationMutationHandler)
-
 	wh.AddHandler("/validate", agonesv1.Kind("GameServer"), admissionv1.Create, ext.creationValidationHandler)
-	logger.Info("[VICENTE] EXTENSION after validate", ext)
-
-	// runtimeschema.GroupVersionKind.GroupKind(corev1.SchemeGroupVersion.WithKind("Pod")),
-	wh.AddHandler("/mutate", corev1.SchemeGroupVersion.WithKind("Pod").GroupKind(), admissionv1.Create, ext.creationMutationHandlerPod)
-
-	logger.Info("[VICENTE] EXTENSION after mutate Pod", ext)
-	//v1.SchemeGroupVersion.WithKind
-	// k8s.io/api/core/v1"
 
+	if runtime.FeatureEnabled(runtime.FeatureAutopilotPassthroughPort) {
+		wh.AddHandler("/mutate", corev1.SchemeGroupVersion.WithKind("Pod").GroupKind(), admissionv1.Create, ext.creationMutationHandlerPod)
+	}
 	return ext
 }
 
@@ -253,8 +248,6 @@ func fastRateLimiter() workqueue.RateLimiter {
 // Should only be called on gameserver create operations.
 // nolint:dupl
 func (ext *Extensions) creationMutationHandler(review admissionv1.AdmissionReview) (admissionv1.AdmissionReview, error) {
-	logger := runtime.NewLoggerWithSource("gke")
-	logger.Info("[VICENTE]INSIDE CREATION MUTATION HANDLERRR FOR GAME SERVER")
 	obj := review.Request.Object
 	gs := &agonesv1.GameServer{}
 	err := json.Unmarshal(obj.Raw, gs)
@@ -327,49 +320,35 @@ func (ext *Extensions) creationValidationHandler(review admissionv1.AdmissionRev
 	return review, nil
 }
 
-// creationValidationHandlerPod that mutates a GameServer pod when it is created
+// creationMutationHandlerPod that mutates a GameServer pod when it is created
 // Should only be called on gameserver pod create operations.
 func (ext *Extensions) creationMutationHandlerPod(review admissionv1.AdmissionReview) (admissionv1.AdmissionReview, error) {
 	logger := runtime.NewLoggerWithSource("gke")
-	logger.Info("[VICENTE] INSIDE CREATION MUTATION HANDLER FOR POD")
 	obj := review.Request.Object
-	gs := &agonesv1.GameServer{}
-	err := json.Unmarshal(obj.Raw, gs)
+	pod := &corev1.Pod{}
+	err := json.Unmarshal(obj.Raw, pod)
 	if err != nil {
 		// If the JSON is invalid during mutation, fall through to validation. This allows OpenAPI schema validation
 		// to proceed, resulting in a more user friendly error message.
 		return review, nil
 	}
-	logger.Info("[VICENTE]INSIDE CREATION MUTATION HANDLER", gs.Spec.Template.Spec)
-
-	//gs.Spec.Template.Spec.Containers[1].Ports[0].ContainerPort = gs.Spec.Ports[0].HostPort
-	logger.Info("[VICENTE]AFTER ASSIGNING PORT", gs.Spec.Template.Spec)
-
-	loggerForGameServer(gs, ext.baseLogger).WithField("review", review).Info("GAME SERVER IS USING PASSTHROUGH PORT POLICY")
-
-	// This is the main logic of this function
-	// the rest is really just json plumbing
-	gs.ApplyDefaults()
-
-	logger.Info("[VICENTE]INSIDE CREATION MUTATION HANDLER", gs.Spec.Template.Spec)
 
-	//gs.Spec.Template.Spec.Containers[1].Ports[0].ContainerPort = gs.Spec.Ports[0].HostPort
-	logger.Info("[VICENTE]AFTER ASSIGNING PORT", gs.Spec.Template.Spec)
-
-	newGS, err := json.Marshal(gs)
+	logger.WithField("review", review).Debug("creationMutationHandlerPod")
+	pod.Spec.Containers[1].Ports[0].ContainerPort = pod.Spec.Containers[1].Ports[0].HostPort
 
+	newPod, err := json.Marshal(pod)
 	if err != nil {
-		return review, errors.Wrapf(err, "error marshalling default applied GameServer %s to json", gs.ObjectMeta.Name)
+		return review, errors.Wrapf(err, "error marshalling changes applied Pod %s to json", pod.ObjectMeta.Name)
 	}
 
-	patch, err := jsonpatch.CreatePatch(obj.Raw, newGS)
+	patch, err := jsonpatch.CreatePatch(obj.Raw, newPod)
 	if err != nil {
-		return review, errors.Wrapf(err, "error creating patch for GameServer %s", gs.ObjectMeta.Name)
+		return review, errors.Wrapf(err, "error creating patch for Pod %s", pod.ObjectMeta.Name)
 	}
 
 	jsonPatch, err := json.Marshal(patch)
 	if err != nil {
-		return review, errors.Wrapf(err, "error creating json for patch for GameServer %s", gs.ObjectMeta.Name)
+		return review, errors.Wrapf(err, "error creating json for patch for Pod %s", pod.ObjectMeta.Name)
 	}
 
 	pt := admissionv1.PatchTypeJSONPatch
@@ -648,24 +627,11 @@ func (c *Controller) syncDevelopmentGameServer(ctx context.Context, gs *agonesv1
 
 // createGameServerPod creates the backing Pod for a given GameServer
 func (c *Controller) createGameServerPod(ctx context.Context, gs *agonesv1.GameServer) (*agonesv1.GameServer, error) {
-	logger := runtime.NewLoggerWithSource("gke")
-
 	sidecar := c.sidecar(gs)
-	logger.Info("[VICENTE]After sidecar", sidecar)
-
-	c.recorder.Event(gs, corev1.EventTypeNormal, string(gs.Status.State), "after sidecar")
-
 	pod, err := gs.Pod(c.controllerHooks, sidecar)
-	logger.Info("[VICENTE]After pod", pod.Spec.Containers)
-	logger.Info("[VICENTE]After pod err", err)
-	c.recorder.Event(gs, corev1.EventTypeNormal, string(gs.Status.State), "after pod")
-
 	if err != nil {
 		// this shouldn't happen, but if it does.
 		loggerForGameServer(gs, c.baseLogger).WithError(err).Error("error creating pod from Game Server")
-		logger.Info("[VICENTE]error is not nil", err)
-		c.recorder.Event(gs, corev1.EventTypeNormal, string(gs.Status.State), "ERROR CREATING POD")
-
 		gs, err = c.moveToErrorState(ctx, gs, err.Error())
 		return gs, err
 	}
@@ -689,15 +655,12 @@ func (c *Controller) createGameServerPod(ctx context.Context, gs *agonesv1.GameS
 
 	loggerForGameServer(gs, c.baseLogger).WithField("pod", pod).Debug("Creating Pod for GameServer")
 	pod, err = c.podGetter.Pods(gs.ObjectMeta.Namespace).Create(ctx, pod, metav1.CreateOptions{})
-	logger.Info("[VICENTE]Pod was created err", err)
-	logger.Info("[VICENTE]Pod was created", pod.Spec.Containers)
 	if err != nil {
 		switch {
 		case k8serrors.IsAlreadyExists(err):
 			c.recorder.Event(gs, corev1.EventTypeNormal, string(gs.Status.State), "Pod already exists, reused")
 			return gs, nil
 		case k8serrors.IsInvalid(err):
-			logger.Info("[VICENTE]Pod was invalid", err)
 			loggerForGameServer(gs, c.baseLogger).WithField("pod", pod).Errorf("Pod created is invalid")
 			gs, err = c.moveToErrorState(ctx, gs, err.Error())
 			return gs, err

pkg/gameservers/controller_test.go

@@ -57,6 +57,7 @@ const (
 )
 
 var GameServerKind = metav1.GroupVersionKind(agonesv1.SchemeGroupVersion.WithKind("GameServer"))
+var PodKind = corev1.SchemeGroupVersion.WithKind("Pod")
 
 func TestControllerSyncGameServer(t *testing.T) {
 	t.Parallel()
@@ -580,6 +581,54 @@ func TestControllerCreationValidationHandler(t *testing.T) {
 	})
 }
 
+func TestControllerCreationMutationHandlerPod(t *testing.T) {
+	t.Parallel()
+	ext := newFakeExtensions()
+
+	type expected struct {
+		patches []jsonpatch.JsonPatchOperation
+	}
+
+	t.Run("valid pod mutation for Passthrough portPolicy, containerPort should be the same as hostPort", func(t *testing.T) {
+		gameServerHostPort := float64(newPassthroughPortSingleContainerSpec().Containers[1].Ports[0].HostPort)
+		fixture := &corev1.Pod{Spec: newPassthroughPortSingleContainerSpec()}
+		raw, err := json.Marshal(fixture)
+		require.NoError(t, err)
+		review := admissionv1.AdmissionReview{
+			Request: &admissionv1.AdmissionRequest{
+				Kind:      metav1.GroupVersionKind(PodKind),
+				Operation: admissionv1.Create,
+				Object: runtime.RawExtension{
+					Raw: raw,
+				},
+			},
+			Response: &admissionv1.AdmissionResponse{Allowed: true},
+		}
+		expected := expected{
+			patches: []jsonpatch.JsonPatchOperation{
+				{Operation: "replace", Path: "/spec/containers/1/ports/0/containerPort", Value: gameServerHostPort}},
+		}
+
+		result, err := ext.creationMutationHandlerPod(review)
+		assert.NoError(t, err)
+		patch := &jsonpatch.ByPath{}
+		err = json.Unmarshal(result.Response.Patch, patch)
+		found := false
+
+		for _, expected := range expected.patches {
+			for _, p := range *patch {
+				if assert.ObjectsAreEqual(p, expected) {
+					found = true
+				}
+			}
+			assert.True(t, found, "Could not find operation %#v in patch %v", expected, *patch)
+		}
+
+		require.NoError(t, err)
+
+	})
+}
+
 func TestControllerSyncGameServerDeletionTimestamp(t *testing.T) {
 	t.Parallel()
 
@@ -2225,3 +2274,9 @@ func newSingleContainerSpec() agonesv1.GameServerSpec {
 		},
 	}
 }
+
+func newPassthroughPortSingleContainerSpec() corev1.PodSpec {
+	return corev1.PodSpec{
+		Containers: []corev1.Container{{Name: "agones-gameserver-sidecar", Image: "container/image", Env: []corev1.EnvVar{{Name: passthroughPortEnvVar, Value: "TRUE"}}}, {Name: "simple-game-server", Image: "container2/image", Ports: []corev1.ContainerPort{{HostPort: 7777, ContainerPort: 555}}, Env: []corev1.EnvVar{{Name: passthroughPortEnvVar, Value: "TRUE"}}}},
+	}
+}

pkg/portallocator/portallocator.go

@@ -248,8 +248,7 @@ func (pa *portRangeAllocator) Allocate(gs *agonesv1.GameServer) *agonesv1.GameSe
 				if p.Protocol == agonesv1.ProtocolTCPUDP {
 					var duplicate = p
 					duplicate.HostPort = a.port
-					logger := runtime.NewLoggerWithSource("gke")
-					logger.Info("[VICENTE]INSIDE PORT ALLOCATOR", a.port)
+
 					if duplicate.PortPolicy == agonesv1.Passthrough {
 						duplicate.ContainerPort = a.port
 					}

pkg/util/webhooks/webhooks.go

@@ -86,7 +86,7 @@ func (wh *WebHook) handle(path string, w http.ResponseWriter, r *http.Request) e
 		review.Response = &admissionv1.AdmissionResponse{Allowed: true}
 	}
 	review.Response.UID = review.Request.UID
-	wh.logger.WithField("name", review.Request.Name).WithField("path", path).WithField("kind", review.Request.Kind.Kind).WithField("group", review.Request.Kind.Group).Info("[VICENTE] INSIDE HANDLE")
+	wh.logger.WithField("name", review.Request.Name).WithField("path", path).WithField("kind", review.Request.Kind.Kind).WithField("group", review.Request.Kind.Group).Info("handling webhook request")
 
 	for _, oh := range wh.handlers[path] {
 		if oh.operation == review.Request.Operation &&
@@ -111,10 +111,7 @@ func (wh *WebHook) handle(path string, w http.ResponseWriter, r *http.Request) e
 					Reason:  metav1.StatusReasonInternalError,
 					Details: &details,
 				}
-				//wh.logger.WithField("name", review.Request.Name).WithField("path", path).WithField("kind", review.Request.Kind.Kind).WithField("group", review.Request.Kind.Group).Info("[VICENTE] INSIDE HANDLE")
 			}
-			//wh.logger.WithField("name", review.Request.Name).WithField("path", path).WithField("kind", review.Request.Kind.Kind).WithField("group", review.Request.Kind.Group).Info("[VICENTE] ERROR", err)
-
 		}
 	}
 	err = json.NewEncoder(w).Encode(review)