Skip to content

Commit

Permalink
Remove serviceaccount for game server container
Browse files Browse the repository at this point in the history
This mounts an emptydir over the service account token
that is automatically mounted in the container that runs
the game server binary.

Since this is exposed to the outside world, removing the serviceaccount
token removes authentication against the rest of the Kubernetes cluster
if it ever gets compromised.

Closes googleforgames#150
  • Loading branch information
markmandel committed Mar 13, 2019
1 parent b133e52 commit f418f5e
Show file tree
Hide file tree
Showing 9 changed files with 224 additions and 108 deletions.
113 changes: 60 additions & 53 deletions cmd/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,24 +53,25 @@ import (
)

const (
enableStackdriverMetricsFlag = "stackdriver-exporter"
enablePrometheusMetricsFlag = "prometheus-exporter"
projectIDFlag = "gcp-project-id"
sidecarImageFlag = "sidecar-image"
sidecarCPURequestFlag = "sidecar-cpu-request"
sidecarCPULimitFlag = "sidecar-cpu-limit"
pullSidecarFlag = "always-pull-sidecar"
minPortFlag = "min-port"
maxPortFlag = "max-port"
certFileFlag = "cert-file"
keyFileFlag = "key-file"
numWorkersFlag = "num-workers"
apiServerSustainedQPSFlag = "api-server-qps"
apiServerBurstQPSFlag = "api-server-qps-burst"
logDirFlag = "log-dir"
logSizeLimitMBFlag = "log-size-limit-mb"
kubeconfigFlag = "kubeconfig"
defaultResync = 30 * time.Second
enableStackdriverMetricsFlag = "stackdriver-exporter"
enablePrometheusMetricsFlag = "prometheus-exporter"
projectIDFlag = "gcp-project-id"
sidecarImageFlag = "sidecar-image"
sidecarCPURequestFlag = "sidecar-cpu-request"
sidecarCPULimitFlag = "sidecar-cpu-limit"
pullSidecarFlag = "always-pull-sidecar"
minPortFlag = "min-port"
maxPortFlag = "max-port"
certFileFlag = "cert-file"
keyFileFlag = "key-file"
numWorkersFlag = "num-workers"
apiServerSustainedQPSFlag = "api-server-qps"
apiServerBurstQPSFlag = "api-server-qps-burst"
logDirFlag = "log-dir"
logSizeLimitMBFlag = "log-size-limit-mb"
disableGameServerContainerServiceAccountFlag = "disable-gameserver-service-account"
kubeconfigFlag = "kubeconfig"
defaultResync = 30 * time.Second
// topNGSForAllocation is used by the GameServerAllocation controller
// to reduce the contention while allocating gameservers.
topNGSForAllocation = 100
Expand Down Expand Up @@ -188,7 +189,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.DisableGameServerContainerServiceAccount,
kubeClient, kubeInformerFactory, extClient, agonesClient, agonesInformerFactory)
gsSetController := gameserversets.NewController(wh, health,
kubeClient, extClient, agonesClient, agonesInformerFactory)
Expand Down Expand Up @@ -241,6 +242,7 @@ func parseEnvFlags() config {
viper.SetDefault(apiServerBurstQPSFlag, 200)
viper.SetDefault(logDirFlag, "")
viper.SetDefault(logSizeLimitMBFlag, 10000) // 10 GB, will be split into 100 MB chunks
viper.SetDefault(disableGameServerContainerServiceAccountFlag, true)

pflag.String(sidecarImageFlag, viper.GetString(sidecarImageFlag), "Flag to overwrite the GameServer sidecar image that is used. Can also use SIDECAR env variable")
pflag.String(sidecarCPULimitFlag, viper.GetString(sidecarCPULimitFlag), "Flag to overwrite the GameServer sidecar container's cpu limit. Can also use SIDECAR_CPU_LIMIT env variable")
Expand All @@ -259,6 +261,8 @@ func parseEnvFlags() config {
pflag.Int32(apiServerBurstQPSFlag, 200, "Maximum burst queries per second to send to the API server")
pflag.String(logDirFlag, viper.GetString(logDirFlag), "If set, store logs in a given directory.")
pflag.Int32(logSizeLimitMBFlag, 1000, "Log file size limit in MB")
pflag.Bool(disableGameServerContainerServiceAccountFlag, viper.GetBool(disableGameServerContainerServiceAccountFlag), "When enabled, mounts an `emptyDir` over the service account token in the GameServer container, to stop game server processes from accessing Kubernetes. Can also use env DISABLE_GAMESERVER_SERVICE_ACCOUNT")

pflag.Parse()

viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
Expand All @@ -280,6 +284,7 @@ func parseEnvFlags() config {
runtime.Must(viper.BindEnv(apiServerBurstQPSFlag))
runtime.Must(viper.BindEnv(logDirFlag))
runtime.Must(viper.BindEnv(logSizeLimitMBFlag))
runtime.Must(viper.BindEnv(disableGameServerContainerServiceAccountFlag))

request, err := resource.ParseQuantity(viper.GetString(sidecarCPURequestFlag))
if err != nil {
Expand All @@ -292,45 +297,47 @@ func parseEnvFlags() config {
}

return config{
MinPort: int32(viper.GetInt64(minPortFlag)),
MaxPort: int32(viper.GetInt64(maxPortFlag)),
SidecarImage: viper.GetString(sidecarImageFlag),
SidecarCPURequest: request,
SidecarCPULimit: limit,
AlwaysPullSidecar: viper.GetBool(pullSidecarFlag),
KeyFile: viper.GetString(keyFileFlag),
CertFile: viper.GetString(certFileFlag),
KubeConfig: viper.GetString(kubeconfigFlag),
PrometheusMetrics: viper.GetBool(enablePrometheusMetricsFlag),
Stackdriver: viper.GetBool(enableStackdriverMetricsFlag),
GCPProjectID: viper.GetString(projectIDFlag),
NumWorkers: int(viper.GetInt32(numWorkersFlag)),
APIServerSustainedQPS: int(viper.GetInt32(apiServerSustainedQPSFlag)),
APIServerBurstQPS: int(viper.GetInt32(apiServerBurstQPSFlag)),
LogDir: viper.GetString(logDirFlag),
LogSizeLimitMB: int(viper.GetInt32(logSizeLimitMBFlag)),
MinPort: int32(viper.GetInt64(minPortFlag)),
MaxPort: int32(viper.GetInt64(maxPortFlag)),
SidecarImage: viper.GetString(sidecarImageFlag),
SidecarCPURequest: request,
SidecarCPULimit: limit,
AlwaysPullSidecar: viper.GetBool(pullSidecarFlag),
KeyFile: viper.GetString(keyFileFlag),
CertFile: viper.GetString(certFileFlag),
KubeConfig: viper.GetString(kubeconfigFlag),
PrometheusMetrics: viper.GetBool(enablePrometheusMetricsFlag),
Stackdriver: viper.GetBool(enableStackdriverMetricsFlag),
GCPProjectID: viper.GetString(projectIDFlag),
NumWorkers: int(viper.GetInt32(numWorkersFlag)),
APIServerSustainedQPS: int(viper.GetInt32(apiServerSustainedQPSFlag)),
APIServerBurstQPS: int(viper.GetInt32(apiServerBurstQPSFlag)),
LogDir: viper.GetString(logDirFlag),
LogSizeLimitMB: int(viper.GetInt32(logSizeLimitMBFlag)),
DisableGameServerContainerServiceAccount: viper.GetBool(disableGameServerContainerServiceAccountFlag),
}
}

// config stores all required configuration to create a game server controller.
type config struct {
MinPort int32
MaxPort int32
SidecarImage string
SidecarCPURequest resource.Quantity
SidecarCPULimit resource.Quantity
AlwaysPullSidecar bool
PrometheusMetrics bool
Stackdriver bool
KeyFile string
CertFile string
KubeConfig string
GCPProjectID string
NumWorkers int
APIServerSustainedQPS int
APIServerBurstQPS int
LogDir string
LogSizeLimitMB int
MinPort int32
MaxPort int32
SidecarImage string
SidecarCPURequest resource.Quantity
SidecarCPULimit resource.Quantity
AlwaysPullSidecar bool
PrometheusMetrics bool
Stackdriver bool
DisableGameServerContainerServiceAccount bool
KeyFile string
CertFile string
KubeConfig string
GCPProjectID string
NumWorkers int
APIServerSustainedQPS int
APIServerBurstQPS int
LogDir string
LogSizeLimitMB int
}

// validate ensures the ctlConfig data is valid.
Expand Down
2 changes: 2 additions & 0 deletions install/helm/agones/templates/controller.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ spec:
value: {{ .Values.agones.controller.apiServerQPS | quote }}
- name: API_SERVER_QPS_BURST
value: {{ .Values.agones.controller.apiServerQPSBurst | quote }}
- name: DISABLE_GAMESERVER_SERVICE_ACCOUNT
value: {{ .Values.gameservers.disableGameServerContainerServiceAccount | quote }}
{{- if .Values.agones.controller.persistentLogs }}
- name: LOG_DIR
value: "/home/agones/logs"
Expand Down
3 changes: 2 additions & 1 deletion install/helm/agones/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ agones:

gameservers:
namespaces:
- default
- default
minPort: 7000
maxPort: 8000
disableGameServerContainerServiceAccount: true

2 changes: 2 additions & 0 deletions install/yaml/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,8 @@ spec:
value: "400"
- name: API_SERVER_QPS_BURST
value: "500"
- name: DISABLE_GAMESERVER_SERVICE_ACCOUNT
value: "true"
- name: LOG_DIR
value: "/home/agones/logs"
- name: LOG_SIZE_LIMIT_MB
Expand Down
13 changes: 13 additions & 0 deletions pkg/apis/stable/v1alpha1/gameserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,19 @@ func (gs *GameServer) FindGameServerContainer() (int, corev1.Container, error) {
return -1, corev1.Container{}, errors.Errorf("Could not find a container named %s", gs.Spec.Container)
}

// ApplyToPodGameServerContainer applies func(v1.Container) to the pod's gameserver container
func (gs *GameServer) ApplyToPodGameServerContainer(pod *corev1.Pod, f func(corev1.Container) corev1.Container) *corev1.Pod {
for i, c := range pod.Spec.Containers {
if c.Name == gs.Spec.Container {
c = f(c)
pod.Spec.Containers[i] = c
break
}
}

return pod
}

// Pod creates a new Pod from the PodTemplateSpec
// attached to the GameServer resource
func (gs *GameServer) Pod(sidecars ...corev1.Container) (*corev1.Pod, error) {
Expand Down
34 changes: 33 additions & 1 deletion pkg/apis/stable/v1alpha1/gameserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ func TestGameServerPatch(t *testing.T) {

assert.Contains(t, string(patch), `{"op":"replace","path":"/spec/container","value":"bear"}`)
}
func TestGetDevAddress(t *testing.T) {
func TestGameServerGetDevAddress(t *testing.T) {
devGs := &GameServer{
ObjectMeta: metav1.ObjectMeta{
Name: "dev-game",
Expand All @@ -451,3 +451,35 @@ func TestGetDevAddress(t *testing.T) {
assert.False(t, isDev, "dev-game should NOT have a dev-address")
assert.Equal(t, "", devAddress, "dev-address IP address should be 127.1.1.1")
}

func TestGameServerApplyToPodGameServerContainer(t *testing.T) {
t.Parallel()

name := "mycontainer"
gs := &GameServer{
Spec: GameServerSpec{
Container: name,
Template: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{Name: name, Image: "foo/mycontainer"},
{Name: "notmycontainer", Image: "foo/notmycontainer"},
},
},
},
},
}

p1 := &corev1.Pod{Spec: *gs.Spec.Template.Spec.DeepCopy()}

p2 := gs.ApplyToPodGameServerContainer(p1, func(c corev1.Container) corev1.Container {
// easy thing to change and test for
c.TTY = true

return c
})

assert.Len(t, p2.Spec.Containers, 2)
assert.True(t, p2.Spec.Containers[0].TTY)
assert.False(t, p2.Spec.Containers[1].TTY)
}
Loading

0 comments on commit f418f5e

Please sign in to comment.