Skip to content
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

Jump to bottom

Use a numeric User ID for the "agones" user in the SDK sidecar #1293

Merged
merged 4 commits into from
Jan 31, 2020
Merged

Use a numeric User ID for the "agones" user in the SDK sidecar #1293

merged 4 commits into from
Jan 31, 2020

Conversation

TBBle
Copy link
Contributor

@TBBle TBBle commented Jan 22, 2020

This fixes failure of the sdk sidecar container to start when run on a system with a PodSecurityPolicy specifying RunAsUser as "MustRunAsNonRoot".

Kubernetes cannot determine if a non-numeric User in the container metadata is 'root' or not.

Fixes: #1287

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 943ae711-0d65-4257-afa0-239772db8c27

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:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1293/head:pr_1293 && git checkout pr_1293
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.3.0-b956bf4

@TBBle
Copy link
Contributor Author

TBBle commented Jan 22, 2020

@TBBle: When you come back to this, push out the same change to the other Docker files, thanks to @Feng-Yin for testing them all.

@TBBle
Copy link
Contributor Author

TBBle commented Jan 22, 2020

Note: Following up with my employer regarding CLA, which we have signed, but I am apparently not added to the authorised contributors list.

@markmandel
Copy link
Member

@TBBle this is awesome! Thanks for taking this on 👍

Are you going to the same for the all the Agones components (and maybe the examples?), either in this PR or others?

Just wondering if I should hold off on reviewing/merging this, in case you were going to expand it further.

@TBBle
Copy link
Contributor Author

TBBle commented Jan 23, 2020 •
edited
Loading

I'll do the same thing for the other services, since my colleague has already tested that the same change applies to them and functions correctly. I'll have a look at the examples, and do them if I'm confident. I'm currently waiting for the CLA to be sorted out at my end, so I'm hopeful for 'tomorrow' but not confident.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 110c6691-0efb-4c52-bba7-ac3edde7522b

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:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1293/head:pr_1293 && git checkout pr_1293
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.3.0-7fbaf13

@TBBle TBBle force-pushed the sidecar_with_numeric_uid branch 2 times, most recently from 5e38ae6 to bc99720 Compare January 23, 2020 07:50
@TBBle
Copy link
Contributor Author

TBBle commented Jan 23, 2020

I've now updated the other Agones components (tested locally) and the examples (not tested locally). So this is all ready to go except the CLA, which is pending an action on our end.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: a5086c5d-8a1f-4629-b0b1-47c75df98d70

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:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1293/head:pr_1293 && git checkout pr_1293
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.4.0-5e38ae6

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: e395a3a2-f6cf-4506-9a7e-46c1bdd229c6

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:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1293/head:pr_1293 && git checkout pr_1293
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.4.0-bc99720

@roberthbailey roberthbailey requested review from markmandel and removed request for roberthbailey and dzlier-gcp January 23, 2020 17:42
@markmandel
Copy link
Member

Everything looks great - I want to run a quick check on the changes to the examples, but once I've done that, happy to approve!

@markmandel
Copy link
Member

Quick thought on this - if you want to have the example images be updated, please go through each example's Makefile and increment the version number 👍

If we're happy leaving the example images alone, then we can continue as is. (Probably better to increment though)

@markmandel
Copy link
Member

Smoke tested the change in the simple-udp example -- looks good!

@markmandel markmandel added area/operations Installation, updating, metrics etc kind/bug These are bugs. labels Jan 24, 2020
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 81b7e1cb-5c9b-4856-a168-68e161ed90a4

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:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1293/head:pr_1293 && git checkout pr_1293
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.4.0-d357f61

@TBBle
Copy link
Contributor Author

TBBle commented Jan 24, 2020

