Conditionally enable mtls for the allocator. #1645

devloop0 · 2020-06-24T21:49:59Z

What type of PR is this?
/kind feature

What this PR does / Why we need it:
Customers can provide their own authentication (or none) instead of being forced to use mtls.

Which issue(s) this PR fixes:
Partially addresses: #1590

Special notes for your reviewer:

agones-bot · 2020-06-24T22:16:31Z

Build Failed 😱

Build Id: 2e4f3fbd-3710-4f9c-9082-1fae9d88205e

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

agones-bot · 2020-06-24T22:22:56Z

Build Succeeded 👏

Build Id: d4b036f8-27c2-41ff-8dfa-e41c6755c852

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.7.0-e659e5a
image: gcr.io/agones-images/agones-sdk:1.7.0-e659e5a
image: gcr.io/agones-images/agones-ping:1.7.0-e659e5a
Linux C++ SDK (build): agonessdk-1.7.0-e659e5a-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.7.0-e659e5a.zip

A preview of the website (the last 30 builds are retained):

https://e659e5a-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1645/head:pr_1645 && git checkout pr_1645
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-e659e5a

agones-bot · 2020-06-24T22:47:15Z

Build Succeeded 👏

Build Id: 730847fe-ca02-432c-bc9c-4d0d4ede3630

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.7.0-89a60ff
image: gcr.io/agones-images/agones-sdk:1.7.0-89a60ff
image: gcr.io/agones-images/agones-ping:1.7.0-89a60ff
Linux C++ SDK (build): agonessdk-1.7.0-89a60ff-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.7.0-89a60ff.zip

A preview of the website (the last 30 builds are retained):

https://89a60ff-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1645/head:pr_1645 && git checkout pr_1645
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-89a60ff

agones-bot · 2020-06-25T02:51:53Z

Build Failed 😱

Build Id: 5d4865d1-a3b9-4c76-96f8-0711502a9e21

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

agones-bot · 2020-06-25T04:35:09Z

Build Failed 😱

Build Id: 833a57e5-7ac4-4bb7-987f-769a3c6af789

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

agones-bot · 2020-06-25T05:13:42Z

Build Succeeded 👏

Build Id: 24e1a8d0-1166-48ec-acfb-99881b8e18e7

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.7.0-24f314c
image: gcr.io/agones-images/agones-sdk:1.7.0-24f314c
image: gcr.io/agones-images/agones-ping:1.7.0-24f314c
Linux C++ SDK (build): agonessdk-1.7.0-24f314c-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.7.0-24f314c.zip

A preview of the website (the last 30 builds are retained):

https://24f314c-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1645/head:pr_1645 && git checkout pr_1645
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-24f314c

pooneh-m

This looks great. My concern is by using feature gate, it is expected that the feature is enabled by default at one point, which is not going to be the case. @markmandel can you please comment on this? The reason for adapting feature flag is the lack of support for testing the helm parameters.

pkg/util/runtime/features.go

pooneh-m · 2020-06-25T16:51:24Z

Also, we should make TLS secret requirement conditional on the flag: https://github.com/googleforgames/agones/blob/master/install/helm/agones/templates/service/allocation.yaml#L136

pkg/util/runtime/features.go

pkg/gameserverallocations/controller_test.go

cmd/allocator/main_test.go

agones-bot · 2020-06-25T21:00:12Z

Build Succeeded 👏

Build Id: e0f2144c-a41a-4dbc-ae69-bd58bc909915

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.7.0-5e3e97b
image: gcr.io/agones-images/agones-sdk:1.7.0-5e3e97b
image: gcr.io/agones-images/agones-ping:1.7.0-5e3e97b
Linux C++ SDK (build): agonessdk-1.7.0-5e3e97b-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.7.0-5e3e97b.zip

A preview of the website (the last 30 builds are retained):

https://5e3e97b-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1645/head:pr_1645 && git checkout pr_1645
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-5e3e97b

agones-bot · 2020-06-25T21:25:55Z

Build Succeeded 👏

