rusackas commented on code in PR #40654:
URL: https://github.com/apache/superset/pull/40654#discussion_r3365924930
##########
superset/cli/update.py:
##########
@@ -110,17 +110,37 @@ def update_api_docs() -> None:
help="An optional previous secret key, if PREVIOUS_SECRET_KEY "
"is not set on the config",
)
-def re_encrypt_secrets(previous_secret_key: Optional[str] = None) -> None:
[email protected](
+ "--engine",
+ "-e",
+ "target_engine_name",
+ required=False,
+ type=click.Choice(sorted(ENCRYPTION_ENGINES), case_sensitive=False),
+ help="Re-encrypt all app-encrypted fields with this encryption engine "
+ "(e.g. 'aes-gcm' for authenticated encryption). The SECRET_KEY is "
+ "unchanged. Take a metadata-DB backup first, then set "
+ "SQLALCHEMY_ENCRYPTED_FIELD_ENGINE to the same value and restart.",
+)
+def re_encrypt_secrets(
+ previous_secret_key: Optional[str] = None,
+ target_engine_name: Optional[str] = None,
+) -> None:
previous_secret_key = previous_secret_key or current_app.config.get(
"PREVIOUS_SECRET_KEY"
)
- if previous_secret_key is None:
+ target_engine = (
+ ENCRYPTION_ENGINES[target_engine_name] if target_engine_name else None
+ )
+ if previous_secret_key is None and target_engine is None:
click.secho(
- "No previous secret key provided; nothing to re-encrypt.",
+ "No previous secret key or target engine provided; nothing to
re-encrypt.",
fg="yellow",
)
return
- secrets_migrator = SecretsMigrator(previous_secret_key=previous_secret_key)
+ secrets_migrator = SecretsMigrator(
+ previous_secret_key=previous_secret_key,
+ target_engine=target_engine,
+ )
try:
stats = secrets_migrator.run()
except Exception as exc: # pylint: disable=broad-except
Review Comment:
Good call — added in 0fe86ea. `tests/integration_tests/cli_tests.py` now
covers the `--engine` CLI path with three tests: that `--engine aes-gcm`
resolves to `AesGcmEngine` and is passed through to `SecretsMigrator` (with
`previous_secret_key=None`), that the option is case-insensitive (`AES-GCM`
resolves the same), and that an unrecognized engine name yields a Click usage
error listing the valid choices. I patched the whole `SecretsMigrator` rather
than just its `run` method so the assertion can inspect the constructor kwargs
without needing real DB introspection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
