PaaS, CI, and production-friendly config; affects [jira]

[paulmelnikow]

This implements the configuration mechanism I described in #2621. The heavy lifting is delegated to node-config with a minor assist from dotenv.

One open is how we deal with Docker. In the old version, the secrets are baked into the image. Now that they can be read from the environment at runtime, it probably would be better to do that. Alternatively we could pull them from the build-time environment into the image. Or both! @calebcartwright perhaps you have thoughts about how we could deal with that.

Some tasks pre-merge:

• Set GH_TOKEN in staging and CI (it was GITHUB_TOKEN)
• Sort out docker
• Update production deploy task, adding .env setting NODE_CONFIG_ENV

Some housekeeping post-merge:

• Prepare the production config file and do the first deploy
• Start a changelog for self hosting
• Inject secrets via the server
• Inject config into trace

Close #1806 #2621

[shields-ci]

	Warnings
⚠️	This PR modified the server but none of its tests. That's okay so long as it's refactoring existing code.

	Messages
📖	✨ Thanks for your contribution to Shields, @paulmelnikow!
📖	Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS against 589cc78

[chris48s]

nice work on this

[chris48s]

One comment I'd have here is that if you're a self-hosting user and you've got all your secrets set up in private/secret.json, upgrading to this revision or later is going to break your install 🔧

I don't really have a clear idea how self-hosting users are managing their installs, but is there a way we can communicate this change well to a self-hosting user? As you've already flagged, one tool available is documentation, although maintaining a changelog when we don't really do releases/tags is a challenge. Maybe we should be doing releases too 🤔 . Perhaps we could also do something like in the bootstrap process could we check for the existence of private/secret.json and if it exists throw an exception with a message saying "you need to move your config to env vars/yaml"?

Also, if everyone has to re-write their config file anyway, maybe we should just drop this

shields/services/jira/jira-base.js

Lines 14 to 25 in 5233a25

	if (serverSecrets.jira_user || serverSecrets.jira_username) {
	/*
	for legacy reasons we still allow
	jira_username, jira_password
	but prefer
	jira_user, jira_pass
	*/
	options.auth = {
	user: serverSecrets.jira_user || serverSecrets.jira_username,
	pass: serverSecrets.jira_pass || serverSecrets.jira_password,
	}
	}

in this PR as there isn't really any backwards-compatibility to preserve at this point.

[paulmelnikow]

I don't really have a clear idea how self-hosting users are managing their installs, but is there a way we can communicate this change well to a self-hosting user? As you've already flagged, one tool available is documentation, although maintaining a changelog when we don't really do releases/tags is a challenge. Maybe we should be doing releases too 🤔 . Perhaps we could also do something like in the bootstrap process could we check for the existence of private/secret.json and if it exists throw an exception with a message saying "you need to move your config to env vars/yaml"?

Yea, I'm a big fan of failing loudly. That's what's happening here: https://github.com/badges/shields/pull/2626/files#diff-e2e5bd472244fbfb4a40d38300d42534R6

This PR should make it possible for PaaS users to use the shields repo as-is, without forking or checking in any files. If we can solve that for Docker users, even better. Maybe we can publish an official docker image. Regardless, tagging releases probably would be a good idea.

Besides failing loudly and linking to a changelog, I'm not sure we have any other way to communicate this. The changelog will provide this answer going forward, though: it gives a single place to look when you're updating your self-hosted install.

I'd like to provide a second path for self-hosting someday, which is for advanced users who want to do something custom. We could ship an npm package with the server logic. Then the user can create their own project and use shields-server as a dependency. They can add their own services, and maybe even pick and choose which shields services should be enabled. This server package would be an ordinary library, and could follow semver. (Obviously this is not going to be tackled here.)

[chris48s]

Yea, I'm a big fan of failing loudly. That's what's happening here: https://github.com/badges/shields/pull/2626/files#diff-e2e5bd472244fbfb4a40d38300d42534R6

oh right yes sorry - you've already done that exact thing and I missed it 🤦‍♂️

[calebcartwright]

There's some changes in here to parts of the ecosystem that I'm admittedly not familiar with (like deployments), but it LGTM and I don't have any additional feedback!

[paulmelnikow]

@chris48s Do you want to have another look at this?

[chris48s]

Looks great. The only outstanding point from me is lets remove the block that references jira_username and jira_password. I think privateConfigSchema is filtering them out anyway now

[paulmelnikow]

This pull request introduces 1 alert when merging aa3c0fd into 566b58a - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

---

Comment posted by LGTM.com

[chris48s]

👍

[shields-deployment]

This pull request was merged to master branch. This change is now waiting for deployment, which will usually happen within a few days. Stay tuned by joining our #ops channel on Discord!

After deployment, changes are copied to gh-pages branch: