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

Add type validation for CIDR values in restricted_to #28

Merged
merged 1 commit into from
Aug 19, 2020

Conversation

micahlee
Copy link
Contributor

This PR updates the type for restricted_to to a new :cidr type, that is validated at policy load to ensure this is a valid IPAddr string. If it is not valid, it returns a correct error to point the author to the line and column of the invalid value.

For example, with the policy:

- !host
  id: a-host
  restricted_to: [ 192.168.1.1, invalid_cidr ]

This will cause a validation error:

Error at line 3, column 32 in spec/round-trip/yaml/restricted_to.yml : Invalid IP address or CIDR range 'invalid_cidr'

Connected to #27

@h-artzi
Copy link
Contributor

h-artzi commented Aug 19, 2020

Currently, I believe if a user enters a list for example restricted_to : [ not_valid1, not_valid2 ] it will return Invalid IP address or CIDR range 'not_valid1' I would want it to return all attributes that are not valid so the user can fix them all at once.

@micahlee
Copy link
Contributor Author

@h-artzi

Currently, I believe if a user enters a list for example restricted_to : [ not_valid1, not_valid2 ] it will return Invalid IP address or CIDR range 'not_valid1' I would want it to return all attributes that are not valid so the user can fix them all at once.

You are correct, and I've added a test case to highlight this behavior. Changing this will likely require a larger structural change to how the policy is parsed and errors are collected. I'm going to call that out of scope for this PR, but I did create an issue for it here: #29

Copy link
Contributor

@h-artzi h-artzi left a comment

Choose a reason for hiding this comment

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

LGTM!

@micahlee micahlee merged commit 5da02ad into master Aug 19, 2020
@micahlee micahlee deleted the 27-validate-cidr-type branch August 19, 2020 18:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants