Add support for comma-delimited destinations in ASGs #3644
Conversation
validator perspective
- the security_group_rule_validator_spec should now cover most of the
validator logic
- begins adding the feature flag for this config
- unfortunately the tests do not like the current implementation,
hence the comments for the alternative implementation.
[#186770494](https://www.pivotaltracker.com/story/show/186770494)
Co-authored-by: Brandon Roberson <[email protected]>
…-in-asgs-186770494
- Refactored behavior from RulesValidator into RuleValidator and then
call those shared methods in RulesValidator
- Also use class functions from ICMPRuleValidator and
TransportRuleValidator
- Comma-delimited destinations are evaluated using recursion instead of
iteration
[#186770494](https://www.pivotaltracker.com/story/show/186770494)
So that we can easily see the stack when using the `pry` debugger.
```diff - message_validators: - security_groups: - enable_comma_delimited_ips: true + security_groups: + enable_comma_delimited_destinations: false ```
comma-delimited lists
This reverts commit 5129057.
…-in-asgs-186770494
|
We probably want to clarify in the docs that destinations can be a comma delimited list in our docs: https://v3-apidocs.cloudfoundry.org/version/3.159.0/index.html#the-security-group-object (if enabled) |
|
|
||
| def port_in_range(port) | ||
| port > 0 && port < 65_536 | ||
| CloudController::TransportRuleValidator.validate_port(ports) |
- `range` is spelled with an e 😅 [#186770494](https://www.pivotaltracker.com/story/show/186770494)
From the feedback in this comment: - https://github.com/cloudfoundry/cloud_controller_ng/pull/3644/files/cedb9e7f214d52395bd9dafc9d6df88ffc7bfbc4#r1513673389 I'm changing the implementation for RulesValidator from recursion (which is present in the RuleValidator), to an implementation using iteration. Hopefully this is easier to read. [#186770494](https://www.pivotaltracker.com/story/show/186770494)
|
Hi @sethboyles |
|
I'm curious if you consider having destinations as an [ ] array as an alternative to a comma-separated string. |
|
@a-b it's worth noting that CCNG already accepts ports in ASG definitions as a comma-separated list: https://v3-apidocs.cloudfoundry.org/version/3.159.0/index.html#the-security-group-object, so to do so with IP addresses is at least consistent with that |
|
@jrussett unfortunately this has gotten conflicts now so I can't merge directly. Sorry that this got delayed so long that this happened. I can attempt to resolve it myself when I have time, but I think the commits should also be squashed/combined into one. Would you like to resolve this or should I give it a shot? |
|
Oops the conflicts were only on the 'rebase' option, seems like I can squash & merge no problem |
A short explanation of the proposed change:
Allow operators to configure Cloud Controller to support ASGs with comma-delimited destinations.
For example, the following ASG definition would be valid:
An explanation of the use cases your change solves
Currently, if a developer wants to open connections from their cf app to two non-consecutive IP addresses (e.g. 8.8.8.8 and 10.10.10.10), they'd have to create two separate ASGs. This creates duplication and performance implications on some large environments with many ASGs that are also running networking fabrics other than silk.
One way to reduce said duplication is to enable comma-delimited destinations, so that the firewall rules are consolidated.
Links
Add config for comma-delimited destinations in ASGs capi-release#386
Add support for comma-delimited destinations in ASGs #3644
Support comma-delimited IP adresses silk-release#107
#186770494
Checklist
I have reviewed the contributing guide
I have viewed, signed, and submitted the Contributor License Agreement
I have made this pull request to the
mainbranchI have run all the unit tests using
bundle exec rakeI have run CF Acceptance Tests