-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Description
Describe the bug
When using PushSecret with the AWS Secrets Manager provider and the secretPushFormat is set to string, ESO calls PutSecretValueWithContext to create a new version of the secret on every refresh cycle, even when the desired value has not changed since the previous refresh. This eventually results in hitting AWS API limits (on version update frequency) and creates numerous extraneous revisions of the AWS secret. This appears to have been introduced in v0.9.14 with #3189 which initially enabled the "string" value format.
Steps to reproduce the behavior:
- Create a
PushSecretresource against aSecretStoreusing the AWS Secrets Manager provider and setspec.data[].metadata.secretPushFormattostring. To make the failure more evident, use a shortspec.refreshIntervallike10s - The incorrect behavior can be observed in several ways:
- Watch the "Versions" tab of the secret in the AWS console and note the version IDs continue to change for the
AWSCURRENTandAWSPREVIOUSlabels. - Get the secret values with the AWS CLI (
aws secretsmanager list-secret-version-ids) - Watch CloudTrail logs for repeated calls to
PutSecretValueWithContextfor the same secret resource. - Watch
externalsecret_provider_api_calls_countESO metrics forPutSecretValueWithContext
Expected behavior
ESO should only create new AWS secret versions when the desired secret value is different from the value observed in the destination secret.
Additional context
It appears that the logic is intended to behave as described in "expected behavior". In line 572 of secretsmanager.go, having read the remote value from the provider, the code checks that the remote secret exists and compares its observed value to the desired value. If those values are the same, it exits the reconcile loop taking no further action:
if awsSecret != nil && bytes.Equal(awsSecret.SecretBinary, value) {
return nil
}Unfortunately, it only compares the SecretBinary field. If the user elected to use secretPushFormat: string, then the API docs indicate SecretBinary will have no value because the value was set using SecretString as in line 590:
if secretPushFormat == SecretPushFormatString {
input.SetSecretBinary(nil).SetSecretString(string(value))
}Presumably, the fix would be fairly straightforward: if the user elected secretPushFormat: string (like it checks in line 589), then instead of comparing value to SecretBinary, we'd want to compare string(value) to SecretString (with appropriate safety checks). I wish I felt confident enough in my golang skills to submit a PR for this, buuuut I don't. :/
Sub-optimal workarounds
A few less-than-ideal workarounds are available while awaiting this fix:
- Use binary format. Users can use binary format for the secret, if the consumer of the secret can tolerate that and do any necessary conversion. It also makes it impossible to view the secret value with the AWS console.
- Set long
refreshIntervalto avoid excessive writes. This can help avoid hitting API limits, but it's no guarantee.