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

fix: AdminService, config & secret CLI refactor, --op=VAULT deprecated #1565

Merged
merged 5 commits into from
Jun 4, 2024

Conversation

safeer
Copy link
Contributor

@safeer safeer commented May 23, 2024

Fixes #1473 by providing an AdminService and refactoring cmd_config.go and cmd_secret.go to use that service. The following CLI changes are included that may break existing clients:

  • ftl config and ftl secret now require a running controller
  • ftl secret set <ref> --op=VAULT is no longer supported
  • ftl-controller and ftl dev accept a 1Password vault using --opvault=VAULT
  • ftl secret set <ref> --op is supported, where op is a bool flag that requires the controller to have an --opvault

@safeer safeer requested a review from alecthomas as a code owner May 23, 2024 21:30
@safeer safeer requested review from a team and matt2e and removed request for a team May 23, 2024 21:30
@safeer safeer marked this pull request as draft May 23, 2024 21:30
@ftl-robot ftl-robot mentioned this pull request May 23, 2024
backend/controller/admin.go Outdated Show resolved Hide resolved
backend/controller/admin.go Outdated Show resolved Hide resolved
backend/protos/xyz/block/ftl/v1/ftl.proto Outdated Show resolved Hide resolved
backend/controller/admin.go Outdated Show resolved Hide resolved
backend/protos/xyz/block/ftl/v1/ftl.proto Outdated Show resolved Hide resolved
backend/protos/xyz/block/ftl/v1/ftl.proto Outdated Show resolved Hide resolved
@safeer safeer force-pushed the config-refactor-adminservice branch 2 times, most recently from 10b4956 to 678853f Compare May 29, 2024 20:37
backend/controller/admin.go Outdated Show resolved Hide resolved
safeer added a commit that referenced this pull request May 31, 2024
…ers, adds `--opvault=VAULT` controller flag (#1604)

This change is a precursor to #1565. It removes the notion of
`MutableProviders` and requires a provider be specified when calling
mutable functions in the manager. It also allows a 1Password vault to be
set at the controller & main.go levels by using `--opvault=VAULT`. To
operate on 1P secrets via CLI, continue to use `ftl secret ...
--op=VAULT`, but this should eventually change to a bool flag.
@safeer safeer force-pushed the config-refactor-adminservice branch 2 times, most recently from c5343d0 to 2ad12ee Compare June 3, 2024 20:51
@safeer safeer changed the title WIP: AdminService that consolidates config/secret CRUD calls fix: AdminService, config & secret CLI refactor, --op=VAULT deprecated Jun 3, 2024
@safeer safeer force-pushed the config-refactor-adminservice branch from 2ad12ee to 95706f6 Compare June 3, 2024 21:10
@safeer safeer marked this pull request as ready for review June 3, 2024 21:14
@safeer safeer requested review from gak and wesbillman June 3, 2024 21:14
Copy link
Contributor

@gak gak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +1 to +2
package controller

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To follow the pattern we have now, maybe this should go in an admin package here in the controller?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we should add some tests for this new admin :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressing in a separate PR :)

backend/controller/controller.go Show resolved Hide resolved
@alecthomas
Copy link
Collaborator

Should this go in?

@safeer safeer force-pushed the config-refactor-adminservice branch from 95706f6 to 55441b0 Compare June 4, 2024 17:21
@safeer safeer merged commit fba50cf into main Jun 4, 2024
30 of 31 checks passed
@safeer safeer deleted the config-refactor-adminservice branch June 4, 2024 17:31
@matt2e matt2e added the approved Marks an already closed PR as approved label Jun 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Marks an already closed PR as approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor how config/secrets are managed so that all operations go through the controller
5 participants