Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alias Parameters #145

Closed
JeGr opened this issue Jul 20, 2021 · 2 comments · Fixed by #150
Closed

Alias Parameters #145

JeGr opened this issue Jul 20, 2021 · 2 comments · Fixed by #150
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation

Comments

@JeGr
Copy link

JeGr commented Jul 20, 2021

Hi!

First thing: great work!

I was trying your API out and found Alias description/handling a bit strange. Creation lists name, type, descr, address & detail. So far, so nice. Read operation lists an alias normally, e.g.

    "status": "ok",
    "code": 200,
    "return": 0,
    "message": "Success",
    "data": {
        "17": {
            "name": "test",
            "type": "host",
            "address": "1.2.3.4",
            "descr": "",
            "detail": "Entry added Tue, 11 Feb 2021 12:43:47 +0100"
        }
    }
}

First thing that's irritating: the 17. In our case, that's the ID of the alias, easily viewable via the UI as GET parameter of the browser. Why isn't the ID listed besides name, type etc?

But as I was checking, how one would update an alias, I found:

{
    "id": "RFC1918",
    "name": "UPDATED_RFC1918",
    "type": "network",
    "descr": "Example description",
    "address": [
        "192.168.0.0/16",
        "172.16.0.0/12",
        "10.0.0.0/8"
    ],
    "detail": [
        "Class A",
        "Class B",
        "Class C"
    ]
}

Here, ID seems to be the name whereas name is also a name? So where did the real ID (numerical) go? And what is that ID?
As the pfSense XML format doesn't list an ID (it's just used in the webUI and the order of the entries in XML format) we only have name, type, address, descr and details. There is no ID, so I'm currently lost as to what you mean by that example in your documentation :)

Cheers
Jens

@JeGr
Copy link
Author

JeGr commented Jul 21, 2021

Found it out via try and error: ID in the update case is the "name" and "name" is the updated name.

I'd propose to perhaps rename those to "name" and "altname" or "newname" and describe it as optional. ID is too easy to confuse with the original numerical id of the data entry. And if you just want to change the address or details, it's not really intuitive to do a GET with "name" and the PUT etc. with an ID that wasn't declared before. And we should add it to the documentation :)

@jaredhendrickson13
Copy link
Owner

Hey!

Thanks for the input! For most endpoints, id strictly refers to the unique identifier for that particular object. Usually this is a numeric ID but in the case of aliases the name of the alias is also required to be unique by pfSense. It was used as the id for the object since it's usually easier to find than the object's pfSense ID, but I do see how that causes confusing. Keeping the name field strictly for interacting with the object's name is ideally since that will remain consistent across alias creations and updates. It may be possible to allow the object to be identified by both it's name or the numeric ID, I will have to play around with that and see if it causes any conflicts.

Looking at the documentation, the field descriptions for the PUT method are missing. These usually explain the use of the ID field which is likely why this nuance is so confusing. I'll keep this issue open as a bug/documentation error so it can be fixed in the next release.

Thanks again!

@jaredhendrickson13 jaredhendrickson13 self-assigned this Jul 23, 2021
@jaredhendrickson13 jaredhendrickson13 added bug Something isn't working documentation Improvements or additions to documentation labels Jul 23, 2021
@jaredhendrickson13 jaredhendrickson13 linked a pull request Jul 30, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants