toggle quoted messageShow quoted text
On Wed, Sep 9, 2015 at 9:46 AM, Voelz, Marco <marco.voelz(a)sap.com> wrote:
we currently encounter a problem which has been discussed briefly before
on the list : Adding security groups in resource pools should be
possible. On a side note: we are dealing with openstack, so references
below might be openstack specific.
Here is a use-case: having machines in the same network, but with
different incoming/outgoing rules. Example: only the runners/DEAs of a CF
deployment should be able to access some service VMs. Currently, this means
that we have to have the same network configuration twice in our manifests,
the only difference being the set of security groups. I’d like to propose a
change to allow specifying them on resource_pool level and discuss some
implementation-specifics and impacts before writing code, so we are all on
the same page.
If we are introducing anything new, my assumption is that current behavior
of specifying security groups in networks should not break or change. If
you have a manifest specifying security groups, you probably expect it to
work also when a new feature is added.
Analysis of current state
* global default security groups can be specified when setting up your
* network security groups override those when specified for a deployment
* security groups are not a first-class concept. They are transported
through the entities known to bosh within the cloud_properties of a
network. Therefore, only methods dealing with the network entities obtained
from the manifest or director db actually know about them implicitly.
* introduce ability to specify security groups for resource pools
* keep current behavior:
** if there are global default security groups only, use them.
** if there are network security groups, use them instead of anything
else. Don’t care about global groups or the new resource pool groups
** if there are resource pool groups AND no network security groups, use
them. Don’t care about global groups.
** probably remove security groups on networks at some point in time with
a heads-up to everyone currently using it. I have no idea if this is
* create_vm and configure_networks of CPI seem to be the relevant calls
setting up the security groups: the former or creating a vm, the latter for
updating an existing one. Any changes done here would be CPI specific!
* adapting create_vm could be done straight forward: it is already using
the network_configurator to merge the security groups , and has access
to the resource_pool for the vm as well. We could simply add logic here to
take security groups within a resource pool into account.
* adapting configure_networks is more tricky: It gets a network spec and
compares the security groups in there with the ones currently present on a
vm . It has no idea about the resource pool of that vm. The CPI is
called by the director’s network_updater  which gets initialized for a
specific instance and is called by the instance_updater .
* The instance entity combines all there is to know in terms of
configuration for a specific vm (e.g. network settings ), so this could
be the point to include the new feature
So, what could be changed now?
* introduce a new method security_groups on
security_groups, providing information about security groups. If there are
secrurity groups on networks, return them, otherwise return security groups
defined on resource pools if there are any. Just like the desired behavior
we assumed above.
* adapt create_vm and configure_networks to accept security_groups as an
additional argument. Instead of having the CPIs extract security groups
from the network’s cloud_properties, take them from the argument and keep
the current logic of the methods.
What are your thoughts on this?
I would love to have the change isolated from the actual CPI coding, so we
don’t need to adapt all of them at the same time. However, this seems like
an API change might be in order, so I’m not sure on how to do it.
Given any form of agreement on how to proceed, we could provide a PR as a
further means for discussion. However, this change will impact the API, so
I wanted to get your feedback on this before actually implementing anything.