Fix some index-references and warn about remaining (#55727)

The ACI MultiSite PATCH API has a deficiency requiring some objects to be referenced by index. This can cause silent corruption on concurrent access when changing/removing on object as the wrong object may be referenced. This PR removes some references by index, and documents this issue for others.
2019-05-07 15:00:04 +02:00 · 2019-05-07 15:00:04 +02:00 · beca3661a4
commit beca3661a4
parent 2007a79952
11 changed files with 39 additions and 12 deletions
--- a/lib/ansible/modules/network/aci/mso_schema_site_anp.py
+++ b/lib/ansible/modules/network/aci/mso_schema_site_anp.py
@ -166,8 +166,7 @@ def main():

    if anp is not None and anp_ref in anps:
        anp_idx = anps.index(anp_ref)
-        # FIXME: Changes based on index are DANGEROUS
-        anp_path = '/sites/{0}/anps/{1}'.format(site_template, anp_idx)
+        anp_path = '/sites/{0}/anps/{1}'.format(site_template, anp)
        mso.existing = schema_obj['sites'][site_idx]['anps'][anp_idx]

    if state == 'query':
--- a/lib/ansible/modules/network/aci/mso_schema_site_anp_epg.py
+++ b/lib/ansible/modules/network/aci/mso_schema_site_anp_epg.py
@ -181,8 +181,7 @@ def main():
    epgs = [e['epgRef'] for e in schema_obj['sites'][site_idx]['anps'][anp_idx]['epgs']]
    if epg is not None and epg_ref in epgs:
        epg_idx = epgs.index(epg_ref)
-        # FIXME: Changes based on index are DANGEROUS
-        epg_path = '/sites/{0}/anps/{1}/epgs/{2}'.format(site_template, anp_idx, epg_idx)
+        epg_path = '/sites/{0}/anps/{1}/epgs/{2}'.format(site_template, anp, epg)
        mso.existing = schema_obj['sites'][site_idx]['anps'][anp_idx]['epgs'][epg_idx]

    if state == 'query':
@ -192,7 +191,7 @@ def main():
            mso.fail_json(msg="EPG '{epg}' not found".format(epg=epg))
        mso.exit_json()

-    epgs_path = '/sites/{0}/anps/{1}/epgs'.format(site_template, anp_idx)
+    epgs_path = '/sites/{0}/anps/{1}/epgs'.format(site_template, anp)
    ops = []

    mso.previous = mso.existing
--- a/lib/ansible/modules/network/aci/mso_schema_site_anp_epg_staticleaf.py
+++ b/lib/ansible/modules/network/aci/mso_schema_site_anp_epg_staticleaf.py
@ -64,6 +64,10 @@ options:
    type: str
    choices: [ absent, present, query ]
    default: present
+notes:
+- The ACI MultiSite PATCH API has a deficiency requiring some objects to be referenced by index.
+  This can cause silent corruption on concurrent access when changing/removing on object as
+  the wrong object may be referenced. This module is affected by this deficiency.
 seealso:
 - module: mso_schema_site_anp_epg
 - module: mso_schema_template_anp_epg
@ -223,7 +227,7 @@ def main():
            mso.fail_json(msg="Static leaf '{leaf}/{vlan}' not found".format(leaf=leaf, vlan=vlan))
        mso.exit_json()

-    leafs_path = '/sites/{0}/anps/{1}/epgs/{2}/staticLeafs'.format(site_template, anp_idx, epg_idx)
+    leafs_path = '/sites/{0}/anps/{1}/epgs/{2}/staticLeafs'.format(site_template, anp, epg)
    ops = []

    mso.previous = mso.existing
--- a/lib/ansible/modules/network/aci/mso_schema_site_anp_epg_staticport.py
+++ b/lib/ansible/modules/network/aci/mso_schema_site_anp_epg_staticport.py
@ -88,6 +88,10 @@ options:
    type: str
    choices: [ absent, present, query ]
    default: present
+notes:
+- The ACI MultiSite PATCH API has a deficiency requiring some objects to be referenced by index.
+  This can cause silent corruption on concurrent access when changing/removing on object as
+  the wrong object may be referenced. This module is affected by this deficiency.
 seealso:
 - module: mso_schema_site_anp_epg
 - module: mso_schema_template_anp_epg
@ -266,7 +270,7 @@ def main():
            mso.fail_json(msg="Static port '{portpath}' not found".format(portpath=portpath))
        mso.exit_json()

-    ports_path = '/sites/{0}/anps/{1}/epgs/{2}/staticPorts'.format(site_template, anp_idx, epg_idx)
+    ports_path = '/sites/{0}/anps/{1}/epgs/{2}/staticPorts'.format(site_template, anp, epg)
    ops = []

    mso.previous = mso.existing
--- a/lib/ansible/modules/network/aci/mso_schema_site_anp_epg_subnet.py
+++ b/lib/ansible/modules/network/aci/mso_schema_site_anp_epg_subnet.py
@ -74,6 +74,10 @@ options:
    type: str
    choices: [ absent, present, query ]
    default: present
+notes:
+- The ACI MultiSite PATCH API has a deficiency requiring some objects to be referenced by index.
+  This can cause silent corruption on concurrent access when changing/removing on object as
+  the wrong object may be referenced. This module is affected by this deficiency.
 seealso:
 - module: mso_schema_site_anp_epg
 - module: mso_schema_template_anp_epg_subnet
@ -230,7 +234,7 @@ def main():
            mso.fail_json(msg="Subnet '{subnet}' not found".format(subnet=subnet))
        mso.exit_json()

-    subnets_path = '/sites/{0}/anps/{1}/epgs/{2}/subnets'.format(site_template, anp_idx, epg_idx)
+    subnets_path = '/sites/{0}/anps/{1}/epgs/{2}/subnets'.format(site_template, anp, epg)
    ops = []

    mso.previous = mso.existing
--- a/lib/ansible/modules/network/aci/mso_schema_site_bd_l3out.py
+++ b/lib/ansible/modules/network/aci/mso_schema_site_bd_l3out.py
@ -52,6 +52,10 @@ options:
    type: str
    choices: [ absent, present, query ]
    default: present
+notes:
+- The ACI MultiSite PATCH API has a deficiency requiring some objects to be referenced by index.
+  This can cause silent corruption on concurrent access when changing/removing on object as
+  the wrong object may be referenced. This module is affected by this deficiency.
 seealso:
 - module: mso_schema_site_bd
 - module: mso_schema_template_bd
@ -181,6 +185,7 @@ def main():
    l3outs = schema_obj['sites'][site_idx]['bds'][bd_idx]['l3Outs']
    if l3out is not None and l3out in l3outs:
        l3out_idx = l3outs.index(l3out)
+        # FIXME: Changes based on index are DANGEROUS
        l3out_path = '/sites/{0}/bds/{1}/l3Outs/{2}'.format(site_template, bd, l3out_idx)
        mso.existing = schema_obj['sites'][site_idx]['bds'][bd_idx]['l3Outs'][l3out_idx]

--- a/lib/ansible/modules/network/aci/mso_schema_site_bd_subnet.py
+++ b/lib/ansible/modules/network/aci/mso_schema_site_bd_subnet.py
@ -71,6 +71,10 @@ options:
    type: str
    choices: [ absent, present, query ]
    default: present
+notes:
+- The ACI MultiSite PATCH API has a deficiency requiring some objects to be referenced by index.
+  This can cause silent corruption on concurrent access when changing/removing on object as
+  the wrong object may be referenced. This module is affected by this deficiency.
 seealso:
 - module: mso_schema_site_bd
 - module: mso_schema_template_bd
--- a/lib/ansible/modules/network/aci/mso_schema_site_vrf_region_cidr.py
+++ b/lib/ansible/modules/network/aci/mso_schema_site_vrf_region_cidr.py
@ -60,6 +60,10 @@ options:
    type: str
    choices: [ absent, present, query ]
    default: present
+notes:
+- The ACI MultiSite PATCH API has a deficiency requiring some objects to be referenced by index.
+  This can cause silent corruption on concurrent access when changing/removing on object as
+  the wrong object may be referenced. This module is affected by this deficiency.
 seealso:
 - module: mso_schema_site_vrf_region
 - module: mso_schema_site_vrf_region_cidr_subnet
@ -204,6 +208,7 @@ def main():
    cidrs = [c['ip'] for c in schema_obj['sites'][site_idx]['vrfs'][vrf_idx]['regions'][region_idx]['cidrs']]
    if cidr is not None and cidr in cidrs:
        cidr_idx = cidrs.index(cidr)
+        # FIXME: Changes based on index are DANGEROUS
        cidr_path = '/sites/{0}/vrfs/{1}/regions/{2}/cidrs/{3}'.format(site_template, vrf, region, cidr_idx)
        mso.existing = schema_obj['sites'][site_idx]['vrfs'][vrf_idx]['regions'][region_idx]['cidrs'][cidr_idx]

--- a/lib/ansible/modules/network/aci/mso_schema_site_vrf_region_cidr_subnet.py
+++ b/lib/ansible/modules/network/aci/mso_schema_site_vrf_region_cidr_subnet.py
@ -65,6 +65,10 @@ options:
    type: str
    choices: [ absent, present, query ]
    default: present
+notes:
+- The ACI MultiSite PATCH API has a deficiency requiring some objects to be referenced by index.
+  This can cause silent corruption on concurrent access when changing/removing on object as
+  the wrong object may be referenced. This module is affected by this deficiency.
 seealso:
 - module: mso_schema_site_vrf_region_cidr
 - module: mso_schema_template_vrf
@ -221,6 +225,7 @@ def main():
    subnets = [s['ip'] for s in schema_obj['sites'][site_idx]['vrfs'][vrf_idx]['regions'][region_idx]['cidrs'][cidr_idx]['subnets']]
    if subnet is not None and subnet in subnets:
        subnet_idx = subnets.index(subnet)
+        # FIXME: Changes based on index are DANGEROUS
        subnet_path = '/sites/{0}/vrfs/{1}/regions/{2}/cidrs/{3}/subnets/{4}'.format(site_template, vrf, region, cidr_idx, subnet_idx)
        mso.existing = schema_obj['sites'][site_idx]['vrfs'][vrf_idx]['regions'][region_idx]['cidrs'][cidr_idx]['subnets'][subnet_idx]

--- a/lib/ansible/modules/network/aci/mso_schema_template_anp_epg_contract.py
+++ b/lib/ansible/modules/network/aci/mso_schema_template_anp_epg_contract.py
@ -214,8 +214,7 @@ def main():
        contract_ref = mso.contract_ref(**contract)
        if (contract_ref, contract['type']) in contracts:
            contract_idx = contracts.index((contract_ref, contract['type']))
-            # FIXME: Changes based on index are DANGEROUS
-            contract_path = '/templates/{0}/anps/{1}/epgs/{2}/contractRelationships/{3}'.format(template, anp, epg, contract_idx)
+            contract_path = '/templates/{0}/anps/{1}/epgs/{2}/contractRelationships/{3}'.format(template, anp, epg, contract)
            mso.existing = schema_obj['templates'][template_idx]['anps'][anp_idx]['epgs'][epg_idx]['contractRelationships'][contract_idx]

    if state == 'query':
--- a/lib/ansible/modules/network/aci/mso_schema_template_contract_filter.py
+++ b/lib/ansible/modules/network/aci/mso_schema_template_contract_filter.py
@ -244,8 +244,7 @@ def main():
        filter_ref = mso.filter_ref(schema_id=filter_schema_id, template=filter_template, filter=filter_name)
        if filter_ref in filters:
            filter_idx = filters.index(filter_ref)
-            # FIXME: Changes based on index are DANGEROUS
-            filter_path = '/templates/{0}/contracts/{1}/{2}/{3}'.format(template, contract, filter_key, filter_idx)
+            filter_path = '/templates/{0}/contracts/{1}/{2}/{3}'.format(template, contract, filter_key, filter_name)
            mso.existing = schema_obj['templates'][template_idx]['contracts'][contract_idx][filter_key][filter_idx]

    if state == 'query':