-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
When using AWS Secrets Manager for persisting secrets, updating a connector deletes its secret #46097
Comments
@natikgadzhi @nataliekwong can you please prioritize the fix? |
That's @bgroff area. |
@bgroff, are there any updates on this? Is there someone else who can handle the fix for the issue? |
@cjwooo Thanks for reporting this. Are you performing the creation and update of the source through the public API? |
We are using update source API from OpenAPI spec: |
I'm also experiencing issues with AWS Secrets Manager and
|
FWIW - still seeing this behavior after upgrading to v1.3.0 |
I'm having this issue too on Airbyte 1.3.0. |
Hello, team is investigating the issue and trying to reproduce it to implement a fix. Any update I'll let you know. |
Hey, we've got this exact issue as well. If anyone makes a change to a Source, it will break any connections. We are on 1.2.0. More users are starting to be onboarded so this is likely going to become critical |
@marcosmarxm any update on this? |
Hey folks, we have a PR up internally with a fix for this. It should get merged this week and should go out in the 1.5.0 release which should happen next week. Stay tuned. |
Fix has been merged, expect it to go out with 1.5.0 next week. Thanks for reporting this and for the patience here everyone. Note that this will take effect for new connections, since the old secrets were saved incorrectly it's difficult for us to remediate them. We're trying to find a way to mitigate for existing connections but it isn't straightforward. Edit: We've got a way to mitigate for existing connections. Summary: For sources/destinations that haven't been touched, there should be no issues either continuing to run connections or updating secrets/config post upgrading to 1.5.0 |
@JonsSpaghetti I'm testing Airbyte 1.5.0. I've notice that if you change a secret (e.g. a destination password) Airbyte deletes the current one but for some reason it creates always a new secret with the new password and another secret with the old password. |
It costs $0.40 per secret per month in AWS Secret Manager. It would be nice to keep these to a minimum if at all possible 🙏 |
Unfortunately yes the extra secrets you see getting created are from our check_connection. We have updates to make to ensure that those are expired and cleaned up correctly which we will do in a follow-up. |
@JonsSpaghetti meanwhile this is fixed, can we assume Airbyte will never use the secrets containing the old password and delete them by just filtering by last accessed date? Assuming that all the ingestions are scheduled to be triggered at least once per day, can we safely delete all the secrets with last accessed date < now - 2 or 3 days? |
@giacomochiarella I'd recommend looking @ the naming scheme of the secret instead. The secrets with a name like airbyte_workspace_00000000-0000-0000-0000-000000000000_secret_* should be ones which are ephemeral - once they've been accessed (usually only once) they should be safe to clean up. Writing secrets with expiry is supported in our existing SecretPersistence interface, but it's only implemented for some of our supported secret managers. I'll add an item to our backlog to make sure we support it for AWS as well so that these secrets will be cleaned up automatically. I'll link the new ticket here. |
@JonsSpaghetti can you be more precise please? I ended up in this situation. All the secrets are there. Even if the one with 0s would be removed (and looks to me it is not removed) there are many others to delete. ![]() This would cause uncontrolled extra costs. |
Thanks you all. Is fixed now on v1.5.0 |
@giacomochiarella that looks correct to me. The salesforce source has multiple secrets in its config and each one creates a single secret. The temp secrets are the ones that should be cleaned up and I created a related ticket to track that. I'm going to close this ticket as last comment states this specific issue is resolved in 1.5.0. |
@JonsSpaghetti could you mention the ticket for the other issue for me to be posted on it? |
Does this change support the previous AWS Secrets Manager secrets which don't have the _vX extension? |
Helm Chart Version
0.64.388
What step the error happened?
Other
Relevant information
Observed behavior:
I believe I have tracked the cause to this commit. The related deletion behavior was behind a feature flag that was defaulted to false, but the commit removed the flag and enabled the deletion behavior permanently.
AFAICT, the problem with the Secrets Manager integration is that the version suffix that Airbyte adds to the ids -- e.g.
<secret-id-base>_v1/v2/etc
is not respected when writing to AWS Secrets Manager, so none of the secrets are versioned properly. As a result, when Airbyte thinks it's deleting an old secret after a connector config update, it's actually deleting the required secret, as there is only one.While we are using an older chart version, and I am not familiar enough with the platform codebase to say anything about it concretely, I don't think this behavior has changed between our version and 1.0.
Relevant log output
No response
The text was updated successfully, but these errors were encountered: