Skip to content
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

bitwarden_secrets_manager: Handle rate limits #8230

Closed
1 task done
mintyhippoxyz opened this issue Apr 17, 2024 · 9 comments · Fixed by #8238
Closed
1 task done

bitwarden_secrets_manager: Handle rate limits #8230

mintyhippoxyz opened this issue Apr 17, 2024 · 9 comments · Fixed by #8238
Labels
bug This issue/PR relates to a bug

Comments

@mintyhippoxyz
Copy link
Contributor

Summary

I'm not finding any official documentation on it yet but Bitwarden's Secret Manager seems to have a rate limit of 5 requests per second. When the rate limit is hit, the lookup fails with an error: 429 Too Many Requests; Slow down! Too many requests. Try again in 1s.

Issue Type

Bug Report

Component Name

bitwarden_secret_manager

Ansible Version

$ ansible --version
ansible [core 2.16.1]
  config file = /mnt/ansible/ansible.cfg
  configured module search path = ['/home/matta/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.11/site-packages/ansible
  ansible collection location = /mnt/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.11.9 (main, Apr 14 2024, 13:40:00) [GCC 13.2.1 20231014] (/usr/bin/python3)
  jinja version = 3.1.2
  libyaml = True

Community.general Version

$ ansible-galaxy collection list community.general

# /mnt/ansible/collections/ansible_collections
Collection        Version
----------------- -------
community.general 8.5.0

# /usr/lib/python3.11/site-packages/ansible_collections
Collection        Version
----------------- -------
community.general 7.5.1

Configuration

$ ansible-config dump --only-changed
COLLECTIONS_PATHS(/mnt/ansible/ansible.cfg) = ['/mnt/ansible/collections']
CONFIG_FILE() = /mnt/ansible/ansible.cfg
DEFAULT_FORKS(/mnt/ansible/ansible.cfg) = 10
DEFAULT_HOST_LIST(/mnt/ansible/ansible.cfg) = ['/mnt/ansible/inventory']
DEFAULT_MANAGED_STR(/mnt/ansible/ansible.cfg) = This file is managed by Ansible. Do not modify directly!%n
template: {file}
date: %Y-%m-%d %H:%M:%S
user: {uid}
host: {host}
DISPLAY_SKIPPED_HOSTS(/mnt/ansible/ansible.cfg) = False
EDITOR(env: EDITOR) = vim
INTERPRETER_PYTHON(/mnt/ansible/ansible.cfg) = auto_silent
PAGER(env: PAGER) = less

OS / Environment

Alpine Linux 3.19

Steps to Reproduce

---
- name: Bitwarden Secrets Manager Rate Limit Reproduction
  hosts:
    - xen01
    - xen02
    - xen03
    - xen04
    - xen05
    - xen06
  become: false
  gather_facts: false
  tasks:
    - debug:
        var: "{{ lookup('community.general.bitwarden_secrets_manager', '<secret id here>').value }}"

Expected Results

I would expect the module to handle the 429 error with a back-off and retry until it succeeds

Actual Results

PLAY [Bitwarden Secrets Manager Rate Limit Reproduction] ******************************************************************************************************************************************************************************************************************
TASK [debug] **************************************************************************************************************************************************************************************************************************************************************
fatal: [xen01]: FAILED! => {"msg": "Error: \n   0: Received error message from server: [429 Too Many Requests] {\"message\":\"Slow down! Too many requests. Try again in 1s.\",\"validationErrors\":null,\"exceptionMessage\":null,\"exceptionStackTrace\":null,\"innerExceptionMessage\":null,\"object\":\"error\"}\n\nLocation:\n   /home/matta/alpine-package-repository/main/bws/src/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bws-0.4.0/src/main.rs:334\n\nBacktrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.\nRun with RUST_BACKTRACE=full to include source snippets.\n"}
ok: [xen03] => {
    "this-is-a-test-secret": "{{this-is-a-test-secret}}"
}
ok: [xen04] => {
    "this-is-a-test-secret": "{{this-is-a-test-secret}}"
}
ok: [xen05] => {
    "this-is-a-test-secret": "{{this-is-a-test-secret}}"
}
ok: [xen06] => {
    "this-is-a-test-secret": "{{this-is-a-test-secret}}"
}
ok: [xen02] => {
    "this-is-a-test-secret": "{{this-is-a-test-secret}}"
}

PLAY RECAP ****************************************************************************************************************************************************************************************************************************************************************
xen01                      : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0
xen02                      : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0
xen03                      : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0
xen04                      : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0
xen05                      : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0
xen06                      : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@ansibullbot
Copy link
Collaborator

Files identified in the description:
None

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibullbot ansibullbot added the bug This issue/PR relates to a bug label Apr 17, 2024
@mintyhippoxyz
Copy link
Contributor Author

I got an implementation of that working...

--- bitwarden_secrets_manager.py.original       2024-04-17 11:37:56.288578282 -0500
+++ bitwarden_secrets_manager.py        2024-04-17 14:36:56.322357518 -0500
@@ -70,6 +70,7 @@
 """

 from subprocess import Popen, PIPE
+from time import sleep

 from ansible.errors import AnsibleLookupError
 from ansible.module_utils.common.text.converters import to_text
@@ -84,11 +85,28 @@
 class BitwardenSecretsManager(object):
     def __init__(self, path='bws'):
         self._cli_path = path
+        self._max_retries = 3
+        self._retry_delay = 1

     @property
     def cli_path(self):
         return self._cli_path

+    def _run_with_retry(self, args, stdin=None, retries=0):
+        if retries > self._max_retries:
+            raise BitwardenSecretsManagerException("Max retries exceeded. Unable to retrieve secret.")
+
+        out, err, rc = self._run(args, stdin)
+
+        if "Too many requests" in err:
+            delay = self._retry_delay * (2 ** retries)
+            sleep(delay)
+            return self._run_with_retry(args, stdin, retries + 1)
+        elif rc != 0:
+            raise BitwardenSecretsManagerException(f"Command failed with return code {rc}: {err}")
+
+        return out, err, rc
+
     def _run(self, args, stdin=None):
         p = Popen([self.cli_path] + args, stdout=PIPE, stderr=PIPE, stdin=PIPE)
         out, err = p.communicate(stdin)
@@ -107,7 +125,7 @@
             'get', 'secret', secret_id
         ]

-        out, err, rc = self._run(params)
+        out, err, rc = self._run_with_retry(params)
         if rc != 0:
             raise BitwardenSecretsManagerException(to_text(err))

I do however think there's a better approach now that I've done this. Instead of being reactive to the rate limit errors it's probably more appropriate to take a more proactive approach and avoid hitting them to begin with.

@felixfontein
Copy link
Collaborator

!component =plugins/lookup/bitwarden_secrets_manager.py

@ansibullbot
Copy link
Collaborator

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibullbot
Copy link
Collaborator

@jantari
Copy link
Contributor

jantari commented Apr 17, 2024

I think avoiding the rate limit will be difficult because I think ansible will do a lookup/templating for every host (no caching?) so unless you set serial: 1 there could always be many requests in parallel.

Reacting and respecting 429 though should be easier, and has to happen anyways I feel.

@felixfontein
Copy link
Collaborator

Using set_fact: to store the value in an ansible facts (instead of using lazy evaluation, which evaluates the lookup potentially many times) with run_once: true (to make sure it's really only done once) also helps minimizing the number of times a value is looked up.

@jantari
Copy link
Contributor

jantari commented Apr 18, 2024

@mintyhippoxyz do you want to contribute your edits as a PR? If not, I would likely implement something pretty close to that.

btw, longer term I would recommend trying the - now finally available - official lookup plugin from bitwarden: https://bitwarden.com/blog/bitwarden-secrets-manager-and-ansible/

felixfontein pushed a commit that referenced this issue Apr 21, 2024
…8238)

* bitwarden_secrets_manager: implement rate limit retry with backoff (#8230)

* bitwarden_secrets_manager: add changelog fragment for 90cd2d6 (#8238)

* bitwarden_secrets_manager: clarify "Too many requests" is an error condition (#8238)

* bitwarden_secrets_manager: avoid an extra _run_with_retry execution after the last (very long) delay

* bitwarden_secrets_manager: changelog fragment key and reference issue url
patchback bot pushed a commit that referenced this issue Apr 21, 2024
…8238)

* bitwarden_secrets_manager: implement rate limit retry with backoff (#8230)

* bitwarden_secrets_manager: add changelog fragment for 90cd2d6 (#8238)

* bitwarden_secrets_manager: clarify "Too many requests" is an error condition (#8238)

* bitwarden_secrets_manager: avoid an extra _run_with_retry execution after the last (very long) delay

* bitwarden_secrets_manager: changelog fragment key and reference issue url

(cherry picked from commit a05a598)
patchback bot pushed a commit that referenced this issue Apr 21, 2024
…8238)

* bitwarden_secrets_manager: implement rate limit retry with backoff (#8230)

* bitwarden_secrets_manager: add changelog fragment for 90cd2d6 (#8238)

* bitwarden_secrets_manager: clarify "Too many requests" is an error condition (#8238)

* bitwarden_secrets_manager: avoid an extra _run_with_retry execution after the last (very long) delay

* bitwarden_secrets_manager: changelog fragment key and reference issue url

(cherry picked from commit a05a598)
felixfontein added a commit that referenced this issue Apr 21, 2024
…lement rate limit retry with backoff (#8260)

* bitwarden_secrets_manager: implement rate limit retry with backoff (#8238)

* bitwarden_secrets_manager: implement rate limit retry with backoff (#8230)

* bitwarden_secrets_manager: add changelog fragment for 90cd2d6 (#8238)

* bitwarden_secrets_manager: clarify "Too many requests" is an error condition (#8238)

* bitwarden_secrets_manager: avoid an extra _run_with_retry execution after the last (very long) delay

* bitwarden_secrets_manager: changelog fragment key and reference issue url

(cherry picked from commit a05a598)

* Make Python 2 compatible.

---------

Co-authored-by: Matt Adams <matt@4dk.me>
Co-authored-by: Felix Fontein <felix@fontein.de>
felixfontein pushed a commit that referenced this issue Apr 21, 2024
…lement rate limit retry with backoff (#8261)

bitwarden_secrets_manager: implement rate limit retry with backoff (#8238)

* bitwarden_secrets_manager: implement rate limit retry with backoff (#8230)

* bitwarden_secrets_manager: add changelog fragment for 90cd2d6 (#8238)

* bitwarden_secrets_manager: clarify "Too many requests" is an error condition (#8238)

* bitwarden_secrets_manager: avoid an extra _run_with_retry execution after the last (very long) delay

* bitwarden_secrets_manager: changelog fragment key and reference issue url

(cherry picked from commit a05a598)

Co-authored-by: Matt Adams <matt@4dk.me>
aretrosen pushed a commit to aretrosen/community.general that referenced this issue Apr 22, 2024
…nsible-collections#8238)

* bitwarden_secrets_manager: implement rate limit retry with backoff (ansible-collections#8230)

* bitwarden_secrets_manager: add changelog fragment for 90cd2d6 (ansible-collections#8238)

* bitwarden_secrets_manager: clarify "Too many requests" is an error condition (ansible-collections#8238)

* bitwarden_secrets_manager: avoid an extra _run_with_retry execution after the last (very long) delay

* bitwarden_secrets_manager: changelog fragment key and reference issue url
@tigattack
Copy link

Hopefully this helps some other people - I use bws-cache, a project that another Ansible user and I created, along with the included lookup plugin to avoid the rate limiting.

I found it pretty difficult to avoid rate limits without such a solution as I use Bitwarden Secrets Manager heavily and would hit the limit very fast in most playbooks.

Massl123 pushed a commit to Massl123/community.general that referenced this issue Feb 7, 2025
…nsible-collections#8238)

* bitwarden_secrets_manager: implement rate limit retry with backoff (ansible-collections#8230)

* bitwarden_secrets_manager: add changelog fragment for 90cd2d6 (ansible-collections#8238)

* bitwarden_secrets_manager: clarify "Too many requests" is an error condition (ansible-collections#8238)

* bitwarden_secrets_manager: avoid an extra _run_with_retry execution after the last (very long) delay

* bitwarden_secrets_manager: changelog fragment key and reference issue url
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants