Particular / NServiceBus

Build, version, and monitor better microservices with the most powerful service platform for .NET
https://particular.net/nservicebus/
Other
2.07k stars 650 forks source link

Providing an empty license string throws an exception #7060

Open dbelcham opened 1 month ago

dbelcham commented 1 month ago

Describe the bug

Description

When using the EndpointConfiguration.License(string) method, providing a null, empty or whitespace string will thrown an exception which bubbles into the user's code that is starting the endpoint. Usually this will be an exception that surfaces in the application's startup code and it will cause the application to fail during startup unless there is specific try...catch code written to handle that exception.

Since the longstanding principle of NSB has been to not prevent running of an endpoint if a license is not provided, this behaviour is both unexpected and contrary to that practice.

While this is not a critical bug, since there is a work around (provide a valid license string), we think it is a high level bug since it is a) not documented, and b) contrary to the behaviour that the framework exhibits elsewhere.

Expected behavior

I would expect that a null, empty or whitespace license string would behave like an endpoint that was configured, and started, with no license provided at all.

Actual behavior

Exception is thrown

Versions

Observed in NServiceBus 9.0.1

Steps to reproduce

Call EndpointConfiguration.License("")

Relevant log output

Unhandled exception. System.ArgumentException: The value cannot be an empty string or composed entirely of whitespace. (Parameter 'licenseText')
   at System.ArgumentException.ThrowNullOrWhiteSpaceException(String argument, String paramName)
   at System.ArgumentException.ThrowIfNullOrWhiteSpace(String argument, String paramName)
   at NServiceBus.ConfigureLicenseExtensions.License(EndpointConfiguration config, String licenseText) in /_/src/NServiceBus.Core/Licensing/ConfigureLicenseExtensions.cs:line 19
   at Customer.Common.Nsb.Extensions.HostBuilderExtensions.<>c__DisplayClass5_0.<ConfigureNServiceBus>b__0(HostBuilderContext context)
   at NServiceBus.HostBuilderExtensions.<>c__DisplayClass0_0.<UseNServiceBus>b__0(HostBuilderContext ctx, IServiceCollection services) in /_/src/NServiceBus.Extensions.Hosting/HostBuilderExtensions.cs:line 30
   at Microsoft.AspNetCore.Builder.ConfigureHostBuilder.ConfigureServices(Action`2 configureDelegate)
   at NServiceBus.HostBuilderExtensions.UseNServiceBus(IHostBuilder hostBuilder, Func`2 endpointConfigurationBuilder) in /_/src/NServiceBus.Extensions.Hosting/HostBuilderExtensions.cs:line 22
   at Customer.Common.Nsb.Extensions.HostBuilderExtensions.ConfigureNServiceBus(IHostBuilder hostBuilder, Boolean isSendOnly, String configurationSectionKey, Action`1 exposeConfiguration)
   at Customer.Common.Nsb.Extensions.HostBuilderExtensions.AddAsSendOnlyEndpoint(IHostBuilder hostBuilder)
   at Program.<Main>$(String[] args) in D:\a\1\s\src\Customer.Accounts.WebApi\Program.cs:line 129

Additional Information

Workarounds

Provide a license value that is not "", null or whitespace.

Possible solutions

Instead of throwing an exception, fall back to the behaviour that an endpoint exhibits when no license at all is provided.

Additional information

ramonsmits commented 1 month ago

The call to License and LicensePath are explicit configuration call. It is expected that data is present. If a license is corrupt or expired we still start but without data we consider this a configuration error.

Since the longstanding principle of NSB has been to not prevent running of an endpoint if a license is not provided,

If no (valid) license is found by our license scanning heuristics or the passed license is invalid we still start. The difference is in the word "provided" vs "present". Invoking License and LicensePath is explicitly selecting that license data to use.

You're asking that if there is no data we should ignore those calls.

If there is not data the user should not invoke those methods:

if (!string.IsNullOrEmpty(licenseText))
{
    endpointConfiguration.License(licenseText);
}

or

if (!File.Exist(licensePath))
{
    endpointConfiguration.LicensePath(licensePath);
}

or

if (!File.Exist(licensePath))
{
    var licenseText = File.ReadAllText(licensePath);
    if (!string.IsNullOrEmpty(licenseText))
    {
        endpointConfiguration.LicensePath(licensePath);
    }
}

The difference is more in an explicit configuration using License and LicensePath versus implicit license loading.

@dbelcham Do you have more information as currently it seems that just ignoring an explicit decision by the user if not the right approach.

dbelcham commented 1 month ago

We use the call to License(string) because we are pulling the license value from a centralized configuration system (Azure App Config and Key Vault in our case). We have many different environments in our development work (DEV/UAT/PROD/etc) and each of those environments has it's own centralized configuration stores. As we progress through our environments towards production, the restrictions around who has access to the configuration stores becomes more and more restricted. Because the lower environments are widely accessible we do not want to put our production license into them where it can be taken/leaked much more easily due to the less restricted nature of that environment.

As such, we rely on NSB's licensing which allows us to run in any non-production environment without a full license. Our expectation is that we can have the exact same code in each of our environments (i.e. no conditional saying "use License(string) only if this is production") and that the endpoints should successfully start in any of those environments, whether the license is in the configuration store or not. Our expectation is that when an empty or null license is pulled from the configuration store the endpoint will behave as if it has not been provided a license.

It seems like a very leaky abstraction that we need to add a conditional check for what our license value is when we retrieve it and then we have to decide how NSB should behave when the license is not available. This isn't our application's concern. This is an NSB concern that should be fully encapsulated with in the apis that NSB provides.

johnsimons commented 1 month ago

We triaged this issue and concluded that this is not a bug. We classify this as a small improvement and it will be considered in a future maintenance release.