Fixes 4362: preserve secret: prefix during Python SDK serialization#28625
Open
pmbrull wants to merge 1 commit into
Open
Fixes 4362: preserve secret: prefix during Python SDK serialization#28625pmbrull wants to merge 1 commit into
pmbrull wants to merge 1 commit into
Conversation
CustomSecretStr serialization resolved `secret:<id>` external references through the configured secrets manager, stripping the `secret:` prefix from model_dump / model_dump_json output. On create/update the SDK then sent the resolved (or, with the default DB manager, prefix-stripped) value to the server, so external references were stored as plain secrets instead of being preserved. handle_secret now serializes with skip_secret_manager=True so the raw stored reference survives transport. Secret resolution still happens at use-time via the direct get_secret_value() call when building a connection, so ingestion keeps resolving secret:<id> as before. Fixes open-metadata/openmetadata-collate#4362 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
Ingestion
safe to test
Code Review ✅ ApprovedUpdates CustomSecretStr serialization to skip secret manager resolution during model dumps, ensuring 'secret:' prefixes are preserved in payloads. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Python SDK secret serialization so external secret references (secret:<id>) are preserved in create/update payloads instead of being resolved (and having the secret: prefix stripped) during model_dump_json().
Changes:
- Update
CustomSecretStrserialization to skip secrets-manager resolution during transport serialization (skip_secret_manager=True), preserving rawsecret:<id>references. - Add unit tests to ensure
secret:references survivemodel_dump_json(context={"mask_secrets": False})and that connect-time resolution viaget_secret_value()still works with an external secrets manager.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ingestion/src/metadata/ingestion/models/custom_pydantic.py | Adjusts secret serialization to preserve secret: references by skipping secrets-manager resolution during serialization. |
| ingestion/tests/unit/models/test_custom_pydantic.py | Adds regression tests for serialization preservation and runtime resolution behavior for external secret references. |
IceS2
approved these changes
Jun 2, 2026
|
Contributor
🔴 Playwright Results — 2 failure(s), 15 flaky✅ 4255 passed · ❌ 2 failed · 🟡 15 flaky · ⏭️ 88 skipped
Genuine Failures (failed on all attempts)❌
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Basically, if we created a service with the YAML putting in secret:redshift-pwd, it got sent deserialized with the password as-is, so then the server saved it with the "internal" secret path like "secret:openmetadata-database-red-sm-new-authtype-password"
So you ended up with the service misaligned with how you have your secrets set up locally
So anyway, this cleans up the serde step when we send the payload
Describe your changes:
Fixes open-metadata/openmetadata-collate#4362
CustomSecretStrserialization resolvedsecret:<id>external secret references through the configured secrets manager, stripping thesecret:prefix frommodel_dump/model_dump_jsonoutput. On create/update the SDK sent that resolved (or, with the default DB manager, prefix-stripped) value to the server, so external references were stored as plain secrets instead of being preserved.handle_secretnow serializes withskip_secret_manager=True, keeping the rawsecret:<id>reference intact for transport; resolution still happens at use-time via the directget_secret_value()call when a connection is built, so ingestion keeps resolving references exactly as before.Type of change:
High-level design:
N/A — small change (2 files). Serialization (transport) preserves the reference; resolution (use-time, e.g.
builders.get_password_secret) is a separate, untouched path.Tests:
Use cases covered
secret:<id>external reference keeps the reference in the serialized create/update payload (so the server stores it as an external reference, not a plain secret).secret:<id>against the configured secrets manager when building a connection.Unit tests
ingestion/tests/unit/models/test_custom_pydantic.pyTestExternalSecretReferenceSerialization— asserts thesecret:prefix survivesmodel_dump_json(context={"mask_secrets": False})for both a plainCustomSecretStrmodel and a typedMysqlConnection.TestExternalSecretReferenceResolution— guards the connect-time path: with an external secrets manager active,get_password_secret().get_secret_value()still resolves the reference, while serialization does not leak the resolved value.Backend integration tests
Ingestion integration tests
Playwright (UI) tests
Manual testing performed
authType.password: secret:redshift-pwdand a Kubernetes secrets manager.GET .../databaseServices/name/<svc>?fields=*returnsauthType.password = secret:redshift-pwd(reference preserved) instead of a server-wrappedsecret:openmetadata-...plain secret.UI screen recording / screenshots:
Not applicable.
Checklist:
Fixesabove.