Skip to content

cisco_duo.auth: Provide option to ignore ingesting API Errors. - #12870

Merged
kcreddy merged 5 commits into
elastic:mainfrom
kcreddy:cisco_duo-ignore-api-erros
Feb 28, 2025
Merged

cisco_duo.auth: Provide option to ignore ingesting API Errors.#12870
kcreddy merged 5 commits into
elastic:mainfrom
kcreddy:cisco_duo-ignore-api-erros

Conversation

@kcreddy

@kcreddy kcreddy commented Feb 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Proposed commit message

Cisco Duo auth API rate limit is very low i.e., once per minute. 
Sometimes 429s are received even when requests are made 
less than once per minute. This leads to users ingesting 429 
API errors very often. 

Provide an option to users to ignore ingesting API Errors.
Currently only 429s are ignored when this option is enabled. 

Also add system tests for CEL input in auth data-stream.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.
  • I have verified that any added dashboard complies with Kibana's Dashboard good practices

Author's Checklist

  • [ ]

How to test this PR locally

System tests pass for auth data-stream:

--- Test results for package: cisco_duo - START ---
╭───────────┬─────────────┬───────────┬───────────┬────────┬───────────────╮
│ PACKAGE   │ DATA STREAM │ TEST TYPE │ TEST NAME │ RESULT │  TIME ELAPSED │
├───────────┼─────────────┼───────────┼───────────┼────────┼───────────────┤
│ cisco_duo │ auth        │ system    │ cel       │ PASS   │ 46.284483459s │
│ cisco_duo │ auth        │ system    │ httpjson  │ PASS   │ 43.683903833s │
╰───────────┴─────────────┴───────────┴───────────┴────────┴───────────────╯
--- Test results for package: cisco_duo - END   ---
Done
@kcreddy kcreddy self-assigned this Feb 24, 2025
@kcreddy kcreddy added enhancement New feature or request Integration:cisco_duo Cisco Duo Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] labels Feb 24, 2025
@kcreddy
kcreddy marked this pull request as ready for review February 24, 2025 06:56
@kcreddy
kcreddy requested a review from a team as a code owner February 24, 2025 06:56
@elasticmachine

elasticmachine commented Feb 24, 2025

Copy link
Copy Markdown

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)
@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Feb 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

🚀 Benchmarks report

Package cisco_duo 👍(3) 💚(0) 💔(5)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
auth 3322.26 2141.33 -1180.93 (-35.55%) 💔
offline_enrollment 31250 17857.14 -13392.86 (-42.86%) 💔
summary 55555.56 27777.78 -27777.78 (-50%) 💔
telephony 58823.53 32258.06 -26565.47 (-45.16%) 💔
telephony_v2 45454.55 14705.88 -30748.67 (-67.65%) 💔

To see the full report comment with /test benchmark fullreport
@kcreddy
kcreddy requested a review from efd6 February 25, 2025 08:00
efd6
efd6 reviewed Feb 26, 2025
View reviewed changes

@efd6 efd6 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor query only, otherwise LGTM
Comment thread packages/cisco_duo/data_stream/auth/agent/stream/cel.yml.hbs Outdated
)
:
bytes(resp.Body).decode_json().as(body,
resp.StatusCode == 429 && (state.ignore_api_errors == "true") ?

@efd6 efd6 Feb 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does state.ignore_api_errors not get interpreted as a boolean by YAML?

@kcreddy kcreddy Feb 27, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it seems like without the quote, I get this error inside system tests:

Unit state changed cel-default-cel-cisco_duo-a76f1110-a53c-4c59-bf68-fec5bf59b33f (STARTING->FAILED): failed to check program: failed compilation: ERROR: <input>:76:64: found no matching overload for '_==_' applied to '(string, bool)'\n |             resp.StatusCode == 429 && (state.ignore_api_errors == true) ?\n | ...............................................................^ accessing config

I defined ignore_api_errors as boolean inside manifest.yml but still the program considers it as string.

@efd6 efd6 Feb 27, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be worth doing the converse conversion, bool(state.ignore_api_errors) in order to catch "ture" and similar.

@kcreddy kcreddy Feb 27, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c3f9c49
@kcreddy
kcreddy requested a review from efd6 February 27, 2025 06:40
@kcreddy
Address PR comment.
c3f9c49 
Convert string option to bool.
efd6
efd6 reviewed Feb 27, 2025
View reviewed changes
Comment thread packages/cisco_duo/data_stream/auth/agent/stream/cel.yml.hbs Outdated
)
:
bytes(resp.Body).decode_json().as(body,
resp.StatusCode == 429 && (bool(state.ignore_api_errors) == true) ?

@efd6 efd6 Feb 27, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
resp.StatusCode == 429 && (bool(state.ignore_api_errors) == true) ?
resp.StatusCode == 429 && bool(state.ignore_api_errors) ?

Here, otherwise madness lies.

resp.StatusCode == 429 && (…((bool(state.ignore_api_errors) == true) == true) = true) …) ?

@kcreddy kcreddy Feb 28, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching. Done in 6fff53b
@kcreddy
kcreddy requested a review from efd6 February 28, 2025 05:00
@elasticmachine

elasticmachine commented Feb 28, 2025

Copy link
Copy Markdown

💚 Build Succeeded

History

cc @kcreddy
@elastic-sonarqube

elastic-sonarqube Bot commented Feb 28, 2025

Copy link
Copy Markdown

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube
@kcreddy
kcreddy merged commit bf201c2 into elastic:main Feb 28, 2025
@elastic-vault-github-plugin-prod

elastic-vault-github-plugin-prod Bot commented Feb 28, 2025

Copy link
Copy Markdown
Contributor

Package cisco_duo - 2.4.0 containing this change is available at https://epr.elastic.co/package/cisco_duo/2.4.0/
flexitrev pushed a commit that referenced this pull request Mar 20, 2025
@kcreddy @flexitrev
cisco_duo.auth: Provide option to ignore ingesting API Errors. (#12870)
dc59749 
Cisco Duo auth API rate limit is very low i.e., once per minute. 
Sometimes 429s are received even when requests are made 
less than once per minute. This leads to users ingesting 429 
API errors very often. 

Provide an option to users to ignore ingesting API Errors.
Currently only 429s are ignored when this option is enabled. 

Also add system tests for CEL input in auth data-stream.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Integration:cisco_duo Cisco Duo Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations]

3 participants

@kcreddy @elasticmachine @efd6