[BUG] boto_vpc.route_table_present - not idempotent

Description

The subnet_names and subnet_ids parameters of the salt.states.boto_vpc.route_table_present method overwrite the existing subnet associations of the route table.

That means that if you have a state file that..

1. creates vpc
2. creates internet gateway
3. creates route table
4. creates subnets (and specify the route table)

This will create all of the objects just fine. However, if you run it again, the third step will remove all subnet associations from the route table, and then the fourth step will add them back.

This creates a hard dependency on the order in which the objects are created. You MUST create the subnets first and then, and only then, can you create the route table. Also, the route table state file must be updated to include the names/ids of any subnets that you try to create.

This also renders the "route_table_name" parameter of "subnet_present" useless.

It also makes it impossible to encapsulate the networking and application objects in a single set of state files. The creation of your route table + subnets MUST be a monolithic state file, which limits its usefulness.

I've put this as a bug, rather than an enhancement mostly because this behavior makes subnet_present::route_table_name useless if you're enforcing state with route_table_present. Otherwise, I would probably have raised this as an enhancement.

Setup

cat <<EOF > /srv/salt/issue-setup.sls
{% set profile = salt['pillar.get']('aws') %}
{% set prefix = salt['pillar.get']('aws:resource_prefix', 'sedemo_') %}
{% set region = 'eu-west-1' %}

{{prefix}}vpc:
  boto_vpc.present:
    - name: {{ prefix }}vpc
    - region: {{ region }}
    - cidr_block: 172.16.0.0/16
    - instance_tenancy: default
    - dns_hostnames: False
    - tags:
        App:  SaltStack
        Name: {{ prefix }}vpc

{{prefix}}ig:
  boto_vpc.internet_gateway_present:
    - name: {{ prefix }}ig
    - vpc_name: {{ prefix }}vpc
    - region: {{ region }}
    - tags:
        Name: {{ prefix }}ig
    - require:
        - {{prefix}}vpc

{{prefix}}rt:
  boto_vpc.route_table_present:
    - name: {{ prefix }}rt
    - region: {{ region }}
    - vpc_name: {{ prefix }}vpc
    - routes:
        - destination_cidr_block: 0.0.0.0/0
          internet_gateway_name: {{ prefix }}ig
    - tags:
        Name: {{ prefix }}rt
    - require:
        - {{prefix}}vpc
        - {{prefix}}ig

{% for net in ['128', '129']  %}
{{prefix}}net_{{net}}:
  boto_vpc.subnet_present:
    - name: {{ prefix }}net_{{ net }}
    - region: {{ region }}
    - cidr_block: 172.16.{{net}}.0/24
    - vpc_name: {{prefix}}vpc
    - availability_zone: eu-west-1b
    - tags:
        Name: {{ prefix }}net_{{ net }}
    - route_table_name: {{ prefix }}rt
    - require:
        - {{prefix}}vpc
        - {{prefix}}rt
{% endfor %}
EOF

Steps to Reproduce the behavior

Run the following twice:

salt-call state.apply issue-setup

On the second run, you will see this...

############## SNIP  ##############
----------
          ID: dkf_rt
    Function: boto_vpc.route_table_present
      Result: True
     Comment: Route table dkf_rt (rtb-0009d30188aebb16b) present.
     Started: 21:49:47.278640
    Duration: 3067.967 ms
     Changes:
              ----------
              new:
              old:
                  - subnet-0bc4a6abaa06b7d04
                  - subnet-0f3e70b41c898fc97

Notice how the subnet associations have been deleted. This is going to cause a very brief network outage for all subnets in this routing table

----------
          ID: dkf_net_128
    Function: boto_vpc.subnet_present
      Result: True
     Comment: Subnet present. Subnet successfully associated with route table dkf_rt.
     Started: 21:49:50.347023
    Duration: 1579.11 ms
     Changes:
              ----------
              new:
                  ----------
                  subnet:
                      ----------
                      availability_zone:
                          eu-west-1b
                      cidr_block:
                          172.16.128.0/24
                      explicit_route_table_association_id:
                          rtbassoc-0156e973854a669ca
                      id:
                          subnet-0bc4a6abaa06b7d04
                      tags:
                          ----------
                          Name:
                              dkf_net_128
                      vpc_id:
                          vpc-0f7082f722d991fd5
              old:
                  ----------
                  subnet:
                      ----------
                      availability_zone:
                          eu-west-1b
                      cidr_block:
                          172.16.128.0/24
                      id:
                          subnet-0bc4a6abaa06b7d04
                      tags:
                          ----------
                          Name:
                              dkf_net_128
                      vpc_id:
                          vpc-0f7082f722d991fd5
----------
          ID: dkf_net_129
    Function: boto_vpc.subnet_present
      Result: True
     Comment: Subnet present. Subnet successfully associated with route table dkf_rt.
     Started: 21:49:51.926422
    Duration: 1662.893 ms
     Changes:
              ----------
              new:
                  ----------
                  subnet:
                      ----------
                      availability_zone:
                          eu-west-1b
                      cidr_block:
                          172.16.129.0/24
                      explicit_route_table_association_id:
                          rtbassoc-09cf473f15103bfe5
                      id:
                          subnet-0f3e70b41c898fc97
                      tags:
                          ----------
                          Name:
                              dkf_net_129
                      vpc_id:
                          vpc-0f7082f722d991fd5
              old:
                  ----------
                  subnet:
                      ----------
                      availability_zone:
                          eu-west-1b
                      cidr_block:
                          172.16.129.0/24
                      id:
                          subnet-0f3e70b41c898fc97
                      tags:
                          ----------
                          Name:
                              dkf_net_129
                      vpc_id:
                          vpc-0f7082f722d991fd5

And now all of the subnets have been re-associated with the route table.

Expected behavior

I would not expect the second execution of the state file to generate any changes.

Suggested solutions:

1. the route_table_present::subnet_* params to be additive, rather than absolute, and then add an "overwrite_subnets={True, FALSE}" (and update the documentation to reflect the behavior change)
2. Add additional parameters (like salt.states.group does for group membership)

My personal preference is the first option rather than the second because it seems to be more intuitive to me - the addition of the "overwrite_subnets" makes the behavior explicit. Where as the second option creates bloat (IMHO).

There are probably other options that I've not thought about but those two are what immediately came to mind.

Versions Report

NOTE: I have applied https://github.com/saltstack/salt/commit/32ed7d05f0af1f07a0144cf5594370a5d65e70b3 to be able to create the routing tables in my sample script.

Salt Version:
          Salt: 3002.5

Dependency Versions:
          cffi: 1.14.5
      cherrypy: unknown
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: 0.6.4
     gitpython: 1.0.1
        Jinja2: 2.11.1
       libgit2: Not Installed
      M2Crypto: 0.35.2
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.20
      pycrypto: Not Installed
  pycryptodome: 3.10.1
        pygit2: Not Installed
        Python: 3.6.8 (default, Nov 16 2020, 16:55:22)
  python-gnupg: Not Installed
        PyYAML: 3.13
         PyZMQ: 17.0.0
         smmap: 0.9.0
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.1.4

System Versions:
          dist: centos 7 Core
        locale: UTF-8
       machine: x86_64
       release: 3.10.0-957.1.3.el7.x86_64
        system: Linux
       version: CentOS Linux 7 Core

Start with the issue-setup.sls example and run salt-call state.apply issue-setup twice to reproduce the route table association changes. Inspect the salt.states.boto_vpc.route_table_present and subnet_present entry points, then add focused coverage for repeated application. Done means the second run reports no changes and preserves existing subnet associations.