CC should validate ASGs when created and/or provide feedback to a user pushing an app to an org/space with a badly formatted ASG

[sunjayBhatia]

Issue

• when a user creates a security group that is incorrectly formatted, they should see an error
• if a user pushes an app in an org/space that has an already incorrectly formatted security group rule, they should see a descriptive error as to why their app does not start
  • this is a distinct choice to consider, but should the app push still succeed and the invalid rule discarded? (I would argue against this as it could cause more security problems than it solves app push failures)

Steps to Reproduce

• create a file with an ASG definition (notice whitespace in IP destination)

[
  {
    "protocol": "tcp",
    "destination": "10.0.11.0/24  ",
    "ports": "80,443",
    "log": true,
    "description": "Allow http and https traffic from ZoneA"
  }
]

• cf create-security-group bad-asg <path-to-asg-file>
  • see that this succeeds
• cf bind-security-group bad-asg <some-org> <some-space>
• push an app in that org and space, see that staging completes and the app start times out but without any specific error

Expected result

• the user gets an error telling them their ASG definition is invalid
• if an incorrect ASG already exists, the app push should fail with a descriptive error

Possible Fix

• validate ASGs more strictly on creation (BBS validations here: https://github.com/cloudfoundry/bbs/blob/b694a507897e81b4bced056b349cce3822f19432/models/security_groups.go#L18)
• bubble out the desiredLRP creation failure error from the BBS or that error wrapped with am message from the CC back to the user?

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/165451086

The labels on this github issue will be updated when the story is started.

[ericpromislow]

CC validates all the fields in an ASG. In particular, it uses Ruby's NetAddr::IPv4Net.parse to verify each proposed destination. The problem is that the Ruby library ignores leading and trailing white-space, while apparently the IPAddr parser Diego is using doesn't. Easy fix.

[ericpromislow]

Fixed in cloudfoundry/cloud_controller_ng@df51c47