Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Adds support for ipc socket permissions #11273

Merged
merged 13 commits into from
Dec 6, 2019
Merged

Conversation

seunlanlege
Copy link
Member

@seunlanlege seunlanlege commented Nov 20, 2019

Adds support for setting file permissions using chmod on IPC sockets through CLI or config files.

closes #9703

@seunlanlege seunlanlege added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M4-core ⛓ Core client code / Rust. labels Nov 20, 2019
@seunlanlege seunlanlege marked this pull request as ready for review November 29, 2019 10:27
@seunlanlege seunlanlege added A0-pleasereview 🤓 Pull request needs code review. M6-rpcapi 📣 RPC API. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Nov 29, 2019
@seunlanlege seunlanlege requested review from dvdplm and ordian and removed request for dvdplm November 29, 2019 10:28
Copy link
Collaborator

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

Looking good, but I have questions!

parity/rpc.rs Outdated Show resolved Hide resolved
rpc/src/lib.rs Outdated Show resolved Hide resolved
rpc/src/lib.rs Outdated Show resolved Hide resolved
parity/cli/mod.rs Outdated Show resolved Hide resolved
parity/cli/mod.rs Outdated Show resolved Hide resolved
@ordian ordian added this to the 2.7 milestone Nov 29, 2019
@ordian ordian added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 29, 2019
parity/cli/mod.rs Outdated Show resolved Hide resolved
parity/rpc.rs Outdated Show resolved Hide resolved
parity/cli/mod.rs Outdated Show resolved Hide resolved
@dvdplm
Copy link
Collaborator

dvdplm commented Nov 29, 2019

@seunlanlege once this lands, make sure you edit the wiki to document the new option.

rpc/src/lib.rs Outdated Show resolved Hide resolved
parity/rpc.rs Outdated Show resolved Hide resolved
@seunlanlege
Copy link
Member Author

seunlanlege commented Dec 3, 2019

Putting this on ice because parity-tokio-ipc doesn't support android builds, yet

@seunlanlege seunlanlege added A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Dec 3, 2019
parity/rpc.rs Outdated Show resolved Hide resolved
parity/rpc.rs Outdated Show resolved Hide resolved
@dvdplm
Copy link
Collaborator

dvdplm commented Dec 4, 2019

Putting this on ice because parity-tokio-ipc doesn't support android builds, yet

error[E0308]: mismatched types
  --> /ci-cache/parity-ethereum/cargo/build-android/registry/src/github.com-1ecc6299db9ec823/parity-tokio-ipc-0.3.0/src/unix_permissions.rs:50:37
   |
50 |             if chmod(path.as_ptr(), mode) == -1 {
   |                                     ^^^^ expected u16, found u32
help: you can convert an `u32` to `u16` and panic if the converted value wouldn't fit
   |
50 |             if chmod(path.as_ptr(), mode.try_into().unwrap()) == -1 {
   |                                     ^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: could not compile `parity-tokio-ipc`.

Is that what you mean? Seems more like a legit error than some incompatibility to me.

@niklasad1
Copy link
Collaborator

@dvdplm it is fixed in paritytech/parity-tokio-ipc#18

I think we can change label to in progress

rpc/Cargo.toml Outdated Show resolved Hide resolved
Copy link
Collaborator

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

LGTM

@niklasad1 niklasad1 added A0-pleasereview 🤓 Pull request needs code review. A8-looksgood 🦄 Pull request is reviewed well. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. A0-pleasereview 🤓 Pull request needs code review. A8-looksgood 🦄 Pull request is reviewed well. labels Dec 4, 2019
@niklasad1
Copy link
Collaborator

niklasad1 commented Dec 4, 2019

@seunlanlege please commit the updated Cargo.lock in order to fix the CI errors.

Copy link
Collaborator

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

lgtm modulo CI

@ordian ordian merged commit 64295fd into master Dec 6, 2019
@ordian ordian deleted the seun-ipc-socket-permissions branch December 6, 2019 15:40
@ordian ordian added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. labels Dec 6, 2019
dvdplm added a commit that referenced this pull request Dec 9, 2019
* master:
  Istanbul activation on xDai (#11299)
  Istanbul activation on POA Core (#11298)
  Adds support for ipc socket permissions (#11273)
  Add check for deserialising hex values over U256 limit (#11309)
  validate-chainspecs: check istanbul eips are in the foundation spec (#11305)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. B7-releasenotes 📜 Changes should be mentioned in the release notes of the next minor version release. M4-core ⛓ Core client code / Rust. M6-rpcapi 📣 RPC API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: IPC file permission
4 participants