Support for --add-host

[Thijmen]

This PR adds support for Nixpacks' build argument --add-host.

--add-host allows multiple hosts being added, just like with the docker build.

For example: nixpacks build --add-host nginx:127.0.0.1 --add-host postgres:127.0.0.1.

I've added tests to validate that:

1. The example could be build when hosts are provided during runtime
2. The example could not be build when no hosts are provided.

It's my first time doing something with Rust, so code might not be up to your standards. However, I am always open for improvements!

[Thijmen]

Ideally I also would like to add --add-hosts-for-network docker-network, to pass all ip's, but that's something up for discussion first.

[github-actions]

This pull request is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days

[Thijmen]

Not stale

[ndneighbor]

Updated the label, going to reflag this internally. Our main maintainer for this repo is on PTO so apologies for the delay.

[Thijmen]

No need for apologies, @ndneighbor. Appreciate this package and only happy to contribute!

[github-actions]

This pull request is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days

[Thijmen]

Not stale

[JakeCooper]

Can you fix lint issues and other stuff?

[Thijmen]

I was not aware of these linting issues before, I will get right on it!

[Thijmen]

I believe there are no linting issues anymore, running cargo fmt --all -- --check does not give any error anymore @JakeCooper.

[coffee-cup]

This is really awesome! Thanks for the PR :D

[coffee-cup]

This is really awesome! Thanks for the PR :D

[coffee-cup]

Sorry about the conflicts 😬

There also seems to be some missing tests and linting errors. Could you please

• Update the snapshot tests with cargo snapshot
• Fix the linting errors with cargo clippy --all-targets --all-features -- -D warnings

After this you should be all good to go

[Thijmen]

@coffee-cup I think this should do! :D

[Thijmen]

I'll dig into the failing test later tonight, or is this something you caused? I'm not sure.

[coffee-cup]

I fixed one of the snapshot tests (by running cargo snapshot), which enabled CI to run all of the docker run tests. These seem to be failing though. Could you please take a look. I don't want to mess with the code too much. Appologizes for changing things. I expected it to be a quick fix and merge

[Thijmen]

Interesting, the tests dont fail locally. I'll spin up a VM with ubuntu and try there. I'll ping you once done :D

[Thijmen]

I'm not done yet, I forgot something!

[Thijmen]

@coffee-cup In order to fully understand what's happening, I have some questions;

1. Can I safely change the snapshot of a given test (in my case: node_fetch_network)?
2. If so, is this the place to add environment variables? It is important that during build phase, a given environment variable is being used (in this case, REMOTE_URL).

[coffee-cup]

@coffee-cup In order to fully understand what's happening, I have some questions;

1. Can I safely change the snapshot of a given test (in my case: node_fetch_network)?
2. If so, is this the place to add environment variables? It is important that during build phase, a given environment variable is being used (in this case, REMOTE_URL).

1. The snapshot should be generated automatically using cargo snapshot. You can change it as part of this PR, but it shouldn't be edited by hand. For more info on snapshot tests please see this
2. You can add the environment variable in the docker_run_tests.rs file when the build is run. Alternatively you can create a nixpacks.toml file in the node_fetch_network example directory with the contents

[variables]
REMOTE_URL = '...'

[Thijmen]

Hi there,

Cheers, now I understand. It seems that on MacOS Docker, the --add-host flag works fine, however on Ubuntu it does not. I need some more time to dive into this issue. I'll ping and un-draft this PR once done.

[Thijmen]

It seems Docker on Linux is much more strict when it comes to adding hosts. I've added --network=host when providing --add-hosts.

However, I can understand if you'd like to see that as an argument for Nixpacks itself. Let me know what you prefer, then I will make sure to add it in as well.

Would be nice though to see if this fixes the CI as well.

[coffee-cup]

Looks great and all tests passing! I don't think adding the --network=host arg is a problem.

Thanks again!