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

Re-add support for configuring server.host: "0" #86806

Merged
merged 1 commit into from
Dec 22, 2020

Conversation

watson
Copy link
Contributor

@watson watson commented Dec 22, 2020

Support for this was lost in #85406.

Fixes #86716

@watson watson added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.12.0 labels Dec 22, 2020
@watson watson requested a review from a team December 22, 2020 17:26
@watson watson self-assigned this Dec 22, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@watson
Copy link
Contributor Author

watson commented Dec 22, 2020

This also has the added benefit out logging a valid URL in the log, so instead of:

Server running at http://0:5601/

It now prints:

Server running at http://0.0.0.0:5601/

@watson watson merged commit 97858d6 into elastic:master Dec 22, 2020
@watson watson deleted the support-0 branch December 22, 2020 19:40
watson added a commit that referenced this pull request Dec 22, 2020
// re-written for v20 and hapi no longer considers '0' a valid host. For
// details, see:
// https://github.com/elastic/kibana/issues/86716#issuecomment-749623781
this.host = rawHttpConfig.host === '0' ? '0.0.0.0' : rawHttpConfig.host;
Copy link
Contributor

Choose a reason for hiding this comment

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

@watson We should probably follow up in all the places where we use this for testing, replace it with 0.0.0.0 and then remove it from 0?

Thanks for the fix BTW

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's best to support this in 7.x and then wait till 8.0.0 to remove it

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree. But the 8.0.0 removal could already happen now. An email could be sent to the teams to give a heads up that it will be removed in x weeks with the fix that needs to happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fine with me 😃 I just wasn't sure what our policy on diverging the branches was. I've heard some say that we want to keep master and 7.x in sync as much as possible to ease backporting. I presume that would mean that we hold off diverging the branches until closer to the 8.0.0 release (hence why we have the big todo-list of things we want to break).

On the other and, it's easier to get it in now when it's all fresh in our heads. I'm personally leaning towards the latter, so I'll create a PR to remove this in master and then people can object if they want in the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

No strong opinion, I let the KB team comment ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've created #87114 - You mentioned that the docs use 0 as the hostname. Where is that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I did mention docs? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strike that... It was @graphaelli

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kibana docker container won't start
5 participants