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

deprecate /v2/keys/_etcd/machines #1279

Closed
yichengq opened this issue Oct 10, 2014 · 9 comments
Closed

deprecate /v2/keys/_etcd/machines #1279

yichengq opened this issue Oct 10, 2014 · 9 comments
Milestone

Comments

@yichengq
Copy link
Contributor

Reasons for the change:

  1. /_etcd/machines will be exposed to users when we remove hidden key feature
  2. we want to provide user a clean and empty key space
  3. user should have no access to internal member list
  4. easy for future change on internal member list

Possible target location:
/v2/members/

Implementation options:

  1. add a wrapper on current store to add prefix for store request and remove prefix for store response to hold user space at '/keys' and member space at '/members'
  2. use two instances of store

I prefer 1.

still digging it... Open for ideas

@yichengq yichengq added this to the v0.5.0-alpha milestone Oct 10, 2014
@jonboulle
Copy link
Contributor

I think 1) is reasonable, then we leave the door open for storing other unexposed things in future (e.g. /config).

@xiang90
Copy link
Contributor

xiang90 commented Oct 13, 2014

I am trying to understand why we need to provide an empty key space at first.
And what is the difference between internal (etcd server facing) and external member list(user facing)?
In both 1 and 2, the only difference I can see is the change of the key path.

In my opinion, it is good to provide the configuration information in a reserved key path. It does not have to be hidden.
Advantages:

  1. key space operation are available: GET and WAIT
  2. only one way to access the same piece of information in the same format

No matter how we change the "path" (as solution 1) or where we put the data (as solution 2), we still need to think about what configuration data we want to expose to user (external vs internal) and what operations we want to support on that (we probably do not want the random user update/remove a member configuration).

@sheldonh
Copy link

Yes, please.

@yichengq
Copy link
Contributor Author

@xiangli-cmu i think i should update the title to deprecate /v2/keys/_etcd/machines to make it clear.
internal usage means that the place to save and load the information for members, external usage means that the way for user to fetch and update members.
mainly this issue will focus on moving old member path to a new place in a reasonable way to solve the reasons for the change. It covers the advantages mentioned by you.
key space is just for user, and it is inconvenient if it returns /_etcd/machines/.... when recursive list in an empty space.

@jonboulle
Copy link
Contributor

@xiangli-cmu it's not just about an empty key space. As you mentioned, we likely do not want to allow users to do arbitrary operations (PUT/DELETE) on the member configuration. So I think it would be a very poor experience to have some part of the /keys/ namespace behaving inconsistently with the rest of it. IMO, /keys/ should be entirely under user control and an rm -fr /keys/ is a reasonable operation to support and have behave predictably.

@yichengq yichengq changed the title deprecate /v2/keys/_etcd/machines for internal usage deprecate /v2/keys/_etcd/machines Oct 14, 2014
@xiang90
Copy link
Contributor

xiang90 commented Oct 15, 2014

@unihorn i do not think so.
the main reasons for removing hidden path:

  1. remove the implication of "security".
  2. user should handling key filtering themselves.

I do not think we MUST provide a "clean" root space. Does any storage/database system ever aim to do that? If the user cares, they can always create a sub dir themselves.
If you are a huge fan of cleanness, you should argue we need to preserve the hidden behavior. I do not think your solution solves the root problem.

@jonboulle Yea. The real problem is permission control. I do not know... Personally I still think it is fine to have a special reversed key special for all etcd public information/configuration.
rm -rf /keys or whatever sub dir is reasonable... But I do not think rm -rf / is reasonable. Our key space starts with /keys/ prefix and the root is still /.

@yichengq
Copy link
Contributor Author

@xiangli-cmu I think our divergence sits on which is the root space. In my view, '/v2/' is the root space, and '/v2/keys/' is the space for users to play with keys. '/v2/members', '/v2/stats' are stuffs mounted for system usage.
It is better for user to start with a clean space, just as we always direct to '/home/$name' when you ssh to linux.

@jonboulle
Copy link
Contributor

+1 to what @unihorn said.
/v2/keys should be clean out of the box and rm -fr /v2/keys should be fine. Hence, machines etc should not be exposed in /v2/keys

@jonboulle jonboulle modified the milestones: v0.6.0maybe, v0.5.0-alpha Oct 17, 2014
@jonboulle
Copy link
Contributor

This is fixed by #1335

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants