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: added REST API for validating yaml VPP-Agent configuration using validate methods on registered descriptors #1786

Merged
merged 8 commits into from
Mar 3, 2021

Conversation

fgschwan
Copy link
Collaborator

No description provided.

…g validate methods on registered descriptors

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
…ield names (fields filled in descriptor validator are not enforced to fill correct field value)

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
client/dynamic_config.go Outdated Show resolved Hide resolved
Co-authored-by: Andrii <a.zakurenyi@gmail.com>

// ValidateSemantically validates given proto messages according to semantic validation(KVDescriptor.Validate)
// from registered KVDescriptors. If all messages are valid, nil is returned. If all message could be
// validated, kvscheduler.InvalidMessagesError is returned. In any other case, error is returned.
Copy link
Collaborator

Choose a reason for hiding this comment

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

If all message could be validated, kvscheduler.InvalidMessagesError is returned.

Probably mistake in the comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 695d734

}
goType := model.LocalGoType() // only for locally known models will return meaningful go type
if goType == nil {
return errors.Errorf("dynamic messages for remote models are not supported due to "+
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could there be an option to skip over items with remotely known models instead of returning error?
(i.e. validate everything that can be validated)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I forgot here about the remote models that are in local registry (the proxied remote models). It will validate everything that can be validated. See 695d734.

fgschwan added 3 commits March 1, 2021 17:08
Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
…or to indicate validation failure

Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
Signed-off-by: Filip Gschwandtner <filip.gschwandtner@pantheon.tech>
@fgschwan
Copy link
Collaborator Author

fgschwan commented Mar 1, 2021

The new 2 commits are basically fixes of this PR that i did as part of other PR dependent on this one (#1788). So i logically moved them here.

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.

3 participants