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

Add reverse_pointer filter and reverse_lookup lookup #228

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

felixfontein
Copy link
Collaborator

SUMMARY

Add plugins for helping with reverse lookups:

  1. A filter to convert IP addresses into DNS names for using with regular lookup plugins;
  2. A reverse lookup plugin which does the whole thing in one step.

Fixes #227.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

reverse_pointer filter

Copy link

github-actions bot commented Nov 10, 2024

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and the docs are now incorporated into main:
https://ansible-collections.github.io/community.dns/branch/main

@felixfontein felixfontein changed the title [WIP] Add reverse_pointer filter Add reverse_pointer filter Nov 18, 2024
@felixfontein felixfontein changed the title Add reverse_pointer filter Add reverse_pointer filter and reverse_lookup lookup Nov 18, 2024
Copy link
Contributor

@oraNod oraNod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I suggested some type hints but I'll leave it to your discretion b/c I know they require import statements. I find them useful anyway. Hope this helps.

pass


def reverse_pointer(ip):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def reverse_pointer(ip):
def reverse_pointer(ip: str) -> str:

class FilterModule(object):
'''Ansible jinja2 filters'''

def filters(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def filters(self):
def filters(self) -> Dict[str, Any]:

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure since when exactly, but since Python 3.8 or 3.9 you can also write dict[str, Any] (same for list and tuple etc.).

server=name,
)

def run(self, terms, variables=None, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def run(self, terms, variables=None, **kwargs):
def run(self, terms: List[str], variables: Optional[dict] = None, **kwargs: Any): -> List[str]:

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of Optional[dict] you can also use dict | None. That requires either 3.9 or 3.10 (I'm not sure anymore), but with the right __future__ import (I think from __future__ import annotations) you can already start using it in Python 3.7+ (as long as the type checking happens with a new enough Python).


class LookupModule(LookupBase):
@staticmethod
def _resolve(resolver, name, rdtype, server_addresses):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def _resolve(resolver, name, rdtype, server_addresses):
def _resolve(resolver, name: str, rdtype: int, server_addresses: Optional[List[str]]): -> List[str]:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the type hint for the resolver parameter would be SimpleResolver right? so resolver: SimpleResolver

@felixfontein
Copy link
Collaborator Author

Type hints are a great idea; the collection requires ansible-core 2.14+, which means Python 3.9+ on the controller side. So type hints are definitely possible! (And already would have been since ansible-core 2.12+.)

I'd prefer to add type hints in a follow-up PR though (and then for everything else than modules and module_utils as well), and figure out there how well type checking works with ansible-test.

@felixfontein felixfontein merged commit b52a0c5 into ansible-collections:main Nov 19, 2024
39 checks passed
@felixfontein felixfontein deleted the reverse branch November 19, 2024 19:41
@felixfontein
Copy link
Collaborator Author

@oraNod thanks for reviewing this!

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.

community.dns.lookup does not resolve ptr lookup request
2 participants