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

Fix nowait poll #2

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix nowait poll #2

wants to merge 4 commits into from

Conversation

miker985
Copy link

@miker985 miker985 commented Oct 28, 2024

This is currently live in production

This remediates the problem sireq was having.

In more detail, this improves logging and fixes a loop that wasn't working as intended. Inside the looper function was a nested while loop. Here's a simplified and annotated version

    while not tp.terminate.isSet():         # while nothing has told me to turn off (main)
        while not tp.terminate.isSet():     # INNER
            tp.timestamp = time.time()
            result = tp.handle.fetch()      # fetch new data
            tp.count = tp.count + 1
            tp.status = '... status ...'
            if not result:                  # if there was no new data, go to WAIT.
                break                       # otherwise repeat INNER and continue fetching new data
        tp.terminate.wait(...)              # WAIT some number of seconds before polling again

The problem being that the auth log was so busy there was always new data.

This led to continuous fetching of new data well beyond what Duo's rate limit allows for. Per Cole on duo support ticket 01567856 (forwarded to Kevin and Jonathan) the maximum API rate is in the range of 2-3 requests per minute and

... the requests are being received [from logger4] every 2-4 seconds.

These API limits are per Duo application integration which left sireq largely locked out because it used to share an integration with this. It no longer does.

@miker985 miker985 requested review from counik and jdiverp October 28, 2024 21:19
@counik
Copy link

counik commented Oct 28, 2024

It all looks good, but could you elaborate on your last comment?

These API limits are per Duo application integration which left sireq largely locked out because it used to share an integration with this. It no longer does.

@miker985
Copy link
Author

I had @chmc3 create another Application Integration and updated duo_watcher to use that. I saved the details is mosler and updated the duo_watcher README to refer to them in this PR

sireq has had no changes and still uses the established integration. This was easiest because sireq is ref-managed and duo_watcher's credentials are not managed at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants