Add validation for Labels on Fleet and GS Template #1257

aLekSer · 2020-01-03T11:42:57Z

Now ObjectMeta Labels part in a Fleet Template and GameServerSpec Template are
validated, as well as for GameServerSet Template field.

Closes #1244 .

agones-bot · 2020-01-03T11:59:42Z

Build Succeeded 👏

Build Id: 3a3c2e98-e767-4776-b082-e2f9b8a715e7

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

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

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

https://951ee33-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1257/head:pr_1257 && git checkout pr_1257
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.3.0-951ee33

roberthbailey · 2020-01-06T07:35:54Z

@aLekSer - is this ready for review or are you still working on it?

aLekSer · 2020-01-06T20:28:07Z

Code is ready for review.
Some tests could be added for GameServerSet, but it should work as for Fleet.
I will add Annotation validation in a separate PR because it is a different way of validating Annotations.

roberthbailey

I don't understand why you added the validation.go and validation_test.go files in the vendor_fixes directory. I diff'd them against what was vendored in and they seemed identical.

roberthbailey · 2020-01-07T23:35:17Z

pkg/apis/agones/v1/common.go

 func validateGSSpec(gs gsSpec) []metav1.StatusCause {
 	gsSpec := gs.GetGameServerSpec()
 	gsSpec.ApplyDefaults()
 	causes, _ := gsSpec.Validate("")

 	return causes
 }
+
+// validateObjectMeta Check ObjectMeta specification
+// Used by Fleet, GameServerSet and Gameserver


nit: GameServer (match case with elsewhere)

roberthbailey · 2020-01-07T23:37:47Z

pkg/apis/agones/v1/gameserver.go

@@ -389,8 +389,12 @@ func (gss GameServerSpec) Validate(devAddress string) ([]metav1.StatusCause, boo
 			})
 		}
 	}
-	return causes, len(causes) == 0
+	objMetaCauses := validateObjectMeta(gss.Template.ObjectMeta)


Should the changes to this file (and gameserverset.go) have unit tests akin to what you added in fleet_test.go?

I have added similar test for gameserver .

roberthbailey · 2020-01-07T23:48:01Z

test/e2e/fleet_test.go

+	for i := 0; i < nameLen; i++ {
+		key[i] = 'f'
+	}
+	flt.Spec.Template.ObjectMeta.Labels["label"] = string(key)


Should be able to just do

flt.Spec.Template.ObjectMeta.Labels["label"] = strings.Repeat("f", validation.LabelValueMaxLength + 1)

See https://golang.org/pkg/strings/#Repeat

Thanks, applying this to all connected code parts

roberthbailey · 2020-01-07T23:48:18Z

pkg/apis/agones/v1/fleet_test.go

+	for i := 0; i < nameLen; i++ {
+		key[i] = 'f'
+	}
+	f.Spec.Template.ObjectMeta.Labels["label"] = string(key)


Should be able to just do f.Spec.Template.ObjectMeta.Labels["label"] = strings.Repeat("f", validation.LabelValueMaxLength + 1)

See https://golang.org/pkg/strings/#Repeat

roberthbailey · 2020-01-07T23:50:17Z

test/e2e/fleet_test.go

+	assert.Equal(t, metav1.CauseTypeFieldValueInvalid, statusErr.Status().Details.Causes[0].Type)
+	goodFlt := defaultFleet(defaultNs)
+	goodFlt.Spec.Template.ObjectMeta.Labels = make(map[string]string)
+	goodFlt.Spec.Template.ObjectMeta.Labels["label"] = string(key[0 : nameLen-1])


goodFlt.Spec.Template.ObjectMeta.Labels["label"] = strings.Repeat("f", validation.LabelValueMaxLength)

aLekSer · 2020-01-08T11:15:41Z

It seems that I did not configure local go properly. make test-go works fine.
A reason for adding api-machinery to both ./vendor and ./vendor_fixes was this issue, we have go.mod which

replace k8s.io/apimachinery => ./vendor_fixes/k8s.io/apimachinery

> go test
common.go:21:2: module k8s.io/apimachinery@latest (v0.17.0) found, but does not contain package k8s.io/apimachinery/pkg/apis/meta/v1/validation

agones-bot · 2020-01-08T11:50:01Z

Build Succeeded 👏

Build Id: 4eb39e22-af92-4c05-b623-3c37f4d887d3

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

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

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

https://882aee5-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1257/head:pr_1257 && git checkout pr_1257
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.3.0-882aee5

