Skip to content
This repository has been archived by the owner on Feb 28, 2019. It is now read-only.

Add RuleSet validation endpoint (in swagger). #49

Merged
merged 9 commits into from
Jan 12, 2018

Conversation

m-sandusky
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.786% when pulling a7e484b on validation-endpoint-swagger into 706b2dd on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.786% when pulling a7e484b on validation-endpoint-swagger into 706b2dd on master.

@@ -771,6 +775,52 @@
}
}
}
},
"/rulesets/validate": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we're validating the full RuleSet, why not have the validation process apart of the normal POST and PUT operations for the mapping and rollup roles.

For example, when you do a PUT on a mapping rules
PUT /namespaces/{namespaceID}/mapping-rules

It can return a HTTP 400: Mapping rule is invalid because {error}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @saminzadeh I've actually updated the diff to include validation for namespace as well so things have changed around. I believe RuleSets are validated on all POST and PUT operations dealing with rules already. The motivation for this endpoint is to:

  1. Provide a validation endpoint for proposed changes to a namespace (ie given a namespace name and its updated RuleSets - validate them). This will be used in CI to validate changes to rules when users have diffs created for their rule changes

  2. Have a way to validate new changes while the user is making them from the UI (without them having to hit create/update.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.786% when pulling 3479f5e on validation-endpoint-swagger into 706b2dd on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.786% when pulling 3479f5e on validation-endpoint-swagger into 706b2dd on master.

@m-sandusky m-sandusky changed the title Add RuleSet validation endpoint. Add RuleSet validation endpoint (in swagger). Jan 3, 2018
@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.786% when pulling ddd736f on validation-endpoint-swagger into 706b2dd on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.786% when pulling ddd736f on validation-endpoint-swagger into 706b2dd on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.786% when pulling 8419c96 on validation-endpoint-swagger into 706b2dd on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.786% when pulling 8419c96 on validation-endpoint-swagger into 706b2dd on master.

Copy link
Collaborator

@saminzadeh saminzadeh left a comment

Choose a reason for hiding this comment

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

LGMT. As discussed, it will be nice to consolidate a single endpoint in the future POST /namespace and it could take a dry_run parameter.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.786% when pulling 84f3c05 on validation-endpoint-swagger into 706b2dd on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.786% when pulling 84f3c05 on validation-endpoint-swagger into 706b2dd on master.

@coveralls
Copy link

coveralls commented Jan 3, 2018

Coverage Status

Coverage remained the same at 52.786% when pulling 89e8a3a on validation-endpoint-swagger into 706b2dd on master.

@coveralls
Copy link

coveralls commented Jan 10, 2018

Coverage Status

Coverage remained the same at 52.786% when pulling 6c8fe19 on validation-endpoint-swagger into 706b2dd on master.

Copy link
Contributor

@xichen2020 xichen2020 left a comment

Choose a reason for hiding this comment

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

LGTM other than the nits.

"tags": [
"namespaces"
],
"summary": "Performs validation for a Namespace's RuleSet.",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: ... for a namespace's ruleset.

"namespaces"
],
"summary": "Performs validation for a Namespace's RuleSet.",
"operationId": "validateNamespaceRuleSet",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: just validateRuleSet?

},
{
"in": "body",
"name": "rule-set",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: just ruleset?

{
"in": "body",
"name": "rule-set",
"description": "The rule-set to validate.",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: The ruleset to validate.

],
"responses": {
"200": {
"description": "The rule-set is valid.",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: rule-set -> ruleset.

}
},
"400": {
"description": "The rule-set is invalid.",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: rule-set -> ruleset.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.786% when pulling b772a74 on validation-endpoint-swagger into 706b2dd on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 52.786% when pulling b772a74 on validation-endpoint-swagger into 706b2dd on master.

@@ -189,8 +189,8 @@
"tags": [
"namespaces"
],
"summary": "Performs validation for a Namespace's RuleSet.",
"operationId": "validateNamespaceRuleSet",
"summary": "Performs validation for a namespace's ruleSet.",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: a namespace's ruleset?

@coveralls
Copy link

coveralls commented Jan 12, 2018

Coverage Status

Coverage increased (+4.5%) to 57.333% when pulling 0715a36 on validation-endpoint-swagger into 706b2dd on master.

@m-sandusky m-sandusky merged commit 1441ef1 into master Jan 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants