You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/08/23 07:39:07 UTC

[GitHub] [superset] EBoisseauSierra opened a new issue #16395: Can't import an instance export on a clone securely (`superset import-dashboards`)

EBoisseauSierra opened a new issue #16395:
URL: https://github.com/apache/superset/issues/16395


   ### Expected results
   
   I want to be able to “clone” a Superset instance via CLI only: first export its configuration (and version-control it), then import and deploy it in another (empty) Superset instance.
   I want to do it securely, so that — notably — secrets (I'm thinking of db connection details) are not stored in plain text.
   
   I proceed the following way:
   
   1. Preparing the export on the instance to be cloned:
     * `superset export-dashboards` (note that in the export, `databases/*.yaml` don't contain the full db URI, but the pw is masked instead).
     * track the archive on a repo
   2. “Reinstalling” the instance on the clone:
     * `superset set-database-uri  --database_name <> --uri <>` (so that secrets — contained in the URI — can be passed securely)
     * `superset import-dashboards -p <the archive tracked on a repo>`
   
   I would then expect all databases, data sources, dashboards, and charts to be recreated in the clone just like they are in the original.
   
   ### Actual results
   
   However, when going through this process, only the db/data sources are created (i.e. `superset import-dashboards` is not actually  working — nor throwing an error†).
   
   ### Workarounds
   
   I have found three workarounds, but they are not satisfactory:
   
   1. Replace the mask (`XXXXXX`) with the actual password in the `databases/*.yaml` files (`sqlalchemy_uri` filed) of the export: this works (and the `superset set-database-uri` step can then be omitted)… yet isn't satisfactory, as it means that the password is stored in plain text — and if it is rotated frequently, it's a pain as you need to `sed` the YAML files to replace the pw at the time of the export with the latest password.
   2. Replace the db UUID with the one of the clone back-end db:
     1. Set-up the database connection (using `superset set-database-uri`) — it's good as this can be done “securely”,
     2. Lookup for the UUID of this record in the `dbs` table of the back-end database (`SELECT uuid FROM dbs WHERE database_name = '<value of the 'database_name' key in the databases/*yaml file you've just imported>';`),
     3. Replace the current value of the `uuid` key in `databases/<db_name>.yaml`, and of the `database_uuid` in the `datasets/<db_name>/*.yaml` files.
     4. Proceed with the import via `superset import-dashboards` using the tweaked Superset instance export.
   3. Use the GUI: in that case, an error message is displayed, yet without giving much details onto why it failed. Plus I'd like to automate the process, so GUI isn't an option in my case.
   
   ### my 2 cents
   
   I looks like `superset import-dashboards` fails to import the charts/dashboards/etc. because the UUID of the db connection record in the back-end database (`dbs` table) isn't the same in the export vs. in the clone.
   
   I can imagine 2 reasons why (and I assume a quick lookup in the codebase would give me the answer but haven't had time to do it yet):
   * the UUID is computed using the value of the `id` column of the record,
   * the UUID is computed using the value of the `created_on` or `changed_on` column of the record.
   
   Because these aren't part of the export (cf. `databases/*.yaml`), they are — and this makes sense — computed again when reinstantiating the db connection on the clone. Yet this yield to a new UUID that is likely causing the `superset import-dashboards` to fail..
   
   ### Environment
   
   (please complete the following information):
   
   - superset version: `1.2.0`
   - python version: `3.8.10`
   - any feature flags active:
     - `"VERSIONED_EXPORT": True`
   
   ### Checklist
   
   Make sure to follow these steps before submitting your issue - thank you!
   
   - [x] I have checked the superset logs for python stacktraces and included it here as text if there are any.
   - [x] I have reproduced the issue with at least the latest released version of superset.
   - [x] I have checked the issue tracker for the same issue and I haven't found one similar.
   
   ### Logs
   
   † Well, I've just tried to reproduce it… and got an error thrown — that's a progress!
   
   ```shell
   (venv) [fedora@my_vm ~]$ superset --version
   Loaded your LOCAL configuration at [/opt/superset/superset_config.py]
   Python 3.8.10
   Flask 1.1.4
   Werkzeug 1.0.1
   (venv) [fedora@my_vm ~]$ export SUPERSET_CONFIG_PATH=/opt/superset/superset_config.py 
   (venv) [fedora@my_vm ~]$ export PYTHONPATH=/opt/superset/
   (venv) [fedora@my_vm ~]$ export FLASK_APP=superset
   (venv) [fedora@my_vm ~]$ superset set-database-uri --database_name foobar --uri postgresql://<username>:<pw>@<url>:5432/<db_name>
   Loaded your LOCAL configuration at [/opt/superset/superset_config.py]
   logging was configured successfully
   2021-08-23 08:31:52,244:INFO:superset.utils.logging_configurator:logging was configured successfully
   2021-08-23 08:31:52,251:INFO:root:Configured event logger of type <class 'superset.utils.log.DBEventLogger'>
   /opt/superset/venv/lib64/python3.8/site-packages/flask_caching/__init__.py:201: UserWarning: Flask-Caching: CACHE_TYPE is set to null, caching is effectively disabled.
     warnings.warn(
   Creating database reference for foobar
   2021-08-23 08:31:53,516:INFO:superset.utils.core:Creating database reference for foobar
   (venv) [fedora@my_vm ~]$ superset import-dashboards --path dashboard_export_20210819T122937.zip
   Loaded your LOCAL configuration at [/opt/superset/superset_config.py]
   logging was configured successfully
   2021-08-23 08:32:55,694:INFO:superset.utils.logging_configurator:logging was configured successfully
   2021-08-23 08:32:55,700:INFO:root:Configured event logger of type <class 'superset.utils.log.DBEventLogger'>
   /opt/superset/venv/lib64/python3.8/site-packages/flask_caching/__init__.py:201: UserWarning: Flask-Caching: CACHE_TYPE is set to null, caching is effectively disabled.
     warnings.warn(
   Command failed validation
   2021-08-23 08:32:56,878:INFO:superset.dashboards.commands.importers.dispatcher:Command failed validation
   There was an error when importing the dashboards(s), please check the exception traceback in the log
   Traceback (most recent call last):
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/cli.py", line 335, in import_dashboards
       ImportDashboardsCommand(contents, overwrite=True).run()
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/dashboards/commands/importers/dispatcher.py", line 65, in run
       raise exc
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/dashboards/commands/importers/dispatcher.py", line 58, in run
       command.run()
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/commands/importers/v1/__init__.py", line 63, in run
       self.validate()
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/commands/importers/v1/__init__.py", line 147, in validate
       raise exception
   superset.commands.exceptions.CommandInvalidError: Error importing dashboard
   2021-08-23 08:32:56,878:ERROR:superset.cli:There was an error when importing the dashboards(s), please check the exception traceback in the log
   Traceback (most recent call last):
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/cli.py", line 335, in import_dashboards
       ImportDashboardsCommand(contents, overwrite=True).run()
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/dashboards/commands/importers/dispatcher.py", line 65, in run
       raise exc
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/dashboards/commands/importers/dispatcher.py", line 58, in run
       command.run()
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/commands/importers/v1/__init__.py", line 63, in run
       self.validate()
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/commands/importers/v1/__init__.py", line 147, in validate
       raise exception
   superset.commands.exceptions.CommandInvalidError: Error importing dashboard
   ```


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] EBoisseauSierra commented on issue #16395: Can't import an instance export on a clone securely (`superset import-dashboards`)

Posted by GitBox <gi...@apache.org>.
EBoisseauSierra commented on issue #16395:
URL: https://github.com/apache/superset/issues/16395#issuecomment-909285432


   Note that (on `1.3.0`) importing the dashboards via the GUI worked just fine (after I manually entered the database password when prompted) — I'm still looking for a CLI-based solution, though.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] EBoisseauSierra commented on issue #16395: Can't import an instance export on a clone securely (`superset import-dashboards`)

Posted by GitBox <gi...@apache.org>.
EBoisseauSierra commented on issue #16395:
URL: https://github.com/apache/superset/issues/16395#issuecomment-909285432


   Note that (on `1.3.0`) importing the dashboards via the GUI worked just fine (after I manually entered the database password when prompted) — I'm still looking for a CLI-based solution, though.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] EBoisseauSierra commented on issue #16395: Can't import an instance export on a clone securely (`superset import-dashboards`)

Posted by GitBox <gi...@apache.org>.
EBoisseauSierra commented on issue #16395:
URL: https://github.com/apache/superset/issues/16395#issuecomment-910259055


   Based on the idea that comparing UUIDs in unlikely to succeed, I've tried to compare `database_name`s instead as follows:
   
   ```diff
   diff --git i/superset/commands/importers/v1/__init__.py w/superset/commands/importers/v1/__init__.py
   index b9ea24414..8f6264cbf 100644
   --- i/superset/commands/importers/v1/__init__.py
   +++ w/superset/commands/importers/v1/__init__.py
   @@ -76,9 +76,9 @@ class ImportModelsCommand(BaseCommand):
    
            # load existing databases so we can apply the password validation
            db_passwords = {
   -            str(uuid): password
   -            for uuid, password in db.session.query(
   -                Database.uuid, Database.password
   +            database_name: password
   +            for database_name, password in db.session.query(
   +                Database.database_name, Database.password
                ).all()
            }
    
   @@ -113,8 +113,8 @@ class ImportModelsCommand(BaseCommand):
                        # populate passwords from the request or from existing DBs
                        if file_name in self.passwords:
                            config["password"] = self.passwords[file_name]
   -                    elif prefix == "databases" and config["uuid"] in db_passwords:
   -                        config["password"] = db_passwords[config["uuid"]]
   +                    elif prefix == "databases" and config["database_name"] in db_passwords:
   +                        config["password"] = db_passwords[config["database_name"]]
    
                        schema.load(config)
                        self._configs[file_name] = config
   ```
   
   (with, obviously, the `"VERSIONED_EXPORT": True` feature flag)
   
   … and this works!
   
   @betodealmeida I'm still a little worried of the possible unforeseen consequences that such approach could have — do think it could cause some security issues? If not, would you like me to submit a PR?


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] EBoisseauSierra commented on issue #16395: Can't import an instance export on a clone securely (`superset import-dashboards`)

Posted by GitBox <gi...@apache.org>.
EBoisseauSierra commented on issue #16395:
URL: https://github.com/apache/superset/issues/16395#issuecomment-909285432


   Note that (on `1.3.0`) importing the dashboards via the GUI worked just fine (after I manually entered the database password when prompted) — I'm still looking for a CLI-based solution, though.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] EBoisseauSierra commented on issue #16395: Can't import an instance export on a clone securely (`superset import-dashboards`)

Posted by GitBox <gi...@apache.org>.
EBoisseauSierra commented on issue #16395:
URL: https://github.com/apache/superset/issues/16395#issuecomment-903946815


   Here are the logs with Superset `1.3.0`:
   
   ```shell
   (venv) [fedora@<my_vm> ~]$ superset import-dashboards --path dashboard_export_20210819T122937.zip --username admin
   Loaded your LOCAL configuration at [/opt/superset/superset_config.py]
   logging was configured successfully
   2021-08-23 17:54:13,418:INFO:superset.utils.logging_configurator:logging was configured successfully
   2021-08-23 17:54:13,435:INFO:root:Configured event logger of type <class 'superset.utils.log.DBEventLogger'>
   /opt/superset/venv/lib64/python3.8/site-packages/flask_caching/__init__.py:201: UserWarning: Flask-Caching: CACHE_TYPE is set to null, caching is effectively disabled.
     warnings.warn(
   Command failed validation
   2021-08-23 17:54:14,747:INFO:superset.dashboards.commands.importers.dispatcher:Command failed validation
   There was an error when importing the dashboards(s), please check the exception traceback in the log
   Traceback (most recent call last):
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/cli.py", line 328, in import_dashboards
       ImportDashboardsCommand(contents, overwrite=True).run()
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/dashboards/commands/importers/dispatcher.py", line 65, in run
       raise exc
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/dashboards/commands/importers/dispatcher.py", line 58, in run
       command.run()
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/commands/importers/v1/__init__.py", line 63, in run
       self.validate()
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/commands/importers/v1/__init__.py", line 147, in validate
       raise exception
   superset.commands.exceptions.CommandInvalidError: Error importing dashboard
   2021-08-23 17:54:14,748:ERROR:superset.cli:There was an error when importing the dashboards(s), please check the exception traceback in the log
   Traceback (most recent call last):
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/cli.py", line 328, in import_dashboards
       ImportDashboardsCommand(contents, overwrite=True).run()
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/dashboards/commands/importers/dispatcher.py", line 65, in run
       raise exc
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/dashboards/commands/importers/dispatcher.py", line 58, in run
       command.run()
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/commands/importers/v1/__init__.py", line 63, in run
       self.validate()
     File "/opt/superset/venv/lib64/python3.8/site-packages/superset/commands/importers/v1/__init__.py", line 147, in validate
       raise exception
   superset.commands.exceptions.CommandInvalidError: Error importing dashboard
   ```


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] EBoisseauSierra commented on issue #16395: Can't import an instance export on a clone securely (`superset import-dashboards`)

Posted by GitBox <gi...@apache.org>.
EBoisseauSierra commented on issue #16395:
URL: https://github.com/apache/superset/issues/16395#issuecomment-910254880


   I believe I have understood where the issue comes from:
   
   Indeed, without prior  `superset set-database-uri`, I inserted a breakpoint [there](https://github.com/apache/superset/blob/1.3.0/superset/commands/importers/v1/__init__.py#L147) and could see that the import is failing as follows:
   
   ```
   >>> exception.message
   'Error importing dashboard'
   
   >>> exception._invalid_exceptions.__repr__()
   "[ValidationError({'_schema': ['Must provide a password for the database']})]"
   ```
   
   So the issue is the makes pw indeed — and, unlike the GUI, the CLI doesn't prompt for the db pw.
   
   An idea would be to accept another parameter in the form of a YAML/JSON file containing a mapping between `database_name` and db pw. This file could then be re-created from a list of (encrypted) secrets.
   
   I then ran the following command and checked that the db appeared in `<FQDN>/databaseview/list`:
   
   ```
   superset set-database-uri -d foobar -u postgresql://user:unmasked_pw@fqdn:5432/db_name
   ```
   (and double-checked that `foobar` is the value of `database_name` in the `dashboard_export_TS/databases/foobar.yaml` file indeed).
   
   … and still got the same error/content of the local variables as above.
   
   I also checked the following:
   
   ```
   >>> config["database_name"]
   'foobar'
   
   >>> config["sqlalchemy_uri"]
   'postgresql://user:XXXXXXXXXX@fqdn:5432/db_name'
   
   >>> config["uuid"]
   '4e5cd567-df42-44f2-be0f-c5035eeca290'
   
   >>> db_passwords
   {'e2ba605a-6141-4961-8...8bb3d2a57d': "unmasked_pw"}
   ```
   
   … so my database password is correctly retrieved from Superset back-end db, and the import works well.
   
   The issue actually happens [here](https://github.com/apache/superset/blob/1.3.0/superset/commands/importers/v1/__init__.py#L116): one tries to match the UUID of the already existing db (I understand it has been defined when I ran `superset set-database-uri`) with… the value of the `uuid` key in the `export.zip/databases/foobar.yaml` file.
   
   I haven't dug into how these UUIDs are generated, but they might include runtime inputs (e.g. the db ID of the record? the last_modified timestamp?) that would make comparing those in the new instance with those of the import very unlikely to succeed.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org