Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redo configuration logic #87

Merged
merged 46 commits into from
Aug 6, 2020
Merged

Redo configuration logic #87

merged 46 commits into from
Aug 6, 2020

Conversation

mayankchhabra
Copy link
Member

@mayankchhabra mayankchhabra commented Aug 3, 2020

The PR rewrites the configuration logic to achieve the following:

  1. Use templates to generate configuration files for bitcoind, lnd and tor (h/t @lukechilds for the awesome idea).

  2. Store RPC + Tor credentials in $UMBREL_ROOT/.env instead of secrets/ directory, allowing us to directly refer them in the Docker services as environment variables vs. having them hardcoded/patched.

  3. Reconfigure the installation over and over, even if it fails.

  4. OTA updates will now configure a new release as a fresh install with new configuration files and the existing (persistent) data.

  5. OTA updates will also pick-up the network (mainnet, testnet or regtest) from the existing .env and configure the new release for it.

  6. Commit rpcauth.py to scripts/ so the installation is more deterministic.

  7. Generate a different password for Tor instead of using the RPC password.

@mayankchhabra mayankchhabra marked this pull request as draft August 3, 2020 12:12
@mayankchhabra mayankchhabra marked this pull request as ready for review August 4, 2020 11:34
If you add the /.. traversal before readlink instead of after it will resolve it into the direct path
The quotes are either not doing anything:

This:
"some string ""$some_var"" some string"

Is the same as:
"some string $some_var some string"

Or are actually unquoting the variable:

This:
"some string "$some_var" some string"

Means $some_var is unquoted and will be subject to word splitting and
glob expansion.
Copy link
Member

@lukechilds lukechilds left a comment

Choose a reason for hiding this comment

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

Great stuff!

Some minor tweaks needed which are mostly resolved by this PR: https://github.com/mayankchhabra/umbrel/pull/7

I really love how this gives us a single source of truth for the configuration and we can use to rebuild the actual config files at any point in time.

templates/bitcoin-sample.conf Show resolved Hide resolved
scripts/configure Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
scripts/configure Outdated Show resolved Hide resolved
scripts/update/00-run.sh Show resolved Hide resolved
scripts/update/01-run.sh Outdated Show resolved Hide resolved
templates/bitcoin-sample.conf Show resolved Hide resolved
scripts/configure Outdated Show resolved Hide resolved
scripts/configure Outdated Show resolved Hide resolved
scripts/configure Outdated Show resolved Hide resolved
@lukechilds
Copy link
Member

One other suggestion, after running the configure script in an active git repo, you get the following:

$ git status
On branch patch/configure
Your branch is up to date with 'origin/patch/configure'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)

        .env
        bitcoin/bitcoin.conf
        lnd/lnd.conf
        tor/torrc

nothing added to commit but untracked files present (use "git add" to track)

We should add these files to .gitignore so we can develop on an active instance of getumbrel/umbrel without worrying about accidentally committing these files.

It would probably be best to add the entire dirs to .gitignore and then just un-ignore the .gitkeep file inside it.

@lukechilds
Copy link
Member

I also really love how docker-compose.yml is just a static file now that inherits everything from the environment.

@mayankchhabra
Copy link
Member Author

Awesome! Thanks for the review and fixes, man.

Also:

It would probably be best to add the entire dirs to .gitignore and then just un-ignore the .gitkeep file inside it.

Great suggestion! Updated.

Looking good to merge :)

@mayankchhabra mayankchhabra merged commit ad2bec7 into getumbrel:master Aug 6, 2020
@mayankchhabra mayankchhabra deleted the patch/configure branch August 6, 2020 03:55
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.

3 participants