I updated the versions in all the Makefiles. I also noticed I'd missed the unity-simple example, as it was running as root, and I'd been looking for adduser or useradd calls. So apart from that this is right to go pending the CLA (which I haven't heard back from at my end yet). If I have more time before the CLA list is updated, I'll take a look at the Unity sample, but I don't think it should otherwise block this PR.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: d99ece12-361a-4981-ba87-d4b776de2d8d

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:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1293/head:pr_1293 && git checkout pr_1293
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.4.0-c2f13ae

@markmandel
Copy link
Member

Sounds good to me! I can take care of updating the images, and pushing them up. 👍 they are probably due for a review to make sure they all still work.

@TBBle
Copy link
Contributor Author

TBBle commented Jan 29, 2020

@googlebot I signed it!

@markmandel
Copy link
Member

Just having a look @TBBle - seems like your account is still not added as a contributor 😞 it might take some time though to propagate, I'll take another look tomorrow.

@roberthbailey roberthbailey mentioned this pull request Jan 29, 2020
Merged
@TBBle
Copy link
Contributor Author

TBBle commented Jan 30, 2020

It's a corporate CLA, apparently we're waiting to hear back from someone at Google. I'll keep chasing at my end.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: b54156e1-b2ae-40c4-b610-83331e892dee

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 4a7d10c2-c830-4471-aba8-5c4000a257d7

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:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1293/head:pr_1293 && git checkout pr_1293
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.4.0-1c2a58f

@TBBle
Copy link
Contributor Author

TBBle commented Jan 31, 2020

@googlebot I signed it!

This time I can see it on https://cla.developers.google.com/clas

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Paul "Hampy" Hampson added 4 commits January 31, 2020 11:34
Use a numeric User ID for the agones user
d30e582 
This fixes failure of the container to start when run on a system with a
PodSecurityPolicy specifying RunAsUser as "MustRunAsNonRoot".

Signed-off-by: Paul "Hampy" Hampson <p_hampson@wargaming.net>
Set group-ownership of files to 'agones' group.
a7077cb 
Creating a non-system user named 'agones' also creates a group named
'agones'. For consistency, the files should be owned by that group.

Signed-off-by: Paul "Hampy" Hampson <p_hampson@wargaming.net>
Use a numeric User ID in the Debian-based examples
96f3399 
This fixes failure of the container to start when run on a system with a
PodSecurityPolicy specifying RunAsUser as "MustRunAsNonRoot".

Signed-off-by: Paul "Hampy" Hampson <p_hampson@wargaming.net>
Use a numeric User ID in the Alpine-based examples
d875db5 
This fixes failure of the container to start when run on a system with a
PodSecurityPolicy specifying RunAsUser as "MustRunAsNonRoot".

Signed-off-by: Paul "Hampy" Hampson <p_hampson@wargaming.net>
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 32b059fd-aceb-489a-9a80-9bbf81e65d0b

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:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1293/head:pr_1293 && git checkout pr_1293
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.4.0-d875db5

markmandel
markmandel approved these changes Jan 31, 2020
View reviewed changes
Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

🔥

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markmandel, TBBle

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@markmandel markmandel merged commit 27edc0e into googleforgames:master Jan 31, 2020
@TBBle TBBle deleted the sidecar_with_numeric_uid branch January 31, 2020 01:00
@markmandel markmandel mentioned this pull request Jan 31, 2020
Closed
10 tasks
@markmandel markmandel added this to the 1.4.0 milestone Feb 26, 2020
ilkercelikyilmaz pushed a commit to ilkercelikyilmaz/agones that referenced this pull request Oct 23, 2020
@TBBle @ilkercelikyilmaz
Use a numeric User ID for the "agones" user in the SDK sidecar (googl…
2d5c8fb 
…eforgames#1293)

* Use a numeric User ID for the agones user

This fixes failure of the container to start when run on a system with a
PodSecurityPolicy specifying RunAsUser as "MustRunAsNonRoot".

* Set group-ownership of files to 'agones' group.
Creating a non-system user named 'agones' also creates a group named
'agones'. For consistency, the files should be owned by that group.

* Use a numeric User ID in the Debian-based examples

This fixes failure of the container to start when run on a system with a
PodSecurityPolicy specifying RunAsUser as "MustRunAsNonRoot".

* Use a numeric User ID in the Alpine-based examples

This fixes failure of the container to start when run on a system with a
PodSecurityPolicy specifying RunAsUser as "MustRunAsNonRoot".

Signed-off-by: Paul "Hampy" Hampson <p_hampson@wargaming.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markmandel markmandel markmandel approved these changes

Assignees

@markmandel markmandel

Labels
approved area/operations Installation, updating, metrics etc kind/bug These are bugs. lgtm size/M
Projects
None yet
Milestone
1.4.0
Development

Successfully merging this pull request may close these issues.

Gameserver failed to start in the namespace with runAsNonRoot pod security context.
5 participants
@TBBle @googlebot @agones-bot @markmandel @google-oss-robot