CPP-2497 Support setting custom Hako environments in containerised-app plugins
#936
Conversation
This allows flexibility for when apps want to deploy to custom environments. We drop the old `multiregion` option as part of this change as it can now be better expressed by setting the `hakoProductionEnvironments` option, rather than carving out a special case for the ft-com environments. We could have preserved backwards-compatibility with this change – or even continued supporting the option – but it feels better to rip the bandage off now whilst not many apps have it set and to encourage one way of defining multiple environments.
ivomurrell
requested review from
ft-dependency-auto-merger and
a team
as code owners
| @@ -21,6 +21,7 @@ | |||
| }, | |||
| "devDependencies": { | |||
| "@jest/globals": "^29.7.0", | |||
| "@relmify/jest-serializer-strip-ansi": "^1.0.2", | |||
There was a problem hiding this comment.
non-blocking: should we use an exact pinned version of this package? Given that it has not been maintained in a while and only has one author. Just to reduce our supply chain attack surface (thankfully it has no deps at the moment)
There was a problem hiding this comment.
I don't think that's necessary for a dev dependency. If a security bug is found in the package presumably it would be easier to update if the version isn't pinned?
| @@ -5,13 +5,16 @@ import * as fs from 'node:fs/promises' | |||
| import type { Valid } from '@dotcom-tool-kit/validated' | |||
There was a problem hiding this comment.
thought: should we also add a test for substituting non-string values?
| environments: | ||
| - ft-com-prod-eu | ||
| - ft-com-prod-us | ||
| environments: !toolkit/option '@dotcom-tool-kit/containerised-app.hakoProductionEnvironments' |
There was a problem hiding this comment.
question: Do we need to have a discussion around how and where we store this environment config? It's currently hardcoded in the Hako plugin but since we are now allowing free user input, I think it would make sense to have a place to store them independently of plugin code changes. Also, I expect eng users would naturally not know what values are supported and from what I can see atm, we are not doing much error-handling in the Hako deploy task for bad user input
There was a problem hiding this comment.
I'd be tempted to leave this for now as the list of environments is likely to change a lot. We don't have a specific error for a nonexistent environment but the hako command will error with a descriptive message that says the environment doesn't exist.
There was a problem hiding this comment.
aha this might be tangential to the original question but this highlighted to me that we'll still get errors if we pass alternative environments to the containerised-app plugin as the HakoDeploy task will fail to validate them. i'll update the HakoDeploy schema in a separate revision to allow any string (or something similar that will still allow the rest of the mapping logic to work.)
back to the original point, what kind of environment config are you thinking of here, Shola?
Description
This allows flexibility for when apps want to deploy to custom environments.
We drop the old
multiregionoption as part of this change as it can now be better expressed by setting thehakoProductionEnvironmentsoption, rather than carving out a special case for the ft-com environments. We could have preserved backwards-compatibility with this change – or even continued supporting the option – but it feels better to rip the bandage off now whilst not many apps have it set and to encourage one way of defining multiple environments.Closes #892.
Checklist:
feat(circleci): add support for nightly workflows,fix: set Heroku app name for staging apps too