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

fix(cmd/influxd): prevent panic on upgrade with V1 users and no V1 conf #20548

Merged
merged 4 commits into from
Jan 19, 2021

Conversation

danxmoran
Copy link
Contributor

Closes #20545

I reworked our existing e2e test for influxd upgrade to actually cover the main body of the command.

@danxmoran danxmoran force-pushed the dm-fix-upgrade-panic-20545 branch 2 times, most recently from 98788c7 to 600d580 Compare January 19, 2021 19:27

// This command is executed multiple times by test code. Initialization can happen only once.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The upgrade process doesn't use flux, I think this was bad copy-pasta

@danxmoran danxmoran force-pushed the dm-fix-upgrade-panic-20545 branch from 600d580 to 15ac25d Compare January 19, 2021 20:38
@@ -361,8 +361,9 @@ func runUpgradeE(cmd *cobra.Command, options *options, verbose bool) error {
}
}

var v1Config *configV1
v1Config := &configV1{}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This (making sure v1Config isn't nil) is the fix for the panic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The panic was occurring on this line (nil dereference on !v1Config.Http.AuthEnabled)

Copy link
Contributor

@lesam lesam left a comment

Choose a reason for hiding this comment

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

lgtm

@danxmoran danxmoran force-pushed the dm-fix-upgrade-panic-20545 branch from a47cf85 to 633d93c Compare January 19, 2021 20:52
@danxmoran danxmoran merged commit c62d3f2 into master Jan 19, 2021
@danxmoran danxmoran deleted the dm-fix-upgrade-panic-20545 branch January 19, 2021 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

influxd upgrade panics when V1 users are present and no V1 config is given
2 participants