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

Replication token needs acl write on all ns's #370

Merged
merged 1 commit into from
Oct 29, 2020

Conversation

lkysow
Copy link
Member

@lkysow lkysow commented Oct 27, 2020

If namespaces are enabled, the replication token will need to be able to
replicate all tokens and policies from all namespaces. Without this
functionality, users won't be able to use namespaced
tokens/policies/roles in secondary datacenters (since they won't be
replicated).

Fixes #364

How I've tested this PR:

  • Federated two clusters with ACLs enabled
  • Created a namespace luke
  • Created a policy and token in namespace luke:
    acl = "read"
  • On secondary DC, exec'd into server and ran:
    curl -H "X-Consul-Token: 5207f7dd-cfb0-7d7f-b0a5-964295bcb3a1" -k https://localhost:8501/v1/acl/token/self
    ACL not found
  • Modified replication ACL policy to have namespace_prefix ""
  • Confirmed that (after a 5m delay) that the API call worked

How I expect reviewers to test this PR:

  • code review

Checklist:

  • Tests added
  • CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

@lkysow lkysow requested review from a team, ndhanushkodi and thisisnotashwin and removed request for a team October 29, 2020 20:57
@lkysow lkysow marked this pull request as ready for review October 29, 2020 20:57
@@ -233,6 +232,7 @@ node_prefix "" {
{{- if .EnableNamespaces }}
namespace_prefix "" {
{{- end }}
acl = "write"
Copy link
Member Author

Choose a reason for hiding this comment

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

The key is here. Moving this inside the namespace_prefix "" block.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does namespace_prefix "" mean the token created from these rules has that permission for any namespace?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes exactly. If it was namespace "blah" then it would be only for blah and if it was namespace_prefix "blah-" it would be for blah-.*

Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

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

🦊

@@ -233,6 +232,7 @@ node_prefix "" {
{{- if .EnableNamespaces }}
namespace_prefix "" {
{{- end }}
acl = "write"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does namespace_prefix "" mean the token created from these rules has that permission for any namespace?

CHANGELOG.md Outdated Show resolved Hide resolved
If namespaces are enabled, the replication token will need to be able to
replicate all tokens and policies from all namespaces. Without this
functionality, users won't be able to use namespaced
tokens/policies/roles in secondary datacenters (since they won't be
replicated).
@lkysow lkysow merged commit 304aa72 into master Oct 29, 2020
@lkysow lkysow deleted the replication-token-global branch October 29, 2020 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replication token cannot replicate namespaced tokens
3 participants