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

feat: validate schema only during the user's input unmarshal #5566

Merged
merged 1 commit into from Jan 5, 2023

Conversation

lobkovilya
Copy link
Contributor

@lobkovilya lobkovilya commented Jan 3, 2023

Problem

OpenAPI schema has required attribute, for example:

required:
  - port
  - address

But since ValidateSchema accepts Go object as an input, it's not possible to check if the field was set by user. So the user's input:

spec:
  port: 80

is unmarshalled to:

&Spec{
  port: 80,
}

and ValidateSchema returns no errors even though the address was not set.

As a result the validation result is different on K8s and Universal.

Solution

Unmarshal user's input to rawObj := map[string]interface{}{} instead of spec := &Spec{}. In that case if the field is not provided by user there is no key in rawObj map.

Implementation

This PR moves schema validation out of the Resource#Validate() method. Schema is validated only inside the rest.Unmarshal().

Signed-off-by: Ilya Lobkov ilya.lobkov@konghq.com

Checklist prior to review

  • Link to docs PR or issue --
  • Link to UI issue or PR --
  • Is the issue worked on linked? --
  • The PR does not hardcode values that might break projects that depend on kuma (e.g. "kumahq" as a image registry) --
  • The PR will work for both Linux and Windows, system specific functions like syscall.Mkfifo have equivalent implementation on the other OS --
  • Unit Tests --
  • E2E Tests --
  • Manual Universal Tests --
  • Manual Kubernetes Tests --
  • Do you need to update UPGRADE.md? --
  • Does it need to be backported according to the backporting policy? --
  • Do you need to explicitly set a > Changelog: entry here or add a ci/ label to run fewer/more tests?

Changelog: feat(policy): validate schema only during the user's input unmarshal

Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya lobkovilya marked this pull request as ready for review January 4, 2023 12:13
@lobkovilya lobkovilya requested review from a team, bartsmykla and lukidzi and removed request for a team January 4, 2023 12:13
Copy link
Contributor

@lahabana lahabana left a comment

Choose a reason for hiding this comment

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

This 3x unmarshaling is a little weird but I guess we can always improve on this later.

@lobkovilya lobkovilya merged commit 59ad51f into kumahq:master Jan 5, 2023
@lobkovilya lobkovilya deleted the feat/validate-rest-schema branch January 5, 2023 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants