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

[v23.3.x] cluster/config_manager: protect cfg.get() behind cfg.contains() #15838

Merged
merged 3 commits into from
Jan 4, 2024
Merged

[v23.3.x] cluster/config_manager: protect cfg.get() behind cfg.contains() #15838

merged 3 commits into from
Jan 4, 2024

Conversation

andijcr
Copy link
Contributor

@andijcr andijcr commented Dec 21, 2023 •
edited
Loading

cfg.get() can throw std::out_of_range if the name is not known by redpanda.

this can be the case for unknown properties that nonetheless gets written to the controller log, and later loaded again during startup

Backport of PR #15840

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x
  • v23.1.x

Release Notes

Bug Fixes

  • safer handle unknown properties in local state

@andijcr
cluster/config_manager: protect cfg.get() behind cfg.contains()
3d8bfcf 
cfg.get() can throw std::out_of_range if the name is not known by
redpanda.

this can be the case for unknown properties that nonetheless gets
written to the controller log, and later loaded again during startup
@andijcr andijcr changed the title cluster/config_manager: protect cfg.get() behind cfg.contains() [23.3.xcluster/config_manager: protect cfg.get() behind cfg.contains() Dec 21, 2023
@andijcr andijcr changed the title [23.3.xcluster/config_manager: protect cfg.get() behind cfg.contains() [23.3.x] cluster/config_manager: protect cfg.get() behind cfg.contains() Dec 21, 2023
@andijcr andijcr changed the title [23.3.x] cluster/config_manager: protect cfg.get() behind cfg.contains() [v23.3.x] cluster/config_manager: protect cfg.get() behind cfg.contains() Dec 21, 2023
dotnwat
dotnwat previously requested changes Dec 21, 2023
View reviewed changes
Copy link
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

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

test?

@andijcr andijcr requested a review from dotnwat December 22, 2023 16:11
@andijcr andijcr mentioned this pull request Dec 22, 2023
Closed
@andijcr andijcr linked an issue Dec 22, 2023 that may be closed by this pull request
[v23.3.x] setting a cluster property during a partial upgrade might prevent the node from starting #15878
Closed
@andijcr
Copy link
Contributor Author

andijcr commented Dec 22, 2023

test?

last commit has a test for this

@piyushredpanda piyushredpanda merged commit f73439d into redpanda-data:v23.3.x Jan 4, 2024
20 checks passed
@github-actions github-actions bot mentioned this pull request Jan 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lazin Lazin Lazin approved these changes

@dotnwat dotnwat Awaiting requested review from dotnwat

Assignees
No one assigned
Labels
area/redpanda
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[v23.3.x] setting a cluster property during a partial upgrade might prevent the node from starting
4 participants
@andijcr @Lazin @dotnwat @piyushredpanda