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

cluster/config_manager: protect cfg.get() behind cfg.contains() #15840

Merged
merged 3 commits into from
Dec 22, 2023
Merged

cluster/config_manager: protect cfg.get() behind cfg.contains() #15840

merged 3 commits into from
Dec 22, 2023

Conversation

andijcr
Copy link
Contributor

@andijcr andijcr commented Dec 21, 2023

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

Fixes #15839

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 andijcr mentioned this pull request Dec 21, 2023
Merged
7 tasks
@andijcr
Copy link
Contributor Author

andijcr commented Dec 21, 2023

/dt

nvartolomei
nvartolomei previously approved these changes Dec 21, 2023
View reviewed changes
@vbotbuildovich
Copy link
Collaborator

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/43231#018c8d5f-4dfb-48df-91f7-1f528304122c

@andijcr
cluster/config_manager: protect cfg.get() behind cfg.contains()
d2dcba9 
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
services/admin: patch_cluster_config typing and default values
d02b426
@andijcr
tests/cluster_config_test: handling of unknown properties
f891fd6
@andijcr
Copy link
Contributor Author

andijcr commented Dec 22, 2023

force push: ducktape test that triggers the issue

@andijcr andijcr mentioned this pull request Dec 22, 2023
Merged
@andijcr andijcr merged commit aed5020 into redpanda-data:dev Dec 22, 2023
20 checks passed
@andijcr andijcr deleted the fix/dev/cfg_unchecked_test branch December 22, 2023 16:19
@vbotbuildovich
Copy link
Collaborator

/backport v23.3.x

@vbotbuildovich
Copy link
Collaborator

/backport v23.2.x

@vbotbuildovich
Copy link
Collaborator

Failed to create a backport PR to v23.2.x branch. I tried:

git remote add upstream https://github.com/redpanda-data/redpanda.git
git fetch --all
git checkout -b backport-pr-15840-v23.2.x-376 remotes/upstream/v23.2.x
git cherry-pick -x d2dcba9745a2c6e20fdaf2ff260bd57773027504 d02b426794a372c551845ee9532db21d77332f84 f891fd6feed481253a07636496d96883496ba76f

Workflow run logs.

This was referenced Dec 22, 2023
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nvartolomei nvartolomei nvartolomei approved these changes

@dotnwat dotnwat Awaiting requested review from dotnwat

@Lazin Lazin Awaiting requested review from Lazin

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

Successfully merging this pull request may close these issues.

setting a cluster property during a partial upgrade might prevent the node from starting
3 participants
@andijcr @vbotbuildovich @nvartolomei