Skip to content

Add rust Dockerfile#80

Open
frnandu wants to merge 2 commits into
lightningdevkit:mainfrom
frnandu:chore/rust-dockerfile
Open

Add rust Dockerfile#80
frnandu wants to merge 2 commits into
lightningdevkit:mainfrom
frnandu:chore/rust-dockerfile

Conversation

@frnandu
Copy link
Copy Markdown

@frnandu frnandu commented Dec 29, 2025

fixes #74

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Dec 29, 2025

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@tnull tnull requested review from tankyleo and removed request for jkczyz December 30, 2025 08:22
@ldk-reviews-bot
Copy link
Copy Markdown

🔔 1st Reminder

Hey @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link
Copy Markdown

🔔 2nd Reminder

Hey @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@ldk-reviews-bot
Copy link
Copy Markdown

🔔 3rd Reminder

Hey @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Copy link
Copy Markdown
Contributor

@tankyleo tankyleo left a comment

Choose a reason for hiding this comment

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

Thanks ! can you make this one commit / remove the merge commit in the current PR ?

Comment thread rust/Dockerfile
Comment thread rust/Dockerfile
Comment thread rust/Dockerfile Outdated
Comment thread rust/Dockerfile Outdated
@frnandu frnandu changed the title Chore/rust dockerfile chore: rust dockerfile Jan 15, 2026
Copy link
Copy Markdown
Contributor

@tankyleo tankyleo left a comment

Choose a reason for hiding this comment

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

As mentioned above, let's bring back ca-certificates for HTTPS requests.

Also squash all the commits into a single one.

@frnandu frnandu force-pushed the chore/rust-dockerfile branch 3 times, most recently from fcec5ff to 2336a2d Compare January 24, 2026 15:35
@frnandu
Copy link
Copy Markdown
Author

frnandu commented Jan 24, 2026

Also squash all the commits into a single one.

Isn't that possible to do when merging by squash and merge ?

@tankyleo
Copy link
Copy Markdown
Contributor

Isn't that possible to do when merging by squash and merge ?

I need to ACK the exact commit that will get merged, should be quite doable on your side no ?

@tankyleo
Copy link
Copy Markdown
Contributor

@frnandu making sure you see this message above thanks for squashing if you can

@frnandu frnandu force-pushed the chore/rust-dockerfile branch from 2336a2d to 7f34829 Compare February 1, 2026 23:06
@frnandu
Copy link
Copy Markdown
Author

frnandu commented Feb 1, 2026

Squashed, thanks!

Copy link
Copy Markdown
Contributor

@tankyleo tankyleo left a comment

Choose a reason for hiding this comment

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

Thanks tested on my machine looks good.

One last nitpick before merge: please set the title to Add rust Dockerfile, no need for the chore prefix thank you, I want to keep the commit log consistent.

@frnandu frnandu force-pushed the chore/rust-dockerfile branch from 7f34829 to 2bbc54c Compare February 6, 2026 00:32
@frnandu
Copy link
Copy Markdown
Author

frnandu commented Feb 6, 2026

One last nitpick before merge: please set the title to Add rust Dockerfile, no need for the chore prefix thank you, I want to keep the commit log consistent.

done

@frnandu
Copy link
Copy Markdown
Author

frnandu commented Mar 27, 2026

ping?

@frnandu frnandu changed the title chore: rust dockerfile Add rust Dockerfile Apr 24, 2026
@tankyleo tankyleo force-pushed the chore/rust-dockerfile branch from 2bbc54c to 834b6b9 Compare June 2, 2026 01:29
@tankyleo
Copy link
Copy Markdown
Contributor

tankyleo commented Jun 2, 2026

@frnandu I cleaned up the branch, used fmar <fmar@fmar.dev> for the author field on the dockerfile commit.

tnull thanks for taking a look I can't merge since I'm the last pusher.

@tankyleo tankyleo requested a review from tnull June 2, 2026 01:37
Comment thread rust/Dockerfile
# Copy default configuration file
COPY server/vss-server-config.toml /app/vss-server-config.toml

EXPOSE 8080
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems by default this will only bind to localhost. Do we need to set VSS_BIND_ADDRESS to make this work outside of docker/reachable by other docker containers?

  • [P2] Use container-reachable network defaults — /home/tnull/workspace/vss-server/rust/Dockerfile:40-40
    When the image is run with its default CMD, e.g. docker run -p 8080:8080 ..., /app/vss-server-config.toml still binds VSS to 127.0.0.1 and points PostgreSQL at 127.0.0.1. Inside Docker those are container-loopback addresses, so the published port cannot reach the server and Postgres on
    another container/host is unreachable unless users override both env vars. Consider shipping a Docker-specific config or setting Docker defaults such as VSS_BIND_ADDRESS=0.0.0.0:8080 and a network-reachable Postgres address.

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.

Dockerfile for rust version

4 participants