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

Configure anonymous token policy for connect #230

Merged
merged 1 commit into from
Mar 19, 2020

Conversation

lkysow
Copy link
Member

@lkysow lkysow commented Mar 10, 2020

When running Consul Connect, cross-dc calls require that the anonymous
token has read permissions on all services. This change updates the
server-acl-init command to give the anonymous token those permissions if
connect is enabled.

Since we already set those permissions in the case of dns being enabled,
the change was to also set those permissions in the case of connect
being enabled. To detect connect being enabled, we used the presence of
the -create-inject-auth-method flag since that's set when connect is
enabled.

The policy was renamed from dns-policy to anonymous-token-policy since
it applies for more than just dns now. In existing installations, a new
policy with that name will be created and attached to the anonymous
token that will duplicate the old dns-policy but will have no
detrimental effects.

@lkysow lkysow force-pushed the wan-federation-acl-anon-policy branch from 2ee7dfe to 11136eb Compare March 10, 2020 21:02
@lkysow lkysow marked this pull request as ready for review March 10, 2020 21:52
@lkysow lkysow requested a review from a team March 12, 2020 13:52
@lkysow lkysow added area/acls Related to ACLs area/multi-dc Related to running with multiple datacenters type/enhancement New feature or request labels Mar 12, 2020
@lkysow lkysow force-pushed the wan-federation-acl-replication branch from 377040e to 433e036 Compare March 16, 2020 16:57
@lkysow lkysow force-pushed the wan-federation-acl-anon-policy branch 2 times, most recently from 35d2e42 to ab7c485 Compare March 16, 2020 17:16
Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

Luke, looks good! I left some comments around using connect without DNS and multi-dc setup and a suggestion.

My other question is around upgrading from an existing installation. Since we'll create a policy with the new name, the old dns policy will stay around. I think Consul API allows us to update the policy's name since the update API is using policy's ID. Do you think it makes sense to do that? The risk is that someone is attaching this policy to the tokens we don't manage.

subcommand/server-acl-init/anonymous_token.go Outdated Show resolved Hide resolved
subcommand/server-acl-init/command.go Outdated Show resolved Hide resolved
@lkysow lkysow force-pushed the wan-federation-acl-anon-policy branch from 93a2cee to e2d7568 Compare March 18, 2020 20:00
@lkysow
Copy link
Member Author

lkysow commented Mar 18, 2020

My other question is around upgrading from an existing installation. Since we'll create a policy with the new name, the old dns policy will stay around. I think Consul API allows us to update the policy's name since the update API is using policy's ID. Do you think it makes sense to do that? The risk is that someone is attaching this policy to the tokens we don't manage.

I think that the increased complexity probably isn't worth it in this case. The policy will still exist and work for the tokens they've attached it to and the policies are the exact same so it's not like they'll miss out on the new rules being attached to those tokens.

Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

LGTM

@lkysow lkysow changed the base branch from wan-federation-acl-replication to wan-federation-base March 19, 2020 16:54
When running Consul Connect, cross-dc calls require that the anonymous
token has read permissions on all services. This change updates the
server-acl-init command to give the anonymous token those permissions if
connect is enabled.

Since we already set those permissions in the case of dns being enabled,
the change was to also set those permissions in the case of connect
being enabled. To detect connect being enabled, we used the presence of
the -create-inject-auth-method flag since that's set when connect is
enabled.

The policy was renamed from dns-policy to anonymous-token-policy since
it applies for more than just dns now. In existing installations, a new
policy with that name will be created and attached to the anonymous
token that will duplicate the old dns-policy but will have no
detrimental effects.
@lkysow lkysow force-pushed the wan-federation-acl-anon-policy branch from 19d17b1 to 7fd044f Compare March 19, 2020 16:56
@lkysow lkysow merged commit ffdf97d into wan-federation-base Mar 19, 2020
@lkysow lkysow deleted the wan-federation-acl-anon-policy branch March 19, 2020 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/acls Related to ACLs area/multi-dc Related to running with multiple datacenters type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants