DefangLabs / defang

Defang CLI and sample projects. Defang is a radically simpler way for developers to develop, deploy, and debug cloud applications.
https://defang.io
MIT License
38 stars 9 forks source link

Config interpolation implementation #721

Closed raphaeltm closed 1 month ago

raphaeltm commented 1 month ago

@nullfunc and I were having a discussion about the new interpolation feature which allows you to bring config values into env vars.

There's a thing that keeps coming up which is that, while it would be ideal to dynamically interpolate values at runtime, that likely isn't feasible for various reasons.

To prevent sensitive values "leaking" it also isn't ideal to interpolate at build time, because we can't be certain that we can prevent those values from ending up in logs in a cloud account.

To me the ideal, given the limitations, is the following.

Imagine a compose file with the following environment variable:

env:
  - CONNECTION_STRING=proto://user:${PASSWORD}@host:1234

Let's say a user run defang config set PASSWORD=potato. That command should just update the PASSWORD config val as usual.

But now, the user is ready to deploy. I think that this should happen:

  1. The CLI should build a little dependency graph, figuring out which env vars depend on which config vals for interpolation.
  2. If the user is in an interactive session and they have the required permissions to read config values in their provider (could they run aws ssm get-parameter --name "/Defang/projectname/beta/PASSWORD" --with-decryption for example)
  3. Then the CLI should pull all the required values, interpolate the value and set the env vars that depend on interpolation as config vals.
  4. There could also be a command like defang config interpolate which could be run by users with the same conditions. Maybe the CLI runs this automatically every time defang config set gets run?
  5. Metadata should show if any interpolated config is older than its dependencies: if so, the CLI should throw an error saying that a user with appropriate permissions needs to rerun defang config interpolate or run a defang compose up manually.
  6. We should store the structure (i.e. proto://user:${PASSWORD}@host:1234 or its hash) in the config's metadata. When running a deployment, we should check if the stored structure matches the structure in the current compose file. If not, we should throw an error saying that a user with appropriate permissions needs to rerun defang config interpolate or run a defang compose up manually.

I assume there are cases where this might not work, but I think it covers a lot of them? I can't think of anything right now where this would fail.

I know @lionello has been hesitant to do anything where we read sensitive config values, for good reason. But without being able to do dynamic interpolation at runtime right before booting up a container... I can't think of anything else. And if we force those two conditions:

  1. interactive sessions only
  2. only users who directly have access to the cloud account and the encrypted values in it

Then they could just as easily run something like aws ssm get-parameter --name "/Defang/projectname/beta/PASSWORD so it's not like that value is more secure in their context.

raphaeltm commented 1 month ago

Tagging @lionello and @nullfunc

lionello commented 1 month ago

The issue text conflates the DX with implementation. Can we rewrite this and focus on the DX? Specifically, I'm not clear what the exact DX-delta is compared to what we have now. @raphaeltm

Adding a CLI command (interpolate) that I've not seen anywhere else is not ideal. Not impossible, but I have the feeling we should be able to solve this without an extra command.

raphaeltm commented 1 month ago

The issue text conflates the DX with implementation. Can we rewrite this and focus on the DX? Specifically, I'm not clear what the exact DX-delta is compared to what we have now. @raphaeltm

Adding a CLI command (interpolate) that I've not seen anywhere else is not ideal. Not impossible, but I have the feeling we should be able to solve this without an extra command.

DX is equivalent to the new stuff, minus the interpolate command. I haven't seen that either anywhere. Looking back, I guess we don't need it: we can just tell people to defang compose up if they want their changes applied. My thinking was you might have an admin who interactively updates some values in an interactive session, but then someone else is deploying through CD and those changes wouldn't get applied. But asking them to run defang compose up seems reasonable.

I can rewrite it around DX, but my initial reason for writing it was the implementation details that @nullfunc and I were discussing (i.e. should the CLI be able to pull encrypted config to update interpolated values).

nullfunc commented 1 month ago

Ref: Original PR for interpolation.

Closing this. Unlikely to implement, revisit if the DX for this become an issue for users.