Warn if ACL is enabled but no token is provided to Envoy #15967
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kisunji
force-pushed
the
kisunji/NET-1826-envoy-warn-no-acl-token
branch
from
299ba29 to
f48b94e
Compare
github-actions
bot
added
theme/cli
kisunji
force-pushed
the
kisunji/NET-1826-envoy-warn-no-acl-token
branch
2 times, most recently
from
98bdd2a to
0874992
Compare
kisunji
changed the title
kisunji
requested review from
a team,
rboyer,
pglass,
DanStough and
wilkermichael
and removed request for
a team
rboyer
reviewed
Jan 12, 2023
rboyer
reviewed
Jan 12, 2023
cizh
reviewed
Jan 12, 2023
kisunji
force-pushed
the
kisunji/NET-1826-envoy-warn-no-acl-token
branch
from
978deec to
48f14ba
Compare
kisunji
commented
Jan 13, 2023
kisunji
commented
Jan 13, 2023
| Help: "Measures the number of active xDS streams handled by the server split by protocol version.", | ||
| }, | ||
| { | ||
| Name: []string{"xds", "server", "streamsUnauthenticated"}, |
There was a problem hiding this comment.
Does this read better as unauthenticatedStreams?
I thought it would be nicer to close to the related streams gauge when sorted alphabetically.
kisunji
commented
Jan 13, 2023
pglass
reviewed
Jan 13, 2023
| @@ -541,6 +541,7 @@ These metrics are used to monitor the health of the Consul servers. | |||
| | `consul.grpc.server.stream.count` | Counts the number of new gRPC streams received by the server. Includes a `server_type` label indicating either the `internal` or `external` gRPC server. | streams | counter | | |||
| | `consul.grpc.server.streams` | Measures the number of active gRPC streams handled by the server. Includes a `server_type` label indicating either the `internal` or `external` gRPC server. | streams | gauge | | |||
| | `consul.xds.server.streams` | Measures the number of active xDS streams handled by the server split by protocol version. | streams | gauge | | |||
| | `consul.xds.server.streamsUnauthenticated` | Measures the number of active xDS streams handled by the server that are unauthenticated because ACLs are not enabled or ACL tokens were missing. | streams | gauge | | |||
There was a problem hiding this comment.
What does this gauge look like if ACLs are enabled. Will we have any (long-lived) xDS streams that are unauthenticated? Or would this basically be a gauge of the attempts to open an xDS stream that are unauthenticated?
There was a problem hiding this comment.
Whether ACLs are on or off this gauge will track streams that do not have tokens associated with them. I'm not entirely sure if any unauthenticated streams will be long-lived, unless the anonymous token is privileged enough to continue operating.
pglass
approved these changes
Jan 13, 2023
Co-authored-by: Jared Kirschner <85913323+jkirschner-hashicorp@users.noreply.github.com>
kisunji
force-pushed
the
kisunji/NET-1826-envoy-warn-no-acl-token
branch
from
7c54180 to
25760a9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When ACLs are enabled, the CLI command
consul connect envoyrequires a valid ACL token for the sidecar proxy to function. However, the command may not error in the absence of a token since the anonymous token fallback will usually be sufficient forconsul connect envoyto complete execution.This PR adds a call to check if ACLs are enabled then outputs a warning if ACLs are enabled but no token was provided.
This PR also adds a metric to count the number of unauthenticated streams in xDS.
PR Checklist