unioslo / mreg

GNU General Public License v3.0
7 stars 13 forks source link

Inconsistent use of Location headers upon creating entries. #528

Open terjekv opened 3 months ago

terjekv commented 3 months ago

When creating a Host via post to the hosts endpoint, we get a Location header telling us were the new host is, which is done manually and repeatedly: https://github.com/unioslo/mreg/blob/d45d25756c5476efa3c8051c77519b7f20687e90/mreg/api/v1/views.py#L349-L408

Now, we have a MregListCreateAPIView implements Location for create...

But for most list views, this class is not inherited. As one of many examples, see IPaddressList which inherits from HostPermissionsListCreateAPIView which inherits from HostLogMixin and MregPermissionsListCreateAPIView... But none of these classes implement Location headers for post. So, using post on the ipaddress endpoint does not return a location header for the newly created entry...

Now, there may be a reason for this. We have models that look like this:

class Ipaddress(BaseModel):
    host = models.ForeignKey(
        Host, on_delete=models.CASCADE, db_column="host", related_name="ipaddresses"
    )
    ipaddress = models.GenericIPAddressField()
    macaddress = models.CharField(
        max_length=17, blank=True, validators=[validate_mac_address]
    )

    class Meta:
        db_table = "ipaddress"
        unique_together = (("host", "ipaddress"),)

So how would a location header look like? Sadly, the best we can do is /api/v1/ipaddresses/?host=<host>&ipaddress=<ipaddress>.

This is 1) inconsistent, 2) very annoying when trying to write automation, 3) missing a unique ID field that would resolve all this.

Addendum: There is an autogenerated id / pk field. We just don't return it in post. :(