lnconfig: Support utilizing Environment Variables in `lnd.conf` for `rpcuser` and `rpcpass` fields.

[mohamedawnallah]

Change Description

• Added supplyEnvValue function in config.go that returns The value of the specified environment variable, or the default value if provided, or the original input string if no matching variable is found or set.
• Called supplyEnvValue function in parseRPCParams function in config.go file where rpcuser and rpcpass fields are being parsed.

Additional Context

supplyEnvValue extracts the value of an environment variable from a string. It supports the following formats:

• $ENVIRONMENT_VARIABLE
• ${ENVIRONMENT_VARIABLE}
• ${ENVIRONMENT_VARIABLE:-DEFAULT_VALUE}

It follows the following naming conventions for the Standard environment variable:

• ENVIRONMENT_VARIABLE contains letters, digits, and underscores, and does not start with a digit.
• DEFAULT_VALUE follows the rule that it can contain any characters except whitespace.

It takes the following parameters:

• value: The input string containing references to environment variables.

Closes #8295

Steps to Test

Try the following tests in lnd.conf on the bitcoind for example:

• $ENVIRONMENT_VARIABLE -> It extracts the value of ENVIRONMENT_VARIABLE if it is not set it raises error.
• $ENVIRONMENT_VARIABLE} or ${ENVIRONMENT_VARIABLE -> As this is not a valid expression for accessing it deals with them as a typical string.
• ${ENVIRONMENT_VARIABLE} -> It extracts the value of ENVIRONMENT_VARIABLE if it is not set it raises error.
• ${ENVIRONMENT_VARIABLE:-DEFAULT_VALUE} -> It extracts the value of ENVIRONMENT_VARIABLE if it is not set it gets the value of DEFAULT_VALUE.
• ${ENVIRONMENT_VARIABLE:-} -> As this is not a valid expression for accessing ${ENVIRONMENT_VARIABLE:-} it deals with it as a typical string.
• ${:-DEFAULT_VALUE} -> If the ENVIRONMENT_VARIABLE is empty here it get the value of DEFAULT_VALUE.

You could also check the logic workflow of the used regular expression in the diagram tool:

Pull Request Checklist

Testing

• [ ] Your PR passes all CI checks.
• [ ] Tests covering the positive and negative (error paths) are included.
• [ ] Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

• [ ] The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
• [ ] Commits follow the Ideal Git Commit Structure.
• [ ] Any new logging statements use an appropriate subsystem and logging level.
• [ ] Any new lncli commands have appropriate tags in the comments for the rpc in the proto file.
• [x] There is a change description in the release notes, or [skip ci] in the commit message for small changes.

📝 Please see our Contribution Guidelines for further guidance.

[mohamedawnallah]

Thanks for the great feedback @guggero @ProofOfKeags I've addressed your suggestions!

[guggero]

Before I'm going to review in detail again, just one comment: It feels weird to me that we would want to support different formats and even fall back values within the parsing code in lnd itself. It makes the regular expression quite complicated and hard to maintain. What's the use case you see for allowing to specify a fall back value? This would need to be documented really well, otherwise I can already see all the support issues being created because users unfamiliar with the bash like syntax remove the - character after the colon because they think it's part of the fall back value instead of the actual bash syntax.

[lightninglabs-deploy]

@mohamedawnallah, remember to re-request review from reviewers when ready

[ProofOfKeags]

I'm going to unsubscribe from this since I have approved. Please ping me if my input is needed again.

[mohamedawnallah]

@guggero I'd like to discuss this issue again anytime you're available. I agree with your thoughts That the changes that this PR proposes add much complexity compared to the value it added. I made the fallback value since the main issue #8295 mentioned that in the demo rpcuser=${BTC_RPCUSER:-btc} also that bitcoind does support this fall-back value and To be honest I don't have any use case in mind in the meantime for allowing to specify a fall-back value. That said I'm very open to suggestions e.g. having one format something like ${ENVIRONMENT_VARIABLE} would solve the issue?

What's the use case you see for allowing to specify a fallback value?

cc: @ProofOfKeags

[guggero]

@guggero I'd like to discuss this issue again anytime you're available. I agree with your thoughts That the changes that this PR proposes add much complexity compared to the value it added. I made the fallback value since the main issue #8295 mentioned that in the demo rpcuser=${BTC_RPCUSER:-btc} also that bitcoind does support this fall-back value and To be honest I don't have any use case in mind in the meantime for allowing to specify a fall-back value. That said I'm very open to suggestions e.g. having one format something like ${ENVIRONMENT_VARIABLE} would solve the issue?

Okay, I guess it could be useful to have a fall back value. Although I think that can also lead to hard-to-find errors if you're not expecting that.

So I guess my main ask here is to simplify the regular expression, so it's easier to read and understand. My suggestion would be to have an expression per supported format, then have a switch statement that checks which one matches. Then the extraction will be easier to understand as well.

[mohamedawnallah]

So I guess my main ask here is to simplify the regular expression, so it's easier to read and understand. My suggestion would be to have an expression per supported format, then have a switch statement that checks which one matches. Then the extraction will be easier to understand as well.

This looks great! I'm gonna submit a patch and will let you know. Thanks for sharing!

[coderabbitai[bot]]

[!IMPORTANT]

Auto Review Skipped

Auto reviews are limited to the following labels: llm-review. Please add one of these labels to enable auto reviews.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[mohamedawnallah]

Thanks a lot, @guggero for your suggestion. It is now much simpler. I've addressed all of your feedback!