Dear Boshers,

we currently encounter a problem which has been discussed briefly before on the list [1]: 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 director
* 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.

Concept proposal
* 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 feasible.

Implementation proposal
* 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 [2], 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 [3]. It has no idea about the resource pool of that vm. The CPI is called by the director’s network_updater [4] which gets initialized for a specific instance and is called by the instance_updater [5].
* The instance entity combines all there is to know in terms of configuration for a specific vm (e.g. network settings [6]), so this could be the point to include the new feature

So, what could be changed now?
* introduce a new method security_groups on bosh-director/lib/bosh/director/deployment_plan/instance.rb called 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.

Warm regards
Marco

[1] https://groups.google.com/a/cloudfoundry.org/forum/#!topic/bosh-users/LJ2Kym6QCak
[2] https://github.com/cloudfoundry-incubator/bosh-openstack-cpi-release/blob/master/src/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb#L226
[3] https://github.com/cloudfoundry-incubator/bosh-openstack-cpi-release/blob/master/src/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb#L397
[4] https://github.com/cloudfoundry/bosh/blob/master/bosh-director/lib/bosh/director/instance_updater/network_updater.rb#L28
[5] https://github.com/cloudfoundry/bosh/blob/master/bosh-director/lib/bosh/director/instance_updater.rb#L283-L286
[6] https://github.com/cloudfoundry/bosh/blob/master/bosh-director/lib/bosh/director/deployment_plan/instance.rb#L186-L222

Would look forward to seeing this go through, it relates to a previous
thread from early this year -
https://groups.google.com/a/cloudfoundry.org/forum/#!topic/bosh-users/LJ2Kym6QCak.
It fell off my radar and have not had time to revisit.

It's a bit unclear to me if you are proposing to security groups feature in
the Director or in the OpenStack CPI specifically.

If in the OpenStack CPI, then I think it makes sense to pull in security
groups config into resource pool's cloud_properties section. For example:

resource_pools:
- name: my-fancy-web
stemcell: { ... }
cloud_properties:
instance_type: m3.xlarge
security_groups: [web]

- name: my-fancy-worker
stemcell: { ... }
cloud_properties:
instance_type: m3.xlarge
security_groups: [worker]

will assign web to VMs of type my-fancy-web and worker to VMs of type
my-fancy-worker. When resource pool's cloud_properties are changed, VMs
will be recreated with new config. Like you point out there is a case of
configure_networks, imho we can just raise an error in create_vm if both
networks' cloud_properties specifies security groups and resource pool cp's
specifies security_groups.

If you are proposing changes in the Director, then I think it gets a bit
more complicated. I'm not sure we have enough good usage patterns to figure
out a good abstraction *yet* (e.g. Azure has two different types of
security groups: network and vm, which imho makes a lot of sense). Also if
it's becoming a first class feature, may be BOSH at this point should be
creating security groups automatically with certain rules and may be even
with links generating these rules dynamically, etc.

Btw we are currently implementing first class support for AZs + links in
the Director, so all of the bosh-director core classes are going through
significant changes on global-net branch. Related to that I am actually
thinking we split up resource pool config into two pieces: vm type and
stemcell. With recent cloud config changes it would make it more sense to
keep stemcell os/version in the deployment manifest (just like releases)
and vm type with iaas specifics in the cloud config file.

On Wed, Sep 9, 2015 at 1:38 PM, john mcteague <john.mcteague(a)gmail.com>
wrote:

Would look forward to seeing this go through, it relates to a previous
thread from early this year -
https://groups.google.com/a/cloudfoundry.org/forum/#!topic/bosh-users/LJ2Kym6QCak.
It fell off my radar and have not had time to revisit.

On Wed, Sep 9, 2015 at 9:46 AM, Voelz, Marco <marco.voelz(a)sap.com> wrote:

Dear Boshers,

we currently encounter a problem which has been discussed briefly before
on the list [1]: 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
director
* 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.

Concept proposal
* 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
feasible.

Implementation proposal
* 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 [2], 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 [3]. It has no idea about the resource pool of that vm. The CPI is
called by the director’s network_updater [4] which gets initialized for a
specific instance and is called by the instance_updater [5].
* The instance entity combines all there is to know in terms of
configuration for a specific vm (e.g. network settings [6]), so this could
be the point to include the new feature

So, what could be changed now?
* introduce a new method security_groups on
bosh-director/lib/bosh/director/deployment_plan/instance.rb called
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.

Warm regards
Marco

[1]
https://groups.google.com/a/cloudfoundry.org/forum/#!topic/bosh-users/LJ2Kym6QCak
[2]
https://github.com/cloudfoundry-incubator/bosh-openstack-cpi-release/blob/master/src/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb#L226
[3]
https://github.com/cloudfoundry-incubator/bosh-openstack-cpi-release/blob/master/src/bosh_openstack_cpi/lib/cloud/openstack/cloud.rb#L397
[4]
https://github.com/cloudfoundry/bosh/blob/master/bosh-director/lib/bosh/director/instance_updater/network_updater.rb#L28
[5]
https://github.com/cloudfoundry/bosh/blob/master/bosh-director/lib/bosh/director/instance_updater.rb#L283-L286
[6]
https://github.com/cloudfoundry/bosh/blob/master/bosh-director/lib/bosh/director/deployment_plan/instance.rb#L186-L222

Dear Dmitriy,

thanks for the quick reply. See my response inline.