Extract secrets out of config.json and into a secrets.json #1374
Conversation
There was a problem hiding this comment.
The general motivation and idea for this makes sense, however:
- iirc there is a precommit hook or sth that attempts to create the config.json out of the template on first launch, prob need to adjust that
- the documenation (wiki) needs to be adjusted. since the wiki is part of the code base, can do that right away in this PR as well
- one main reason we had the config outside of github control so far was that we are able to change it without needing to do a PR, merging that through branches and making a release. We must remain able to "oh shit, we need to deactivate this feature, its on rampage!" without having to go through a release. Im not sure if this new approach has that in mind. Maybe a hybrid would work as well. I guess you could argue that in such a case you would just "force" push a commit straight on the master branch, forcing the config change without proper a proper release flow. That technically works but then develop and master history diverge, which will cause unecessary trouble. Anyways, sth to think about.
|
hey @Zabuzard , thanks for pointing that out. I did the changes to the docs, couldn't find anything in the pre-commit script for the config.json. On your last point, if we need to really update a config, we should do it via a PR and keep that tracked (better for reverts anyway). It's a standard practise in most organisations and I want to uphold this, especially for our junior members jumping into TJ Bot. Like you mentioned, we can just force push to master in a serious event. |
Zabuzard
left a comment
Zabuzard
left a comment
There was a problem hiding this comment.
(seems it was the cp call in the devcontainer i was thinking about, so u found it)
im not really into the idea of having to go through a PR for an important config change like disabling a feature that goes on rampage.
that said, that only happened twice so far, so we are probably good.
do you set it up in a way that in worst case someone could login to the VPS and edit the file there manually (obviously being auto-overriden on next master push)? if so, then we should be good.
and yeah, straight push on master would work as well.
long story short, ill approve it and we can just give it a try and see if we run into issues or not. the benefits are definitely strong, so lets go :)
|
Not at all, it's now bundled into the jar :D |
There was a problem hiding this comment.
Sure, I guess ¯\(ツ)/¯
Would simplify some things, and it's generally saner approach. You shouldn't need to ssh into prod to do any live editing as standard procedure.
Since we chose to have a linear history for making things simple and clean, and we only have master and dev branch, there are some drawbacks to that approach. If you want to quickly make some config changes, and merge to master, you have to merge everything before it. Alternative is diverging history, or rewriting history in a public branch. So that's where Zabu's point is valid, not sure if that scenario is simpler than quickly editing a file on vps.
Executing an emergency release cleanly, without disruption for other contributors or getting in hard to recover state.. is not simple, git can be messy if you are not experienced. Not only that it is tricky, but force-pushing master is pretty dangerous as well.
Again, not a big deal, since those situations are rare, and generally handled by experienced people that can deal with them efficiently. We never had any significant incident that we haven't dealt with expediently. Pretty stable, high availability so far.
tj-wazei
dismissed stale reviews from marko-radosavljevic and Zabuzard
via
5c0eca4
|
Updated the config.json to match what's on the VPS. And on the VPS I have added the secrets.json for dev/master |
Zabuzard
left a comment
Zabuzard
left a comment
There was a problem hiding this comment.
a problem i notice is that we have 3 separate use cases and they have a need for separate configs in some cases:
- prod bot
- test bot
- local developer setup
in most cases the config is identical but in some its different on purpose. i am not sure if we can represent that well with the setup.
maybe it wont be a problem but it is a downside i just noticed
| @@ -1,111 +1,45 @@ | |||
| { | |||
| "databasePath": "local-database.db", | |||
| "databasePath": "/home/bot/database/database.db", | |||
There was a problem hiding this comment.
problematic for users local setup? especially if not linux id imagine
There was a problem hiding this comment.
How do you suggest doing this? Updating docs? Changing the path? With a git-ops route, whatever is versioned controlled is what will be run on the VPS. Unless, I move that into secrets since that's technically that's different between environments.
There was a problem hiding this comment.
Right here's what I've done. I've tried to make this backward compatible so you can still use a disk based config file (which fits your local dev example).
This feels like a sweet spot/middle ground for what you're asking. Let me know what you think.
| "mode": "AUTO_DELETE_BUT_APPROVE_QUARANTINE", | ||
| "reportChannelPattern": "commands", | ||
| "botTrapChannelPattern": "bot-trap", | ||
| "mode": "AUTO_DELETE_AND_QUARANTINE", |
There was a problem hiding this comment.
mh, that one doesnt fit the local use-case that well. namely people working on the feature, for example testing new scam code, they shouldnt get autoquarantined by the bot
|
Pull-request
Changes
Closes Issue: NaN
Description
The problem we currently have is that whenever there is a config change, somebody with access to the VPS has to manually update the
config.jsonso that we can add config the bot needs to start.This for obvious reasons doesn't make sense for non-secret values (e.g. API URLs, channel names etc.) thus this PR separates anything secretive like API keys into a
secrets.json.config.jsonhas been moved underapplication/src/main/resourcesGoal here is that we only touch the VPS when needed and not for trivial changes that can be git-ops.
Before this PR can be merged we need to:
config.json.templatetoconfig.jsonand copy over the values from the VPS to this repository.This has not yet been done as I want confirmation that nothing else secretive remains in the
config.json.template!Post merge:
secrets.jsonon the VPS containing just the secrets that were removed from theconfig.jsonin this PR.