[DO NOT MERGE] Allow disabling RabbitMQ broker requirement checks#5353
Draft
[DO NOT MERGE] Allow disabling RabbitMQ broker requirement checks#5353
Conversation
…tring For multi-tenant RabbitMQ deployments where tenants cannot be granted management API access, operators can now add DisableBrokerRequirementChecks=true to the connection string.
Delivery limit validation also requires management API access, so it must be disabled together with broker requirement checks.
When DisableBrokerRequirementChecks=true, do not register RabbitMQQuery (broker throughput) or QueueLengthProvider, since both require management API access that the user has indicated is unavailable. A NoOpQueueLengthProvider is registered instead to satisfy the DI requirement.
dvdstelt
changed the title
dvdstelt
changed the title
| protected override void AddTransportForPrimaryCore(IServiceCollection services, TransportSettings transportSettings) | ||
| => services.AddSingleton<IBrokerThroughputQuery, RabbitMQQuery>(); | ||
| { | ||
| if (!RabbitMQTransportExtensions.HasBrokerRequirementChecksDisabled(transportSettings.ConnectionString)) |
Member
There was a problem hiding this comment.
What happens if that's false? I mean what uses the IBrokerThroughputQuery and is that dependency optional?
Member
Author
There was a problem hiding this comment.
Several classes use it, but it's null by default. You can verify here:
- https://github.com/Particular/ServiceControl/blob/master/src/Particular.LicensingComponent/ThroughputCollector.cs#L16
- https://github.com/Particular/ServiceControl/blob/master/src/Particular.LicensingComponent/AuditThroughput/AuditThroughputCollectorHostedService.cs#L16
- https://github.com/Particular/ServiceControl/blob/master/src/Particular.LicensingComponent/MonitoringThroughput/MonitoringService.cs#L11
Extract ParseConnectionString helper to eliminate duplicated DbConnectionStringBuilder logic. Add warning log on startup when queue length monitoring is disabled. Mark NoOpQueueLengthProvider as sealed with explanatory comment.
dvdstelt
changed the title
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Motivation
In multi-tenant RabbitMQ deployments, granting management API access to individual tenants is a security risk. ServiceControl currently performs broker requirement checks that depend on management API access, with no way to disable them.
The RabbitMQ transport (v11.0.0) already supports
DisabledBrokerRequirementChecks, but ServiceControl did not expose it.What this does
Adds DisableBrokerRequirementChecks=true as a connection string parameter:
host=rabbitmq;username=tenant1;password=secret;DisableBrokerRequirementChecks=trueWhen enabled, this:
RabbitMQQuery(broker throughput collection) andQueueLengthProvider(queue length monitoring), both of which require management API accessNoOpQueueLengthProviderto satisfy DI requirementsWhat still works without management API access
What becomes unavailable