Fix panic when MaxRetryInterval is specified, but RetryInterval is not #35820

tibordp · 2023-06-19T15:23:14Z

What does this PR do?

In disk queue configuration, it seems like the MaxRetryInterval is read from the wrong field, which leads to a panic when RetryInterval is not provided.

Also, MaxRetryInterval doesn't seem configurable at all as it is.

Why is it important?

So MaxRetryInterval can be configured

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.

Logs

{"panic": "runtime error: invalid memory address or nil pointer dereference", "stack": "github.com/elastic/beats/v7/libbeat/cmd/instance.Run.func1.1\n\t/go/src/github.com/elastic/beats/libbeat/cmd/instance/beat.go:170\nruntime.gopanic\n\t/usr/local/go/src/runtime/panic.go:1038\nruntime.panicmem\n\t/usr/local/go/src/runtime/panic.go:221\nruntime.sigpanic\n\t/usr/local/go/src/runtime/signal_unix.go:735\ngit.luolix.top/elastic/beats/v7/libbeat/publisher/queue/diskqueue.SettingsForUserConfig\n\t/go/src/github.com/elastic/beats/libbeat/publisher/queue/diskqueue/config.go:165\ngit.luolix.top/elastic/beats/v7/libbeat/publisher/queue/diskqueue.queueFactory\n\t/go/src/github.com/elastic/beats/libbeat/publisher/queue/diskqueue/queue.go:104\ngit.luolix.top/elastic/beats/v7/libbeat/publisher/pipeline.createQueueBuilder.func1\n\t/go/src/github.com/elastic/beats/libbeat/publisher/pipeline/module.go:197\ngit.luolix.top/elastic/beats/v7/libbeat/publisher/pipeline.New\n\t/go/src/github.com/elastic/beats/libbeat/publisher/pipeline/pipeline.go:176\ngit.luolix.top/elastic/beats/v7/libbeat/publisher/pipeline.LoadWithSettings\n\t/go/src/github.com/elastic/beats/libbeat/publisher/pipeline/module.go:108\ngit.luolix.top/elastic/beats/v7/libbeat/publisher/pipeline.Load\n\t/go/src/github.com/elastic/beats/libbeat/publisher/pipeline/module.go:76\ngit.luolix.top/elastic/beats/v7/libbeat/cmd/instance.(*Beat).createBeater\n\t/go/src/github.com/elastic/beats/libbeat/cmd/instance/beat.go:383\ngit.luolix.top/elastic/beats/v7/libbeat/cmd/instance.(*Beat).launch\n\t/go/src/github.com/elastic/beats/libbeat/cmd/instance/beat.go:459\ngit.luolix.top/elastic/beats/v7/libbeat/cmd/instance.Run.func1\n\t/go/src/github.com/elastic/beats/libbeat/cmd/instance/beat.go:215\ngit.luolix.top/elastic/beats/v7/libbeat/cmd/instance.Run\n\t/go/src/github.com/elastic/beats/libbeat/cmd/instance/beat.go:216\ngit.luolix.top/elastic/beats/v7/libbeat/cmd.genRunCmd.func1\n\t/go/src/github.com/elastic/beats/libbeat/cmd/run.go:36\ngit.luolix.top/spf13/cobra.(*Command).execute\n\t/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:846\ngit.luolix.top/spf13/cobra.(*Command).ExecuteC\n\t/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:950\ngit.luolix.top/spf13/cobra.(*Command).Execute\n\t/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:887\nmain.main\n\t/go/src/github.com/elastic/beats/x-pack/filebeat/main.go:22\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:255"}

cla-checker-service · 2023-06-19T15:23:19Z

💚 CLA has been signed

mergify · 2023-06-19T15:24:36Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @tibordp? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

elasticmachine · 2023-06-19T15:28:05Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-06-19T20:49:14.703+0000
Duration: 75 min 40 sec

Test stats 🧪

Test	Results
Failed	0
Passed	27380
Skipped	2013
Total	29393

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2023-06-19T15:39:36Z

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

cmacknz · 2023-06-19T20:48:59Z

/test

tibordp · 2023-06-20T14:57:01Z

Is there any chance that this fix be backported to 7.x?

pierrehilbert · 2023-06-20T15:19:44Z

Let's wait for @faec's review here but no blocking point to have it backported in 7.17 on my side.

faec

Looks great, thanks for finding this!

faec · 2023-06-20T19:12:42Z

Looks good, I will start the backports, I think we're on track to make the impending release cuts for 7.x and 8.9

#35820) * Fix panic when MaxRetryInterval is specified, but RetryInterval is not * add changelog (cherry picked from commit 3b73003)

#35820) (#35839) * Fix panic when MaxRetryInterval is specified, but RetryInterval is not * add changelog (cherry picked from commit 3b73003) Co-authored-by: Tibor Djurica Potpara <tibor.djurica@ojdip.net>

#35820) (#35838) * Fix panic when MaxRetryInterval is specified, but RetryInterval is not * add changelog (cherry picked from commit 3b73003) Co-authored-by: Tibor Djurica Potpara <tibor.djurica@ojdip.net>

faec · 2023-06-20T21:07:38Z

Backports merged! I will follow up soon with a unit test but this fix should appear in the releases later this month

Fix panic when MaxRetryInterval is specified, but RetryInterval is not

a10c44a

tibordp requested a review from a team as a code owner June 19, 2023 15:23

tibordp requested review from pierrehilbert and fearful-symmetry June 19, 2023 15:23

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jun 19, 2023

mergify bot assigned tibordp Jun 19, 2023

add changelog

03141eb

pierrehilbert requested review from faec and removed request for pierrehilbert June 19, 2023 15:39

pierrehilbert added the Team:Elastic-Agent Label for the Agent team label Jun 19, 2023

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 19, 2023

cmacknz added backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.8.0 Automated backport with mergify labels Jun 20, 2023

faec approved these changes Jun 20, 2023

View reviewed changes

faec merged commit 3b73003 into elastic:main Jun 20, 2023

mergify bot pushed a commit that referenced this pull request Jun 20, 2023

Fix panic when MaxRetryInterval is specified, but RetryInterval is not (

459a154

#35820) * Fix panic when MaxRetryInterval is specified, but RetryInterval is not * add changelog (cherry picked from commit 3b73003)

mergify bot pushed a commit that referenced this pull request Jun 20, 2023

Fix panic when MaxRetryInterval is specified, but RetryInterval is not (

5d429b3

#35820) * Fix panic when MaxRetryInterval is specified, but RetryInterval is not * add changelog (cherry picked from commit 3b73003)

This was referenced Jun 20, 2023

[7.17](backport #35820) Fix panic when MaxRetryInterval is specified, but RetryInterval is not #35838

Merged

[8.8](backport #35820) Fix panic when MaxRetryInterval is specified, but RetryInterval is not #35839

Merged

reakaleek mentioned this pull request Jul 19, 2023

Fix ironbank validation in 8.8 #36115

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix panic when MaxRetryInterval is specified, but RetryInterval is not #35820

Fix panic when MaxRetryInterval is specified, but RetryInterval is not #35820

tibordp commented Jun 19, 2023 •

edited

Loading

cla-checker-service bot commented Jun 19, 2023 •

edited

Loading

mergify bot commented Jun 19, 2023

elasticmachine commented Jun 19, 2023 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

elasticmachine commented Jun 19, 2023

cmacknz commented Jun 19, 2023

tibordp commented Jun 20, 2023

pierrehilbert commented Jun 20, 2023

faec left a comment

faec commented Jun 20, 2023

faec commented Jun 20, 2023

Fix panic when MaxRetryInterval is specified, but RetryInterval is not #35820

Fix panic when MaxRetryInterval is specified, but RetryInterval is not #35820

Conversation

tibordp commented Jun 19, 2023 • edited Loading

What does this PR do?

Why is it important?

Checklist

Logs

cla-checker-service bot commented Jun 19, 2023 • edited Loading

mergify bot commented Jun 19, 2023

elasticmachine commented Jun 19, 2023 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

elasticmachine commented Jun 19, 2023

cmacknz commented Jun 19, 2023

tibordp commented Jun 20, 2023

pierrehilbert commented Jun 20, 2023

faec left a comment

Choose a reason for hiding this comment

faec commented Jun 20, 2023

faec commented Jun 20, 2023

tibordp commented Jun 19, 2023 •

edited

Loading

cla-checker-service bot commented Jun 19, 2023 •

edited

Loading

elasticmachine commented Jun 19, 2023 •

edited by jenkins-beats-ci bot

Loading