Skip to content

Add usable dockerfile to clightning - #1187

Closed
NicolasDorier wants to merge 2 commits into
ElementsProject:masterfrom
NicolasDorier:dockerfile
Closed

Add usable dockerfile to clightning#1187
NicolasDorier wants to merge 2 commits into
ElementsProject:masterfrom
NicolasDorier:dockerfile

Conversation

@NicolasDorier

Copy link
Copy Markdown
Contributor

@cdecker, you are providing a dockerfile on https://hub.docker.com/r/cdecker/lightningd .
Sadly, this docker file is useless for developers as it is impossible to connect to the RPC unix domain socket.

This PR propose a Dockerfile which expose clightning on TCP with socat. This allow developers not using linux to still develop stuff on top of CLightning by dockerizing the app and connecting via TCP.

It will also simplify lightning-charge which currently is starting clightning and charge in the same container though a very hacky manner.

This port is not meant to be mapped to the host (except for testing) for obvious security reasons.

@NicolasDorier

Copy link
Copy Markdown
Contributor Author

Example of dockerfile using it:

version: "3"

services:
  bitcoind:
    image: nicolasdorier/docker-bitcoin:0.16.0
    environment:
      BITCOIN_EXTRA_ARGS: |
        rpcuser=ceiwHEbqWI83
        rpcpassword=DwubwWsoo3
        regtest=1
        server=1
        rpcport=43782
        port=39388
        whitelist=0.0.0.0/0
        zmqpubrawblock=tcp://0.0.0.0:29000
        zmqpubrawtx=tcp://0.0.0.0:29000
        txindex=1
        # Eclair is still using addwitnessaddress
        deprecatedrpc=addwitnessaddress 
    expose:
      - "43782" # RPC
      - "39388" # P2P
    volumes:
      - "lightning_datadir:/data"

  lightning:
    image: nicolasdorier/clightning
    environment:
      LIGHTNINGD_OPT: |
        network=regtest
        bitcoin-rpcconnect=bitcoind
    volumes:
      - "lightning_datadir:/root/.bitcoin"
    expose:
      - "9835" # RPC
      - "9735" # Lightning
    links:
      - bitcoind

volumes:
  lightning_datadir:

@rustyrussell

Copy link
Copy Markdown
Contributor

@shesek?

@cdecker cdecker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we should provide a docker image that exposes the RPC interface without any form of authentication to the world. lightningd does not have any authentication simply because we want higher up layers to implement whatever the environment it is running in requires.

Even if we were to host the dockerfile, it should be in contrib/ not the root directory.

So for me this is a NACK.

Comment thread .gitattributes
@@ -0,0 +1,4 @@
# Declare files that will always have CRLF line endings on checkout.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a duplicate commit from #1185

@hbasria

hbasria commented Mar 8, 2018

Copy link
Copy Markdown

If you want to use as a rest api

pip install lightning-rest
python -m lightning_rest.server 8000 ~/.lightning/lightning-rpc

or

docker run -it --rm -p 8000:8000 -v /path/to/lightning-rpc:/tmp/lightning-rpc siriuslabs/lightning-rest 8000 /tmp/lightning-rpc

https://github.com/hbasria/lightning-rest

@NicolasDorier

NicolasDorier commented Mar 8, 2018

Copy link
Copy Markdown
Contributor Author

lightningd does not have any authentication simply because we want higher up layers to implement whatever the environment it is running in requires.

This is exactly what I want to do... except that it is impossible without an hell of pain because I am on windows. I can't just code in visual studio and connect to clightning.

If you want to use as a rest api

This is cool. Though my use case is to use TCP only for dev time purposes, this should go away in prod by using shared volumes as you are doing.

@NicolasDorier

Copy link
Copy Markdown
Contributor Author

closing this, will maintain my own nicolasdorier/clightning and make things depend on this instead.

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.

4 participants