Skip to content

Add security context to Agones containers #3856

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 10, 2024
Merged

Add security context to Agones containers #3856

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
969863e
Add security context
peterzhongyi Jun 5, 2024
3cc787a
mend
peterzhongyi Jun 10, 2024
8089a39
Merge branch 'main' into sec
zmerlynn Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions install/helm/agones/templates/controller.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ spec:
- name: agones-controller
image: "{{ .Values.agones.image.registry }}/{{ .Values.agones.image.controller.name}}:{{ default .Values.agones.image.tag .Values.agones.image.controller.tag }}"
imagePullPolicy: {{ .Values.agones.image.controller.pullPolicy }}
securityContext:
runAsNonRoot: true
runAsUser: 1000
allowPrivilegeEscalation: false
env:
# minimum port that can be exposed to GameServer traffic
- name: MIN_PORT
Expand Down
4 changes: 4 additions & 0 deletions install/helm/agones/templates/extensions-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ spec:
- name: agones-extensions
image: "{{ .Values.agones.image.registry }}/{{ .Values.agones.image.extensions.name}}:{{ default .Values.agones.image.tag .Values.agones.image.extensions.tag }}"
imagePullPolicy: {{ .Values.agones.image.extensions.pullPolicy }}
securityContext:
runAsNonRoot: true
runAsUser: 1001
allowPrivilegeEscalation: false
env:
- name: PROMETHEUS_EXPORTER
value: {{ .Values.agones.metrics.prometheusEnabled | quote }}
Expand Down
4 changes: 4 additions & 0 deletions install/helm/agones/templates/ping.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ spec:
- name: agones-ping
image: "{{ .Values.agones.image.registry }}/{{ .Values.agones.image.ping.name}}:{{ default .Values.agones.image.tag .Values.agones.image.ping.tag }}"
imagePullPolicy: {{ .Values.agones.image.ping.pullPolicy }}
securityContext:
runAsNonRoot: true
runAsUser: 1002
allowPrivilegeEscalation: false
{{- if .Values.agones.ping.resources }}
resources:
{{ toYaml .Values.agones.ping.resources | indent 12 }}
Expand Down
4 changes: 4 additions & 0 deletions install/helm/agones/templates/service/allocation.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ spec:
- name: agones-allocator
image: "{{ .Values.agones.image.registry }}/{{ .Values.agones.image.allocator.name}}:{{ default .Values.agones.image.tag .Values.agones.image.allocator.tag }}"
imagePullPolicy: {{ .Values.agones.image.allocator.pullPolicy }}
securityContext:
runAsNonRoot: true
runAsUser: 1003
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why this is a different userid on each container? I would have thought they would be the same?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, do we need to specify the uid at all? That seems brittle. or will it work with just runAsNonRoot ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh I thought each pod needs to have an unique ID across the cluster (https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#:~:text=Every%20Kubernetes%20object%20also%20has,are%20each%20named%20myapp%2D1234%20.) but apparently it is referring to some other UID because I tried setting the runAsUser 1000 for all 4 binaries and they can turn up just fine.

Yes we need to specify the UID otherwise the pods can't turn up, and the error message would be Error: container has runAsNonRoot and image has non-numeric user (appuser), cannot verify user is non-root

https://stackoverflow.com/questions/49720308/kubernetes-podsecuritypolicy-set-to-runasnonroot-container-has-runasnonroot-and for reference. Actually I also added a comment on the pr to explain this:)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should pick a uid out of the blue. I was hoping this would just work with runAsNonRoot since the image specifies a username, but that appears to be wrong, as you discovered.

If we have to specify an arbitrary uid, I think it should be configurable by helm, in case someone has a weird identity setup. I'd just default it to e.g. 1000.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We talked offline, I think it's ok just to see if anyone really needs this to be configurable. LGTM!

allowPrivilegeEscalation: false
livenessProbe:
httpGet:
path: /live
Expand Down
16 changes: 16 additions & 0 deletions install/yaml/install.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17061,6 +17061,10 @@ spec:
- name: agones-controller
image: "us-docker.pkg.dev/agones-images/release/agones-controller:1.41.0"
imagePullPolicy: IfNotPresent
securityContext:
runAsNonRoot: true
runAsUser: 1000
allowPrivilegeEscalation: false
env:
# minimum port that can be exposed to GameServer traffic
- name: MIN_PORT
Expand Down Expand Up @@ -17221,6 +17225,10 @@ spec:
- name: agones-extensions
image: "us-docker.pkg.dev/agones-images/release/agones-extensions:1.41.0"
imagePullPolicy: IfNotPresent
securityContext:
runAsNonRoot: true
runAsUser: 1001
allowPrivilegeEscalation: false
env:
- name: PROMETHEUS_EXPORTER
value: "true"
Expand Down Expand Up @@ -17358,6 +17366,10 @@ spec:
- name: agones-ping
image: "us-docker.pkg.dev/agones-images/release/agones-ping:1.41.0"
imagePullPolicy: IfNotPresent
securityContext:
runAsNonRoot: true
runAsUser: 1002
allowPrivilegeEscalation: false
livenessProbe:
httpGet:
port: 8080
Expand Down Expand Up @@ -17432,6 +17444,10 @@ spec:
- name: agones-allocator
image: "us-docker.pkg.dev/agones-images/release/agones-allocator:1.41.0"
imagePullPolicy: IfNotPresent
securityContext:
runAsNonRoot: true
runAsUser: 1003
allowPrivilegeEscalation: false
livenessProbe:
httpGet:
path: /live
Expand Down