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

REST API docs: several issues which need clarification #2064

Open
4 of 7 tasks
jpmens opened this issue Jan 9, 2015 · 13 comments
Open
4 of 7 tasks

REST API docs: several issues which need clarification #2064

jpmens opened this issue Jan 9, 2015 · 13 comments

Comments

@jpmens
Copy link
Contributor

jpmens commented Jan 9, 2015

I noticed a few things related to documentation and/or code in the REST API. I think (hope) they all belong together:

Comments

  • What are name / type (record type?) for? If they are for an RR why domain_id only, and not record id?
  • What is account used for? Is there an association with domains.account?
  • Re 1. above. If I set type=DNSKEY type is shown correctly in table. If I set to ohoh, it contains TYPE0.
  • Re 1. name: if this should be a zone name, it isn't checked to actually exist.

Domains

  • When adding a new zone, master is empty string. Shouldn't that be NULL?
  • Clarify id/zone_id wording

Records

  • Adding a record with a lower-cased type (a instead of A) produces a very strange message:
{"error":"Record www.b1.net/TYPE0 192.168.1.42: Unknown record was stored incorrectly, need 3 fields, got 1: 192.168.1.42"}
@Habbie Habbie added the docs label Jan 12, 2015
@zeha
Copy link
Collaborator

zeha commented Jan 16, 2015

The record (type='a') error should probably get the same handling as for comments on type 'ohoh', i.e. both should fail with "unknown type" instead.

@zeha
Copy link
Collaborator

zeha commented Jan 16, 2015

I'm not sure what you're saying about name/type for comments. comments are always bound to an RRset, and as such, they inherit the RRsets name and type.

account for comments needs explicit documentation; short story: it's a freeform text field with no relation to anything else.

@jpmens
Copy link
Contributor Author

jpmens commented Jan 16, 2015

Be that as it may, it needs documentation / clarification.
Inherit? IIRC I had to explicitly set name/type.

@zeha
Copy link
Collaborator

zeha commented Jan 16, 2015

FTR, I agree on 'needs better docs' :)

@jpmens
Copy link
Contributor Author

jpmens commented Jan 18, 2015

It appears the sentence Changes made through the Zones API will always yield valid zone data, and the zone will be properly "rectified". in api spec is incorrect.

@zeha
Copy link
Collaborator

zeha commented Jan 18, 2015

Indeed, this should be marked as a TODO.

@zeha
Copy link
Collaborator

zeha commented Jan 18, 2015

Regarding comments, the following snippet (from the reg. tests) shows how name/type for comments should just come from the RRset. What did you send? Or rather, what exactly needs clarification? (Maybe I'm looking at the wrong section in the spec, or it's just "obvious" to me because I wrote it.)

        rrset = {
            'changetype': 'replace',
            'name': name,
            'type': 'NS',
            'comments': [
                {
                    'account': 'test1',
                    'content': 'blah blah',
                },
                {
                    'account': 'test2',
                    'content': 'blah blah bleh',
                }
            ]
        }

@jpmens
Copy link
Contributor Author

jpmens commented Jan 18, 2015

I sent

{
    "comments": [
        {
            "account": "JP",
            "content": "My first API-created zone",
            "name": "uhuh",
            "type": "dunno"
        }
    ],
    "kind": "Native",
    "masters": [],
    "name": "example.net",
    "nameservers": [
        "ns1.example.net",
        "ns2.example.net"
    ],
    "records": [
        {
            "content": "ns.example.net. hostmaster.example.com. 1 1800 900 604800 86400",
            "disabled": false,
            "name": "example.net",
            "ttl": 86400,
            "type": "SOA"
        },
        {
            "content": "192.168.1.42",
            "disabled": false,
            "name": "www.example.net",
            "ttl": 3600,
            "type": "A"
        }
    ]
}

@zeha
Copy link
Collaborator

zeha commented Jan 18, 2015

Oh, at zone creation! Alright. (I was looking at PATCH.)

@cmouse
Copy link
Contributor

cmouse commented Jan 18, 2015

About the record types, since all of them are UPPER CASE, would it make sense to just convert the given type to uppercase?

zeha added a commit to zeha/pdns that referenced this issue Jan 25, 2015
Based on feedback from issue PowerDNS#2064; no idea so far how to better
describe how comments work.
@zeha
Copy link
Collaborator

zeha commented Jan 25, 2015

Upcasing types is now #2128

@zeha
Copy link
Collaborator

zeha commented Feb 26, 2015

Going to tick off the 'master is not NULL' thing, as I have no way of fixing this without a lot of changes and I don't see any benefit from it. If anything, the schema should be made NOT NULL DEFAULT ''.

@Habbie Habbie added this to the auth-4.0.0 milestone Dec 15, 2015
@Habbie Habbie modified the milestones: auth-4.0.x, auth-4.0.0 Jul 7, 2016
@Habbie Habbie modified the milestones: auth-helpneeded, auth-4.0.x May 3, 2017
@Habbie
Copy link
Member

Habbie commented Jul 6, 2017

#3417 is related

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants