codeaffen / phpipam-ansible-modules

Ansible Modules to manage phpIPAM installations
https://codeaffen.org/projects/phpipam-ansible-modules
GNU General Public License v3.0
22 stars 10 forks source link

Trouble creating subnet with a vrf #93

Closed ishuguru closed 1 year ago

ishuguru commented 1 year ago

Describe the bug Cant assign a subnet with a custom vrf

To Reproduce Steps to reproduce the behavior:

  - name: "Create subnet"
    codeaffen.phpipam.subnet:
       app_id: "ansible"
       username: "ansible"
       password: "{{password}}"
       server_url: "https://ipam.internal"
       cidr: "192.168.1.0/27"
       parent: "192.168.1.0/24"
       vlan: "1337"
       vrf: FW-Linknets
       location: "DC1"
       validate_certs: no
       section: "Linknets"
       routing_domain: DC1

Expected behavior A clear and concise description of what you expected to happen. Have the subnet created in the correct subnets

Versions:

Additional context I see this error:

       }
    },
    "msg": "Can not resolve 'vrf' to an existing ID"
}
cmeissner commented 1 year ago

Hi @ishuguru,

Thank you for bringing this to our attention. Can you confirm that the vrf you want to assign to that subnet already exists.

ishuguru commented 1 year ago

Yeah, even setting to "None" gives the same error. Also tried other vrf:s Also tried the "id" i get when browsing a vrf (for example 126 https://ipam.internal/tools/vrf/126/) same error.

cmeissner commented 1 year ago

Ok, I was able to reproduce the issue. I will start to work on it asap. Currently there is no way workaround it. I need to have a look into the resolving logic for vrf's. Please stay tuned.

cmeissner commented 1 year ago

I investigate the issue a bit. Currently I try to understand the phpipam behavior. My test environment is configured as followed:

  1. phpipam 1.5.0 installation
  2. configured 2 sections
  3. created 4 vrfs with different sections assigned a. my vrf, section 1 & 3 b. my vrf, sections 3 c. my vrf, sections 1 c. my vrf, sections null (all sections)

I'm not very sure if this is a real world scenario but it is possible to configure vrfs with same name with different variants of sections. This made it very difficult to search for the correct vrf.

$ IPAM_AUTH="phpipam-token: $(curl -ks -XPOST --user admin https://localhost:8443/api/ansible/user/ | jq -r '.data.token')"
$ curl -ks -XGET -H $IPAM_AUTH https://localhost:8443/api/ansible/vrfs/\?filter_by\=sections\&filter_value\=1\&filter_match\=partial | jq ".data"
[
  {
    "id": "1",
    "name": "my vrf",
    "rd": null,
    "description": null,
    "sections": "1;3",
    "editDate": "2023-01-11 09:41:25",
    "customer_id": null
  },
  {
    "id": "6",
    "name": "my vrf",
    "rd": null,
    "description": null,
    "sections": "1",
    "editDate": null,
    "customer_id": null
  }
]

May be you understand the problem. A subnet belongs to a single section and currently I'm not sure how to differentiate the correct vrf.

ishuguru commented 1 year ago

Yes that was messy, i guess a superugly workaround would be to have to refer to the id not the name?

cmeissner commented 1 year ago

Yes that was messy, i guess a superugly workaround would be to have to refer to the id not the name?

Hm, this would break the way we work with entities in our collection. First of all we need to clarify if the behavior of phpipam is the way vrf's should be handled. As far as I'm concerned this phpipam behaves a bit wrong here. Adding my vrf with only section 1 should lead to an error if a equal named vrf in section 1 and 3 already exists.

cmeissner commented 1 year ago

I opened https://github.com/phpipam/phpipam/issues/3727 to get hopefully the situation clarified.

cmeissner commented 1 year ago

@ishuguru can you please test the attached fix with your environment. The test case created to reproduce the described issue runs now without errors. make test-all also result in a clean state. So from my point of view the issue should be fixed with this implementation.

ishuguru commented 1 year ago

@ishuguru can you please test the attached fix with your environment. The test case created to reproduce the described issue runs now without errors. make test-all also result in a clean state. So from my point of view the issue should be fixed with this implementation.

Yeah seems to work! 👍

cmeissner commented 1 year ago

Yeah seems to work! +1

It would be appreciated if you also leave a comment in the attached pull request.