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

Bind RPC to localhost by default, add to sample config #7835

Merged
merged 3 commits into from
May 15, 2017

Conversation

mark-rushakoff
Copy link
Contributor

@mark-rushakoff mark-rushakoff commented Jan 13, 2017

Bind the RPC port to localhost by default so it is not potentially exposed to the public internet.

Add toplevel bind-address to the sample config because it was missing, and fix a couple typos in the config.

  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated

Copy link
Contributor

@jwilder jwilder left a comment

Choose a reason for hiding this comment

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

This will break upgrades and has been problematic in the past when the address is saved in the meta store and is changed.

I don't think we should make this change.

@jwilder
Copy link
Contributor

jwilder commented Jan 13, 2017

Actually, just realized this is the 8088 port and not the 8086 one. This might be ok.

@gunnaraasen
Copy link
Member

We should also reevaluate whether we need to use a separate port 8088 at some point, since it only handles backup and restore (I think).

@jwilder jwilder modified the milestones: 1.3.0, 1.2.0 Jan 23, 2017
CHANGELOG.md Outdated
@@ -45,6 +45,7 @@ The stress tool `influx_stress` will be removed in a subsequent release. We reco
- [#7585](https://github.com/influxdata/influxdb/pull/7585): Return Error instead of panic when decoding point values.
- [#7812](https://github.com/influxdata/influxdb/issues/7812): Fix slice out of bounds panic when pruning shard groups. Thanks @vladlopes
- [#7822](https://github.com/influxdata/influxdb/issues/7822): Drop database will delete /influxdb/data directory
- [#7835](https://github.com/influxdata/influxdb/pull/7835): Bind backup and restore port to localhost by default
Copy link
Contributor

@jwilder jwilder Mar 21, 2017

Choose a reason for hiding this comment

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

Needs to be moved to 1.3 section now.

Also, I think we need to add a note under a configuration changes section (like prior releases) that notes the change in default value. Some people may be relying on the current default for remote backups.

Prior to this change, the default configuration would listen on all
interfaces, potentially exposing the RPC to the public internet.
@mark-rushakoff
Copy link
Contributor Author

Finally updated the changelog and rebased, @jwilder.

@mark-rushakoff mark-rushakoff merged commit 51b60e2 into master May 15, 2017
@mark-rushakoff mark-rushakoff deleted the mr-rpc-bind branch May 15, 2017 18:03
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