Build Id: 5919332f-0592-4a39-b89a-756d48b9ea4f

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.7.0-d563119
image: gcr.io/agones-images/agones-sdk:1.7.0-d563119
image: gcr.io/agones-images/agones-ping:1.7.0-d563119
Linux C++ SDK (build): agonessdk-1.7.0-d563119-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.7.0-d563119.zip

A preview of the website (the last 30 builds are retained):

https://d563119-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1645/head:pr_1645 && git checkout pr_1645
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-d563119

agones-bot · 2020-06-25T21:40:40Z

Build Failed 😱

Build Id: add65d9a-f7ed-4cc7-94ed-19ca35210dac

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

agones-bot · 2020-06-25T22:08:03Z

Build Succeeded 👏

Build Id: e3e04c92-c11a-4287-bac4-61c050afc832

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.7.0-0cb1f98
image: gcr.io/agones-images/agones-sdk:1.7.0-0cb1f98
image: gcr.io/agones-images/agones-ping:1.7.0-0cb1f98
Linux C++ SDK (build): agonessdk-1.7.0-0cb1f98-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.7.0-0cb1f98.zip

A preview of the website (the last 30 builds are retained):

https://0cb1f98-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1645/head:pr_1645 && git checkout pr_1645
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-0cb1f98

markmandel · 2020-06-25T22:17:07Z

One thing I'm confused about - if I'm reading this right, the feature flag is what is turning the mTLS off or on?

Shouldn't there be a helm argument that sets this value that is part of the config as well? I know it's a bit of a double-set, but otherwise, would we need to introduce a configuration param to Helm (to be passed down to the allocator as an env/argument) to disable mTLS when we go GA with this feature?

I can definitely see arguments for not having both a feature flag and a helm config that essentially does the same thing, but just wanted to be sure that was the long term plan.

devloop0 · 2020-06-25T22:22:35Z

