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

Allow comments in security group specifications #382

Closed
sdqali opened this issue May 21, 2015 · 7 comments
Closed

Allow comments in security group specifications #382

sdqali opened this issue May 21, 2015 · 7 comments

Comments

@sdqali
Copy link

sdqali commented May 21, 2015

As security group specifications become fine grained, the specification file becomes harder to understand without any comments to show the intend of each rule.

For example, the security group applied on one of my spaces look like this:

[
  {"protocol":"tcp","destination":"<ip-foo>","ports":"3306"},
  {"protocol":"tcp","destination":"<some-ip>-<another-ip>","ports":"55882"},
  {"protocol":"tcp","destination":"<ip-bar>","ports":"443"}
]

There is no easy way to understand the intend behind each rule.

Allowing a comment field will greatly improve this. Like so:

[
  {"protocol":"tcp","destination":"<ip-foo>","ports":"3306", "comment": "Allow database connection to PostgreSQL at hosted-postgres-service.com"},
  {"protocol":"tcp","destination":"<some-ip>-<another-ip>","ports":"55882", "comment": "Allow logging to hosted-logging-service.com"},
  {"protocol":"tcp","destination":"<ip-bar>","ports":"443", "comment": "Allow monitoring service at hosted-monitoring-service.com"}
]

As things stand, if I were to attempt this, I will get the following error:

Server error, status code: 400, error code: 300001, message: The security group is invalid: rules rule number 1 contains the invalid field 'comment', rules rule number 2 contains the invalid field 'comment', rules rule number 3 contains the invalid field 'comment'

This is as intended.

The comment field should be marked as a valid field.

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/95123330.

@zaksoup
Copy link
Contributor

zaksoup commented May 26, 2015

Hi @sdqali That seems like a good idea. I've asked our PM to prioritize a story in our backlog to get this feature done. We'd also be happy to merge in any pull request with this functionality!

Thanks!

Zak, CF Runtime Team

@sdqali
Copy link
Author

sdqali commented May 27, 2015

Thank you, @zaksoup. I have a patch I will push shortly.

@sdqali
Copy link
Author

sdqali commented May 27, 2015

Just noticed that I need to sign the CLA. Running it past my employer. :-)

@jpalermo
Copy link
Member

jpalermo commented Jun 4, 2015

Hi @sdqali,
Another option is to keep comments in your manifest.

This is still valid yaml, so it shouldn't cause any problems.

[
  # Allow database connection to PostgreSQL at hosted-postgres-service.com
  {"protocol":"tcp","destination":"<ip-foo>","ports":"3306"},
  # Allow logging to hosted-logging-service.com
  {"protocol":"tcp","destination":"<some-ip>-<another-ip>","ports":"55882"},
  # Allow monitoring service at hosted-monitoring-service.com
  {"protocol":"tcp","destination":"<ip-bar>","ports":"443"}
]

@zaksoup
Copy link
Contributor

zaksoup commented Jun 26, 2015

Hi @sdqali
Just wanted to follow up about the patch you said you might have?

@utako
Copy link
Contributor

utako commented Jul 23, 2015

Hi @sdqali,

We have added this as a story. Let us know if you have that patch and submit a PR; otherwise, the feature will be completed eventually.

Thanks for bringing this to our attention,
@Quintaminant and @utako, CF CAPI Team

@utako utako closed this as completed Jul 23, 2015
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

No branches or pull requests

5 participants