-
Notifications
You must be signed in to change notification settings - Fork 823
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.
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
Add security context to Agones containers #3856
Conversation
This LGTM but I'd like to see it pass CI before I approve |
Build Failed 😱 Build Id: c6935f4c-7aec-47aa-b350-1783009d55e9 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: 65712bed-b83c-4aa0-b0df-9c1324359379 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
I added UIDs because the pods won't start if I don't provide them (Error is https://stackoverflow.com/questions/49720308/kubernetes-podsecuritypolicy-set-to-runasnonroot-container-has-runasnonroot-and for reference |
@@ -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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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:)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!
Build Succeeded 👏 Build Id: d5c3bfaa-8b8a-4ff8-8d7f-74935d6d1752 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Failed 😱 Build Id: 8264d9ca-2b1d-4fcd-b974-78f72231123e To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: cf7d21eb-6607-49cf-8ff3-6d6d92dc76c6 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: 57410224-9f6d-469e-b482-aaed1ec61303 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [agones](https://agones.dev) ([source](https://github.com/googleforgames/agones)) | minor | `1.41.0` -> `1.42.0` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>googleforgames/agones (agones)</summary> ### [`v1.42.0`](https://github.com/googleforgames/agones/blob/HEAD/CHANGELOG.md#v1420-2024-07-16) [Compare Source](https://github.com/googleforgames/agones/compare/v1.41.0...v1.42.0) [Full Changelog](https://github.com/googleforgames/agones/compare/v1.41.0...v1.42.0) **Breaking changes:** - Update csharp.md to indicate ConnectAsync is deprecated by [@​aallbrig](https://github.com/aallbrig) in [https://github.com/googleforgames/agones/pull/3866](https://github.com/googleforgames/agones/pull/3866) **Implemented enhancements:** - Add security context to Agones containers by [@​peterzhongyi](https://github.com/peterzhongyi) in [https://github.com/googleforgames/agones/pull/3856](https://github.com/googleforgames/agones/pull/3856) - Add Security Context to game server sidecar by [@​peterzhongyi](https://github.com/peterzhongyi) in [https://github.com/googleforgames/agones/pull/3869](https://github.com/googleforgames/agones/pull/3869) - Drop CountsAndLists Data from the Fleet and Game Server Set When the Flag is False by [@​igooch](https://github.com/igooch) in [https://github.com/googleforgames/agones/pull/3881](https://github.com/googleforgames/agones/pull/3881) - Adds tests to confirm that Fleet, Fleet Autoscaler, and Fleet Allocation apply defaults code is idempotent by [@​igooch](https://github.com/igooch) in [https://github.com/googleforgames/agones/pull/3888](https://github.com/googleforgames/agones/pull/3888) - feat: Add CRD Changes and Feature Flag for chain policy by [@​indexjoseph](https://github.com/indexjoseph) in [https://github.com/googleforgames/agones/pull/3880](https://github.com/googleforgames/agones/pull/3880) **Fixed bugs:** - sdk-server expects SDK_LOG_LEVEL by [@​KAllan357](https://github.com/KAllan357) in [https://github.com/googleforgames/agones/pull/3858](https://github.com/googleforgames/agones/pull/3858) - this will resolve From/layer extraction issue on ltsc2019 in examples by [@​ashutosji](https://github.com/ashutosji) in [https://github.com/googleforgames/agones/pull/3873](https://github.com/googleforgames/agones/pull/3873) - featuregate: adds validation if PortPolicyNone is not enabled by [@​daniellee](https://github.com/daniellee) in [https://github.com/googleforgames/agones/pull/3871](https://github.com/googleforgames/agones/pull/3871) - added local as default for registry when registry is not specified by [@​kamaljeeti](https://github.com/kamaljeeti) in [https://github.com/googleforgames/agones/pull/3876](https://github.com/googleforgames/agones/pull/3876) - Buffer Unity SDK ReceiveData when watching for configuration changes by [@​ZeroParticle](https://github.com/ZeroParticle) in [https://github.com/googleforgames/agones/pull/3872](https://github.com/googleforgames/agones/pull/3872) - agones-{extensions,allocator}: Make servers context aware by [@​zmerlynn](https://github.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3845](https://github.com/googleforgames/agones/pull/3845) - added condition for distributed logic by [@​ashutosji](https://github.com/ashutosji) in [https://github.com/googleforgames/agones/pull/3877](https://github.com/googleforgames/agones/pull/3877) **Security fixes:** - Bump [@​grpc/grpc-js](https://github.com/grpc/grpc-js) from 1.10.7 to 1.10.9 in /sdks/nodejs by [@​dependabot](https://github.com/dependabot) in [https://github.com/googleforgames/agones/pull/3863](https://github.com/googleforgames/agones/pull/3863) **Other:** - Preparation for Release v1.42.0 by [@​ashutosji](https://github.com/ashutosji) in [https://github.com/googleforgames/agones/pull/3854](https://github.com/googleforgames/agones/pull/3854) - Add helpful note to edit-first-gameserver-go by [@​peterzhongyi](https://github.com/peterzhongyi) in [https://github.com/googleforgames/agones/pull/3846](https://github.com/googleforgames/agones/pull/3846) - Moved Passthrough feature description to the correct section in Feature Stages by [@​vicentefb](https://github.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3861](https://github.com/googleforgames/agones/pull/3861) - Updated Node.js Page to Reflect that Counters and Lists is Implemented by [@​ashutosji](https://github.com/ashutosji) in [https://github.com/googleforgames/agones/pull/3865](https://github.com/googleforgames/agones/pull/3865) - Change Slack channel description from #developers to #development by [@​branhoff](https://github.com/branhoff) in [https://github.com/googleforgames/agones/pull/3868](https://github.com/googleforgames/agones/pull/3868) - updated UpdateList documentation for local sdk server and sdk server by [@​ashutosji](https://github.com/ashutosji) in [https://github.com/googleforgames/agones/pull/3878](https://github.com/googleforgames/agones/pull/3878) - Add zio-agones to the list of third party client SDKs by [@​ghostdogpr](https://github.com/ghostdogpr) in [https://github.com/googleforgames/agones/pull/3875](https://github.com/googleforgames/agones/pull/3875) - refactor simple game server by [@​ashutosji](https://github.com/ashutosji) in [https://github.com/googleforgames/agones/pull/3817](https://github.com/googleforgames/agones/pull/3817) - Update Slack invite link by [@​markmandel](https://github.com/markmandel) in [https://github.com/googleforgames/agones/pull/3896](https://github.com/googleforgames/agones/pull/3896) - Added cleanup for app-engine services in cloudbuild script by [@​kamaljeeti](https://github.com/kamaljeeti) in [https://github.com/googleforgames/agones/pull/3890](https://github.com/googleforgames/agones/pull/3890) - Adds a command to generate the zz_generated.deepcopy.go files for the apis by [@​igooch](https://github.com/igooch) in [https://github.com/googleforgames/agones/pull/3900](https://github.com/googleforgames/agones/pull/3900) - update go version to 1.21.12 by [@​ashutosji](https://github.com/ashutosji) in [https://github.com/googleforgames/agones/pull/3894](https://github.com/googleforgames/agones/pull/3894) **New Contributors:** - [@​KAllan357](https://github.com/KAllan357) made their first contribution in [https://github.com/googleforgames/agones/pull/3858](https://github.com/googleforgames/agones/pull/3858) - [@​branhoff](https://github.com/branhoff) made their first contribution in [https://github.com/googleforgames/agones/pull/3868](https://github.com/googleforgames/agones/pull/3868) - [@​aallbrig](https://github.com/aallbrig) made their first contribution in [https://github.com/googleforgames/agones/pull/3866](https://github.com/googleforgames/agones/pull/3866) - [@​ZeroParticle](https://github.com/ZeroParticle) made their first contribution in [https://github.com/googleforgames/agones/pull/3872](https://github.com/googleforgames/agones/pull/3872) - [@​ghostdogpr](https://github.com/ghostdogpr) made their first contribution in [https://github.com/googleforgames/agones/pull/3875](https://github.com/googleforgames/agones/pull/3875) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MzIuMCIsInVwZGF0ZWRJblZlciI6IjM3LjQzMi4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9oZWxtIiwidHlwZS9taW5vciJdfQ==-->
Added runAsNonRoot(true) and allowPrivilegeEscalation(false) into the containers field of controller, extensions, ping, and allocator.
Left out sidecar because it is generated by the controller and require another set of changes.
Towards #3848