search

foo-software / lighthouse-check-action

GitHub Action for running @GoogleChromeLabs Lighthouse audits with all the bells and whistles 🔔 Multiple audits, Slack notifications, and more!
https://github.com/marketplace/actions/lighthouse-check
MIT License
478 stars 24 forks source link

Feature request: Allow to use the host network #69

Closed dlehmhus closed 2 years ago

dlehmhus commented 2 years ago

Feature request

Problem

In my current setup I'm running the lighthouse-check-action against a local server (e.g. localhost:3000). This worked fine with v5, but since v6 all tests always score 0 points. My explanation for this is, that v6 now runs inside a docker container, and therefore by default can not access the host's network.

Request

Config to customize the docker container or add --network="host" by default.

adamhenson commented 2 years ago

Thanks for raising this issue @dlehmhus. Your theory makes a lot of sense to me. Let me see what I can do.

adamhenson commented 2 years ago

@dlehmhus - I'm having difficulty in coming up with a way to reproduce this. Can you provide an example?

dlehmhus commented 2 years ago

Hey @adamhenson, here is an example repo. It is basic create-next-app boilerplate with two feature branches, one that adds v5 and one that adds v6.

adamhenson commented 2 years ago

Perfect, thanks @dlehmhus. I'll do some experimenting on my end. At the moment, I'm not sure if it's possible to use --network="host" in the action.yml metadata file... using runs, but I'll do some experimenting. I'll try using runs.args, but I'm not very hopeful since it's only used with ENTRYPOINT.

adamhenson commented 2 years ago

@dlehmhus - I'm not so sure we can support this. I haven't found a way. I tried passing --network="host" as a runs.args value in the GitHub Action via metadata file (this is what is used to define the GitHub Action), as this was the only thing I could come up with... but it's failing for this reason:

GitHub passes the args to the container's ENTRYPOINT when the container starts up.

So, it doesn't actually pass the arg to docker run like we would need it to.

Here's the PR I've been using for experimenting. Feel free to fork it and play around. Otherwise, I'm open to suggestions. I was looking for a way to do this in the Dockerfile but I didn't find anything.

I've spent most of my day working on this, and I think I've exhausted most of the time I'm willing to spend on this. Definitely open to suggestions.

adamhenson commented 2 years ago

The reason for converting this project to Docker was mainly because GitHub took forever in providing a Node.js runner later than 12. I was starting to doubt it would ever happen.

https://github.com/actions/runner/issues/772

But, it looks like there was a recent comment on that issue and it appears that they've released a Node 16 runner. I'm not so sure the runner is available, but it appears that corresponding PR has been merged: https://github.com/actions/runner/pull/1439

I think my next step will be to confirm if Node 16 runner is actually available and then I can convert this project back to it. Perhaps we could hold on this until then.

adamhenson commented 2 years ago

Okay, the rumors are true. Node 16 runner is now available 😅

This PR should fix this issue once it's merged and released... hopefully within the next couple hours: https://github.com/foo-software/lighthouse-check-action/pull/71

adamhenson commented 2 years ago

I confirmed that v7.0.0 fixes this issue as reproduced in https://github.com/foo-software/lighthouse-check-action/pull/70.

Thanks again for raising this and providing a reproduction @dlehmhus.