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.

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

GameServerTemplate validation: no description when used big port values #1770

Closed
aLekSer opened this issue Aug 25, 2020 · 5 comments · Fixed by #2865
Closed

GameServerTemplate validation: no description when used big port values #1770

aLekSer opened this issue Aug 25, 2020 · 5 comments · Fixed by #2865
Assignees
Labels
good first issue These are great first issues. If you are looking for a place to start, start here! help wanted We would love help on these issues. Please come help us! kind/bug These are bugs.
Milestone

Comments

@aLekSer
Copy link
Collaborator

aLekSer commented Aug 25, 2020

What happened:
Inconsistent error message when using a big value for grpcPort field grpcPort: 33332229357.

The Fleet "" is invalid

What you expected to happen:
Consistent error message without dependency on port field length.

How to reproduce it (as minimally and precisely as possible):

Change example/fleet.yaml lines starting from 65 to :

      sdkServer:
        # sdkServer log level parameter has three options:
        #  - "Info" (default) The SDK server will output all messages except for debug messages
        #  - "Debug" The SDK server will output all messages including debug messages
        #  - "Error" The SDK server will only output error messages
        logLevel: Info
        grpcPort: 33332229357
        httpPort: 9358

kubectl apply -f resulted fleet:

The Fleet "" is invalid

Do the same with smaller grpcPort value 2229357:

spec.template.spec.sdkServer.grpcPort in body should be less than or equal to 65535

Anything else we need to know?:

Environment:

  • Agones version: 1.8 (master)
  • Kubernetes version (use kubectl version): 1.15
  • Cloud provider or hardware configuration:
  • Install method (yaml/helm): Terraform Helm
  • Troubleshooting guide log(s):
  • Others:
@aLekSer aLekSer added the kind/bug These are bugs. label Aug 25, 2020
@aLekSer
Copy link
Collaborator Author

aLekSer commented Sep 15, 2020

I assume that this bug is more general and specific to Helm templates and out of scope of Agones. Need more investigation.

@markmandel
Copy link
Member

We should retest this.

The https://github.com/googleforgames/agones/blob/main/install/helm/agones/templates/crds/_gameserverspecschema.yaml has min/max values, so in theory, you should get good validation - but we are leaning on K8s to provide it. Not sure how much control we have in Agones.

@markmandel markmandel added help wanted We would love help on these issues. Please come help us! good first issue These are great first issues. If you are looking for a place to start, start here! labels Dec 8, 2022
@roberthbailey
Copy link
Member

I just tried this with an install of Agones 1.25.0 and the error message still doesn't tell us what is invalid, just that it is:

$ kubectl apply -f fleet.yaml
The Fleet "fleet-example" is invalid

@roberthbailey
Copy link
Member

I've installed the latest release (1.28.0) and the behavior is the same as was originally reported:

# grpcPort: 33332229357
$ kubectl apply -f fleet.yaml 
The Fleet "fleet-example" is invalid
# grpcPort: 2229357
$ kubectl apply -f fleet2.yaml 
The Fleet "fleet-example" is invalid: spec.template.spec.sdkServer.grpcPort: Invalid value: 2229357: spec.template.spec.sdkServer.grpcPort in body should be less than or equal to 65535

My guess is that the small number is convertible to an integer type (which can then be compared to the min/max values) whereas the larger number overflows and as such can not be converted to an integer type at all, producing the generic error message.

@zmerlynn zmerlynn self-assigned this Dec 12, 2022
@zmerlynn
Copy link
Collaborator

I stumbled into this the other day and put up #2863, but I'm playing with a slightly different mechanism now. It seems weird that the OpenAPI validation isn't catching this, but I think that create webhooks take place before schema validation - so I'm also trying to see if we just "fail open" in creation webhooks when the JSON can't be unmarshalled whether we get the nice OpenAPI errors.

zmerlynn added a commit that referenced this issue Dec 13, 2022
Request processing by kube-apiserver is roughly:
  { mutating webhooks } => { OpenAPI schema validation } => { validating webhooks }

In this PR, we disable validation errors (at least, JSON unmarshalling
validation errors) during mutating webhook processing. It seems
counter-intuitive not to return an error in mutating webhooks for a
serious violation such as invalid JSON, but doing so allows OpenAPI
schema validation to proceed. Schema validation gives us nice errors,
so e.g.:

instead of:

```
The Fleet "" is invalid
```

we get:

```
The Fleet "fleet-example" is invalid: spec.template.spec.sdkServer.grpcPort: Invalid value: 33332229357: spec.template.spec.sdkServer.grpcPort in body should be less than or equal to 65535
```

This will become all the more important if we eventually move to CEL
based validations.

I'm also changing the error in #2863 to an internal error - after this
PR, any hard error from a handler (vs a cause) is unexpected.

Fixes #1770
@Kalaiselvi84 Kalaiselvi84 added this to the 1.29.0 milestone Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue These are great first issues. If you are looking for a place to start, start here! help wanted We would love help on these issues. Please come help us! kind/bug These are bugs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants