rusackas opened a new pull request, #40584:
URL: https://github.com/apache/superset/pull/40584

   ### SUMMARY
   
   TDD-style regression test for #31983.
   
   Issue #31983 reports that `superset import_datasources` stores 
`sqlalchemy_uri` as cleartext instead of encrypting it through the same 
`EncryptedType` path used by the REST API. A workaround using the API to 
re-save connections post-import exists, but the CLI should handle this 
transparently.
   
   This PR adds a regression test against the CLI import path. If CI is green, 
the bug is already fixed and merging closes #31983. If CI is red, the failing 
test confirms the bug is still present.
   
   **Root cause (traced during test authoring):**
   The `superset import_datasources -p file.yaml` CLI command dispatches to the 
legacy v0 `ImportDatasetsCommand`, which calls 
`Database.import_from_dict(database, sync=sync)`. The generic 
`import_from_dict` helper in `superset/models/helpers.py` sets `sqlalchemy_uri` 
directly on the model (via `setattr`) rather than routing through 
`Database.set_sqlalchemy_uri()`. The `set_sqlalchemy_uri` method is the only 
place that extracts the password, stores it in the encrypted `password` column, 
and replaces the password in the URI with `XXXXXXXXXX`. Without that call, the 
plaintext password persists in the `sqlalchemy_uri` column.
   
   The v1 ZIP import path (`ImportDatabasesCommand`) is NOT affected — it 
explicitly calls `database.set_sqlalchemy_uri(sqlalchemy_uri)` in 
`superset/commands/database/importers/v1/utils.py`.
   
   ### How to interpret CI
   
   - **CI green** → `import_datasources` now encrypts passwords correctly; 
merging closes #31983.
   - **CI red** → bug is still live. Fix target: 
`superset/commands/dataset/importers/v0.py`, function `import_from_dict` (line 
~213), which calls `Database.import_from_dict` instead of routing through 
`Database.set_sqlalchemy_uri`.
   
   ### TESTING INSTRUCTIONS
   
   ```bash
   pytest 
tests/unit_tests/databases/commands/importers/v1/import_test.py::test_import_datasources_cli_encrypts_password
 -v
   ```
   
   ### ADDITIONAL INFORMATION
   
   - [x] Has associated issue: closes #31983
   - [ ] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API


-- 
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]

Reply via email to