I think the long-term plan should be to include a configuration parameter as part of the values.yaml file.
This offers us more functionality like completely avoiding the need to mount volumes (i.e. only conditionally enabling these lines: https://github.com/googleforgames/agones/blob/master/install/helm/agones/templates/service/allocation.yaml#L136). Currently, all this does is it avoids the need to verify and send certificates when the flag is set, but the certificates still need to present. I think having this flag in the values.yaml file is the best long-term solution.
The reason this isn't being done now is because it's hard to check what happens when yaml files are configured differently at test time.

markmandel · 2020-06-25T22:27:48Z

So when we go to beta, then this feature flag will be on by default, so mTLS will be disabled for all users when this feature goes to beta? Because that's the beta feature level contract.

This seems like it's not going to work with our feature gate flow. Also, when the feature goes GA, users will need to change their config from beta->GA because there will be a whole new configuration value - which seems to go against the spirit of the feature levels (i.e. beta should eventually close onto what GA will be before we switch).

Maybe this doesn't need a feature flag? Since it's a straight introduction of a new feature without it impacting existing functionality?

agones-bot · 2020-06-26T09:47:52Z

Build Failed 😱

Build Id: 30bc2e39-be54-4cec-bbab-83816c20220c

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

agones-bot · 2020-06-26T10:02:01Z

Build Failed 😱

Build Id: fba57667-68a8-4bdd-bb68-20dc272714da

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

agones-bot · 2020-06-26T10:15:06Z

Build Failed 😱

Build Id: 7fc00f71-7027-457a-b766-120bb112cbed

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

markmandel · 2020-06-26T16:45:21Z

Will take a look, but looks like tests are failing because you'll need to run make gen-install 👍

(Since you edited the helm config, we check if the install.yaml in git still lines up)

agones-bot · 2020-06-26T18:30:55Z

Build Failed 😱

Build Id: 1f8d997c-7e27-4812-8fe2-ace1993ea2a2

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

agones-bot · 2020-06-26T18:44:41Z

Build Failed 😱

Build Id: 562f59bf-1abd-4544-8aa4-09107e138333

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

agones-bot · 2020-06-26T18:56:06Z

Build Failed 😱

Build Id: 2f270371-2f23-4840-ab76-e1525a83a1c1

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

pooneh-m

Thanks for the change.

pkg/gameserverallocations/allocator.go

agones-bot · 2020-06-26T22:43:33Z

Build Succeeded 👏

Build Id: 8b4ce118-cceb-4e02-8e10-a4077c46eef6

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.7.0-737a50e
image: gcr.io/agones-images/agones-sdk:1.7.0-737a50e
image: gcr.io/agones-images/agones-ping:1.7.0-737a50e
Linux C++ SDK (build): agonessdk-1.7.0-737a50e-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.7.0-737a50e.zip

A preview of the website (the last 30 builds are retained):

https://737a50e-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1645/head:pr_1645 && git checkout pr_1645
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-737a50e

install/helm/agones/templates/controller.yaml

agones-bot · 2020-06-26T23:42:41Z

Build Failed 😱

Build Id: 93c007b6-72c0-461e-8a89-3ed116682186

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

agones-bot · 2020-06-27T00:24:40Z

Build Failed 😱

Build Id: 4dd91a33-c895-41ed-b6f2-69b6542404b5

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

agones-bot · 2020-06-27T00:39:26Z

Build Failed 😱

Build Id: 9203dde4-1403-4596-93f5-4936cf0cbc22

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

agones-bot · 2020-06-27T00:56:36Z

Build Failed 😱

Build Id: 79db9af8-7a69-42dc-8693-0234108a86bc

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

google-oss-robot · 2020-06-29T17:04:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: devloop0, pooneh-m

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:

~~OWNERS~~ [pooneh-m]

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

pooneh-m · 2020-06-29T17:10:19Z

Thanks for the change @devloop0. Please also add a note to the issue that e2e testing for this environment variable needs to be added after the test infrastructure is supporting the environment variable testing. Then we need to document this value in the helm parameters.

agones-bot · 2020-06-29T17:39:44Z

Build Succeeded 👏

Build Id: 183134db-6e3e-45f7-9956-b175b0d77f55

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.7.0-7d48bb0
image: gcr.io/agones-images/agones-sdk:1.7.0-7d48bb0
image: gcr.io/agones-images/agones-ping:1.7.0-7d48bb0
Linux C++ SDK (build): agonessdk-1.7.0-7d48bb0-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.7.0-7d48bb0.zip

A preview of the website (the last 30 builds are retained):

https://7d48bb0-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1645/head:pr_1645 && git checkout pr_1645
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-7d48bb0

devloop0 · 2020-06-29T17:41:49Z

Thanks! I'll be sure to add the note once this is merged.

markmandel · 2020-06-29T18:01:02Z

Merging!

* Removed the need for having certificates with mTLS disabled. Co-authored-by: Nikhil Athreya <nathreya@google.com>

google-oss-robot requested review from markmandel and roberthbailey June 24, 2020 21:50

google-oss-robot added the size/M label Jun 24, 2020

googlebot added the cla: yes label Jun 24, 2020

pooneh-m reviewed Jun 25, 2020

View reviewed changes

pkg/util/runtime/features.go Outdated Show resolved Hide resolved

roberthbailey reviewed Jun 25, 2020

View reviewed changes

pkg/util/runtime/features.go Outdated Show resolved Hide resolved

pkg/util/runtime/features.go Outdated Show resolved Hide resolved

pkg/gameserverallocations/controller_test.go Outdated Show resolved Hide resolved

cmd/allocator/main_test.go Outdated Show resolved Hide resolved

google-oss-robot added size/L size/S and removed size/M size/L labels Jun 25, 2020

google-oss-robot added size/L and removed size/S labels Jun 26, 2020

Removed duplicate disableMTLS flag in values.yaml

737a50e

pooneh-m reviewed Jun 26, 2020

View reviewed changes

pkg/gameserverallocations/allocator.go Outdated Show resolved Hide resolved

nathreya-google added 2 commits June 26, 2020 23:23

Still want to keep MTLS for multi-cluster.

f865947

Clarified the comment a bit more.

fce7043

pooneh-m reviewed Jun 26, 2020

View reviewed changes

install/helm/agones/templates/controller.yaml Outdated Show resolved Hide resolved

Removed mTLS disabling from the game server allocator.

326750c

Merge branch 'master' into mtls-flag

7d48bb0

pooneh-m approved these changes Jun 29, 2020

View reviewed changes

google-oss-robot assigned pooneh-m Jun 29, 2020

google-oss-robot added the lgtm label Jun 29, 2020

google-oss-robot added the approved label Jun 29, 2020

markmandel merged commit 6998d88 into googleforgames:master Jun 29, 2020

markmandel added area/operations Installation, updating, metrics etc kind/feature New features for Agones labels Jun 29, 2020

markmandel added this to the 1.7.0 milestone Jun 29, 2020

devloop0 mentioned this pull request Jun 29, 2020

Proposal: Provide a flag to disable mTLS for agones-allocator #1590

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Conditionally enable mtls for the allocator. #1645

Conditionally enable mtls for the allocator. #1645

devloop0 commented Jun 24, 2020 •

edited

Loading

agones-bot commented Jun 24, 2020

agones-bot commented Jun 24, 2020

agones-bot commented Jun 24, 2020

agones-bot commented Jun 25, 2020

agones-bot commented Jun 25, 2020

agones-bot commented Jun 25, 2020

pooneh-m left a comment

pooneh-m commented Jun 25, 2020

agones-bot commented Jun 25, 2020

agones-bot commented Jun 25, 2020

agones-bot commented Jun 25, 2020

agones-bot commented Jun 25, 2020

markmandel commented Jun 25, 2020 •

edited

Loading

devloop0 commented Jun 25, 2020

markmandel commented Jun 25, 2020

agones-bot commented Jun 26, 2020

agones-bot commented Jun 26, 2020

agones-bot commented Jun 26, 2020

markmandel commented Jun 26, 2020

agones-bot commented Jun 26, 2020

agones-bot commented Jun 26, 2020

agones-bot commented Jun 26, 2020

pooneh-m left a comment

agones-bot commented Jun 26, 2020

agones-bot commented Jun 26, 2020

agones-bot commented Jun 27, 2020

agones-bot commented Jun 27, 2020

agones-bot commented Jun 27, 2020

google-oss-robot commented Jun 29, 2020

pooneh-m commented Jun 29, 2020

agones-bot commented Jun 29, 2020

devloop0 commented Jun 29, 2020

markmandel commented Jun 29, 2020

Conditionally enable mtls for the allocator. #1645

Conditionally enable mtls for the allocator. #1645

Conversation

devloop0 commented Jun 24, 2020 • edited Loading

agones-bot commented Jun 24, 2020

agones-bot commented Jun 24, 2020

agones-bot commented Jun 24, 2020

agones-bot commented Jun 25, 2020

agones-bot commented Jun 25, 2020

agones-bot commented Jun 25, 2020

pooneh-m left a comment

Choose a reason for hiding this comment

pooneh-m commented Jun 25, 2020

agones-bot commented Jun 25, 2020

agones-bot commented Jun 25, 2020

agones-bot commented Jun 25, 2020

agones-bot commented Jun 25, 2020

markmandel commented Jun 25, 2020 • edited Loading

devloop0 commented Jun 25, 2020

markmandel commented Jun 25, 2020

agones-bot commented Jun 26, 2020

agones-bot commented Jun 26, 2020

agones-bot commented Jun 26, 2020

markmandel commented Jun 26, 2020

agones-bot commented Jun 26, 2020

agones-bot commented Jun 26, 2020

agones-bot commented Jun 26, 2020

pooneh-m left a comment

Choose a reason for hiding this comment

agones-bot commented Jun 26, 2020

agones-bot commented Jun 26, 2020

agones-bot commented Jun 27, 2020

agones-bot commented Jun 27, 2020

agones-bot commented Jun 27, 2020

google-oss-robot commented Jun 29, 2020

pooneh-m commented Jun 29, 2020

agones-bot commented Jun 29, 2020

devloop0 commented Jun 29, 2020

markmandel commented Jun 29, 2020

devloop0 commented Jun 24, 2020 •

edited

Loading

markmandel commented Jun 25, 2020 •

edited

Loading