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.

Sign up for GitHub

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

Jump to bottom

[Filebeat][okta] Fix okta pagination #21797

Merged
merged 2 commits into from
Oct 16, 2020
Merged

[Filebeat][okta] Fix okta pagination #21797

merged 2 commits into from
Oct 16, 2020

Conversation

marc-gr
Copy link
Contributor

@marc-gr marc-gr commented Oct 14, 2020 •
edited
Loading

What does this PR do?

  • Fixed header checking for the pagination used by okta
  • Add sanity check when initializing the date cursor
  • Advance cursor at every page
  • Store okta cursor

Why is it important?

Okta was failing to get pagination information correctly from headers

Checklist

  • My code follows the style guidelines of this project
    - [ ] I have commented my code, particularly in hard-to-understand areas
    - [ ] I have made corresponding changes to the documentation
    - [ ] I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
    - [ ] I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

@marc-gr marc-gr requested a review from a team October 14, 2020 13:51
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Oct 14, 2020
@marc-gr marc-gr added the in progress Pull request is currently in progress. label Oct 14, 2020
@marc-gr marc-gr force-pushed the fix_okta_pagination branch from 779936e to 50c6cef Compare October 14, 2020 14:25
@marc-gr marc-gr added review and removed in progress Pull request is currently in progress. labels Oct 14, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 14, 2020 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #21797 updated]

  • Start Time: 2020-10-14T14:26:41.141+0000

  • Duration: 68 min 5 sec

Test stats 🧪

Test Results
Failed 0
Passed 1944
Skipped 259
Total 2203

andrewkroh
andrewkroh reviewed Oct 14, 2020
View reviewed changes
x-pack/filebeat/input/httpjson/pagination.go
@@ -72,7 +72,7 @@ func (p *pagination) nextRequestInfo(ri *requestInfo, response response, lastObj

// getNextLinkFromHeader retrieves the next URL for pagination from the HTTP Header of the response
func getNextLinkFromHeader(header http.Header, fieldName string, re *regexp.Regexp) (string, error) {
links, ok := header[fieldName]
links, ok := header[http.CanonicalHeaderKey(fieldName)]
Copy link
Member

Choose a reason for hiding this comment

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

What if the service didn't implement its headers using the canonical form? Should our code look for both the value that the user configured and the canonical form (if they differ)?

Copy link
Contributor Author

@marc-gr marc-gr Oct 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think from the docs http.Header keys are always in the canonical form (https://golang.org/pkg/net/http/#Header, https://golang.org/src/net/http/response.go), I added this to take this into account since we are accessing the map directly instead of using the Get method which takes care of doing it itself.

@marc-gr marc-gr requested a review from andrewkroh October 15, 2020 14:25
@marc-gr marc-gr merged commit 80d4209 into elastic:master Oct 16, 2020
@marc-gr marc-gr deleted the fix_okta_pagination branch October 16, 2020 07:01
marc-gr added a commit to marc-gr/beats that referenced this pull request Oct 16, 2020
@marc-gr
[Filebeat][okta] Fix okta pagination (elastic#21797)
c43b8dc 
* Fix okta pagination

* Use cursor storage

(cherry picked from commit 80d4209)
@marc-gr marc-gr mentioned this pull request Oct 16, 2020
Merged
2 tasks
marc-gr added a commit to marc-gr/beats that referenced this pull request Oct 16, 2020
@marc-gr
[Filebeat][okta] Fix okta pagination (elastic#21797)
4bcd5a4 
* Fix okta pagination

* Use cursor storage

(cherry picked from commit 80d4209)
@marc-gr marc-gr mentioned this pull request Oct 16, 2020
Merged
2 tasks
v1v added a commit to v1v/beats that referenced this pull request Oct 19, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/windows-10
0c494b2 
* upstream/master: (23 commits)
  [Ingest Manager] Prevent reporting ecs version twice (elastic#21616)
  [CI] Use google storage to keep artifacts (elastic#21910)
  Update docs.asciidoc (elastic#21849)
  Kubernetes leaderelection improvements (elastic#21896)
  Apply name changes to elastic agent docs (elastic#21549)
  Add 7.7.1 relnotes to 7.8 docs (elastic#21937) (elastic#21941)
  [libbeat] Fix potential deadlock in the disk queue + add more unit tests (elastic#21930)
  Refactor docker watcher to fix flaky test and other small issues (elastic#21851)
  [CI] Add stage name in the step (elastic#21887)
  [docs] Remove extra word in autodiscover docs (elastic#21871)
  [CI] lint stage doesn't produce test reports (elastic#21888)
  Add tests of reader of filestream input (elastic#21814)
  [Ingest Manager] Use local temp instead of system one (elastic#21883)
  chore: delegate variant pushes to the right method (elastic#21861)
  [CI] kind setup fails sometimes (elastic#21857)
  Fix panic on add_docker_metadata close (elastic#21882)
  Add tests for fileProspector in filestream input (elastic#21712)
  [Filebeat][okta] Fix okta pagination (elastic#21797)
  Add cloud.account.id into add_cloud_metadata for gcp (elastic#21776)
  Fix syslog RFC 5424 parsing in CheckPoint module (elastic#21854)
  ...
v1v added a commit to v1v/beats that referenced this pull request Oct 19, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/windows-2008
40f297d 
* upstream/master: (23 commits)
  [Ingest Manager] Prevent reporting ecs version twice (elastic#21616)
  [CI] Use google storage to keep artifacts (elastic#21910)
  Update docs.asciidoc (elastic#21849)
  Kubernetes leaderelection improvements (elastic#21896)
  Apply name changes to elastic agent docs (elastic#21549)
  Add 7.7.1 relnotes to 7.8 docs (elastic#21937) (elastic#21941)
  [libbeat] Fix potential deadlock in the disk queue + add more unit tests (elastic#21930)
  Refactor docker watcher to fix flaky test and other small issues (elastic#21851)
  [CI] Add stage name in the step (elastic#21887)
  [docs] Remove extra word in autodiscover docs (elastic#21871)
  [CI] lint stage doesn't produce test reports (elastic#21888)
  Add tests of reader of filestream input (elastic#21814)
  [Ingest Manager] Use local temp instead of system one (elastic#21883)
  chore: delegate variant pushes to the right method (elastic#21861)
  [CI] kind setup fails sometimes (elastic#21857)
  Fix panic on add_docker_metadata close (elastic#21882)
  Add tests for fileProspector in filestream input (elastic#21712)
  [Filebeat][okta] Fix okta pagination (elastic#21797)
  Add cloud.account.id into add_cloud_metadata for gcp (elastic#21776)
  Fix syslog RFC 5424 parsing in CheckPoint module (elastic#21854)
  ...
v1v added a commit to v1v/beats that referenced this pull request Oct 19, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/support-f…
4039f89 
…laky-test-analyser

* upstream/master: (22 commits)
  [Ingest Manager] Prevent reporting ecs version twice (elastic#21616)
  [CI] Use google storage to keep artifacts (elastic#21910)
  Update docs.asciidoc (elastic#21849)
  Kubernetes leaderelection improvements (elastic#21896)
  Apply name changes to elastic agent docs (elastic#21549)
  Add 7.7.1 relnotes to 7.8 docs (elastic#21937) (elastic#21941)
  [libbeat] Fix potential deadlock in the disk queue + add more unit tests (elastic#21930)
  Refactor docker watcher to fix flaky test and other small issues (elastic#21851)
  [CI] Add stage name in the step (elastic#21887)
  [docs] Remove extra word in autodiscover docs (elastic#21871)
  [CI] lint stage doesn't produce test reports (elastic#21888)
  Add tests of reader of filestream input (elastic#21814)
  [Ingest Manager] Use local temp instead of system one (elastic#21883)
  chore: delegate variant pushes to the right method (elastic#21861)
  [CI] kind setup fails sometimes (elastic#21857)
  Fix panic on add_docker_metadata close (elastic#21882)
  Add tests for fileProspector in filestream input (elastic#21712)
  [Filebeat][okta] Fix okta pagination (elastic#21797)
  Add cloud.account.id into add_cloud_metadata for gcp (elastic#21776)
  Fix syslog RFC 5424 parsing in CheckPoint module (elastic#21854)
  ...
marc-gr added a commit that referenced this pull request Oct 22, 2020
@marc-gr
[Filebeat][okta] Fix okta pagination (#21797) (#21878)
528d200 
* Fix okta pagination

* Use cursor storage

(cherry picked from commit 80d4209)
marc-gr added a commit that referenced this pull request Oct 22, 2020
@marc-gr
[Filebeat][okta] Fix okta pagination (#21797) (#21879)
05c1aa6 
* Fix okta pagination

* Use cursor storage

(cherry picked from commit 80d4209)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewkroh andrewkroh andrewkroh approved these changes

Assignees
No one assigned
Labels
bug Filebeat Filebeat review v7.10.0 v7.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@marc-gr @elasticmachine @andrewkroh