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

Support DNS Zone File Syntax #1355

Closed
BenMcH opened this issue Jan 27, 2024 · 8 comments · Fixed by #1360
Closed

Support DNS Zone File Syntax #1355

BenMcH opened this issue Jan 27, 2024 · 8 comments · Fixed by #1360
Labels
🔨 enhancement New feature or request
Milestone

Comments

@BenMcH
Copy link
Contributor

BenMcH commented Jan 27, 2024

It was pointed out in #1352 that we could change DNS mappings from the custom YAML formatting to rely on DNS Zone File syntax instead.

Adapted from @ThinkChaos's comment:
This could be implemented using dns.ZoneParser which supports the standard zonefile format. This would also have the benefit of allowing to split the mapping into another file via $INCLUDE.

Is this something that the project wants to move towards? Initial concerns would be the potential for new users who rely on private DNS entries not necessarily being familiar with the syntax. On the other hand, this is a standard syntax that can be linked to rather than maintaining custom syntax and documentation within this project.

@kwitsch
Copy link
Collaborator

kwitsch commented Jan 27, 2024

I'm a bit conflicted since we would need additional files (I like the "only one config" approach).
Additionally it would consequently mean additional overhead for very simple mappings.

How about we let the current config as-is, add an zonefile-include section and merge those internally?

That way backward compatibility is ensured and no additional files are needed for simple setups but complex could be realized through zonefiles.

@kwitsch kwitsch added the 🔨 enhancement New feature or request label Jan 27, 2024
@kwitsch kwitsch added this to the future milestone Jan 27, 2024
@ThinkChaos
Copy link
Collaborator

I think we should just have the option be a string in the YAML, and if the user wants they just set that string to $INCLUDE a-file. The docs can mention that so it's obvious you can split if you want.

@kwitsch
Copy link
Collaborator

kwitsch commented Jan 27, 2024

I think we should just have the option be a string in the YAML, and if the user wants they just set that string to $INCLUDE a-file. The docs can mention that so it's obvious you can split if you want.

Another option using the suggested string entry would be to just input the zonefile as multiline string directly into the yaml. 🤔

@ThinkChaos
Copy link
Collaborator

Is this something that the project wants to move towards? Initial concerns would be the potential for new users who rely on private DNS entries not necessarily being familiar with the syntax.

I think that's a valid concern, but we can keep the simple mapping for A/AAAA.
And then having docs with examples and maybe links to other references/docs about the format should be enough to get people started, even from scratch.

Another option using the suggested string entry would be to just input the zonefile as multiline string directly into the yaml. 🤔

Yeah that's what I was envisioning too :)
I think the string is pretty versatile option and should fit everyone's needs.

@BenMcH
Copy link
Contributor Author

BenMcH commented Jan 28, 2024

Would support for DNS Zone files come with additional support for all DNS record types or would there still only be a subset of supported record types with helpful error messages output during zone file parsing?

@ThinkChaos
Copy link
Collaborator

I think we can start with the existing subset (UnmarshalYAML can loop through and error if there's some we don't support).
It would be interesting to see which record types are trivial to support, and add those. I'm thinking of stuff like TXT where you just need to send the data, as opposed to CNAME where blocky needs extra processing.

@kwitsch
Copy link
Collaborator

kwitsch commented Jan 28, 2024

I think we can start with the existing subset (UnmarshalYAML can loop through and error if there's some we don't support).
It would be interesting to see which record types are trivial to support, and add those. I'm thinking of stuff like TXT where you just need to send the data, as opposed to CNAME where blocky needs extra processing.

The Zone parser already has an iterator methode Next() where the RR is returned and can be type checked. 😉

@kwitsch
Copy link
Collaborator

kwitsch commented Jan 28, 2024

The directives $INCLUDE, $ORIGIN, $TTL and $GENERATE are all supported. Although $INCLUDE is disabled by default. Note that $GENERATE's range support up to a maximum of 65535 steps.

In my opinion $INCLUDE should be enabled by default and the $GENERATE` part should be mentioned in the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants