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

Validate that the secrets and config required by the schema exactly match defined secrets #1173

Open
alecthomas opened this issue Apr 4, 2024 · 6 comments
Labels
good first issue Good for newcomers

Comments

@alecthomas
Copy link
Collaborator

alecthomas commented Apr 4, 2024

Error for missing entries, and warn for entries that are defined but not required?

- [ ] Adding secrets/configs validates against the list of modules defined in the ftl-project.toml file - quick scan to just load the ftl.toml and cross-check - [ ] Add a `--force, -f` flag to override this check - [ ] Then a future PR to improve schema validation - [ ] Error if a module references a config/secret that is not defined - [ ] Warn if a config/secret is defined but is not used

^^ this is likely all wrong

@github-actions github-actions bot added the triage Issue needs triaging label Apr 4, 2024
@alecthomas alecthomas mentioned this issue Apr 4, 2024
@alecthomas alecthomas added next Work that will be be picked up next good first issue Good for newcomers and removed triage Issue needs triaging labels Apr 8, 2024
@wesbillman
Copy link
Collaborator

@deniseli This is probably a good issue to take a look at since you're working in the ftl-project world now. (cc: @worstell)

@deniseli deniseli self-assigned this Apr 26, 2024
@github-actions github-actions bot removed the next Work that will be be picked up next label Apr 26, 2024
@deniseli
Copy link
Contributor

Does this all sound right?

  • ftl config set should warn if the value being added is not referenced in any modules yet.
  • ftl config unset should fail if the value being removed is still referenced in a module, unless --force or -f is present, in which case we should warn instead of erroring.
  • The force flag shorthand should be -f, not -F.
  • The 3 commands dev, build, and deploy (which call instantiate a new buildengine) should all log warnings (for unnecessary configs/secrets) and errors (for configs/secrets required but not provided).

@alecthomas
Copy link
Collaborator Author

I don't think 1 is necessary, it will be true most of the time. I think most of the validation should be done at schema validation/build time.

@deniseli
Copy link
Contributor

Perfect, I'll plan to build everything except the first thing. Thanks!

@alecthomas
Copy link
Collaborator Author

alecthomas commented May 23, 2024

Let's defer this until we have all the changes (eg. #1473, #1545, #1548) around secret management merged in. I think the solution to this will become clearer at that point. My suspicion is that we'll end up validating this when a deployment is pushed to the controller.

@matt2e
Copy link
Collaborator

matt2e commented Jul 21, 2024

Maybe this is going to be solved with this one then: #1906

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants