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

Fixes #28738: Reset bind host if :: is present #447

Merged
merged 1 commit into from
Jan 15, 2020

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Jan 13, 2020

This is necessary with smart proxy moving away from Ruby 2.0 via
RPM packaging changes to use SCLs.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

You're not guaranteed it's an array. A string is also a valid input. Technically that's deprecated since 1.15 but we haven't made the change yet to drop it. I'd suggest checking with .is_a?(Array) just to be sure, also in case it's nil.

Technically this was a valid config on Debian-based systems to bind to IPv6-only and only affected EL7. Though it's quite unlikely users actually ran into this, it may make sense to add an OS check in config/foreman.migrations.

@ehelms
Copy link
Member Author

ehelms commented Jan 13, 2020

This check will com ein handy for that -- https://github.com/theforeman/foreman-installer/pull/444/files#diff-c6fdf0194de166ebab0dde67cd968490R32

Would it be safe to reset the value of this in all cases? Or are there instances where this will have been configured by a user?

@ehelms
Copy link
Member Author

ehelms commented Jan 14, 2020

Added array and EL7 check to the migrations.

@@ -0,0 +1,6 @@
if answers['foreman_proxy']['bind_host'].is_a?(Array) &&
Copy link
Member

Choose a reason for hiding this comment

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

answers['foreman_proxy'] can be false in which case it crashes. I don't think we can use answers.dig('foreman_proxy', 'bind_host') in Ruby 2.0 yet, but that would be the defensive way.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added an is_a?(Hash) check

This is necessary with smart proxy moving away from Ruby 2.0 via
RPM packaging changes to use SCLs.
@ehelms ehelms merged commit 172f51e into theforeman:develop Jan 15, 2020
@ehelms ehelms deleted the fixes-28738 branch January 15, 2020 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants