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

DNS utils improvements #46184

Merged
merged 22 commits into from Apr 30, 2018

Conversation

Projects
None yet
5 participants
@The-Loeki
Copy link
Contributor

commented Feb 25, 2018

What does this PR do?

Fix various issues with the dns utils, expand it's functionality and increase it's testing coverage

New Behavior

  • Add TLSA, CAA and SSHFP 'intelligent' records
  • Various bugs and corner cases fixed
  • remove duplicate code and functions
  • add solid TLD determination when available

Previous PR's

#39639
#40269

Tests written?

Yes

@garethgreenaway

This comment has been minimized.

Copy link
Member

commented Feb 26, 2018

@The-Loeki Looks like some of the DNS tests are failing.

@The-Loeki The-Loeki force-pushed the The-Loeki:dns-impr branch 2 times, most recently from 895652c to eb5240d Feb 26, 2018

The-Loeki added some commits May 2, 2017

@The-Loeki The-Loeki force-pushed the The-Loeki:dns-impr branch from 9d7c08d to ee6c483 Feb 27, 2018

The-Loeki added some commits Mar 7, 2018

@The-Loeki

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2018

@garethgreenaway / @terminalmage
I'd like to ask you guys to merge this if that's OK; I've got the equivalent replacement module WIP as well pending that.

I'd like your opinion before continuing as well:
I've noticed for 2018.3 the refresh_dns() and dns_check() functions got moved to salt.utils.network
That IMHO isn't that logical considering salt.utils.dns.etcetera()

So:

  • could you care less?

  • or make salt.utils.network.dns and refactor the lot into that

  • or refactor dns_check() & refresh_dns() to salt.utils.dns.check()|refresh_resolv(()

  • and/or make salt.utils(.network).dns a directory to move more stuff into in expectation of future development

@terminalmage

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2018

I'm not familiar enough with this DNS utils module to comment on it at the moment.

@terminalmage terminalmage removed their request for review Mar 9, 2018

@garethgreenaway

This comment has been minimized.

Copy link
Member

commented Mar 27, 2018

@The-Loeki Probably makes more sense and more consistent to move all dns related utils into salt.utils.dns

@rallytime rallytime merged commit ebe7fd4 into saltstack:develop Apr 30, 2018

5 of 10 checks passed

jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #4483 — ABORTED
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #9408 — ABORTED
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #18535 — ABORTED
Details
codeclimate 2 issues to fix
Details
default Build finished.
Details
WIP ready for review
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #24655 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #16784 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #22363 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #21404 — SUCCESS
Details
@@ -887,6 +966,21 @@ def sshfp_data(key_t, hash_t, pub):
return _rec2data(key_t, hash_t, ssh_fp)


def sshfp_rec(rdata):
'''
Validate and parse DNS record data for TLSA record(s)

This comment has been minimized.

Copy link
@skorpy2009

skorpy2009 Mar 4, 2019

shouldn't this be sshfp record(s)

@The-Loeki The-Loeki deleted the The-Loeki:dns-impr branch Apr 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.