Closed tarwich closed 2 years ago
Well, curiously, the cron timer ran, and it attempted to validate the files. I haven't gotten a IP geolocation key, because I don't want it to update. So it appears to have deleted all entries (presumably due to them being invalid).
I'm not going to undelete these files, because they're just going to get re-ran every hour, since the cron is on running on my box. If you choose to merge this PR, I can remove them at that time.
Awesome, thanks for this PR. I'm seeing that in the autogenerated PRs the valid_*.txt files are always empty. This may (or may not!) mean that the Github Actions runtime doesn't provide UDP connectivity, at least not beyond standard DNS traffic. I can't find anything about this in the documentation. Can you, @tarwich?
BTW, I love the emoticon <3
I think it's because I'm not providing an IP geolocation key. I can add one this week and see what happens.
On Mon, Jun 13, 2022 at 10:24 AM pradt2 @.***> wrote:
Awesome, thanks for this PR. I'm seeing that in the autogenerated PRs the valid_*.txt files are always empty. This may (or may not!) mean that the Github Actions runtime doesn't provide UDP connectivity, at least not beyond standard DNS traffic, I can't find anything about this in the documentation. Can you, @tarwich https://github.com/tarwich?
— Reply to this email directly, view it on GitHub https://github.com/pradt2/always-online-stun/pull/6#issuecomment-1154062923, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACRSRJE44AJEU77BZWWIGTVO5HEJANCNFSM5YQDNN5A . You are receiving this because you were mentioned.Message ID: @.***>
Hey, I added the API key to secrets, but this PR wont be able to access it since secrets aren't exposed to actions that come from a fork.
I opened PR #8 with your .github/workflows/check-list.yaml hoping that it would trigger the job, and that the job would have access to the api key. However, it seems that the action isn't triggered at all. Strange.
That does seem odd @pradt2. Are they enabled in the settings?
Also, you could unblock the push
trigger to test easier. There are other ways, but that's the easiest.
It's super sad but it seems that using Github's runners will not cut it. See my concluding comment in #8.
We could potentially work around this by using a self hosted runner, but I can see that Github recommends against it for public repos. I guess we'll just stick to the current setup for now.
However, there are other Github Actions that this project could certainly use. E.g. compiling/testing on a new PR, periodic dependabot scans, etc. Do you think you could set these actions up? Just whatever crosses your mind that you think a project like this could use.
@pradt2 based on your message https://github.com/pradt2/always-online-stun/pull/8#issuecomment-1155618955 I wonder if we can find a workaround. Are the current updates running from your own server?
What about running something like this? https://webrtc.github.io/samples/src/content/peerconnection/trickle-ice/
What about launching puppeteer (a headless Chrome) in the action and contacting the STUN server that way?
This PR could be closed, and we can resurrect it later if you like. It does the CRON job #8 is requesting. However, in order for it to work we might need to make a different testing harness and I don't understand STUN well enough to understand what you're even testing or why it won't work behind NAT.
Nice thinking outside the box with the Pupeteer stuff, but I'm afraid that won't work either.
So a bit of a background, maybe that'll make it clearer.
In basic terms, what a STUN server does is tell the caller what the caller's IP and port is. This project wants to test that each of the STUN servers that we know of does exactly that.
This is what I call 'tests' BTW. It's the process of checking which STUN servers from our list are alive and well. Not to be confused with unit testing, integration testing, etc.
We do this 'testing' by querying each STUN server from the candidates.txt
list. For a server to be healthy (i.e. to pass the test), all of the below must be true:
Now, let's pause for a moment. Verifying 1 and 2 can easily be done from any network (behind a NAT or not).
But 3 is different. If I'm not behind NAT, the task is easy - I can just compare the returned mapping with the IP:port values of the local socket I used to send the STUN query. Job done.
But if I'm behind NAT, the local IP:port is useless. We know for a fact that it will not be the same as the mapping returned by the STUN server. So how can I tell that the mapping returned is correct? What can I compare it to? I can't magically query my NAT for the mapping - if I could, I'd never bother with STUN in the first place).
In the past I thought about querying multiple STUN servers using the same local socket and cross-checking the mappings. However, that would make the project more complex, increase the testing time, and even then it would only work on certain NATs that do Endpoint Independent Mapping, so I decided against it. However, now that I'm thinking about it, adding this cross-checking mode for doing the testing behind NAT is probably our best bet on being able to run the tests in the Github Actions ecosystem.
Coming back to Puppeteer - yes we could in principle use it together with https://webrtc.github.io/samples/src/content/peerconnection/trickle-ice/ and test each STUN server this way. But this is a dead end way of doing so. In the not so distant future I want to start publishing new lists with more information about the STUN servers, e.g. their physical geographic location (hence the need for the IPGEOLOCAITONIO API key), what RFC standards they conform to, etc. None of this info is exposed by the website you propose.
@tarwich I just pushed some changes that enable STUN testing behind NAT. Environment variable IS_BEHIND_NAT must be set to true for it to work.
So the only thing left is to wait for GHA to support IPv6. There's an issue about this already: https://github.com/actions/virtual-environments/issues/668 (it's closed because they refuse to support IPv6 but surely this will be reopened someday)
Ok... So do we want to close this since IPV6 will never be a thing, meaning we can't reliably test this?
Yeah I will close it for now, and re-open it when they start supporting IPv6.
Anyway, thanks for looking into it. If you are up for it, we could still add other actions e.g. compile and unit testing on PRs etc.
I took it upon myself to try to implement a solution for #2. I use GitHub actions, so the process is fairly familiar to me. I'm not familiar with RUST, but since this is a simple shell script, I assume it should just work.
You'll need to make some secrets for any private keys your build process needs. The secrets documentation is helpful, or you can just follow the example I made for IPGEOLOCATIONIO_API_KEY
fixes #2