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

auth, e2e: the root role should be granted access to every key #6356

Merged
merged 1 commit into from
Sep 6, 2016

Conversation

mitake
Copy link
Contributor

@mitake mitake commented Sep 6, 2016

This commit changes the semantics of the root role. The role should be
able to access to every key.

Partially fixes #6355

/cc @glycerine @soyking @xiang90

@xiang90
Copy link
Contributor

xiang90 commented Sep 6, 2016

We need to fix CI. Some of the tests are failing. The change looks good.

@mitake
Copy link
Contributor Author

mitake commented Sep 6, 2016

@xiang90 fixed the CI failure (it was caused by the semantics of the root role), PTAL.

@@ -666,6 +666,10 @@ func (as *authStore) isOpPermitted(userName string, revision uint64, key, rangeE
return ErrPermissionDenied
}

if hasRootRole(user) {
Copy link
Contributor

Choose a reason for hiding this comment

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

add a comment here: root role should have permission on all ranges.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

@xiang90
Copy link
Contributor

xiang90 commented Sep 6, 2016

LGTM

This commit changes the semantics of the root role. The role should be
able to access to every key.

Partially fixes etcd-io#6355
@xiang90
Copy link
Contributor

xiang90 commented Oct 9, 2016

@gyuho Have we backport this yet?

@gyuho
Copy link
Contributor

gyuho commented Oct 9, 2016

@xiang90 It's not backported yet. Should we include this in the next patch release?

@xiang90
Copy link
Contributor

xiang90 commented Oct 9, 2016

@gyuho yes. we should.

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

Successfully merging this pull request may close these issues.

etcdctl/ctlv3: auth: cannot create a root role who can readwrite all keys
3 participants