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.

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

ec_deployment: Fix panic on null user_settings_* #355

Merged
merged 7 commits into from
Aug 26, 2021

Conversation

marclop
Copy link
Contributor

@marclop marclop commented Aug 24, 2021

Description

Fixes a panic when any elasticsearch.config.user_settings_* are set to
null.

Related Issues

Closes #345

How Has This Been Tested?

Unit and acceptance tested.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)

Fixes a panic when any `elasticsearch.config.user_settings_*` are set to
`null`.

Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
@marclop marclop added bug Something isn't working Team:Delivery labels Aug 24, 2021
@marclop marclop self-assigned this Aug 24, 2021
@marclop marclop requested a review from a team as a code owner August 24, 2021 08:42
Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
@marclop marclop changed the title ec_deployment: Fix panic on null user_settings_* ec_deployment: Fix panic on null user_settings_* Aug 24, 2021
Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
Copy link

@rhass rhass left a comment

Choose a reason for hiding this comment

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

Seems fine... but I there are some details in the tests which I would like to learn about.

Does not store the `[]interface{}{map[string]interface{}}` to the state
if the underlying map has no keys. This avoids unnecessary state storage
and the previous import test failure.

Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
@marclop
Copy link
Contributor Author

marclop commented Aug 26, 2021

Chatted about it in Slack. Merging.

@marclop marclop merged commit 8d45c61 into elastic:master Aug 26, 2021
@marclop marclop deleted the b/fix-empty-config branch August 26, 2021 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config blocks unable to handle default empty or omitted settings
2 participants