SDK Service Account was Hardcoded

The value of agones.serviceaccount.sdk was not propagated
down to the creation of the GameServer Pod, so there actually
was no way to edit the service account information without
Agones breaking.

This is now fixed!

cmd/controller/main.go

@@ -55,6 +55,7 @@ const (
 	sidecarImageFlag             = "sidecar-image"
 	sidecarCPURequestFlag        = "sidecar-cpu-request"
 	sidecarCPULimitFlag          = "sidecar-cpu-limit"
+	sdkServerAccountFlag         = "sdk-service-account"
 	pullSidecarFlag              = "always-pull-sidecar"
 	minPortFlag                  = "min-port"
 	maxPortFlag                  = "max-port"
@@ -152,7 +153,7 @@ func main() {
 
 	gsController := gameservers.NewController(wh, health,
 		ctlConf.MinPort, ctlConf.MaxPort, ctlConf.SidecarImage, ctlConf.AlwaysPullSidecar,
-		ctlConf.SidecarCPURequest, ctlConf.SidecarCPULimit,
+		ctlConf.SidecarCPURequest, ctlConf.SidecarCPULimit, ctlConf.SdkServiceAccount,
 		kubeClient, kubeInformerFactory, extClient, agonesClient, agonesInformerFactory)
 	gsSetController := gameserversets.NewController(wh, health,
 		kubeClient, extClient, agonesClient, agonesInformerFactory)
@@ -195,6 +196,7 @@ func parseEnvFlags() config {
 	viper.SetDefault(sidecarCPURequestFlag, "0")
 	viper.SetDefault(sidecarCPULimitFlag, "0")
 	viper.SetDefault(pullSidecarFlag, false)
+	viper.SetDefault(sdkServerAccountFlag, "agones-sdk")
 	viper.SetDefault(certFileFlag, filepath.Join(base, "certs/server.crt"))
 	viper.SetDefault(keyFileFlag, filepath.Join(base, "certs/server.key"))
 	viper.SetDefault(enablePrometheusMetricsFlag, true)
@@ -208,6 +210,7 @@ func parseEnvFlags() config {
 	pflag.String(sidecarCPULimitFlag, viper.GetString(sidecarCPULimitFlag), "Flag to overwrite the GameServer sidecar container's cpu limit. Can also use SIDECAR_CPU_LIMIT env variable")
 	pflag.String(sidecarCPURequestFlag, viper.GetString(sidecarCPURequestFlag), "Flag to overwrite the GameServer sidecar container's cpu request. Can also use SIDECAR_CPU_REQUEST env variable")
 	pflag.Bool(pullSidecarFlag, viper.GetBool(pullSidecarFlag), "For development purposes, set the sidecar image to have a ImagePullPolicy of Always. Can also use ALWAYS_PULL_SIDECAR env variable")
+	pflag.String(sdkServerAccountFlag, viper.GetString(sdkServerAccountFlag), "Overwrite what service account default for GameServer Pods. Defaults to Can also use SDK_SERVICE_ACCOUNT")
 	pflag.Int32(minPortFlag, 0, "Required. The minimum port that that a GameServer can be allocated to. Can also use MIN_PORT env variable.")
 	pflag.Int32(maxPortFlag, 0, "Required. The maximum port that that a GameServer can be allocated to. Can also use MAX_PORT env variable")
 	pflag.String(keyFileFlag, viper.GetString(keyFileFlag), "Optional. Path to the key file")
@@ -226,6 +229,7 @@ func parseEnvFlags() config {
 	runtime.Must(viper.BindEnv(sidecarCPULimitFlag))
 	runtime.Must(viper.BindEnv(sidecarCPURequestFlag))
 	runtime.Must(viper.BindEnv(pullSidecarFlag))
+	runtime.Must(viper.BindEnv(sdkServerAccountFlag))
 	runtime.Must(viper.BindEnv(minPortFlag))
 	runtime.Must(viper.BindEnv(maxPortFlag))
 	runtime.Must(viper.BindEnv(keyFileFlag))
@@ -255,6 +259,7 @@ func parseEnvFlags() config {
 		SidecarImage:          viper.GetString(sidecarImageFlag),
 		SidecarCPURequest:     request,
 		SidecarCPULimit:       limit,
+		SdkServiceAccount:     viper.GetString(sdkServerAccountFlag),
 		AlwaysPullSidecar:     viper.GetBool(pullSidecarFlag),
 		KeyFile:               viper.GetString(keyFileFlag),
 		CertFile:              viper.GetString(certFileFlag),
@@ -275,6 +280,7 @@ type config struct {
 	SidecarImage          string
 	SidecarCPURequest     resource.Quantity
 	SidecarCPULimit       resource.Quantity
+	SdkServiceAccount     string
 	AlwaysPullSidecar     bool
 	PrometheusMetrics     bool
 	Stackdriver           bool

install/helm/agones/templates/controller.yaml

@@ -84,6 +84,8 @@ spec:
           value: {{ .Values.agones.image.sdk.alwaysPull | quote }}
         - name: SIDECAR_CPU_REQUEST
           value: {{ .Values.agones.image.sdk.cpuRequest | quote }}
+        - name: SDK_SERVICE_ACCOUNT
+          value: {{ .Values.agones.serviceaccount.sdk | quote }}
         - name: PROMETHEUS_EXPORTER
           value: {{ .Values.agones.metrics.prometheusEnabled | quote }}
         - name: STACKDRIVER_EXPORTER

install/yaml/install.yaml

@@ -1065,6 +1065,8 @@ spec:
           value: "false"
         - name: SIDECAR_CPU_REQUEST
           value: "30m"
+        - name: SDK_SERVICE_ACCOUNT
+          value: "agones-sdk"
         - name: PROMETHEUS_EXPORTER
           value: "true"
         - name: STACKDRIVER_EXPORTER

pkg/apis/stable/v1alpha1/gameserver.go

@@ -75,8 +75,6 @@ const (
 	// GameServerContainerAnnotation is the annotation that stores
 	// which container is the container that runs the dedicated game server
 	GameServerContainerAnnotation = stable.GroupName + "/container"
-	// SidecarServiceAccountName is the default service account for managing access to get/update GameServers
-	SidecarServiceAccountName = "agones-sdk"
 	// DevAddressAnnotation is an annotation to indicate that a GameServer hosted outside of Agones.
 	// A locally hosted GameServer is not managed by Agones it is just simply registered.
 	DevAddressAnnotation = "stable.agones.dev/dev-address"
@@ -344,10 +342,6 @@ func (gs *GameServer) Pod(sidecars ...corev1.Container) (*corev1.Pod, error) {
 
 	gs.podObjectMeta(pod)
 
-	if pod.Spec.ServiceAccountName == "" {
-		pod.Spec.ServiceAccountName = SidecarServiceAccountName
-	}
-
 	i, gsContainer, err := gs.FindGameServerContainer()
 	// this shouldn't happen, but if it does.
 	if err != nil {

pkg/apis/stable/v1alpha1/gameserver_test.go

@@ -291,7 +291,6 @@ func TestGameServerPod(t *testing.T) {
 	assert.Equal(t, "gameserver", pod.ObjectMeta.Labels[stable.GroupName+"/role"])
 	assert.Equal(t, fixture.ObjectMeta.Name, pod.ObjectMeta.Labels[GameServerPodLabel])
 	assert.Equal(t, fixture.Spec.Container, pod.ObjectMeta.Annotations[GameServerContainerAnnotation])
-	assert.Equal(t, "agones-sdk", pod.Spec.ServiceAccountName)
 	assert.True(t, metav1.IsControlledBy(pod, fixture))
 	assert.Equal(t, fixture.Spec.Ports[0].HostPort, pod.Spec.Containers[0].Ports[0].HostPort)
 	assert.Equal(t, fixture.Spec.Ports[0].ContainerPort, pod.Spec.Containers[0].Ports[0].ContainerPort)

pkg/gameservers/controller.go

@@ -64,6 +64,7 @@ type Controller struct {
 	alwaysPullSidecarImage bool
 	sidecarCPURequest      resource.Quantity
 	sidecarCPULimit        resource.Quantity
+	sdkServiceAccount      string
 	crdGetter              v1beta1.CustomResourceDefinitionInterface
 	podGetter              typedcorev1.PodsGetter
 	podLister              corelisterv1.PodLister
@@ -91,6 +92,7 @@ func NewController(
 	alwaysPullSidecarImage bool,
 	sidecarCPURequest resource.Quantity,
 	sidecarCPULimit resource.Quantity,
+	sdkServiceAccount string,
 	kubeClient kubernetes.Interface,
 	kubeInformerFactory informers.SharedInformerFactory,
 	extClient extclientset.Interface,
@@ -106,6 +108,7 @@ func NewController(
 		sidecarCPULimit:        sidecarCPULimit,
 		sidecarCPURequest:      sidecarCPURequest,
 		alwaysPullSidecarImage: alwaysPullSidecarImage,
+		sdkServiceAccount:      sdkServiceAccount,
 		crdGetter:              extClient.ApiextensionsV1beta1().CustomResourceDefinitions(),
 		podGetter:              kubeClient.CoreV1(),
 		podLister:              pods.Lister(),
@@ -512,6 +515,11 @@ func (c *Controller) createGameServerPod(gs *v1alpha1.GameServer) (*v1alpha1.Gam
 		return gs, err
 	}
 
+	// apply the sdk service account
+	if pod.Spec.ServiceAccountName == "" {
+		pod.Spec.ServiceAccountName = c.sdkServiceAccount
+	}
+
 	c.addGameServerHealthCheck(gs, pod)
 
 	c.logger.WithField("pod", pod).Info("creating Pod for GameServer")

pkg/gameservers/controller_test.go

@@ -775,6 +775,7 @@ func TestControllerCreateGameServerPod(t *testing.T) {
 
 			assert.Equal(t, fixture.ObjectMeta.Name, pod.ObjectMeta.Name)
 			assert.Equal(t, fixture.ObjectMeta.Namespace, pod.ObjectMeta.Namespace)
+			assert.Equal(t, "sdk-service-account", pod.Spec.ServiceAccountName)
 			assert.Equal(t, "gameserver", pod.ObjectMeta.Labels[stable.GroupName+"/role"])
 			assert.Equal(t, fixture.ObjectMeta.Name, pod.ObjectMeta.Labels[v1alpha1.GameServerPodLabel])
 			assert.True(t, metav1.IsControlledBy(pod, fixture))
@@ -1185,7 +1186,8 @@ func newFakeController() (*Controller, agtesting.Mocks) {
 	wh := webhooks.NewWebHook("", "")
 	c := NewController(wh, healthcheck.NewHandler(),
 		10, 20, "sidecar:dev", false,
-		resource.MustParse("0.05"), resource.MustParse("0.1"), m.KubeClient, m.KubeInformerFactory, m.ExtClient, m.AgonesClient, m.AgonesInformerFactory)
+		resource.MustParse("0.05"), resource.MustParse("0.1"), "sdk-service-account",
+		m.KubeClient, m.KubeInformerFactory, m.ExtClient, m.AgonesClient, m.AgonesInformerFactory)
 	c.recorder = m.FakeRecorder
 	return c, m
 }