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

feat: import/export of config and secrets #1982

Merged
merged 11 commits into from
Jul 8, 2024
Merged

Conversation

matt2e
Copy link
Collaborator

@matt2e matt2e commented Jul 5, 2024

closes #1948

Usage:

Exporting
Prints json of all secrets to stdout:
ftl secret export

Exports all inline secrets to a json file
ftl secret export --inline > exported.json

Importing
Imports json from a file:
ftl secret import --inline exported.json

Migrating
Migrates inline secrets to 1Password:
ftl secret export --inline | ftl secret import --op --opvault <vauld id>

JSON Format

{
    "<module>.<name>": <secret>,
    ...
}

@matt2e matt2e requested a review from alecthomas as a code owner July 5, 2024 03:07
@matt2e matt2e requested review from a team and worstell and removed request for a team July 5, 2024 03:07
@matt2e matt2e force-pushed the matt2e/secret-import-export branch from 862ed85 to cee8568 Compare July 5, 2024 03:07
@ftl-robot ftl-robot mentioned this pull request Jul 5, 2024
@matt2e matt2e changed the title faet: import & export secrets feat: import & export secrets Jul 5, 2024
cmd/ftl/cmd_secret.go Show resolved Hide resolved
cmd/ftl/cmd_secret.go Outdated Show resolved Hide resolved
cmd/ftl/cmd_secret.go Outdated Show resolved Hide resolved
cmd/ftl/cmd_secret.go Outdated Show resolved Hide resolved
common/configuration/manager.go Outdated Show resolved Hide resolved
in.ExpectError(
in.ExecWithOutput("ftl", "deploy", "notexportedverb"),
"call first argument must be a function but is an unresolved reference to echo.Echo, does it need to be exported?"),
in.ExecWithExpectedError("call first argument must be a function but is an unresolved reference to echo.Echo, does it need to be exported?",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the change? The previous function was generalisable to any Action.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I went in circles a bit on this:

  • When I used ExecWithOutput I expected to be able to use the output. It didn't indicate that it was only capturing the output in case of an error
  • I then tried to make ExecWithOutput work for both success success and error cases:
    • For success cases, call a func with the output (like the replacement ExecWithOutput action I added)
    • For error cases throw the error with the output (old behaviour)
  • It then felt a bit messy for the error case so I tried cleaning it up.

I can go back to ExecWithOutput handling both cases. Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's fine to introduce new functionality, but that doesn't seem like a reason to remove in.ExpectError()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeh I see what you mean. I've moved them back into one func called ExecWithOutput which works like before for the error case (used in conjunction with ExpectError and an empty capture func).

@alecthomas
Copy link
Collaborator

What's the behaviour when restoring, I assume it will just write the values in the dump, not remove any extra keys in the target?

@alecthomas
Copy link
Collaborator

FYI @AlexSzlavik

@matt2e
Copy link
Collaborator Author

matt2e commented Jul 5, 2024

What's the behaviour when restoring, I assume it will just write the values in the dump, not remove any extra keys in the target?

That's right. Existing refs that aren't in the dump aren't touched. So it's less of a "restore" and more of an "import"

@matt2e matt2e force-pushed the matt2e/secret-import-export branch from 093aaea to 7d5d345 Compare July 5, 2024 05:51
@matt2e matt2e changed the title feat: import & export secrets feat: import/export of secrets and secrets Jul 5, 2024
@matt2e matt2e changed the title feat: import/export of secrets and secrets feat: import/export of config and secrets Jul 5, 2024
@matt2e matt2e requested a review from alecthomas July 5, 2024 06:00
@matt2e matt2e force-pushed the matt2e/secret-import-export branch from 06ca81d to f9a77be Compare July 8, 2024 03:18
@matt2e matt2e merged commit e6d8499 into main Jul 8, 2024
47 checks passed
@matt2e matt2e deleted the matt2e/secret-import-export branch July 8, 2024 03:24
matt2e added a commit that referenced this pull request Jul 12, 2024
closes #1947
closes #1772
Creates an entry in 1Password called `<projectname>.secrets` with each
secret stored in a password field called `<modulename>.<secretname>`
Username is set to a warning string as that is presented at the top of
the 1Password UI.

This will break existing secrets stored in 1Password. Migration can be
done using commands made available in:
#1982
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 this pull request may close these issues.

Add a way to dump & restore secrets
2 participants