Occasionally we had seen the TestWatchServers_ACLToken_PermissionDenied be flagged as flaky in circleci. This change should fix that.
Why it fixes it is complicated. The test was failing with a panic when a mocked ACL Resolver was being called more times than expected. I struggled for a while to determine how that could be. This test should call authorize once and only once and the error returned should cause the stream to be terminated and the error returned to the gRPC client. Another oddity was no amount of running this test locally seemed to be able to reproduce the issue. I ran the test hundreds of thousands of time and it always passed.
It turns out that there is nothing wrong with the test. It just so happens that the panic from unexpected invocation of a mocked call happened during the test but was caused by a previous test (specifically the TestWatchServers_StreamLifecycle test)
The stream from the previous test remained open after all the test Cleanup functions were run and it just so happened that when the EventPublisher eventually picked up that the context was cancelled during cleanup, it force closes all subscriptions which causes some loops to be re-entered and the streams to be reauthorized. Its that looping in response to forced subscription closures that causes the mock to eventually panic. All the components, publisher, server, client all operate based on contexts. We cancel all those contexts but there is no syncrhonous way to know when they are stopped.
We could have implemented a syncrhonous stop but in the context of an actual running Consul, context cancellation + async stopping is perfectly fine. What we (Dan and I) eventually thought was that the behavior of grpc streams such as this when a server was shutting down wasn’t super helpful. What we would want is for a client to be able to distinguish between subscription closed because something may have changed requiring re-authentication and subscription closed because the server is shutting down. That way we can send back appropriate error messages to detail that the server is shutting down and not confuse users with potentially needing to resubscribe.
So thats what this PR does. We have introduced a shutting down state to our event subscriptions and the various streaming gRPC services that rely on the event publisher will all just behave correctly and actually stop the stream (not attempt transparent reauthorization) if this particular error is the one we get from the stream. Additionally the error that gets transmitted back through gRPC when this does occur indicates to the consumer that the server is going away. That is more helpful so that a client can then attempt to reconnect to another server.
OSS portion of enterprise PR 1857.
This removes (most) references to the `cache.UpdateEvent` type in the
`proxycfg` package.
As we're going to be direct usage of the agent cache with interfaces that
can be satisfied by alternative server-local datasources, it doesn't make
sense to depend on this type everywhere anymore (particularly on the
`state.ch` channel).
We also plan to extract `proxycfg` out of Consul into a shared library in
the future, which would require removing this dependency.
Aside from a fairly rote find-and-replace, the main change is that the
`cache.Cache` and `health.Client` types now accept a callback function
parameter, rather than a `chan<- cache.UpdateEvents`. This allows us to
do the type conversion without running another goroutine.
- Introduce a new telemetry configurable parameter retry_failed_connection. User can set the value to true to let consul agent continue its start process on failed connection to datadog server. When set to false, agent will stop on failed start. The default behavior is true.
Co-authored-by: Dan Upton <daniel@floppy.co>
Co-authored-by: Evan Culver <eculver@users.noreply.github.com>
Treat each exported service as a "discovery chain" and replicate one
synthetic CheckServiceNode for each chain and remote mesh gateway.
The health will be a flattened generated check of the checks for that
mesh gateway node.
Modify node and service identities paragraphs on ACL index to better
conform with the style guide.
Co-authored-by: Jeff Boruszak <104028618+boruszak@users.noreply.github.com>
* add a github workflow to trigger ent->oss merge on every PR merged
* remove the workflow automation ref to trigger-oss-merge in circle-ci
* remove workflow automation
* revert circle-ci changes
* add actor
* remove cherrypicker
* add condition to avoid running in enterprise
Due to build changes in Consul 1.12.0 the `+ent` modifier is missing
from the version reported by `/v1/agent/self`.
Nomad looks for the `ent` modifier when determining whether to reconcile
services in non-default namespaces. Without the modifier Nomad will only
end up removing services from the default Consul namespace.
Changing from `pull_request` to `pull_request_target` so that forks can get the write permissions to add labels. See [this thread](https://github.com/actions/labeler/issues/121) in the action repo.
* Fix JSON formatted example ACL policies
* Remove HCL examples of roles since only JSON payloads can be used
* Remove unnecessary quotes from HCL variables
* Clarify purpose of example ACL rules and policies
* Redirect /api-docs/acl/acl to the new URL
Commit 9333fad added JSON formatted examples for all ACL polices.
Most of these these examples are not valid JSON, and thus an error is
raised when attempting to create the example policies/rules in Consul.
This commit fixes the example JSON formatted ACL rules so that they
are valid JSON. This enables readers to use the policies as-is from
the documentation to successfully create policies in Consul.
It also removes unnecessary arrays from the example policies so that
the policies are easier for practitioners to read and write.
By changing to use WatchCtx we will actually block for changes to the peering list. WatchCh creates a goroutine to collect errors from WatchCtx and returns immediately.
The existing behavior wouldn't result in a tight loop because of the rate limiting in the surrounding function, but it would still lead to more work than is necessary.
