Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor how config/secrets are managed so that all operations go through the controller #1473

Open
alecthomas opened this issue May 14, 2024 · 1 comment · May be fixed by #1565
Open

Refactor how config/secrets are managed so that all operations go through the controller #1473

alecthomas opened this issue May 14, 2024 · 1 comment · May be fixed by #1565
Assignees

Comments

@alecthomas
Copy link
Collaborator

alecthomas commented May 14, 2024

Currently the controller and CLI both operate directly on ftl-project.toml, as there was previously no mechanism for propagating config down to modules. But now that we have ModuleContext we should change this. We'll need to for the production launch to support AWS anyway, but also because it's quite annoying.

This will entail adding new gRPC endpoints for secrets/config. Another consideration is that currently the CLI allows users to select a different provider (eg. 1Password, Keychain, etc.) per secret. To preserve this behaviour the desired provider will need to be passed to the Controller. However, the valid set of providers will vary depending on the target environment, so the Controller may error if an invalid provider is specified. eg. only ASM will be valid in production, so setting --keychain must error.

We want to start separating out "backend traffic" from "admin traffic", so to that end this new endpoint should be part of a new AdminService gRPC service rather than being part of the ControllerService.

@github-actions github-actions bot added the triage Issue needs triaging label May 14, 2024
@deniseli deniseli added next Work that will be be picked up next and removed triage Issue needs triaging labels May 15, 2024
@safeer safeer self-assigned this May 21, 2024
@github-actions github-actions bot removed the next Work that will be be picked up next label May 21, 2024
@deniseli
Copy link
Contributor

When we pick this up, should we also think about possibly renaming the existing structs in backend/controller/controller.go named CommonConfig and Config? I don't have strong feelings at the moment, but it could get confusing if we have a ton of different data types in there all named variations of "Config". That said, I don't find the cronjob.Config reference confusing since its package name clearly delimits its scope, so possibly this won't be an issue at all.

gak added a commit that referenced this issue Jun 3, 2024
 (#1575)

Please note that this is not integrated into the CLI and controller to
prevent potential conflicts with
#1473

Related issue: #1545
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants