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

portability: permit arpa address #5411

Merged
merged 1 commit into from
Jul 4, 2024
Merged

Conversation

oliver-sanders
Copy link
Member

@oliver-sanders oliver-sanders commented Mar 14, 2023

Closes #6147

Frustratingly, the arpa address returned by socket.getfqdn on Mac OS is different with Python 3.9 than 3.7 (conda-forge).

For context see #4296

I'm not especially keen on fiddling this exception every time it changes, but can't really think of a better way to do this safely.

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@hjoliver
Copy link
Member

Can't say I understand the problem very well, but the "fix" seems fine. I take it we're safe from users on different Mac OS version having different variants of the problem? (It's entirely Python version dependent?).

@oliver-sanders
Copy link
Member Author

oliver-sanders commented Mar 16, 2023

This was the first time I've seen this new address. I'm assuming it's Python version dependent as that's the only thing I've changed but there is a small chance that it's network related, will test that assumption when I get the chance. Unfortunately there's not very much literature on this problem, only a couple of posts from people asking why socket.getfqdn() doesn't match hostname -f and a cPython issue.

@oliver-sanders oliver-sanders self-assigned this Mar 21, 2023
@oliver-sanders
Copy link
Member Author

oliver-sanders commented Mar 21, 2023

Q: Why does socket.getfqdn() not match hostname -f?
A: Because of a long-standing Python bug.

Q: What the hell is "arpa"?
A: The in-addr.arpa and ip6.arpa are the IP V4 and V6 variants of a special domain used for reverse DNS lookups.

What can we do about it?

  1. Blacklist known localhost references (has worked so far but is annoying and ugly).
  2. Here's a patch associated with the Python bug report, but not accepted. It uses gethostbyaddr which is V4/6 compliant.
  3. Subprocess call to hostname -f 😁

Will try out gethostbyaddr when I get a chance, should be fine, but any change to this sensitive code is a risk.

@hjoliver
Copy link
Member

hjoliver commented Mar 21, 2023

A: Because of a python/cpython#49254.

Damn, disappointing that that has not been fixed yet.

Could we use the patch referred to at the end of that Python issue?

@oliver-sanders
Copy link
Member Author

oliver-sanders commented Mar 22, 2023

Could we use the patch referred to at the end of that Python issue?

Yep, a couple of other systems appear to be doing this. It is a little risky though as we know this code to be sensitive.

@oliver-sanders oliver-sanders added the question Flag this as a question for the next Cylc project meeting. label Apr 11, 2023
@oliver-sanders
Copy link
Member Author

oliver-sanders commented May 16, 2023

FYI: This is waiting on me, I need to confirm this new arpa I encountered is due to the default Mac OS DNS config and not due the to configuration of the wider network.

If it is, we need to decide whether to go for the more general, but riskier patch above, or just to hardcode the new exception as done before.

Low priority until we start getting bug reports.

@oliver-sanders
Copy link
Member Author

I think the differing result I was seeing may have been a result of local network / ISP both of which can influence a fully IP based setup.

I'm going to put this one to sleep for now until it gets reported.

@oliver-sanders
Copy link
Member Author

I'm going to put this one to sleep for now until it gets reported.

It has been reported! #6147

@oliver-sanders oliver-sanders added this to the 8.3.1 milestone Jun 17, 2024
@oliver-sanders oliver-sanders linked an issue Jun 17, 2024 that may be closed by this pull request
@MetRonnie MetRonnie changed the base branch from master to 8.3.x June 19, 2024 12:34
@elliotfontaine
Copy link

Thank you so much guys ! 🙏

* The arpa address returned by `socket.getfqdn` on Mac OS is different
  with Python 3.9 than 3.7 (conda-forge).
@oliver-sanders oliver-sanders marked this pull request as ready for review June 26, 2024 11:50
@oliver-sanders
Copy link
Member Author

@hjoliver, suggest going ahead with the workaround for now and looking into more advanced methods in the future, possibly in combination with an overhaul of Cylc's DNS use in general.

@oliver-sanders oliver-sanders removed the question Flag this as a question for the next Cylc project meeting. label Jul 1, 2024
Comment on lines 124 to +125
'1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.'
'0.0.0.0.0.0.ip6.arpa'
)
'0.0.0.0.0.0.ip6.arpa',
Copy link
Member

@MetRonnie MetRonnie Jul 3, 2024

Choose a reason for hiding this comment

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

Could make this tidier if desired

f'1.{"0." * 31}ip6.arpa',

@oliver-sanders oliver-sanders requested review from wxtim and removed request for hjoliver July 4, 2024 08:34
@wxtim wxtim merged commit 1af3999 into cylc:8.3.x Jul 4, 2024
27 checks passed
@oliver-sanders oliver-sanders deleted the macos-dns-fix-2 branch July 4, 2024 08:49
@MetRonnie
Copy link
Member

Do we want a changelog entry for this one?

@oliver-sanders
Copy link
Member Author

Nah

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

Successfully merging this pull request may close these issues.

Host name resolution error on MacOS
5 participants