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

Update validations of hostnames and node names #1

Merged
merged 3 commits into from
Dec 16, 2015

Conversation

spinza
Copy link
Contributor

@spinza spinza commented Dec 14, 2015

This checks the first and last character of node names.
Also adds checking for hostnames.

@jammin84
Copy link
Contributor

why limit length to 48chars?

63 is maximum allowed per level.
https://en.wikipedia.org/wiki/Domain_Name_System

Some of our nodes have multiple domains assigned to them. This allows them to match which one they were referring.
Would this check then restrict me from entering hostname.domainname due to length or due to it containing a "."?
I guess ideally there should be another dropdown next to hostname for the domain portion.

@spinza
Copy link
Contributor Author

spinza commented Dec 15, 2015

Length
nodes.name_ns is varchars(50) in the db. Need an extra 2 for the duplicates. So if name_ns already exists it adds a -1 or -2 etc to the name. So
ip_addresses.hostname is also 50. Could add 2 for you buy making hostname max 50 and name_ns 48.
Subdomains
I can allow a . as a valid char in the text. Would allow people to do subdomains. But this would still be a subdomain of name_ns.

So our fqdn for an ip_adddress entry is ip_addresses.hostname + '.' + nodes.name_ns + '.ctwug.za.net'

I guess you are ignoring name_ns? So allowing dots will work for you?

@jammin84
Copy link
Contributor

thanks for the fqdn, now i understand what your doing.

We use ip_addresses.hostname + '.' + dns_zones.name + '.wafn'

Which allows us a single node to have many domain names.

IE a special "Admin" node has the following domains allocated to it

@spinza
Copy link
Contributor Author

spinza commented Dec 15, 2015

This PR
I think allowing "." will allow both of us to do what we want for now?

Bigger discussion again
I think this is again a bigger discussion. I think your dns zone setup would be quite complex for us. We have many many nodes for example.

To me the best possible model is ip_addresses.hostname + '.' +nodes.name_ns '.' + dns_zones.name.
But there would be a join between ip_addresses and dns_zones. I.e. I want to set the ip address for this hostname.name_ns for this zone.

You'd lose duplicate names to some extent, but doesn't seem to be used that often. Note you could also just edit the name_ns field to the "easy version" of a node name.

Then we could based on your zone data add zone for wafn and zone for say wafn.co.au or so to make your dns available on the web.

Also we "calculate" reverse data based on ip_addresses/nodes data. Our nodes are too many so we can't give each node a /24 to have a separate reverse zone. We share a common address space with other wireless networks in South Africa (allowing peering). We "only" get 172.18 and 172.26.

@spinza
Copy link
Contributor Author

spinza commented Dec 15, 2015

Forgot to mention with either model we may need a separate screen to edit the top level dns data. E.g. MX SPF etc.

@jammin84
Copy link
Contributor

agreed "." for now.

I'll put further comments in issue #9

jammin84 added a commit that referenced this pull request Dec 16, 2015
Update validations of hostnames and node names
@jammin84 jammin84 merged commit 1c481ae into southern-wind:master Dec 16, 2015
@jammin84
Copy link
Contributor

err forgot the "." needs doing. If you can add and merge. Then this can be closed.

@spinza
Copy link
Contributor Author

spinza commented Dec 16, 2015

Away for couple of days. Will do when I'm back.

On Wed, 16 Dec 2015, 06:05 jammin84 [email protected] wrote:

err forgot the "." needs doing. If you can add and merge. Then this can be
closed.


Reply to this email directly or view it on GitHub
#1 (comment).

spinza pushed a commit that referenced this pull request Mar 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants