go icon indicating copy to clipboard operation
go copied to clipboard

services/friendbot: allow configuring friendbot with env vars and set some sensible defaults

Open leighmcculloch opened this issue 4 years ago • 1 comments

PR Checklist

PR Structure

  • [ ] This PR has reasonably narrow scope (if not, break it down into smaller PRs).
  • [ ] This PR avoids mixing refactoring changes with feature changes (split into two PRs otherwise).
  • [ ] This PR's title starts with name of package that is most changed in the PR, ex. services/friendbot, or all or doc if the changes are broad or impact many packages.

Thoroughness

  • [ ] This PR adds tests for the most critical parts of the new functionality or fixes.
  • [ ] I've updated any docs (developer docs, .md files, etc... affected by this change). Take a look in the docs folder for a given service, like this one.

Release planning

  • [ ] I've updated the relevant CHANGELOG (here for Horizon) if needed with deprecations, added features, breaking changes, and DB schema changes.
  • [ ] I've decided if this PR requires a new major/minor version according to semver, or if it's mainly a patch change. The PR is targeted at the next release branch if it's not a patch change.

What

Allow configuring friendbot with environment variables in addition to via a .cfg file. Remove the default ./friendbot.cfg value from the --config option, so that to use a file the user must specify it explicitly.

Why

It is inconvenient to deploy friendbot in some environments because it is not configurable via environment variables. While in K8s environments it is extremely convenient to mount a cfg file, that is not the case in many other PaaS environments where environment variables are the most common approach to configuration and secret injection.

Environment variables have become common place because of their simplicity, and things like the 12factor manifesto.

We should make it easy to deploy friendbot. It's a remarkably simple service and its deployment should be so too.

This arose out of my own attempt to deploy friendbot.

Known limitations

This is in someway a risky change given that someone could have deployed friendbot with a .cfg file mounted in the default location. If they upgrade to a version of friendbot after this they'll see an error with a list of configurations they haven't set. Given that there are few, that we know of, deployments of friendbot, this is a very low risk. In any modern blue-green or canary deployment this failure to boot should be identified quickly without a production issue, so I think the risk is low. Also, given friendbot is only used in test environments, the risk of impact is significantly low.

I checked SDF's deployment to check if it explicitly specifies the location of the config file, and it does, so this change should have no impact on the SDF testnet deployment.

leighmcculloch avatar Nov 09 '21 04:11 leighmcculloch

You can consider using ConfigOption.

Ah of course. I can't believe I forgot this existed. I've used it in the recoverysigner and webauth applications too. I'll redo this using that. Thanks for pointing this out.

leighmcculloch avatar Nov 10 '21 22:11 leighmcculloch

@leighmcculloch do you plan to keep working on this one ?

tsachiherman avatar Dec 19 '22 15:12 tsachiherman