Now ObjectMeta in a Fleet Template and GameServerSpec Template are validated, as well as for GameServerSet Template field.

aLekSer · 2020-01-08T11:58:38Z

Removed unnecessary duplicate files out of ./vendor_fixes. Added more UTs for GSS, Fleet and GS.

agones-bot · 2020-01-08T12:11:10Z

Build Succeeded 👏

Build Id: d56975cd-5161-46b5-b221-706fe894f766

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

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

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

https://3443da6-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1257/head:pr_1257 && git checkout pr_1257
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.3.0-3443da6

google-oss-robot · 2020-01-08T17:11:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aLekSer, roberthbailey

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~~ [roberthbailey]

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

Now ObjectMeta in a Fleet Template and GameServerSpec Template are validated, as well as for GameServerSet Template field.

google-oss-robot added the do-not-merge/work-in-progress label Jan 3, 2020

google-oss-robot requested review from cyriltovena and roberthbailey January 3, 2020 11:43

google-oss-robot added the size/XL label Jan 3, 2020

aLekSer marked this pull request as ready for review January 6, 2020 20:24

google-oss-robot removed the do-not-merge/work-in-progress label Jan 6, 2020

markmandel added area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/feature New features for Agones labels Jan 7, 2020

roberthbailey reviewed Jan 7, 2020

View reviewed changes

aLekSer force-pushed the fix/fleet-label branch from 951ee33 to 882aee5 Compare January 8, 2020 11:37

google-oss-robot added size/L and removed size/XL labels Jan 8, 2020

Add validation for Labels on Fleet and GS Template

3443da6

Now ObjectMeta in a Fleet Template and GameServerSpec Template are validated, as well as for GameServerSet Template field.

aLekSer force-pushed the fix/fleet-label branch from 882aee5 to 3443da6 Compare January 8, 2020 11:57

roberthbailey approved these changes Jan 8, 2020

View reviewed changes

google-oss-robot assigned roberthbailey Jan 8, 2020

google-oss-robot added the lgtm label Jan 8, 2020

roberthbailey merged commit e3ae311 into googleforgames:master Jan 8, 2020

google-oss-robot added the approved label Jan 8, 2020

markmandel added this to the 1.3.0 milestone Jan 10, 2020

aLekSer deleted the fix/fleet-label branch January 13, 2020 15:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add validation for Labels on Fleet and GS Template #1257

Add validation for Labels on Fleet and GS Template #1257

aLekSer commented Jan 3, 2020 •

edited

Loading

agones-bot commented Jan 3, 2020

roberthbailey commented Jan 6, 2020

aLekSer commented Jan 6, 2020

roberthbailey left a comment

roberthbailey Jan 7, 2020

roberthbailey Jan 7, 2020

aLekSer Jan 8, 2020

roberthbailey Jan 7, 2020

aLekSer Jan 8, 2020

roberthbailey Jan 7, 2020

roberthbailey Jan 7, 2020

aLekSer commented Jan 8, 2020 •

edited

Loading

agones-bot commented Jan 8, 2020

aLekSer commented Jan 8, 2020

agones-bot commented Jan 8, 2020

google-oss-robot commented Jan 8, 2020

Add validation for Labels on Fleet and GS Template #1257

Add validation for Labels on Fleet and GS Template #1257

Conversation

aLekSer commented Jan 3, 2020 • edited Loading

agones-bot commented Jan 3, 2020

roberthbailey commented Jan 6, 2020

aLekSer commented Jan 6, 2020

roberthbailey left a comment

Choose a reason for hiding this comment

roberthbailey Jan 7, 2020

Choose a reason for hiding this comment

roberthbailey Jan 7, 2020

Choose a reason for hiding this comment

aLekSer Jan 8, 2020

Choose a reason for hiding this comment

roberthbailey Jan 7, 2020

Choose a reason for hiding this comment

aLekSer Jan 8, 2020

Choose a reason for hiding this comment

roberthbailey Jan 7, 2020

Choose a reason for hiding this comment

roberthbailey Jan 7, 2020

Choose a reason for hiding this comment

aLekSer commented Jan 8, 2020 • edited Loading

agones-bot commented Jan 8, 2020

aLekSer commented Jan 8, 2020

agones-bot commented Jan 8, 2020

google-oss-robot commented Jan 8, 2020

aLekSer commented Jan 3, 2020 •

edited

Loading

aLekSer commented Jan 8, 2020 •

edited

Loading